From a33d8ebac472d5bd1115eb942a83e6cc5cc29291 Mon Sep 17 00:00:00 2001 From: ianray <193674599+ianrayianray@users.noreply.github.com> Date: Thu, 30 Jul 2026 12:50:17 -0700 Subject: [PATCH 1/4] feat: add on-demand source and symbols --- README.md | 26 +- ...e2e-on-demand-source-symbols-2026-07-30.md | 49 ++ scripts/e2e.md | 20 +- skills/bc-al-source-symbols/SKILL.md | 46 ++ src/core/package-download.ts | 446 ++++++++++++++++++ src/core/source-content.ts | 3 +- src/mcp/skills.generated.ts | 9 +- src/mcp/tools/agent-response.ts | 8 +- src/mcp/tools/source-tools.ts | 56 +++ tests/core/package-download.test.ts | 302 ++++++++++++ tests/fixtures/app-package.ts | 75 +++ tests/mcp/agent-response.test.ts | 24 + tests/mcp/server.test.ts | 3 +- tests/mcp/skills.test.ts | 3 +- tests/mcp/tools.test.ts | 38 ++ 15 files changed, 1101 insertions(+), 7 deletions(-) create mode 100644 scripts/e2e-on-demand-source-symbols-2026-07-30.md create mode 100644 skills/bc-al-source-symbols/SKILL.md create mode 100644 src/core/package-download.ts create mode 100644 tests/core/package-download.test.ts create mode 100644 tests/fixtures/app-package.ts diff --git a/README.md b/README.md index 4bfbb0c..9da3489 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,7 @@ MCP server for Business Central AL development: run tests (with code coverage) a | **Interactive debugging** | Next-session, user-filtered, or exact-session attach; file/line breakpoints, break on all or unhandled errors only, record-write breaks (optionally skipping temp records), stack + variables + watch, stepping, abort/release at a break | | **Record-write triage** | Arm a bounded capture for one numeric table ID, automatically continue every global record-write stop, and group exact matching writer stacks while failing closed on unresolved evidence | | **BC native MCP passthrough** | Dynamically discover and invoke BC28 business-action, AL-runtime, and paused-debugger native MCP tools from the same agent | +| **On-demand source & symbols** | Read one deployed server object or download one validated installed `.app` package into the project’s `.alpackages` cache | | **Debug-a-test** | Test run bound to the debug session — breakpoints fire during test execution | | **Config auto-discovery** | Server/instance/tenant read from the AL project's `.vscode/launch.json` | | **Preflight diagnostics** | `bcdev_status` distinguishes unreachable / bad credentials / unsupported dev API | @@ -79,6 +80,7 @@ Running from source instead: `git clone` → `bun install && bun run build` → 5. `bcdev_debug_attach { breakOnError: true }` → trigger the workload → `bcdev_debug_wait` for `sessionBound` and breaks → inspect with `bcdev_debug_variables` / `bcdev_debug_eval` → `bcdev_debug_continue` → `bcdev_debug_detach`. For a debug-bound test run, trigger with `bcdev_debug_run_tests { codeunits: [{ id: 50100 }] }`. 6. To find writers of one table without stepping through every stop: `bcdev_record_writes_start { tableId: 18 }` → trigger the matching workload → check `bcdev_record_writes_status` → call `bcdev_record_writes_finish` for grouped stacks. 7. To use Business Central's own MCP catalog: `bcdev_native_list { company: "...", context: "business" }` → inspect the returned schemas → `bcdev_native_call` with one exact returned tool name. +8. For off-disk code or missing symbols: use `bcdev_source { objectType, objectId }`, or `bcdev_package_download { publisher, appName, version, appId }` for one installed package. Debugger attach returns as soon as Business Central accepts the request; binding is asynchronous. With no selector it binds the next session of the `breakOnNext` client type. Pass `userId` to filter that next session by Business Central user, or pass a known positive `sessionId` to attach to an existing NST session. `sessionId` and `userId` are mutually exclusive; exact `sessionId` targeting takes precedence over `breakOnNext`. `bcdev_debug_wait` reports `{ kind: "sessionBound", sessionId, hostId }` after binding; `hostId` may be null when Business Central omits that optional field. If identity lookup fails, it reports a nonfatal warning-form `sessionBound` event and debugging remains active. If Business Central emits a fatal user-filter rejection before `sessionBound`, the debugger tears down without binding or delivering breaks and reports an actionable `fatal` event. @@ -127,6 +129,7 @@ src/core/ (pure library — typed returns, injected deps) |-- authorization.ts Basic or cached Azure CLI authorization provider |-- server-info.ts GET dev/metadata, feature gates |-- native-mcp.ts BC cloud Streamable HTTP MCP client and trusted routing + |-- package-download.ts bounded, validated dev/packages download into .alpackages |-- al-objects.ts file <-> (objectType, objectId) index, test discovery |-- hubs/test-runner-hub.ts ──> /BC/dev/TestRunnerHub (SignalR) |-- hubs/debugger-hub.ts ──> /BC/dev/DebuggerHub (SignalR) @@ -153,6 +156,7 @@ src/core/ (pure library — typed returns, injected deps) | `bcdev_record_writes_status` | Read current write-triage lifecycle and classification counts without driving collection | | `bcdev_record_writes_finish` | Release/stop collection and return grouped exact writer stacks plus unresolved evidence | | `bcdev_source` | Read the server’s deployed AL source for an object not on local disk | +| `bcdev_package_download` | Download one validated installed dependency/symbol `.app` into `/.alpackages` | | `bcdev_native_list` | List the dynamic BC28 native tool catalog for business, AL runtime, or a paused debugger | | `bcdev_native_call` | Invoke one exact native tool and preserve its complete upstream result | | `bcdev_profile_status` | Preflight the snapshot-debugger endpoint; report whether sampling CPU profiling is supported | @@ -215,6 +219,24 @@ This passthrough supports cloud Sandbox and Production targets. Live acceptance Its contexts intentionally match the verified BC28 surface and do not include native profiling; the existing `bcdev_profile_*` snapshot tools are separate and unchanged. +## On-demand source and symbols + +`bcdev_source` reads the deployed source for one numeric object identity. Use it when a debugger +frame or coverage row has no local file. Empty content with `isAlContent:false` means Business +Central did not expose deployed AL source; the tool does not substitute potentially stale local +source. + +`bcdev_package_download` retrieves one explicitly identified installed `.app` through +`dev/packages` and writes it to the selected project’s `.alpackages` directory. Supply the +publisher, name, four-part minimum version, and app ID when known. Business Central may resolve a +higher installed version; the result reports both requested and resolved versions. + +The package is size- and time-bounded, validated through `SymbolReference.json`, checked against +the requested identity, hashed, and only then installed under a filename derived from the returned +metadata. A byte-identical file returns `unchanged`; different validated server bytes replace the +same package path. This is deliberately a single-package operation, not dependency enumeration or +full Download Symbols synchronization. + ## Profiling `bcdev_profile_*` **captures** a CPU profile; it does not itself drive BC. The division of labour is @@ -252,6 +274,7 @@ Validated end-to-end against a live BC28 in `scripts/e2e-profile-results-2026-07 The server ships its own operational manual as [Agent Skills](https://agentskills.io) over MCP resources (draft [SEP-2640](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2640)): `skill://bc-al-testing/SKILL.md`, `skill://bc-al-debugging/SKILL.md`, +`skill://bc-al-source-symbols/SKILL.md`, `skill://bc-native-mcp/SKILL.md`, discovery index at `skill://index.json`. Clients that understand the `io.modelcontextprotocol/skills` extension pick these up automatically; everything else sees them as plain readable resources. @@ -358,6 +381,7 @@ seconds, preventing a stale or hung preflight from holding that slot indefinitel | `src/core/coverage-gaps.ts` | Exact join between changed procedures and TestRunnerHub coverage evidence | | `src/core/record-write-triage.ts` | Source-aware runtime table classification and bounded writer-stack collector | | `src/core/native-mcp.ts` | BC cloud native MCP routing, lifecycle, and SDK transport | +| `src/core/package-download.ts` | Bounded package retrieval, symbol identity validation, and safe `.alpackages` installation | | `scripts/e2e.md` | Real-server wire-assumption checklist + known server behaviours | ## Roadmap @@ -372,7 +396,7 @@ Ordered by intent, not commitment: 6. **Test orchestration** — **shipped.** Repeat one selected test plan, retain every enriched run, diff adjacent passed/failed sets, and flag flaky or incomplete method outcomes. 7. **Break-on-record-write triage** — **shipped.** Arm bounded write triage for one numeric table ID and automatically collect grouped exact writer stacks. 8. **BC native MCP passthrough** — **shipped.** Dynamically list and invoke BC28 cloud business-action, AL-runtime, and active-debugger native tools through one agent-facing bridge while retaining complete upstream results. -9. **On-demand source & symbols** — fetch a server object's source, or download a single dependency package, without leaving the agent. +9. **On-demand source & symbols** — **shipped.** Read a deployed server object with `bcdev_source`, or download one validated installed dependency package into `.alpackages` with `bcdev_package_download`. ## Development diff --git a/scripts/e2e-on-demand-source-symbols-2026-07-30.md b/scripts/e2e-on-demand-source-symbols-2026-07-30.md new file mode 100644 index 0000000..455c968 --- /dev/null +++ b/scripts/e2e-on-demand-source-symbols-2026-07-30.md @@ -0,0 +1,49 @@ +# On-demand source and symbols — SaaS Sandbox evidence + +- Date: 2026-07-30 +- Target: Business Central SaaS Sandbox +- Client: built `dist/index.js` over MCP stdio +- Server mutations: none + +## Redaction + +The validation output retained only booleans, lifecycle statuses, stable error +codes, file counts, and equality checks. Tenant, environment-specific identity, +user, token, Authorization header, authenticated URL, package selector values, +returned source text, package filename, SHA-256 value, and machine paths were +not recorded. + +The target package was an already-published disposable demo with source exposure +enabled. The harness created and removed a temporary local AL project. Every +Business Central operation in this run was an HTTP GET. + +## Built-server results + +- `tools/list` exposed both `bcdev_source` and `bcdev_package_download`. +- `resources/list` exposed `skill://bc-al-source-symbols/SKILL.md`. +- `bcdev_source` returned `source:"rest"`, `isAlContent:true`, and deployed + source matching the requested disposable codeunit identity. +- The exact package request returned `status:"downloaded"`. +- The written file existed only under the temporary project’s `.alpackages` + directory, began with NAVX, contained parseable `SymbolReference.json`, and + its returned name/app ID matched the requested package in memory. +- The returned byte count and SHA-256 matched the written file. +- Repeating the exact request returned `status:"unchanged"`. +- Requesting a lower minimum version returned `status:"unchanged"` and resolved + to the same higher installed version. +- A too-new version, wrong app ID, and unknown package name each returned the + stable `NOT_FOUND` error code. +- The final directory contained one `.app` and zero `.tmp`/`.backup` files. +- The temporary project was deleted after the MCP client and server closed. + +## Deterministic gates + +- `bun test`: 638 passed, 0 failed. +- `bun run typecheck`: passed. +- `bun run build`: passed; four embedded skills generated. +- Package URL/query, Application app-ID omission, authentication, timeout, + streamed/declared size bounds, corrupt/missing symbols, identity/version + mismatch, unsafe names, safe replacement, and cleanup are covered by unit + tests. + +No Production call was made. diff --git a/scripts/e2e.md b/scripts/e2e.md index 57c6a98..4abe1da 100644 --- a/scripts/e2e.md +++ b/scripts/e2e.md @@ -34,6 +34,9 @@ Never record access tokens, Authorization headers, authenticated URLs, or unreda - [x] The cloud developer-service base is `https://api.businesscentral.dynamics.com/v2.0//dev`, with the environment path segment URL-encoded and `tenant` on metadata/query requests. - [x] The SaaS snapshot-debugger base is `https://api.businesscentral.dynamics.com/v2.0//snapshotdebugger` on the standard HTTPS origin, not the on-prem snapshot port. - [x] SaaS SignalR negotiation accepts the same authorization value in the HTTP `Authorization` header and the `Authentication` query parameter. +- [x] SaaS `GET dev/sourcecontent` returns deployed AL source for an exposed object using the same encoded environment base and tenant query. +- [x] `GET dev/packages` accepts `publisher`, `appName`, `versionText`, optional `appId`, and tenant; a lower `versionText` can resolve a higher installed package. +- [x] A successful package response is a NAVX/ZIP containing top-level `SymbolReference.json` identity fields `AppId`, `Name`, `Publisher`, and `Version`. ## Scenarios @@ -43,7 +46,7 @@ Never record access tokens, Authorization headers, authenticated URLs, or unreda - [x] bcdev_test_run with two codeunits: results for both, sequential execution. - [x] bcdev_debug_attach + bcdev_debug_run_tests + bcdev_debug_wait → break → bcdev_debug_variables → bcdev_debug_continue → testRunFinished. - [x] bcdev_debug_detach mid-break: BC session released (check server). -- [ ] tools/list shows title/annotations/outputSchema for all 23 tools; resources/list shows the three skill:// resources. +- [x] tools/list shows title/annotations/outputSchema for all 24 tools; resources/list shows the four skill:// resources. ## Test orchestration (SaaS Sandbox, BC28) @@ -70,6 +73,21 @@ Evidence belongs in `scripts/e2e-test-orchestration-2026-07-29.md`. - [x] No Production or on-premises call is made during live acceptance. +## On-demand source and symbols (SaaS Sandbox) + +Use one disposable package in a temporary AL project. The workflow is read-only on Business +Central and writes only the local temporary project’s `.alpackages` directory. Never retain +tenant, environment, user, token, authorization, authenticated URL, or machine-path values. +Evidence belongs in `scripts/e2e-on-demand-source-symbols-2026-07-30.md`. + +- [x] `bcdev_source` returns the deployed AL source and `source:"rest"` for one exposed object in the disposable package. +- [x] `bcdev_package_download` with exact publisher/name/version/app ID returns `downloaded`, a real NAVX/ZIP file, matching `SymbolReference.json` identity, byte count, and SHA-256. +- [x] Repeating the same request returns `unchanged` and retains the same digest. +- [x] A lower minimum version resolves the installed higher version and reports distinct requested/resolved versions. +- [x] Too-new version, wrong app ID, and unknown name selectors return typed `NOT_FOUND` errors and leave no temporary files. +- [x] Corrupt/missing symbols, identity/version mismatch, size/timeout bounds, unsafe names, safe replacement, cleanup, and on-prem/cloud URL shapes are deterministic unit tests. +- [x] No Production call is made. + ## Break-on-record-write triage (SaaS Sandbox) Run only against a disposable app and Business Central Sandbox. Never record tenant, environment, diff --git a/skills/bc-al-source-symbols/SKILL.md b/skills/bc-al-source-symbols/SKILL.md new file mode 100644 index 0000000..195659d --- /dev/null +++ b/skills/bc-al-source-symbols/SKILL.md @@ -0,0 +1,46 @@ +--- +name: bc-al-source-symbols +description: Retrieve deployed AL source or download one installed Business Central dependency symbol package without leaving the agent. Use when a debugger or coverage frame has no local file, or an AL project is missing one .app package in .alpackages. +--- + +# Retrieve deployed AL source and package symbols + +## Read one server object + +Call `bcdev_source { objectType, objectId }` with the numeric identity reported by a +debugger frame or coverage result. A nonempty `content` value is the server's deployed +source. `source: "hub"` means the REST endpoint had no answer and an active debugger +provided the source instead. + +`isAlContent: false` with empty content means Business Central did not return deployed AL +source. This is normal for compiled-only applications such as the base application; do +not invent source from local files or treat an empty result as a successful source fetch. + +## Download one symbol package + +1. Read the missing package identity from `app.json`, an AL compiler diagnostic, or other + trusted project metadata. +2. Call `bcdev_package_download` with `publisher`, `appName`, the four-part minimum + `version`, and `appId` when known. +3. Use `resolvedVersion` rather than assuming the requested minimum was returned. + `packagePath` is the validated package under `/.alpackages`. +4. Rerun the compile, source mapping, or coverage operation that needed the symbols. + +`status: "downloaded"` created the local package, `"replaced"` updated different bytes +for the same returned identity/version, and `"unchanged"` retained a byte-identical file. + +## Guardrails + +- The package tool downloads exactly one installed `.app`; it does not enumerate apps, + resolve transitive dependencies, or reproduce the AL extension's full Download Symbols + command. Repeat it only for package identities the project actually needs. +- `version` is a minimum selector. Business Central can return a higher installed version, + and the tool rejects a lower one or a mismatched name, publisher, or requested app ID. +- The output directory is fixed to `.alpackages`; there is no arbitrary destination. + Packages are bounded, validated through `SymbolReference.json`, hashed, and installed + only after the complete response passes validation. +- A downloaded package supplies compiler symbols and may contain source according to the + publisher's resource-exposure policy. The tool does not extract or claim source that + Business Central did not expose through `bcdev_source`. +- Connection defaults come from `.vscode/launch.json`. SaaS uses the current Azure CLI + identity; on-premises uses `BC_DEV_USER` and `BC_DEV_PASSWORD`. diff --git a/src/core/package-download.ts b/src/core/package-download.ts new file mode 100644 index 0000000..d52e40a --- /dev/null +++ b/src/core/package-download.ts @@ -0,0 +1,446 @@ +import { createHash, randomUUID } from "node:crypto"; +import { lstat, mkdir, readFile, rename, rm, writeFile } from "node:fs/promises"; +import { join, resolve } from "node:path"; +import type { AuthorizationProvider } from "./authorization"; +import { BcDevError } from "./agent-errors"; +import { extractEntry } from "./snapshot/zip"; +import type { ConnectionConfig } from "./types"; +import { baseClientUrl } from "./urls"; + +const DEFAULT_TIMEOUT_MS = 120_000; +const DEFAULT_MAX_BYTES = 256 * 1024 * 1024; +const MAX_VERSION_PART = 2_147_483_647; +const GUID = /^[0-9a-f]{8}(?:-[0-9a-f]{4}){3}-[0-9a-f]{12}$/i; + +export interface PackageSelector { + publisher: string; + appName: string; + version: string; + appId?: string; +} + +interface NormalizedPackageSelector extends PackageSelector { + versionParts: readonly [number, number, number, number]; +} + +export interface PackageDownloadOptions { + timeoutMs?: number; + maxBytes?: number; +} + +export interface PackageDownloadResult { + status: "downloaded" | "replaced" | "unchanged"; + packagePath: string; + publisher: string; + appName: string; + appId: string; + requestedVersion: string; + resolvedVersion: string; + bytes: number; + sha256: string; +} + +interface PackageIdentity { + publisher: string; + appName: string; + appId: string; + version: string; + versionParts: readonly [number, number, number, number]; +} + +function invalid(message: string): BcDevError { + return new BcDevError("INVALID_ARGUMENT", message, "validation"); +} + +function nonblank(value: unknown, field: "publisher" | "appName"): string { + if (typeof value !== "string" || value.trim() === "") throw invalid(`${field} must be a nonblank string`); + const trimmed = value.trim(); + if (/[\r\n]/.test(trimmed)) throw invalid(`${field} must not contain newline characters`); + return trimmed; +} + +function parseVersion(value: unknown, field = "version"): { + text: string; + parts: readonly [number, number, number, number]; +} { + if (typeof value !== "string" || !/^\d+\.\d+\.\d+\.\d+$/.test(value.trim())) { + throw invalid(`${field} must be a four-part numeric version such as 28.0.0.0`); + } + const parsed = value.trim().split(".").map(Number); + if (parsed.some((part) => !Number.isSafeInteger(part) || part < 0 || part > MAX_VERSION_PART)) { + throw invalid(`${field} contains an invalid numeric component`); + } + const parts = parsed as [number, number, number, number]; + return { text: parts.join("."), parts }; +} + +function normalizeSelector(selector: PackageSelector): NormalizedPackageSelector { + const publisher = nonblank(selector.publisher, "publisher"); + const appName = nonblank(selector.appName, "appName"); + const version = parseVersion(selector.version); + let appId: string | undefined; + if (selector.appId !== undefined) { + if (typeof selector.appId !== "string" || !GUID.test(selector.appId.trim())) { + throw invalid("appId must be a GUID"); + } + appId = selector.appId.trim().toLowerCase(); + } + return { publisher, appName, version: version.text, versionParts: version.parts, ...(appId ? { appId } : {}) }; +} + +function packageUrl(c: ConnectionConfig, selector: NormalizedPackageSelector): string { + const params = new URLSearchParams({ + publisher: selector.publisher, + appName: selector.appName, + versionText: selector.version, + }); + // WIRE: PackagesApiClient.DownloadPackage sends appId except for the case-insensitive + // "Application" concept reference (dep-decomp PackagesApiClient.cs and + // al-codeanalysis-decomp SymbolReferenceSpecification.IsApplicationConceptReference). + if (selector.appId && selector.appName.toLowerCase() !== "application") { + params.set("appId", selector.appId); + } + if (c.tenant) params.set("tenant", c.tenant); + // WIRE: the official AL client downloads one package with GET dev/packages and the + // publisher/appName/versionText selector above (dep-decomp PackagesApiClient.cs). + // Validated live on BC28 on-prem 2026-07-04 and SaaS Sandbox 2026-07-30. + return `${baseClientUrl(c)}dev/packages?${params.toString()}`; +} + +export function packageDownloadUrl(c: ConnectionConfig, selector: PackageSelector): string { + return packageUrl(c, normalizeSelector(selector)); +} + +function upperInvariantUtf16(value: string): string { + let result = ""; + for (let index = 0; index < value.length; index++) { + const unit = value[index]!; + const upper = unit.toUpperCase(); + result += upper.length === 1 ? upper : unit; + } + return result; +} + +function sameText(left: string, right: string): boolean { + return upperInvariantUtf16(left) === upperInvariantUtf16(right); +} + +function compareVersion( + left: readonly [number, number, number, number], + right: readonly [number, number, number, number], +): number { + for (let i = 0; i < 4; i++) { + const difference = left[i]! - right[i]!; + if (difference !== 0) return difference; + } + return 0; +} + +function stringField(object: Record, wanted: string): string | null { + const key = Object.keys(object).find((candidate) => candidate.toLowerCase() === wanted.toLowerCase()); + const value = key === undefined ? undefined : object[key]; + return typeof value === "string" && value.trim() !== "" ? value.trim() : null; +} + +function protocolError( + message: string, + details: Record = {}, + cause?: unknown, +): BcDevError { + return new BcDevError( + "PROTOCOL_ERROR", + message, + "protocol", + false, + details, + cause === undefined ? undefined : { cause }, + ); +} + +function packageIdentity(bytes: Buffer, selector: NormalizedPackageSelector): PackageIdentity { + let symbolBytes: Buffer | null; + try { + symbolBytes = extractEntry(bytes, "SymbolReference.json"); + } catch (error) { + throw protocolError("Business Central returned an invalid application package archive", {}, error); + } + if (!symbolBytes) { + throw protocolError("Business Central returned an application package without SymbolReference.json"); + } + + let symbols: Record; + try { + const parsed = JSON.parse(symbolBytes.toString("utf8").replace(/^\uFEFF/, "")) as unknown; + if (parsed === null || typeof parsed !== "object" || Array.isArray(parsed)) throw new Error("not an object"); + symbols = parsed as Record; + } catch (error) { + throw protocolError("Business Central returned an application package with invalid SymbolReference.json", {}, error); + } + + // WIRE: AL compiler .app packages identify their module at the top level of + // SymbolReference.json as AppId, Name, Publisher, and Version. Confirmed against genuine + // AL 18/BC28 packages and live SaaS download 2026-07-30. + const publisher = stringField(symbols, "Publisher"); + const appName = stringField(symbols, "Name"); + const appId = stringField(symbols, "AppId"); + const resolvedVersionText = stringField(symbols, "Version"); + if (!publisher || !appName || !appId || !resolvedVersionText || !GUID.test(appId)) { + throw protocolError("Business Central returned incomplete package identity metadata"); + } + + let resolvedVersion: ReturnType; + try { + resolvedVersion = parseVersion(resolvedVersionText, "returned package version"); + } catch (error) { + throw protocolError("Business Central returned an invalid package version", {}, error); + } + if (!sameText(publisher, selector.publisher) || !sameText(appName, selector.appName)) { + throw protocolError("Business Central returned a package with a different publisher or name", { + requestedPublisher: selector.publisher, + requestedAppName: selector.appName, + returnedPublisher: publisher, + returnedAppName: appName, + }); + } + if (selector.appId && appId.toLowerCase() !== selector.appId) { + throw protocolError("Business Central returned a package with a different app ID", { + requestedAppId: selector.appId, + returnedAppId: appId, + }); + } + // WIRE: versionText is a minimum-version selector, not exact. A lower version request + // resolved to the installed higher version on SaaS Sandbox 2026-07-30. + if (compareVersion(resolvedVersion.parts, selector.versionParts) < 0) { + throw protocolError("Business Central returned a package older than the requested minimum version", { + requestedVersion: selector.version, + returnedVersion: resolvedVersion.text, + }); + } + + return { + publisher, + appName, + appId: appId.toLowerCase(), + version: resolvedVersion.text, + versionParts: resolvedVersion.parts, + }; +} + +function safeFilenameComponent(value: string): string { + const safe = value.replace(/[\u0000-\u001f<>:"/\\|?*]/g, "_").replace(/[ .]+$/g, ""); + if (safe === "") throw protocolError("Business Central returned package identity that cannot form a safe filename"); + return safe; +} + +function packageFilename(identity: PackageIdentity): string { + return [ + safeFilenameComponent(identity.publisher), + safeFilenameComponent(identity.appName), + safeFilenameComponent(identity.version), + ].join("_") + ".app"; +} + +async function readBoundedBody(response: Response, controller: AbortController, maxBytes: number): Promise { + const contentLength = response.headers.get("content-length"); + if (contentLength && /^\d+$/.test(contentLength)) { + const declaredBytes = BigInt(contentLength); + if (declaredBytes > BigInt(maxBytes)) { + controller.abort(); + throw protocolError("Business Central package exceeds the download size limit", { + maxBytes, + contentLength: declaredBytes <= BigInt(Number.MAX_SAFE_INTEGER) ? Number(declaredBytes) : contentLength, + }); + } + } + if (!response.body) return Buffer.alloc(0); + + const reader = response.body.getReader(); + const chunks: Buffer[] = []; + let total = 0; + while (true) { + const next = await reader.read(); + if (next.done) break; + const chunk = Buffer.from(next.value); + total += chunk.length; + if (total > maxBytes) { + controller.abort(); + await reader.cancel().catch(() => undefined); + throw protocolError("Business Central package exceeds the download size limit", { + maxBytes, + receivedBytes: total, + }); + } + chunks.push(chunk); + } + return Buffer.concat(chunks, total); +} + +function errorCode(error: unknown): string | undefined { + return typeof error === "object" && error !== null && "code" in error + ? String((error as { code?: unknown }).code) + : undefined; +} + +function filesystemError(message: string, path: string, cause: unknown): BcDevError { + return new BcDevError("CONFIGURATION_ERROR", message, "configuration", false, { path }, { cause }); +} + +async function existingFile(path: string): Promise { + try { + return await readFile(path); + } catch (error) { + if (errorCode(error) === "ENOENT") return null; + throw filesystemError("Unable to read an existing AL package destination", path, error); + } +} + +async function replaceValidatedFile(tempPath: string, destination: string, hadExisting: boolean): Promise { + try { + await rename(tempPath, destination); + return; + } catch (error) { + // POSIX rename replaces an existing file atomically. Windows can reject that with + // EEXIST/EPERM, so use a recoverable backup swap only for that known case. + if (!hadExisting || !["EEXIST", "EPERM"].includes(errorCode(error) ?? "")) throw error; + } + + const backup = `${destination}.${randomUUID()}.backup`; + await rename(destination, backup); + try { + await rename(tempPath, destination); + } catch (error) { + await rename(backup, destination).catch(() => undefined); + throw error; + } + await rm(backup, { force: true }); +} + +async function installPackage(project: string, filename: string, bytes: Buffer, sha256: string): Promise<{ + status: PackageDownloadResult["status"]; + packagePath: string; +}> { + const packageDir = join(resolve(project), ".alpackages"); + try { + await mkdir(packageDir, { recursive: true }); + const packageDirStat = await lstat(packageDir); + if (!packageDirStat.isDirectory() || packageDirStat.isSymbolicLink()) { + throw new Error(".alpackages is not a direct directory"); + } + } catch (error) { + throw filesystemError("Unable to create the AL package directory", packageDir, error); + } + + const packagePath = join(packageDir, filename); + const existing = await existingFile(packagePath); + if (existing && createHash("sha256").update(existing).digest("hex") === sha256) { + return { status: "unchanged", packagePath }; + } + + const tempPath = join(packageDir, `.${filename}.${randomUUID()}.tmp`); + try { + await writeFile(tempPath, bytes, { flag: "wx" }); + await replaceValidatedFile(tempPath, packagePath, existing !== null); + } catch (error) { + throw filesystemError("Unable to install the downloaded AL package", packagePath, error); + } finally { + await rm(tempPath, { force: true }).catch(() => undefined); + } + return { status: existing === null ? "downloaded" : "replaced", packagePath }; +} + +export async function downloadPackage( + c: ConnectionConfig, + authorization: AuthorizationProvider, + project: string, + requested: PackageSelector, + fetchFn: typeof fetch = fetch, + options: PackageDownloadOptions = {}, +): Promise { + const selector = normalizeSelector(requested); + const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; + const maxBytes = options.maxBytes ?? DEFAULT_MAX_BYTES; + if (!Number.isSafeInteger(timeoutMs) || timeoutMs <= 0) throw invalid("package download timeout must be positive"); + if (!Number.isSafeInteger(maxBytes) || maxBytes <= 0) throw invalid("package download size limit must be positive"); + + const controller = new AbortController(); + let timedOut = false; + const timeout = setTimeout(() => { + timedOut = true; + controller.abort(); + }, timeoutMs); + + let bytes: Buffer; + try { + const response = await fetchFn(packageUrl(c, selector), { + method: "GET", + headers: { Authorization: await authorization.getAuthorizationHeader() }, + signal: controller.signal, + }); + if (response.status === 401 || response.status === 403) { + const hint = c.authentication === "UserPassword" + ? "verify BC_DEV_USER and BC_DEV_PASSWORD" + : "verify the Azure CLI login, tenant, and Business Central account access"; + throw new BcDevError("AUTHENTICATION_FAILED", `Business Central rejected package download authentication; ${hint}`, "auth"); + } + if (response.status === 404) { + throw new BcDevError( + "NOT_FOUND", + `No installed Business Central package matched ${selector.publisher}/${selector.appName} at version ${selector.version} or newer`, + "server", + false, + { + publisher: selector.publisher, + appName: selector.appName, + version: selector.version, + appId: selector.appId ?? null, + }, + ); + } + if (!response.ok) { + throw new BcDevError( + "SERVER_REJECTED", + `Business Central package download returned HTTP ${response.status}`, + "server", + response.status >= 500, + { status: response.status }, + ); + } + bytes = await readBoundedBody(response, controller, maxBytes); + } catch (error) { + if (error instanceof BcDevError) throw error; + if (timedOut) { + throw new BcDevError( + "TIMEOUT", + `Business Central package download timed out after ${timeoutMs} ms`, + "network", + true, + { timeoutMs }, + { cause: error }, + ); + } + throw new BcDevError( + "ENDPOINT_UNREACHABLE", + "Business Central package endpoint is unreachable", + "network", + true, + {}, + { cause: error }, + ); + } finally { + clearTimeout(timeout); + } + + const identity = packageIdentity(bytes, selector); + const sha256 = createHash("sha256").update(bytes).digest("hex"); + const installed = await installPackage(project, packageFilename(identity), bytes, sha256); + return { + ...installed, + publisher: identity.publisher, + appName: identity.appName, + appId: identity.appId, + requestedVersion: selector.version, + resolvedVersion: identity.version, + bytes: bytes.length, + sha256, + }; +} diff --git a/src/core/source-content.ts b/src/core/source-content.ts index e52d0a9..2d5cccf 100644 --- a/src/core/source-content.ts +++ b/src/core/source-content.ts @@ -10,7 +10,8 @@ export interface SourceContentResult { export function sourceContentUrl(c: ConnectionConfig, objectType: number, objectId: number): string { // WIRE: GET dev/sourcecontent?type=&id=[&tenant=] (dep-decomp SourceContentApiClient.GetSource), - // requires DevApiFeature.GetSourceCode => dev API 2.0 (dep-decomp DevApiFeatureExtensions.cs). Validated live 2026-07-04. + // requires DevApiFeature.GetSourceCode => dev API 2.0 (dep-decomp DevApiFeatureExtensions.cs). + // Validated live on BC28 on-prem 2026-07-04 and SaaS Sandbox 2026-07-30. const params = new URLSearchParams({ type: String(objectType), id: String(objectId) }); if (c.tenant) params.set("tenant", c.tenant); return `${baseClientUrl(c)}dev/sourcecontent?${params.toString()}`; diff --git a/src/mcp/skills.generated.ts b/src/mcp/skills.generated.ts index 4c19baf..a004d07 100644 --- a/src/mcp/skills.generated.ts +++ b/src/mcp/skills.generated.ts @@ -16,6 +16,13 @@ export const skills: EmbeddedSkill[] = [ "mimeType": "text/markdown", "text": "---\nname: bc-al-debugging\ndescription: Interactively debug AL code on a Business Central server with the bc-dev-mcp tools — attach, breakpoints, break-on-error, step, inspect variables and watches, or collect the stacks that write a table. Use when asked to debug AL code, find why an AL test fails, identify table writers, or inspect live state on a BC server.\n---\n\n# Debugging AL on Business Central\n\n## The loop\n\n1. `bcdev_debug_attach { breakOnError: true }` — arm the next WebClient session WITHOUT\n file/line breakpoints (see \"session must pause first\" below). To filter that next session,\n add `userId`; to attach to a known active NST session, add its positive `sessionId` instead.\n The selectors are mutually exclusive, and exact `sessionId` targeting takes precedence over\n `breakOnNext`. One debugger at a time; `bcdev_debug_detach` before re-attaching.\n2. `bcdev_debug_run_tests { codeunits: [{ id: }] }` — starts the run bound to\n this debug session and returns immediately.\n3. `bcdev_debug_wait` — long-poll for events. Attach returns before a workload binds; a successful\n bind reports `kind: sessionBound` with `sessionId` and `hostId` (the optional `hostId` may be null). **A timeout is a normal result**\n (`timedOut: true`): just call it again. Other events carry `kind`: `break`,\n `testRunFinished` (with the test results embedded), `detached`, `fatal`.\n If waiting times out before binding, confirm that you actually created or triggered the matching\n WebClient/workload, then wait again; waiting alone cannot create a next-session target.\n4. At a `break`: `bcdev_debug_variables { frameId: 0 }` for locals,\n `bcdev_debug_eval { frameId: 0, expression: \"...\" }` for watches,\n `bcdev_debug_breakpoints { add: [...] }` to place file/line breakpoints.\n5. `bcdev_debug_continue { action: \"continue\" | \"stepOver\" | \"stepInto\" | \"stepOut\" }`,\n then `bcdev_debug_wait` again.\n6. `bcdev_debug_detach` when done.\n\n## Find everything that writes one table\n\nUse the dedicated background workflow instead of manual `bcdev_debug_*` stepping:\n\n1. `bcdev_record_writes_start { tableId: }`.\n Add exactly one of `sessionId` or `userId` when needed, or choose `breakOnNext`.\n The tool returns after attach is armed; it does not wait for the session or workload.\n2. Trigger the matching session and operation. Business Central breaks on every record write\n globally; the collector identifies the runtime receiver, counts unrelated writes, groups exact\n target-table stacks, and automatically continues. Do not call `bcdev_debug_wait` or\n `bcdev_debug_continue`.\n3. `bcdev_record_writes_status` reads progress without driving it. When the target operation has\n finished, call `bcdev_record_writes_finish`.\n4. Review `writers`, `unresolved`, `warnings`, and `complete`. A complete report covers only the\n observed breaks in this one attached-session capture window; it is not a tenant-wide proof.\n\nTemporary-record writes are excluded by default. Pass `includeTemporary: true` only when they\nmatter. Deployed source is trusted; local source can prove a receiver only after you have published\nthat exact source and explicitly pass `changesDeployed: true`. Without that assertion, local-only\nmapping remains unresolved. `maxObservedWrites` defaults to 500; reaching it releases the workload\nand returns a truncated, incomplete report. Missing source/span, an unsupported receiver, watch\nfailure, unknown runtime table identity, or debugger failure also keeps the result fail-closed.\nIf the session detaches before `bcdev_record_writes_finish`, retained exact groups remain useful\nbut `complete` is false because later writes cannot be ruled out.\nFinish always clears the shared debugger slot; manual debugging and record-write triage cannot run\nat the same time.\n\n## Facts that save you time\n\n- **Choose one targeting mode.** Omit both selectors for the next client of `breakOnNext` type;\n use `userId` for the next matching client owned by that user; use `sessionId` only when you\n already know the positive ID of an active NST session. Arm next-session/user-filtered attach\n before triggering the workload. A warning-form `sessionBound` with null identity means only\n correlation metadata failed — debugging remains active.\n- If a user-filtered attach reports `fatal` before `sessionBound`, Business Central rejected the\n user filter. The client tears down that debugger and does not bind or deliver breaks; fix the\n user ID or permissions before retrying.\n- **Session must pause once before breakpoints bind.** `AddBreakpoint` fails server-side\n (\"tenant '' not found\") until the session has paused at least once. That is why you\n attach with `breakOnError: true` and add breakpoints at the first break. Once the\n session is live, breakpoints added for later code in the same run fire normally.\n- **Line numbers are 1-based** everywhere in these tools — use the line you see in your\n editor. (The BC wire is 0-based; conversion is internal.)\n- **Check breakpoint verification.** Each added breakpoint returns `verification.status`.\n `verified` means the server accepted the requested line, `relocated` means it moved to the\n returned 1-based span, and `unverified` means the server returned a usable breakpoint ID without\n a resolved span, including the all-zero value it uses for an unset span. Use a non-null returned\n span rather than assuming the requested line will fire.\n- **Watch expressions are paths, not code.** `bcdev_debug_eval` resolves identifier /\n member paths only (`CustomerName`, `Customer.\"No.\"`). Operators come back\n `` — and leave a synthetic empty-method entry in the test-run summary.\n- **Variable expansion:** nodes with `hasChildren: true` expand via\n `bcdev_debug_variables { frameId, expand: \"\" }`, e.g. `\"Customer\"` or\n `\"Customer.Fields\"`. Globals live under the `` node (or pass\n `globals: true`).\n- **Variable changes come from Business Central.** Each variable/watch node has `changeState`\n (`unchanged`, `new`, `valueChanged`, `descendantChanged`, or `unknown`) and `changed` is true\n for the three changed states. On a first read, or when the server omits the wire flag,\n `unknown` is not proof that a value stayed the same.\n- **Pages double-break:** page triggers fire twice per interaction on BC28 — expect two\n identical breaks, continue through the second.\n- **Test results arrive on the `testRunFinished` event** from `bcdev_debug_wait`, not from\n `bcdev_debug_run_tests` (which only starts the run).\n- `break` events map object IDs back to local `.al` files (`file` on the event and each\n stack frame) when the objects exist in the project.\n- **A break frame with no local `file`** (third-party extension, generated object) is readable\n anyway: `bcdev_source { objectType, objectId }` fetches the server's deployed source. Empty\n content with `isAlContent: false` means the object ships without source (base application).\n- **SQL triage:** attach with `sqlInsight: true` (optionally `longRunningSqlThresholdMs`), then\n at each break `bcdev_debug_sql` returns current latency, execute count, and the last SQL\n statements with duration and approximate rows read. It errors if insight was not enabled at\n attach — the option cannot be turned on mid-session.\n- **abort vs release at a break** (both end debugging with a `detached` event): `abort` kills\n the paused operation — a bound test run records it as failed; `release` lets it finish\n undebugged — a passing test still passes. Neither leaves a reusable session: a debug-bound\n test session ends with its run, so re-attach fresh instead of exact-targeting the old ID.\n- **Precision breaks:** `breakOnError: \"unhandled\"` skips errors a try function catches and\n still breaks on uncaught ones; `breakOnRecordWrite: \"nonTemporary\"` skips temporary-record\n writes and still breaks on real-table writes.\n- **Responses guide the loop.** Every successful tool result includes `nextSteps` (possibly\n empty for terminal results). MCP errors have `isError: true`, no `structuredContent`, and a\n JSON text body with stable `error.code`, `retryable`, redacted detail, and recovery steps.\n" }, + { + "name": "bc-al-source-symbols", + "description": "Retrieve deployed AL source or download one installed Business Central dependency symbol package without leaving the agent. Use when a debugger or coverage frame has no local file, or an AL project is missing one .app package in .alpackages.", + "uri": "skill://bc-al-source-symbols/SKILL.md", + "mimeType": "text/markdown", + "text": "---\nname: bc-al-source-symbols\ndescription: Retrieve deployed AL source or download one installed Business Central dependency symbol package without leaving the agent. Use when a debugger or coverage frame has no local file, or an AL project is missing one .app package in .alpackages.\n---\n\n# Retrieve deployed AL source and package symbols\n\n## Read one server object\n\nCall `bcdev_source { objectType, objectId }` with the numeric identity reported by a\ndebugger frame or coverage result. A nonempty `content` value is the server's deployed\nsource. `source: \"hub\"` means the REST endpoint had no answer and an active debugger\nprovided the source instead.\n\n`isAlContent: false` with empty content means Business Central did not return deployed AL\nsource. This is normal for compiled-only applications such as the base application; do\nnot invent source from local files or treat an empty result as a successful source fetch.\n\n## Download one symbol package\n\n1. Read the missing package identity from `app.json`, an AL compiler diagnostic, or other\n trusted project metadata.\n2. Call `bcdev_package_download` with `publisher`, `appName`, the four-part minimum\n `version`, and `appId` when known.\n3. Use `resolvedVersion` rather than assuming the requested minimum was returned.\n `packagePath` is the validated package under `/.alpackages`.\n4. Rerun the compile, source mapping, or coverage operation that needed the symbols.\n\n`status: \"downloaded\"` created the local package, `\"replaced\"` updated different bytes\nfor the same returned identity/version, and `\"unchanged\"` retained a byte-identical file.\n\n## Guardrails\n\n- The package tool downloads exactly one installed `.app`; it does not enumerate apps,\n resolve transitive dependencies, or reproduce the AL extension's full Download Symbols\n command. Repeat it only for package identities the project actually needs.\n- `version` is a minimum selector. Business Central can return a higher installed version,\n and the tool rejects a lower one or a mismatched name, publisher, or requested app ID.\n- The output directory is fixed to `.alpackages`; there is no arbitrary destination.\n Packages are bounded, validated through `SymbolReference.json`, hashed, and installed\n only after the complete response passes validation.\n- A downloaded package supplies compiler symbols and may contain source according to the\n publisher's resource-exposure policy. The tool does not extract or claim source that\n Business Central did not expose through `bcdev_source`.\n- Connection defaults come from `.vscode/launch.json`. SaaS uses the current Azure CLI\n identity; on-premises uses `BC_DEV_USER` and `BC_DEV_PASSWORD`.\n" + }, { "name": "bc-al-testing", "description": "Run AL tests against a Business Central dev endpoint with the bc-dev-mcp tools — discover test codeunits, run them with structured per-method results, optionally collect code coverage. Use when asked to run, re-run, or triage AL tests on a BC server.", @@ -32,4 +39,4 @@ export const skills: EmbeddedSkill[] = [ } ]; -export const skillsIndexJson: string = "{\n \"$schema\": \"https://schemas.agentskills.io/discovery/0.2.0/schema.json\",\n \"skills\": [\n {\n \"name\": \"bc-al-debugging\",\n \"type\": \"skill-md\",\n \"description\": \"Interactively debug AL code on a Business Central server with the bc-dev-mcp tools — attach, breakpoints, break-on-error, step, inspect variables and watches, or collect the stacks that write a table. Use when asked to debug AL code, find why an AL test fails, identify table writers, or inspect live state on a BC server.\",\n \"url\": \"skill://bc-al-debugging/SKILL.md\"\n },\n {\n \"name\": \"bc-al-testing\",\n \"type\": \"skill-md\",\n \"description\": \"Run AL tests against a Business Central dev endpoint with the bc-dev-mcp tools — discover test codeunits, run them with structured per-method results, optionally collect code coverage. Use when asked to run, re-run, or triage AL tests on a BC server.\",\n \"url\": \"skill://bc-al-testing/SKILL.md\"\n },\n {\n \"name\": \"bc-native-mcp\",\n \"type\": \"skill-md\",\n \"description\": \"Discover and invoke Business Central 28 native MCP tools through bc-dev-mcp. Use when an agent needs BC business actions, the native AL runtime test tool, or native troubleshooting tools for an already paused AL debugger session.\",\n \"url\": \"skill://bc-native-mcp/SKILL.md\"\n }\n ]\n}"; +export const skillsIndexJson: string = "{\n \"$schema\": \"https://schemas.agentskills.io/discovery/0.2.0/schema.json\",\n \"skills\": [\n {\n \"name\": \"bc-al-debugging\",\n \"type\": \"skill-md\",\n \"description\": \"Interactively debug AL code on a Business Central server with the bc-dev-mcp tools — attach, breakpoints, break-on-error, step, inspect variables and watches, or collect the stacks that write a table. Use when asked to debug AL code, find why an AL test fails, identify table writers, or inspect live state on a BC server.\",\n \"url\": \"skill://bc-al-debugging/SKILL.md\"\n },\n {\n \"name\": \"bc-al-source-symbols\",\n \"type\": \"skill-md\",\n \"description\": \"Retrieve deployed AL source or download one installed Business Central dependency symbol package without leaving the agent. Use when a debugger or coverage frame has no local file, or an AL project is missing one .app package in .alpackages.\",\n \"url\": \"skill://bc-al-source-symbols/SKILL.md\"\n },\n {\n \"name\": \"bc-al-testing\",\n \"type\": \"skill-md\",\n \"description\": \"Run AL tests against a Business Central dev endpoint with the bc-dev-mcp tools — discover test codeunits, run them with structured per-method results, optionally collect code coverage. Use when asked to run, re-run, or triage AL tests on a BC server.\",\n \"url\": \"skill://bc-al-testing/SKILL.md\"\n },\n {\n \"name\": \"bc-native-mcp\",\n \"type\": \"skill-md\",\n \"description\": \"Discover and invoke Business Central 28 native MCP tools through bc-dev-mcp. Use when an agent needs BC business actions, the native AL runtime test tool, or native troubleshooting tools for an already paused AL debugger session.\",\n \"url\": \"skill://bc-native-mcp/SKILL.md\"\n }\n ]\n}"; diff --git a/src/mcp/tools/agent-response.ts b/src/mcp/tools/agent-response.ts index cf5325d..11bb654 100644 --- a/src/mcp/tools/agent-response.ts +++ b/src/mcp/tools/agent-response.ts @@ -80,7 +80,13 @@ function generatedNextSteps(name: string, result: Record, param case "bcdev_debug_breakpoints": return ["Continue the current investigation or call bcdev_debug_continue if the debugger is paused."]; case "bcdev_source": - return []; + return result["isAlContent"] === false + ? ["Call bcdev_package_download with the owning app identity if package symbols are sufficient even though deployed source is unavailable."] + : []; + case "bcdev_package_download": + return [ + "Use the validated package in .alpackages, then rerun the AL compile, source mapping, or coverage operation that required its symbols.", + ]; case "bcdev_native_list": { const catalog = result["catalog"] as Record | undefined; return Array.isArray(catalog?.["tools"]) && catalog.tools.length > 0 diff --git a/src/mcp/tools/source-tools.ts b/src/mcp/tools/source-tools.ts index d85f15c..12317af 100644 --- a/src/mcp/tools/source-tools.ts +++ b/src/mcp/tools/source-tools.ts @@ -1,4 +1,5 @@ import { z } from "zod"; +import { downloadPackage } from "../../core/package-download"; import { fetchSourceContent, type SourceContentResult } from "../../core/source-content"; import type { ServerState } from "../state"; import { connectionShape, resolve, type ToolDefinition, type ToolDeps } from "./shared"; @@ -51,5 +52,60 @@ export function createSourceTools(state: ServerState, deps: ToolDeps): ToolDefin return shapeResult(result, objectType, objectId, "rest"); }, }, + { + name: "bcdev_package_download", + title: "Download one AL dependency package", + description: + "Download one explicitly identified installed Business Central .app dependency/symbol package into /.alpackages. The version is a minimum; this does not enumerate or synchronize dependencies.", + annotations: { + readOnlyHint: false, + destructiveHint: false, + idempotentHint: true, + openWorldHint: true, + }, + schema: { + ...connectionShape, + publisher: z.string().trim().min(1) + .regex(/^[^\r\n]+$/, "publisher must not contain newline characters") + .describe("Installed package publisher"), + appName: z.string().trim().min(1) + .regex(/^[^\r\n]+$/, "appName must not contain newline characters") + .describe("Installed package name"), + version: z.string().trim() + .regex(/^\d+\.\d+\.\d+\.\d+$/, "version must contain four numeric parts") + .describe("Minimum installed package version, for example 28.0.0.0"), + appId: z.string().trim() + .regex(/^[0-9a-f]{8}(?:-[0-9a-f]{4}){3}-[0-9a-f]{12}$/i, "appId must be a GUID") + .optional() + .describe("Package app ID when known; omitted for publisher/name selection and the special Application package"), + }, + outputSchema: z.object({ + status: z.enum(["downloaded", "replaced", "unchanged"]) + .describe("Whether the local package was newly written, replaced, or already byte-identical"), + packagePath: z.string().describe("Absolute path of the validated .app under /.alpackages"), + publisher: z.string().describe("Publisher read from the downloaded SymbolReference.json"), + appName: z.string().describe("Package name read from the downloaded SymbolReference.json"), + appId: z.string().describe("Package app ID read from the downloaded SymbolReference.json"), + requestedVersion: z.string().describe("Normalized minimum version requested from Business Central"), + resolvedVersion: z.string().describe("Installed package version returned by Business Central"), + bytes: z.number().int().nonnegative().describe("Downloaded package size in bytes"), + sha256: z.string().describe("SHA-256 digest of the installed package"), + }), + handler: async (params) => { + const { config, authorization, project } = resolve(params, deps); + return downloadPackage( + config, + authorization, + project, + { + publisher: params["publisher"] as string, + appName: params["appName"] as string, + version: params["version"] as string, + appId: params["appId"] as string | undefined, + }, + deps.fetchFn, + ); + }, + }, ]; } diff --git a/tests/core/package-download.test.ts b/tests/core/package-download.test.ts new file mode 100644 index 0000000..bb6f6e8 --- /dev/null +++ b/tests/core/package-download.test.ts @@ -0,0 +1,302 @@ +import { describe, expect, test } from "bun:test"; +import { createHash } from "node:crypto"; +import { mkdirSync, mkdtempSync, readFileSync, readdirSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { basename, join } from "node:path"; +import { BasicAuthorizationProvider } from "../../src/core/authorization"; +import { + downloadPackage, + packageDownloadUrl, + type PackageSelector, +} from "../../src/core/package-download"; +import type { ConnectionConfig } from "../../src/core/types"; +import { buildAppPackage, buildStoredZip } from "../fixtures/app-package"; + +const APP_ID = "63ca2fa4-4f03-4f2b-a480-172fef340d3f"; +const OTHER_ID = "00000000-0000-0000-0000-000000000001"; +const selector: PackageSelector = { + publisher: "Microsoft", + appName: "System Application", + version: "28.0.0.0", + appId: APP_ID, +}; +const config: ConnectionConfig = { + environmentType: "OnPrem", + authentication: "UserPassword", + server: "http://localhost", + serverInstance: "BC", + tenant: "default", + username: "u", + password: "p", +}; +const auth = new BasicAuthorizationProvider("u", "p"); + +function appPackage( + overrides: Partial<{ publisher: string; name: string; appId: string; version: string }> = {}, + extra = "", +): Buffer { + return buildAppPackage({ + publisher: overrides.publisher ?? selector.publisher, + name: overrides.name ?? selector.appName, + appId: overrides.appId ?? selector.appId!, + version: overrides.version ?? "28.2.3.4", + extra, + }); +} + +function project(): string { + return mkdtempSync(join(tmpdir(), "bcmcp-package-")); +} + +function responseFetch( + body: BodyInit | Buffer | null, + status = 200, + capture?: { url?: string; authorization?: string; signal?: AbortSignal | null }, + headers?: HeadersInit, +): typeof fetch { + return (async (input: RequestInfo | URL, init?: RequestInit) => { + if (capture) { + capture.url = input.toString(); + capture.authorization = (init?.headers as Record | undefined)?.["Authorization"]; + capture.signal = init?.signal; + } + return new Response(body as BodyInit | null, { status, headers }); + }) as unknown as typeof fetch; +} + +describe("packageDownloadUrl", () => { + test("builds the official on-prem selector and tenant query", () => { + expect(packageDownloadUrl(config, selector)).toBe( + "http://localhost:7049/BC/dev/packages?publisher=Microsoft&appName=System+Application&versionText=28.0.0.0&appId=63ca2fa4-4f03-4f2b-a480-172fef340d3f&tenant=default", + ); + }); + + test("builds the SaaS route and omits appId for the Application concept", () => { + const cloud: ConnectionConfig = { + environmentType: "Sandbox", + authentication: "EntraId", + environmentName: "My Sandbox", + tenant: "tenant-id", + }; + expect(packageDownloadUrl(cloud, { + publisher: "Microsoft", + appName: "application", + version: "28.0.0.0", + appId: APP_ID, + })).toBe( + "https://api.businesscentral.dynamics.com/v2.0/My%20Sandbox/dev/packages?publisher=Microsoft&appName=application&versionText=28.0.0.0&tenant=tenant-id", + ); + }); + + test("rejects malformed selectors before making a request", () => { + expect(() => packageDownloadUrl(config, { ...selector, publisher: " \n" })).toThrow(/publisher/); + expect(() => packageDownloadUrl(config, { ...selector, version: "28.0" })).toThrow(/four-part/); + expect(() => packageDownloadUrl(config, { ...selector, appId: "not-a-guid" })).toThrow(/GUID/); + }); +}); + +describe("downloadPackage", () => { + test("downloads, validates, safely names, hashes, and installs one package", async () => { + const capture: { url?: string; authorization?: string } = {}; + const bytes = appPackage(); + const result = await downloadPackage(config, auth, project(), selector, responseFetch(bytes, 200, capture)); + + expect(result).toMatchObject({ + status: "downloaded", + publisher: "Microsoft", + appName: "System Application", + appId: APP_ID, + requestedVersion: "28.0.0.0", + resolvedVersion: "28.2.3.4", + bytes: bytes.length, + sha256: createHash("sha256").update(bytes).digest("hex"), + }); + expect(basename(result.packagePath)).toBe("Microsoft_System Application_28.2.3.4.app"); + expect(readFileSync(result.packagePath).equals(bytes)).toBe(true); + expect(capture.authorization).toBe("Basic " + Buffer.from("u:p").toString("base64")); + expect(capture.url).toContain("dev/packages?"); + }); + + test("returns unchanged for identical bytes and replaces only with another validated package", async () => { + const dir = project(); + const firstBytes = appPackage({}, "first"); + const secondBytes = appPackage({}, "second"); + const first = await downloadPackage(config, auth, dir, selector, responseFetch(firstBytes)); + const unchanged = await downloadPackage(config, auth, dir, selector, responseFetch(firstBytes)); + const replaced = await downloadPackage(config, auth, dir, selector, responseFetch(secondBytes)); + + expect(first.status).toBe("downloaded"); + expect(unchanged.status).toBe("unchanged"); + expect(replaced.status).toBe("replaced"); + expect(replaced.packagePath).toBe(first.packagePath); + expect(readFileSync(replaced.packagePath).equals(secondBytes)).toBe(true); + }); + + test("uses only validated metadata for the destination filename", async () => { + const unsafe = { + publisher: "Pub/lish:er", + appName: "App.", + version: "1.0.0.0", + appId: APP_ID, + }; + const result = await downloadPackage( + config, + auth, + project(), + unsafe, + responseFetch(appPackage({ publisher: unsafe.publisher, name: unsafe.appName, version: unsafe.version })), + ); + expect(basename(result.packagePath)).toBe("Pub_lish_er_App_Name__1.0.0.0.app"); + }); + + test("accepts a higher resolved version but rejects wrong identity and an older package", async () => { + const higher = await downloadPackage(config, auth, project(), selector, responseFetch(appPackage())); + expect(higher.resolvedVersion).toBe("28.2.3.4"); + + await expect(downloadPackage( + config, + auth, + project(), + selector, + responseFetch(appPackage({ appId: OTHER_ID })), + )).rejects.toMatchObject({ code: "PROTOCOL_ERROR" }); + await expect(downloadPackage( + config, + auth, + project(), + selector, + responseFetch(appPackage({ name: "Other App" })), + )).rejects.toThrow(/different publisher or name/); + await expect(downloadPackage( + config, + auth, + project(), + selector, + responseFetch(appPackage({ version: "27.9.9.9" })), + )).rejects.toThrow(/older than/); + await expect(downloadPackage( + config, + auth, + project(), + { ...selector, appName: "Straße" }, + responseFetch(appPackage({ name: "Strasse" })), + )).rejects.toThrow(/different publisher or name/); + }); + + test("rejects invalid archives and symbols without leaving a package or temporary file", async () => { + for (const bytes of [ + Buffer.from("not a package"), + Buffer.concat([Buffer.from("NAVX"), Buffer.alloc(36), buildStoredZip([{ name: "other.txt", content: Buffer.from("x") }])]), + Buffer.concat([ + Buffer.from("NAVX"), + Buffer.alloc(36), + buildStoredZip([{ name: "SymbolReference.json", content: Buffer.from("{") }]), + ]), + ]) { + const dir = project(); + await expect(downloadPackage(config, auth, dir, selector, responseFetch(bytes))).rejects.toMatchObject({ + code: "PROTOCOL_ERROR", + }); + expect(readdirSync(dir)).toEqual([]); + } + }); + + test("does not replace an existing package when the new response fails validation", async () => { + const dir = project(); + const good = appPackage(); + const installed = await downloadPackage(config, auth, dir, selector, responseFetch(good)); + await expect(downloadPackage(config, auth, dir, selector, responseFetch(Buffer.from("bad")))).rejects.toMatchObject({ + code: "PROTOCOL_ERROR", + }); + expect(readFileSync(installed.packagePath).equals(good)).toBe(true); + expect(readdirSync(join(dir, ".alpackages")).filter((name) => name.endsWith(".tmp"))).toEqual([]); + }); + + test("returns typed authentication, not-found, server, and network errors", async () => { + await expect(downloadPackage(config, auth, project(), selector, responseFetch(null, 401))).rejects.toMatchObject({ + code: "AUTHENTICATION_FAILED", + }); + await expect(downloadPackage(config, auth, project(), selector, responseFetch(null, 404))).rejects.toMatchObject({ + code: "NOT_FOUND", + details: { appId: APP_ID }, + }); + await expect(downloadPackage(config, auth, project(), selector, responseFetch(null, 503))).rejects.toMatchObject({ + code: "SERVER_REJECTED", + retryable: true, + }); + const offline = (async () => { + throw new TypeError("offline"); + }) as unknown as typeof fetch; + await expect(downloadPackage(config, auth, project(), selector, offline)).rejects.toMatchObject({ + code: "ENDPOINT_UNREACHABLE", + retryable: true, + }); + }); + + test("enforces both declared and streamed size bounds", async () => { + await expect(downloadPackage( + config, + auth, + project(), + selector, + responseFetch("12345", 200, undefined, { "Content-Length": "5" }), + { maxBytes: 4 }, + )).rejects.toMatchObject({ code: "PROTOCOL_ERROR", details: { maxBytes: 4, contentLength: 5 } }); + + const body = new ReadableStream({ + start(controller) { + controller.enqueue(new Uint8Array([1, 2, 3])); + controller.enqueue(new Uint8Array([4, 5, 6])); + controller.close(); + }, + }); + await expect(downloadPackage( + config, + auth, + project(), + selector, + responseFetch(body), + { maxBytes: 4 }, + )).rejects.toMatchObject({ code: "PROTOCOL_ERROR", details: { maxBytes: 4, receivedBytes: 6 } }); + }); + + test("times out an unfinished request and passes an abort signal", async () => { + let signal: AbortSignal | null | undefined; + const pending = (async (_input: RequestInfo | URL, init?: RequestInit) => { + signal = init?.signal; + return await new Promise((_resolve, reject) => { + init?.signal?.addEventListener("abort", () => reject(new DOMException("aborted", "AbortError")), { once: true }); + }); + }) as unknown as typeof fetch; + await expect(downloadPackage( + config, + auth, + project(), + selector, + pending, + { timeoutMs: 5 }, + )).rejects.toMatchObject({ code: "TIMEOUT", retryable: true, details: { timeoutMs: 5 } }); + expect(signal?.aborted).toBe(true); + }); + + test("reports an unwritable project destination as a configuration error", async () => { + const file = join(project(), "not-a-directory"); + writeFileSync(file, "x"); + await expect(downloadPackage(config, auth, file, selector, responseFetch(appPackage()))).rejects.toMatchObject({ + code: "CONFIGURATION_ERROR", + }); + }); + + if (process.platform !== "win32") { + test("rejects a symlinked .alpackages directory instead of writing outside the project", async () => { + const dir = project(); + const outside = project(); + mkdirSync(outside, { recursive: true }); + symlinkSync(outside, join(dir, ".alpackages"), "dir"); + await expect(downloadPackage(config, auth, dir, selector, responseFetch(appPackage()))).rejects.toMatchObject({ + code: "CONFIGURATION_ERROR", + }); + expect(readdirSync(outside)).toEqual([]); + }); + } +}); diff --git a/tests/fixtures/app-package.ts b/tests/fixtures/app-package.ts new file mode 100644 index 0000000..b9464d0 --- /dev/null +++ b/tests/fixtures/app-package.ts @@ -0,0 +1,75 @@ +interface ZipEntry { + name: string; + content: Buffer; +} + +export interface AppPackageFixture { + publisher: string; + name: string; + appId: string; + version: string; + extra?: string; + bom?: boolean; + preamble?: boolean; +} + +export function buildStoredZip(entries: ZipEntry[]): Buffer { + const locals: Buffer[] = []; + const central: Buffer[] = []; + let offset = 0; + for (const entry of entries) { + const name = Buffer.from(entry.name); + const local = Buffer.alloc(30); + local.writeUInt32LE(0x04034b50, 0); + local.writeUInt16LE(20, 4); + local.writeUInt16LE(0, 6); + local.writeUInt16LE(0, 8); + local.writeUInt32LE(entry.content.length, 18); + local.writeUInt32LE(entry.content.length, 22); + local.writeUInt16LE(name.length, 26); + locals.push(local, name, entry.content); + + const cd = Buffer.alloc(46); + cd.writeUInt32LE(0x02014b50, 0); + cd.writeUInt16LE(20, 4); + cd.writeUInt16LE(20, 6); + cd.writeUInt16LE(0, 8); + cd.writeUInt16LE(0, 10); + cd.writeUInt32LE(entry.content.length, 20); + cd.writeUInt32LE(entry.content.length, 24); + cd.writeUInt16LE(name.length, 28); + cd.writeUInt32LE(offset, 42); + central.push(cd, name); + offset += local.length + name.length + entry.content.length; + } + + const localBytes = Buffer.concat(locals); + const centralBytes = Buffer.concat(central); + const eocd = Buffer.alloc(22); + eocd.writeUInt32LE(0x06054b50, 0); + eocd.writeUInt16LE(entries.length, 8); + eocd.writeUInt16LE(entries.length, 10); + eocd.writeUInt32LE(centralBytes.length, 12); + eocd.writeUInt32LE(localBytes.length, 16); + return Buffer.concat([localBytes, centralBytes, eocd]); +} + +export function buildAppPackage(fixture: AppPackageFixture): Buffer { + const symbols = { + RuntimeVersion: "17.0", + Publisher: fixture.publisher, + Name: fixture.name, + AppId: fixture.appId, + Version: fixture.version, + Codeunits: [], + Extra: fixture.extra ?? "", + }; + const prefix = fixture.bom === false ? "" : "\uFEFF"; + const zip = buildStoredZip([ + { name: "NavxManifest.xml", content: Buffer.from("") }, + { name: "SymbolReference.json", content: Buffer.from(`${prefix}${JSON.stringify(symbols)}`) }, + ]); + return fixture.preamble === false + ? zip + : Buffer.concat([Buffer.from("NAVX"), Buffer.alloc(36), zip]); +} diff --git a/tests/mcp/agent-response.test.ts b/tests/mcp/agent-response.test.ts index cdaf254..a5118a0 100644 --- a/tests/mcp/agent-response.test.ts +++ b/tests/mcp/agent-response.test.ts @@ -88,4 +88,28 @@ describe("withAgentResponses", () => { (await wrapped("passed").handler({}) as { nextSteps: string[] }).nextSteps, ).toEqual([]); }); + + test("source and package guidance connect missing source to installed symbols", async () => { + const missingSource = withAgentResponses(tool({ + name: "bcdev_source", + handler: async () => ({ isAlContent: false }), + })); + expect((await missingSource.handler({}) as { nextSteps: string[] }).nextSteps.join(" ")).toContain( + "bcdev_package_download", + ); + + const sourceFound = withAgentResponses(tool({ + name: "bcdev_source", + handler: async () => ({ isAlContent: true }), + })); + expect((await sourceFound.handler({}) as { nextSteps: string[] }).nextSteps).toEqual([]); + + const downloaded = withAgentResponses(tool({ + name: "bcdev_package_download", + handler: async () => ({ status: "downloaded" }), + })); + expect((await downloaded.handler({}) as { nextSteps: string[] }).nextSteps.join(" ")).toContain( + ".alpackages", + ); + }); }); diff --git a/tests/mcp/server.test.ts b/tests/mcp/server.test.ts index 3b64996..ba889e8 100644 --- a/tests/mcp/server.test.ts +++ b/tests/mcp/server.test.ts @@ -156,6 +156,7 @@ describe("server wiring", () => { const uris = resources.map((r) => r.uri).sort(); expect(uris).toEqual([ "skill://bc-al-debugging/SKILL.md", + "skill://bc-al-source-symbols/SKILL.md", "skill://bc-al-testing/SKILL.md", "skill://bc-native-mcp/SKILL.md", "skill://index.json", @@ -165,7 +166,7 @@ describe("server wiring", () => { const parsed = JSON.parse((index.contents[0] as { text: string }).text) as { skills: Array<{ url: string; type: string }>; }; - expect(parsed.skills).toHaveLength(3); + expect(parsed.skills).toHaveLength(4); for (const s of parsed.skills) expect(uris).toContain(s.url); const skill = await client.readResource({ uri: "skill://bc-al-debugging/SKILL.md" }); diff --git a/tests/mcp/skills.test.ts b/tests/mcp/skills.test.ts index 69ed62a..f03761f 100644 --- a/tests/mcp/skills.test.ts +++ b/tests/mcp/skills.test.ts @@ -17,6 +17,7 @@ describe("skills embedding", () => { const skills = await collectSkills(join(repoRoot, "skills")); expect(skills.map((s) => s.name)).toEqual([ "bc-al-debugging", + "bc-al-source-symbols", "bc-al-testing", "bc-native-mcp", ]); @@ -37,7 +38,7 @@ describe("skills embedding", () => { skills: Array<{ name: string; type: string; description: string; url: string }>; }; expect(index.$schema).toBe("https://schemas.agentskills.io/discovery/0.2.0/schema.json"); - expect(index.skills).toHaveLength(3); + expect(index.skills).toHaveLength(4); for (const entry of index.skills) { expect(entry.type).toBe("skill-md"); expect(entry.url).toBe(`skill://${entry.name}/SKILL.md`); diff --git a/tests/mcp/tools.test.ts b/tests/mcp/tools.test.ts index 3338bf8..720adb3 100644 --- a/tests/mcp/tools.test.ts +++ b/tests/mcp/tools.test.ts @@ -10,6 +10,7 @@ import { FakeHub, fakeHubFactory } from "../fakes/fake-hub"; import type { GitChangeSet } from "../../src/core/git-changes"; import { calculateProcedureMethodId } from "../../src/core/al-procedures"; import { FakeNativeMcpGateway } from "../fakes/fake-native-mcp"; +import { buildAppPackage } from "../fixtures/app-package"; function makeProject(): string { const dir = mkdtempSync(join(tmpdir(), "bcmcp-tools-")); @@ -80,6 +81,7 @@ describe("tools", () => { "bcdev_debug_wait", "bcdev_native_call", "bcdev_native_list", + "bcdev_package_download", "bcdev_profile_finish", "bcdev_profile_poll", "bcdev_profile_start", @@ -707,6 +709,42 @@ describe("tools", () => { expect(viaHub).toMatchObject({ content: "from hub", source: "hub" }); }); + test("bcdev_package_download installs one validated package and returns agent guidance", async () => { + const bytes = buildAppPackage({ + publisher: "SShadowS", + name: "Demo Symbols", + appId: "2ca1721e-459e-4745-b96a-803517331326", + version: "1.2.0.0", + }); + let requestUrl = ""; + const packageFetch = (async (input: RequestInfo | URL) => { + requestUrl = input.toString(); + return new Response(Uint8Array.from(bytes)); + }) as unknown as typeof fetch; + const { tools } = setup(hub, packageFetch); + const tool = tools.get("bcdev_package_download")!; + const result = await tool.handler({ + publisher: "SShadowS", + appName: "Demo Symbols", + version: "1.0.0.0", + appId: "2ca1721e-459e-4745-b96a-803517331326", + }) as Record; + + expect(result).toMatchObject({ + status: "downloaded", + publisher: "SShadowS", + appName: "Demo Symbols", + requestedVersion: "1.0.0.0", + resolvedVersion: "1.2.0.0", + }); + expect(result["packagePath"]).toBeString(); + expect(result["nextSteps"]).toEqual([ + expect.stringContaining("rerun the AL compile"), + ]); + expect(requestUrl).toContain("dev/packages?"); + expect(() => tool.outputSchema.parse(result)).not.toThrow(); + }); + test("profiling next steps reflect unreachable, unsupported, and empty-capture results", async () => { const unreachable = setup(hub, (async () => { throw new Error("offline"); }) as unknown as typeof fetch); const unreachableStatus = await unreachable.tools.get("bcdev_profile_status")!.handler({ From f2b2c970dcee047cdb014f72752f35d6100c963b Mon Sep 17 00:00:00 2001 From: ianray <193674599+ianrayianray@users.noreply.github.com> Date: Thu, 30 Jul 2026 13:57:05 -0700 Subject: [PATCH 2/4] fix: harden on-demand source and symbols --- README.md | 5 +- ...e2e-on-demand-source-symbols-2026-07-30.md | 29 +++- scripts/e2e.md | 10 +- skills/bc-al-source-symbols/SKILL.md | 9 +- src/core/package-download.ts | 127 ++++++++++++++---- src/core/source-content.ts | 18 ++- src/mcp/skills.generated.ts | 2 +- src/mcp/tools/source-tools.ts | 17 ++- tests/core/package-download.test.ts | 127 +++++++++++++++++- tests/core/source-content.test.ts | 46 ++++++- tests/mcp/tools.test.ts | 1 + 11 files changed, 334 insertions(+), 57 deletions(-) diff --git a/README.md b/README.md index 9da3489..751b077 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ MCP server for Business Central AL development: run tests (with code coverage) a [![Bun](https://img.shields.io/badge/bun-1.x-black)](https://bun.sh) [![TypeScript](https://img.shields.io/badge/typescript-strict-blue)](https://typescriptlang.org) [![BC dev API](https://img.shields.io/badge/BC%20dev%20API-%E2%89%A57.0-purple)]() -[![Tests](https://img.shields.io/badge/tests-641%20passing-green)]() +[![Tests](https://img.shields.io/badge/tests-645%20passing-green)]() ## Overview @@ -229,7 +229,8 @@ source. `bcdev_package_download` retrieves one explicitly identified installed `.app` through `dev/packages` and writes it to the selected project’s `.alpackages` directory. Supply the publisher, name, four-part minimum version, and app ID when known. Business Central may resolve a -higher installed version; the result reports both requested and resolved versions. +higher installed version; the result reports both requested and resolved versions. The selected +project must already contain `app.json`. The package is size- and time-bounded, validated through `SymbolReference.json`, checked against the requested identity, hashed, and only then installed under a filename derived from the returned diff --git a/scripts/e2e-on-demand-source-symbols-2026-07-30.md b/scripts/e2e-on-demand-source-symbols-2026-07-30.md index 455c968..d423026 100644 --- a/scripts/e2e-on-demand-source-symbols-2026-07-30.md +++ b/scripts/e2e-on-demand-source-symbols-2026-07-30.md @@ -23,27 +23,44 @@ Business Central operation in this run was an HTTP GET. - `resources/list` exposed `skill://bc-al-source-symbols/SKILL.md`. - `bcdev_source` returned `source:"rest"`, `isAlContent:true`, and deployed source matching the requested disposable codeunit identity. +- A known no-source object returned the empty result with neutral guidance that + covers compiled-only applications and disabled source exposure. - The exact package request returned `status:"downloaded"`. - The written file existed only under the temporary project’s `.alpackages` directory, began with NAVX, contained parseable `SymbolReference.json`, and its returned name/app ID matched the requested package in memory. - The returned byte count and SHA-256 matched the written file. - Repeating the exact request returned `status:"unchanged"`. +- Omitting the app ID for that uniquely named ordinary package also returned + `status:"unchanged"` for the same validated package. - Requesting a lower minimum version returned `status:"unchanged"` and resolved to the same higher installed version. +- After the harness deliberately replaced the temporary local cache file with + invalid bytes, the next request returned `status:"replaced"` and restored a + NAVX package whose returned digest matched the installed bytes. +- Two simultaneous identical requests against an empty local destination + returned one `downloaded` and one `unchanged` result for the same path. +- A large Microsoft Base Application dependency downloaded and validated within + the default size and time limits. +- The special Microsoft Application concept downloaded with no app ID. A repeat + carrying an irrelevant app ID still omitted that ID from selection and + returned `unchanged` at the same path. - A too-new version, wrong app ID, and unknown package name each returned the stable `NOT_FOUND` error code. -- The final directory contained one `.app` and zero `.tmp`/`.backup` files. +- A mistyped project path returned `CONFIGURATION_ERROR` and was not created. +- The final directory contained three `.app` files and zero `.tmp`/`.backup` + files. - The temporary project was deleted after the MCP client and server closed. ## Deterministic gates -- `bun test`: 638 passed, 0 failed. +- `bun test`: 645 passed, 0 failed. - `bun run typecheck`: passed. - `bun run build`: passed; four embedded skills generated. -- Package URL/query, Application app-ID omission, authentication, timeout, - streamed/declared size bounds, corrupt/missing symbols, identity/version - mismatch, unsafe names, safe replacement, and cleanup are covered by unit - tests. +- Package URL/query, Application app-ID omission, typed authentication, + pre-request project validation, timeout, streamed/declared size bounds, + corrupt/missing symbols, identity/version mismatch, compiler-compatible safe + naming, simultaneous installation, safe replacement, response-body + cancellation, symlink refusal, and cleanup are covered by unit tests. No Production call was made. diff --git a/scripts/e2e.md b/scripts/e2e.md index 4abe1da..1dcdedf 100644 --- a/scripts/e2e.md +++ b/scripts/e2e.md @@ -37,6 +37,7 @@ Never record access tokens, Authorization headers, authenticated URLs, or unreda - [x] SaaS `GET dev/sourcecontent` returns deployed AL source for an exposed object using the same encoded environment base and tenant query. - [x] `GET dev/packages` accepts `publisher`, `appName`, `versionText`, optional `appId`, and tenant; a lower `versionText` can resolve a higher installed package. - [x] A successful package response is a NAVX/ZIP containing top-level `SymbolReference.json` identity fields `AppId`, `Name`, `Publisher`, and `Version`. +- [x] The case-insensitive `Application` concept request omits `appId`; the SaaS endpoint resolves the installed aggregate package by publisher/name/minimum version. ## Scenarios @@ -84,8 +85,15 @@ Evidence belongs in `scripts/e2e-on-demand-source-symbols-2026-07-30.md`. - [x] `bcdev_package_download` with exact publisher/name/version/app ID returns `downloaded`, a real NAVX/ZIP file, matching `SymbolReference.json` identity, byte count, and SHA-256. - [x] Repeating the same request returns `unchanged` and retains the same digest. - [x] A lower minimum version resolves the installed higher version and reports distinct requested/resolved versions. +- [x] Omitting app ID for an ordinary uniquely named package resolves the same validated package. +- [x] Replacing deliberately corrupt local cache bytes returns `replaced`, restores a NAVX package, and reports its matching digest. +- [x] Two simultaneous identical calls against an empty destination serialize into one `downloaded` and one `unchanged` result at the same path. +- [x] A large Microsoft Base Application dependency downloads and validates within the default bounds. +- [x] The special Microsoft Application concept downloads without app ID; repeating with an irrelevant supplied ID omits it and returns `unchanged` at the same path. +- [x] A no-source object returns the neutral empty result and does not claim that every base-application object lacks source. +- [x] A mistyped project path returns `CONFIGURATION_ERROR` and is not created. - [x] Too-new version, wrong app ID, and unknown name selectors return typed `NOT_FOUND` errors and leave no temporary files. -- [x] Corrupt/missing symbols, identity/version mismatch, size/timeout bounds, unsafe names, safe replacement, cleanup, and on-prem/cloud URL shapes are deterministic unit tests. +- [x] Corrupt/missing symbols, identity/version mismatch, size/timeout bounds, compiler-compatible safe naming, authorization propagation, pre-request project validation, error-body cancellation, symlink refusal, safe replacement, cleanup, and on-prem/cloud URL shapes are deterministic unit tests. - [x] No Production call is made. ## Break-on-record-write triage (SaaS Sandbox) diff --git a/skills/bc-al-source-symbols/SKILL.md b/skills/bc-al-source-symbols/SKILL.md index 195659d..06886bd 100644 --- a/skills/bc-al-source-symbols/SKILL.md +++ b/skills/bc-al-source-symbols/SKILL.md @@ -13,8 +13,9 @@ source. `source: "hub"` means the REST endpoint had no answer and an active debu provided the source instead. `isAlContent: false` with empty content means Business Central did not return deployed AL -source. This is normal for compiled-only applications such as the base application; do -not invent source from local files or treat an empty result as a successful source fetch. +source. The application may be compiled-only or its source-exposure policy may disable +retrieval; do not invent source from local files or treat an empty result as a successful +source fetch. ## Download one symbol package @@ -39,6 +40,10 @@ for the same returned identity/version, and `"unchanged"` retained a byte-identi - The output directory is fixed to `.alpackages`; there is no arbitrary destination. Packages are bounded, validated through `SymbolReference.json`, hashed, and installed only after the complete response passes validation. +- The selected project must already be an AL project with an `app.json`. A missing or + mistyped project path is rejected before authentication or download and is never created. +- For the special Microsoft `Application` concept package, omit `appId`; the official + package request selects it by publisher, name, and minimum version. - A downloaded package supplies compiler symbols and may contain source according to the publisher's resource-exposure policy. The tool does not extract or claim source that Business Central did not expose through `bcdev_source`. diff --git a/src/core/package-download.ts b/src/core/package-download.ts index d52e40a..36f516c 100644 --- a/src/core/package-download.ts +++ b/src/core/package-download.ts @@ -1,5 +1,5 @@ import { createHash, randomUUID } from "node:crypto"; -import { lstat, mkdir, readFile, rename, rm, writeFile } from "node:fs/promises"; +import { lstat, mkdir, readFile, rename, rm, stat, writeFile } from "node:fs/promises"; import { join, resolve } from "node:path"; import type { AuthorizationProvider } from "./authorization"; import { BcDevError } from "./agent-errors"; @@ -11,6 +11,7 @@ const DEFAULT_TIMEOUT_MS = 120_000; const DEFAULT_MAX_BYTES = 256 * 1024 * 1024; const MAX_VERSION_PART = 2_147_483_647; const GUID = /^[0-9a-f]{8}(?:-[0-9a-f]{4}){3}-[0-9a-f]{12}$/i; +const installTails = new Map>(); export interface PackageSelector { publisher: string; @@ -83,7 +84,9 @@ function normalizeSelector(selector: PackageSelector): NormalizedPackageSelector if (typeof selector.appId !== "string" || !GUID.test(selector.appId.trim())) { throw invalid("appId must be a GUID"); } - appId = selector.appId.trim().toLowerCase(); + // WIRE: Application is a concept reference rather than an app-ID reference, so the + // official AL client omits appId even when its specification carries one. + if (appName.toLowerCase() !== "application") appId = selector.appId.trim().toLowerCase(); } return { publisher, appName, version: version.text, versionParts: version.parts, ...(appId ? { appId } : {}) }; } @@ -226,18 +229,14 @@ function packageIdentity(bytes: Buffer, selector: NormalizedPackageSelector): Pa }; } -function safeFilenameComponent(value: string): string { - const safe = value.replace(/[\u0000-\u001f<>:"/\\|?*]/g, "_").replace(/[ .]+$/g, ""); - if (safe === "") throw protocolError("Business Central returned package identity that cannot form a safe filename"); - return safe; -} - function packageFilename(identity: PackageIdentity): string { - return [ - safeFilenameComponent(identity.publisher), - safeFilenameComponent(identity.appName), - safeFilenameComponent(identity.version), - ].join("_") + ".app"; + // WIRE: NavAppManifest.DefaultPackageName builds Publisher_Name_Version, then + // FileNameUtilities.SanitizeFilename removes invalid filename/path characters + // (al-codeanalysis-decomp NavAppManifest.cs and FileNameUtilities.cs). + const defaultName = `${identity.publisher}_${identity.appName}_${identity.version}`; + const safe = defaultName.replace(/[\u0000-\u001f<>:"/\\|?*]/g, ""); + if (safe === "") throw protocolError("Business Central returned package identity that cannot form a safe filename"); + return `${safe}.app`; } async function readBoundedBody(response: Response, controller: AbortController, maxBytes: number): Promise { @@ -285,6 +284,43 @@ function filesystemError(message: string, path: string, cause: unknown): BcDevEr return new BcDevError("CONFIGURATION_ERROR", message, "configuration", false, { path }, { cause }); } +async function requireAlProject(project: string): Promise { + const root = resolve(project); + let rootStat; + try { + rootStat = await stat(root); + } catch (error) { + throw filesystemError("AL project directory does not exist or cannot be read", root, error); + } + if (!rootStat.isDirectory()) { + throw new BcDevError( + "CONFIGURATION_ERROR", + "AL package downloads require project to identify an existing directory", + "configuration", + false, + { path: root }, + ); + } + + const manifestPath = join(root, "app.json"); + let manifestStat; + try { + manifestStat = await stat(manifestPath); + } catch (error) { + throw filesystemError("AL package downloads require an existing project app.json", manifestPath, error); + } + if (!manifestStat.isFile()) { + throw new BcDevError( + "CONFIGURATION_ERROR", + "AL package downloads require app.json to be a file", + "configuration", + false, + { path: manifestPath }, + ); + } + return root; +} + async function existingFile(path: string): Promise { try { return await readFile(path); @@ -315,11 +351,28 @@ async function replaceValidatedFile(tempPath: string, destination: string, hadEx await rm(backup, { force: true }); } -async function installPackage(project: string, filename: string, bytes: Buffer, sha256: string): Promise<{ +async function withInstallLock(destination: string, operation: () => Promise): Promise { + const previous = installTails.get(destination) ?? Promise.resolve(); + let release!: () => void; + const current = new Promise((resolveCurrent) => { + release = resolveCurrent; + }); + const tail = previous.then(() => current); + installTails.set(destination, tail); + await previous; + try { + return await operation(); + } finally { + release(); + if (installTails.get(destination) === tail) installTails.delete(destination); + } +} + +async function installPackage(projectRoot: string, filename: string, bytes: Buffer, sha256: string): Promise<{ status: PackageDownloadResult["status"]; packagePath: string; }> { - const packageDir = join(resolve(project), ".alpackages"); + const packageDir = join(projectRoot, ".alpackages"); try { await mkdir(packageDir, { recursive: true }); const packageDirStat = await lstat(packageDir); @@ -331,21 +384,32 @@ async function installPackage(project: string, filename: string, bytes: Buffer, } const packagePath = join(packageDir, filename); - const existing = await existingFile(packagePath); - if (existing && createHash("sha256").update(existing).digest("hex") === sha256) { - return { status: "unchanged", packagePath }; - } + return withInstallLock(packagePath, async () => { + const existing = await existingFile(packagePath); + if (existing && createHash("sha256").update(existing).digest("hex") === sha256) { + return { status: "unchanged", packagePath }; + } + + const tempPath = join(packageDir, `.${filename}.${randomUUID()}.tmp`); + try { + await writeFile(tempPath, bytes, { flag: "wx" }); + await replaceValidatedFile(tempPath, packagePath, existing !== null); + } catch (error) { + throw filesystemError("Unable to install the downloaded AL package", packagePath, error); + } finally { + await rm(tempPath, { force: true }).catch(() => undefined); + } + return { status: existing === null ? "downloaded" : "replaced", packagePath }; + }); +} - const tempPath = join(packageDir, `.${filename}.${randomUUID()}.tmp`); +async function cancelUnusedBody(response: Response): Promise { try { - await writeFile(tempPath, bytes, { flag: "wx" }); - await replaceValidatedFile(tempPath, packagePath, existing !== null); - } catch (error) { - throw filesystemError("Unable to install the downloaded AL package", packagePath, error); - } finally { - await rm(tempPath, { force: true }).catch(() => undefined); + await response.body?.cancel(); + } catch { + // The response is already being discarded; cancellation failure does not replace + // the typed HTTP error that follows. } - return { status: existing === null ? "downloaded" : "replaced", packagePath }; } export async function downloadPackage( @@ -361,6 +425,8 @@ export async function downloadPackage( const maxBytes = options.maxBytes ?? DEFAULT_MAX_BYTES; if (!Number.isSafeInteger(timeoutMs) || timeoutMs <= 0) throw invalid("package download timeout must be positive"); if (!Number.isSafeInteger(maxBytes) || maxBytes <= 0) throw invalid("package download size limit must be positive"); + const projectRoot = await requireAlProject(project); + const authorizationHeader = await authorization.getAuthorizationHeader(); const controller = new AbortController(); let timedOut = false; @@ -373,16 +439,18 @@ export async function downloadPackage( try { const response = await fetchFn(packageUrl(c, selector), { method: "GET", - headers: { Authorization: await authorization.getAuthorizationHeader() }, + headers: { Authorization: authorizationHeader }, signal: controller.signal, }); if (response.status === 401 || response.status === 403) { + await cancelUnusedBody(response); const hint = c.authentication === "UserPassword" ? "verify BC_DEV_USER and BC_DEV_PASSWORD" : "verify the Azure CLI login, tenant, and Business Central account access"; throw new BcDevError("AUTHENTICATION_FAILED", `Business Central rejected package download authentication; ${hint}`, "auth"); } if (response.status === 404) { + await cancelUnusedBody(response); throw new BcDevError( "NOT_FOUND", `No installed Business Central package matched ${selector.publisher}/${selector.appName} at version ${selector.version} or newer`, @@ -397,6 +465,7 @@ export async function downloadPackage( ); } if (!response.ok) { + await cancelUnusedBody(response); throw new BcDevError( "SERVER_REJECTED", `Business Central package download returned HTTP ${response.status}`, @@ -432,7 +501,7 @@ export async function downloadPackage( const identity = packageIdentity(bytes, selector); const sha256 = createHash("sha256").update(bytes).digest("hex"); - const installed = await installPackage(project, packageFilename(identity), bytes, sha256); + const installed = await installPackage(projectRoot, packageFilename(identity), bytes, sha256); return { ...installed, publisher: identity.publisher, diff --git a/src/core/source-content.ts b/src/core/source-content.ts index 2d5cccf..c231449 100644 --- a/src/core/source-content.ts +++ b/src/core/source-content.ts @@ -8,6 +8,14 @@ export interface SourceContentResult { isAlContent: boolean; } +async function cancelUnusedBody(response: Response): Promise { + try { + await response.body?.cancel(); + } catch { + // Preserve the endpoint result when an already-discarded body cannot be cancelled. + } +} + export function sourceContentUrl(c: ConnectionConfig, objectType: number, objectId: number): string { // WIRE: GET dev/sourcecontent?type=&id=[&tenant=] (dep-decomp SourceContentApiClient.GetSource), // requires DevApiFeature.GetSourceCode => dev API 2.0 (dep-decomp DevApiFeatureExtensions.cs). @@ -25,28 +33,32 @@ export async function fetchSourceContent( fetchFn: typeof fetch = fetch, ): Promise { const url = sourceContentUrl(c, objectType, objectId); + const authorizationHeader = await authorization.getAuthorizationHeader(); let response: Response; try { - response = await fetchFn(url, { headers: { Authorization: await authorization.getAuthorizationHeader() } }); - } catch (err) { + response = await fetchFn(url, { headers: { Authorization: authorizationHeader } }); + } catch { throw new DevEndpointError( - `Dev endpoint unreachable at ${url} — is the BC server running and the developer service port open? (${String(err)})`, + "Business Central source-content endpoint is unreachable — is the server running and the developer service reachable?", "unreachable", ); } if (response.status === 401 || response.status === 403) { + await cancelUnusedBody(response); const hint = c.authentication === "UserPassword" ? "verify BC_DEV_USER and BC_DEV_PASSWORD" : "verify the Azure CLI login, tenant, and Business Central account access"; throw new DevEndpointError(`Dev endpoint rejected authentication; ${hint}`, "auth"); } if (response.status === 404) { + await cancelUnusedBody(response); // WIRE: BC28 returns 404 from dev/sourcecontent for objects without deployed source (live E2E // 2026-07-16: codeunit 1 -> 404, published app object -> 200 + JSON). Indistinguishable from a // pre-2.0 server missing the route entirely, so both surface as the empty no-source result. return { content: "", isAlContent: false }; } if (!response.ok) { + await cancelUnusedBody(response); throw new DevEndpointError(`dev/sourcecontent returned HTTP ${response.status}`, "http"); } const body = await response.text(); diff --git a/src/mcp/skills.generated.ts b/src/mcp/skills.generated.ts index a004d07..a7e23f6 100644 --- a/src/mcp/skills.generated.ts +++ b/src/mcp/skills.generated.ts @@ -21,7 +21,7 @@ export const skills: EmbeddedSkill[] = [ "description": "Retrieve deployed AL source or download one installed Business Central dependency symbol package without leaving the agent. Use when a debugger or coverage frame has no local file, or an AL project is missing one .app package in .alpackages.", "uri": "skill://bc-al-source-symbols/SKILL.md", "mimeType": "text/markdown", - "text": "---\nname: bc-al-source-symbols\ndescription: Retrieve deployed AL source or download one installed Business Central dependency symbol package without leaving the agent. Use when a debugger or coverage frame has no local file, or an AL project is missing one .app package in .alpackages.\n---\n\n# Retrieve deployed AL source and package symbols\n\n## Read one server object\n\nCall `bcdev_source { objectType, objectId }` with the numeric identity reported by a\ndebugger frame or coverage result. A nonempty `content` value is the server's deployed\nsource. `source: \"hub\"` means the REST endpoint had no answer and an active debugger\nprovided the source instead.\n\n`isAlContent: false` with empty content means Business Central did not return deployed AL\nsource. This is normal for compiled-only applications such as the base application; do\nnot invent source from local files or treat an empty result as a successful source fetch.\n\n## Download one symbol package\n\n1. Read the missing package identity from `app.json`, an AL compiler diagnostic, or other\n trusted project metadata.\n2. Call `bcdev_package_download` with `publisher`, `appName`, the four-part minimum\n `version`, and `appId` when known.\n3. Use `resolvedVersion` rather than assuming the requested minimum was returned.\n `packagePath` is the validated package under `/.alpackages`.\n4. Rerun the compile, source mapping, or coverage operation that needed the symbols.\n\n`status: \"downloaded\"` created the local package, `\"replaced\"` updated different bytes\nfor the same returned identity/version, and `\"unchanged\"` retained a byte-identical file.\n\n## Guardrails\n\n- The package tool downloads exactly one installed `.app`; it does not enumerate apps,\n resolve transitive dependencies, or reproduce the AL extension's full Download Symbols\n command. Repeat it only for package identities the project actually needs.\n- `version` is a minimum selector. Business Central can return a higher installed version,\n and the tool rejects a lower one or a mismatched name, publisher, or requested app ID.\n- The output directory is fixed to `.alpackages`; there is no arbitrary destination.\n Packages are bounded, validated through `SymbolReference.json`, hashed, and installed\n only after the complete response passes validation.\n- A downloaded package supplies compiler symbols and may contain source according to the\n publisher's resource-exposure policy. The tool does not extract or claim source that\n Business Central did not expose through `bcdev_source`.\n- Connection defaults come from `.vscode/launch.json`. SaaS uses the current Azure CLI\n identity; on-premises uses `BC_DEV_USER` and `BC_DEV_PASSWORD`.\n" + "text": "---\nname: bc-al-source-symbols\ndescription: Retrieve deployed AL source or download one installed Business Central dependency symbol package without leaving the agent. Use when a debugger or coverage frame has no local file, or an AL project is missing one .app package in .alpackages.\n---\n\n# Retrieve deployed AL source and package symbols\n\n## Read one server object\n\nCall `bcdev_source { objectType, objectId }` with the numeric identity reported by a\ndebugger frame or coverage result. A nonempty `content` value is the server's deployed\nsource. `source: \"hub\"` means the REST endpoint had no answer and an active debugger\nprovided the source instead.\n\n`isAlContent: false` with empty content means Business Central did not return deployed AL\nsource. The application may be compiled-only or its source-exposure policy may disable\nretrieval; do not invent source from local files or treat an empty result as a successful\nsource fetch.\n\n## Download one symbol package\n\n1. Read the missing package identity from `app.json`, an AL compiler diagnostic, or other\n trusted project metadata.\n2. Call `bcdev_package_download` with `publisher`, `appName`, the four-part minimum\n `version`, and `appId` when known.\n3. Use `resolvedVersion` rather than assuming the requested minimum was returned.\n `packagePath` is the validated package under `/.alpackages`.\n4. Rerun the compile, source mapping, or coverage operation that needed the symbols.\n\n`status: \"downloaded\"` created the local package, `\"replaced\"` updated different bytes\nfor the same returned identity/version, and `\"unchanged\"` retained a byte-identical file.\n\n## Guardrails\n\n- The package tool downloads exactly one installed `.app`; it does not enumerate apps,\n resolve transitive dependencies, or reproduce the AL extension's full Download Symbols\n command. Repeat it only for package identities the project actually needs.\n- `version` is a minimum selector. Business Central can return a higher installed version,\n and the tool rejects a lower one or a mismatched name, publisher, or requested app ID.\n- The output directory is fixed to `.alpackages`; there is no arbitrary destination.\n Packages are bounded, validated through `SymbolReference.json`, hashed, and installed\n only after the complete response passes validation.\n- The selected project must already be an AL project with an `app.json`. A missing or\n mistyped project path is rejected before authentication or download and is never created.\n- For the special Microsoft `Application` concept package, omit `appId`; the official\n package request selects it by publisher, name, and minimum version.\n- A downloaded package supplies compiler symbols and may contain source according to the\n publisher's resource-exposure policy. The tool does not extract or claim source that\n Business Central did not expose through `bcdev_source`.\n- Connection defaults come from `.vscode/launch.json`. SaaS uses the current Azure CLI\n identity; on-premises uses `BC_DEV_USER` and `BC_DEV_PASSWORD`.\n" }, { "name": "bc-al-testing", diff --git a/src/mcp/tools/source-tools.ts b/src/mcp/tools/source-tools.ts index 12317af..7738718 100644 --- a/src/mcp/tools/source-tools.ts +++ b/src/mcp/tools/source-tools.ts @@ -4,7 +4,8 @@ import { fetchSourceContent, type SourceContentResult } from "../../core/source- import type { ServerState } from "../state"; import { connectionShape, resolve, type ToolDefinition, type ToolDeps } from "./shared"; -const NO_SOURCE_MESSAGE = "No deployed source for this object — base-application objects ship without source"; +const NO_SOURCE_MESSAGE = + "No deployed source was returned for this object — it may be compiled-only or source exposure may be disabled"; function shapeResult(r: SourceContentResult, objectType: number, objectId: number, source: "rest" | "hub") { return { @@ -85,11 +86,17 @@ export function createSourceTools(state: ServerState, deps: ToolDeps): ToolDefin packagePath: z.string().describe("Absolute path of the validated .app under /.alpackages"), publisher: z.string().describe("Publisher read from the downloaded SymbolReference.json"), appName: z.string().describe("Package name read from the downloaded SymbolReference.json"), - appId: z.string().describe("Package app ID read from the downloaded SymbolReference.json"), - requestedVersion: z.string().describe("Normalized minimum version requested from Business Central"), - resolvedVersion: z.string().describe("Installed package version returned by Business Central"), + appId: z.string() + .regex(/^[0-9a-f]{8}(?:-[0-9a-f]{4}){3}-[0-9a-f]{12}$/i) + .describe("Package app ID read from the downloaded SymbolReference.json"), + requestedVersion: z.string() + .regex(/^\d+\.\d+\.\d+\.\d+$/) + .describe("Normalized minimum version requested from Business Central"), + resolvedVersion: z.string() + .regex(/^\d+\.\d+\.\d+\.\d+$/) + .describe("Installed package version returned by Business Central"), bytes: z.number().int().nonnegative().describe("Downloaded package size in bytes"), - sha256: z.string().describe("SHA-256 digest of the installed package"), + sha256: z.string().regex(/^[0-9a-f]{64}$/).describe("SHA-256 digest of the installed package"), }), handler: async (params) => { const { config, authorization, project } = resolve(params, deps); diff --git a/tests/core/package-download.test.ts b/tests/core/package-download.test.ts index bb6f6e8..35d3dec 100644 --- a/tests/core/package-download.test.ts +++ b/tests/core/package-download.test.ts @@ -1,9 +1,10 @@ import { describe, expect, test } from "bun:test"; import { createHash } from "node:crypto"; -import { mkdirSync, mkdtempSync, readFileSync, readdirSync, symlinkSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { basename, join } from "node:path"; import { BasicAuthorizationProvider } from "../../src/core/authorization"; +import { BcDevError } from "../../src/core/agent-errors"; import { downloadPackage, packageDownloadUrl, @@ -45,7 +46,9 @@ function appPackage( } function project(): string { - return mkdtempSync(join(tmpdir(), "bcmcp-package-")); + const dir = mkdtempSync(join(tmpdir(), "bcmcp-package-")); + writeFileSync(join(dir, "app.json"), "{}"); + return dir; } function responseFetch( @@ -117,6 +120,29 @@ describe("downloadPackage", () => { expect(capture.url).toContain("dev/packages?"); }); + test("treats Application as a concept selector and does not enforce a supplied app ID", async () => { + const application = { + publisher: "Microsoft", + appName: "Application", + version: "28.0.0.0", + appId: APP_ID, + }; + const capture: { url?: string } = {}; + const result = await downloadPackage( + config, + auth, + project(), + application, + responseFetch( + appPackage({ name: "Application", appId: OTHER_ID }), + 200, + capture, + ), + ); + expect(capture.url).not.toContain("appId="); + expect(result.appId).toBe(OTHER_ID); + }); + test("returns unchanged for identical bytes and replaces only with another validated package", async () => { const dir = project(); const firstBytes = appPackage({}, "first"); @@ -132,6 +158,20 @@ describe("downloadPackage", () => { expect(readFileSync(replaced.packagePath).equals(secondBytes)).toBe(true); }); + test("serializes simultaneous identical installs into downloaded then unchanged", async () => { + const dir = project(); + const bytes = appPackage(); + const [first, second] = await Promise.all([ + downloadPackage(config, auth, dir, selector, responseFetch(bytes)), + downloadPackage(config, auth, dir, selector, responseFetch(bytes)), + ]); + + expect([first.status, second.status].sort()).toEqual(["downloaded", "unchanged"]); + const names = readdirSync(join(dir, ".alpackages")); + expect(names.filter((name) => name.endsWith(".app"))).toHaveLength(1); + expect(names.filter((name) => name.endsWith(".tmp") || name.endsWith(".backup"))).toEqual([]); + }); + test("uses only validated metadata for the destination filename", async () => { const unsafe = { publisher: "Pub/lish:er", @@ -146,7 +186,7 @@ describe("downloadPackage", () => { unsafe, responseFetch(appPackage({ publisher: unsafe.publisher, name: unsafe.appName, version: unsafe.version })), ); - expect(basename(result.packagePath)).toBe("Pub_lish_er_App_Name__1.0.0.0.app"); + expect(basename(result.packagePath)).toBe("Publisher_AppName._1.0.0.0.app"); }); test("accepts a higher resolved version but rejects wrong identity and an older package", async () => { @@ -197,7 +237,7 @@ describe("downloadPackage", () => { await expect(downloadPackage(config, auth, dir, selector, responseFetch(bytes))).rejects.toMatchObject({ code: "PROTOCOL_ERROR", }); - expect(readdirSync(dir)).toEqual([]); + expect(readdirSync(dir)).toEqual(["app.json"]); } }); @@ -233,6 +273,46 @@ describe("downloadPackage", () => { }); }); + test("preserves authorization failures and does not start a package request", async () => { + const authFailure = new BcDevError( + "AUTHENTICATION_FAILED", + "Azure CLI login is unavailable", + "auth", + ); + let fetched = false; + const fetchFn = (async () => { + fetched = true; + return new Response(); + }) as unknown as typeof fetch; + const rejectingAuth = { + getAuthorizationHeader: async () => { + throw authFailure; + }, + }; + + const error = await downloadPackage(config, rejectingAuth, project(), selector, fetchFn).catch((caught) => caught); + expect(error).toBe(authFailure); + expect(fetched).toBe(false); + }); + + test("cancels an unused error response body", async () => { + let cancelled = false; + const fetchFn = (async () => ({ + status: 404, + ok: false, + headers: new Headers(), + body: { + cancel: async () => { + cancelled = true; + }, + }, + })) as unknown as typeof fetch; + await expect(downloadPackage(config, auth, project(), selector, fetchFn)).rejects.toMatchObject({ + code: "NOT_FOUND", + }); + expect(cancelled).toBe(true); + }); + test("enforces both declared and streamed size bounds", async () => { await expect(downloadPackage( config, @@ -279,12 +359,45 @@ describe("downloadPackage", () => { expect(signal?.aborted).toBe(true); }); - test("reports an unwritable project destination as a configuration error", async () => { + test("rejects a non-directory project before making a package request", async () => { const file = join(project(), "not-a-directory"); writeFileSync(file, "x"); - await expect(downloadPackage(config, auth, file, selector, responseFetch(appPackage()))).rejects.toMatchObject({ + let fetched = false; + const fetchFn = (async () => { + fetched = true; + return new Response(Uint8Array.from(appPackage())); + }) as unknown as typeof fetch; + await expect(downloadPackage(config, auth, file, selector, fetchFn)).rejects.toMatchObject({ code: "CONFIGURATION_ERROR", }); + expect(fetched).toBe(false); + }); + + test("rejects a missing or non-AL project without creating directories or fetching", async () => { + const parent = project(); + const missing = join(parent, "typo-project"); + const noManifest = mkdtempSync(join(tmpdir(), "bcmcp-not-al-")); + let authorizations = 0; + let fetches = 0; + const countingAuth = { + getAuthorizationHeader: async () => { + authorizations += 1; + return "Basic test"; + }, + }; + const fetchFn = (async () => { + fetches += 1; + return new Response(Uint8Array.from(appPackage())); + }) as unknown as typeof fetch; + + for (const candidate of [missing, noManifest]) { + await expect(downloadPackage(config, countingAuth, candidate, selector, fetchFn)).rejects.toMatchObject({ + code: "CONFIGURATION_ERROR", + }); + } + expect(existsSync(missing)).toBe(false); + expect(authorizations).toBe(0); + expect(fetches).toBe(0); }); if (process.platform !== "win32") { @@ -296,7 +409,7 @@ describe("downloadPackage", () => { await expect(downloadPackage(config, auth, dir, selector, responseFetch(appPackage()))).rejects.toMatchObject({ code: "CONFIGURATION_ERROR", }); - expect(readdirSync(outside)).toEqual([]); + expect(readdirSync(outside).filter((name) => name.endsWith(".app"))).toEqual([]); }); } }); diff --git a/tests/core/source-content.test.ts b/tests/core/source-content.test.ts index ba88084..4eb5424 100644 --- a/tests/core/source-content.test.ts +++ b/tests/core/source-content.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "bun:test"; import { fetchSourceContent, sourceContentUrl } from "../../src/core/source-content"; import { BasicAuthorizationProvider } from "../../src/core/authorization"; +import { BcDevError } from "../../src/core/agent-errors"; import type { ConnectionConfig } from "../../src/core/types"; const config: ConnectionConfig = { @@ -71,10 +72,53 @@ describe("fetchSourceContent", () => { expect(result).toEqual({ content: "", isAlContent: false }); }); + test("cancels an unused error response body", async () => { + let cancelled = false; + const fetchFn = (async () => ({ + status: 404, + ok: false, + body: { + cancel: async () => { + cancelled = true; + }, + }, + })) as unknown as typeof fetch; + expect(await fetchSourceContent(config, auth, 5, 1, fetchFn)).toEqual({ + content: "", + isAlContent: false, + }); + expect(cancelled).toBe(true); + }); + test("network failure throws unreachable", async () => { const failing = (async () => { throw new TypeError("fetch failed"); }) as unknown as typeof fetch; - await expect(fetchSourceContent(config, auth, 5, 1, failing)).rejects.toMatchObject({ kind: "unreachable" }); + const error = await fetchSourceContent(config, auth, 5, 1, failing).catch((caught) => caught); + expect(error).toMatchObject({ kind: "unreachable" }); + expect((error as Error).message).not.toContain("tenant=default"); + expect((error as Error).message).not.toContain("http://"); + }); + + test("preserves a typed authorization-provider failure and never starts fetch", async () => { + const authFailure = new BcDevError( + "AUTHENTICATION_FAILED", + "Azure CLI login is unavailable", + "auth", + ); + const rejectingAuth = { + getAuthorizationHeader: async () => { + throw authFailure; + }, + }; + let fetched = false; + const fetchFn = (async () => { + fetched = true; + return new Response(); + }) as unknown as typeof fetch; + + const error = await fetchSourceContent(config, rejectingAuth, 5, 1, fetchFn).catch((caught) => caught); + expect(error).toBe(authFailure); + expect(fetched).toBe(false); }); }); diff --git a/tests/mcp/tools.test.ts b/tests/mcp/tools.test.ts index 720adb3..6878f55 100644 --- a/tests/mcp/tools.test.ts +++ b/tests/mcp/tools.test.ts @@ -691,6 +691,7 @@ describe("tools", () => { const baseApp = (await tools.get("bcdev_source")!.handler({ objectType: 5, objectId: 1 })) as Record; expect(baseApp).toMatchObject({ content: "", isAlContent: false, source: "rest" }); expect(baseApp["message"]).toContain("No deployed source"); + expect(baseApp["message"]).not.toContain("base-application"); expect(hub.invoked("GetSourceContent")).toHaveLength(0); }); From edc6466a5b2784820b9fd3532d23889a6d6c3219 Mon Sep 17 00:00:00 2001 From: ianray <193674599+ianrayianray@users.noreply.github.com> Date: Fri, 31 Jul 2026 10:14:09 -0700 Subject: [PATCH 3/4] fix: close source and symbol review findings --- README.md | 7 +- ...e2e-on-demand-source-symbols-2026-07-30.md | 17 +- scripts/e2e.md | 4 +- skills/bc-al-source-symbols/SKILL.md | 10 +- src/core/package-download.ts | 152 +++++++++++++----- src/core/snapshot/zip.ts | 27 +++- src/mcp/skills.generated.ts | 2 +- src/mcp/tools/source-tools.ts | 25 ++- tests/core/package-download.test.ts | 129 +++++++++++++-- tests/core/snapshot/zip.test.ts | 11 ++ tests/fixtures/app-package.ts | 25 ++- tests/mcp/tools.test.ts | 4 + 12 files changed, 336 insertions(+), 77 deletions(-) diff --git a/README.md b/README.md index 751b077..614d863 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ MCP server for Business Central AL development: run tests (with code coverage) a [![Bun](https://img.shields.io/badge/bun-1.x-black)](https://bun.sh) [![TypeScript](https://img.shields.io/badge/typescript-strict-blue)](https://typescriptlang.org) [![BC dev API](https://img.shields.io/badge/BC%20dev%20API-%E2%89%A57.0-purple)]() -[![Tests](https://img.shields.io/badge/tests-645%20passing-green)]() +[![Tests](https://img.shields.io/badge/tests-649%20passing-green)]() ## Overview @@ -236,7 +236,10 @@ The package is size- and time-bounded, validated through `SymbolReference.json`, the requested identity, hashed, and only then installed under a filename derived from the returned metadata. A byte-identical file returns `unchanged`; different validated server bytes replace the same package path. This is deliberately a single-package operation, not dependency enumeration or -full Download Symbols synchronization. +full Download Symbols synchronization. The default request bounds are 120 seconds and 256 MiB; +`timeoutMs` and `maxBytes` can raise them to hard limits of 300 seconds and 512 MiB. Inflated symbol +metadata is independently capped at 512 MiB. A `NOT_FOUND` response can mean either no installed +package matched or the target server does not expose `dev/packages`. ## Profiling diff --git a/scripts/e2e-on-demand-source-symbols-2026-07-30.md b/scripts/e2e-on-demand-source-symbols-2026-07-30.md index d423026..64c77a5 100644 --- a/scripts/e2e-on-demand-source-symbols-2026-07-30.md +++ b/scripts/e2e-on-demand-source-symbols-2026-07-30.md @@ -54,13 +54,22 @@ Business Central operation in this run was an HTTP GET. ## Deterministic gates -- `bun test`: 645 passed, 0 failed. +- `bun test`: 649 passed, 0 failed after the deterministic review corrections. - `bun run typecheck`: passed. - `bun run build`: passed; four embedded skills generated. - Package URL/query, Application app-ID omission, typed authentication, pre-request project validation, timeout, streamed/declared size bounds, - corrupt/missing symbols, identity/version mismatch, compiler-compatible safe - naming, simultaneous installation, safe replacement, response-body - cancellation, symlink refusal, and cleanup are covered by unit tests. + corrupt/missing symbols, identity/version mismatch, identity-derived safe + naming, simultaneous installation, safe replacement and restoration, + response-body cancellation, symlink/junction refusal, bounded DEFLATE output, + and cleanup are covered by unit tests. + +The post-review corrections were deterministic and made no additional Sandbox +call: `SymbolReference.json` inflation is capped, the Windows backup-swap success +and restore paths use injected filesystem faults, the link guard runs as a +junction test on Windows, selector lengths and caller-adjustable request bounds +are validated, and 404 details distinguish a supplied app ID from an ID sent on +the wire. Wire comments now cite this tracked evidence instead of an unavailable +local decompilation directory. No Production call was made. diff --git a/scripts/e2e.md b/scripts/e2e.md index 1dcdedf..c4804a5 100644 --- a/scripts/e2e.md +++ b/scripts/e2e.md @@ -37,7 +37,7 @@ Never record access tokens, Authorization headers, authenticated URLs, or unreda - [x] SaaS `GET dev/sourcecontent` returns deployed AL source for an exposed object using the same encoded environment base and tenant query. - [x] `GET dev/packages` accepts `publisher`, `appName`, `versionText`, optional `appId`, and tenant; a lower `versionText` can resolve a higher installed package. - [x] A successful package response is a NAVX/ZIP containing top-level `SymbolReference.json` identity fields `AppId`, `Name`, `Publisher`, and `Version`. -- [x] The case-insensitive `Application` concept request omits `appId`; the SaaS endpoint resolves the installed aggregate package by publisher/name/minimum version. +- [x] The case-insensitive Microsoft `Application` concept request omits `appId`; the SaaS endpoint resolves the installed aggregate package by publisher/name/minimum version. Third-party apps with that name retain ordinary app-ID selection. ## Scenarios @@ -93,7 +93,7 @@ Evidence belongs in `scripts/e2e-on-demand-source-symbols-2026-07-30.md`. - [x] A no-source object returns the neutral empty result and does not claim that every base-application object lacks source. - [x] A mistyped project path returns `CONFIGURATION_ERROR` and is not created. - [x] Too-new version, wrong app ID, and unknown name selectors return typed `NOT_FOUND` errors and leave no temporary files. -- [x] Corrupt/missing symbols, identity/version mismatch, size/timeout bounds, compiler-compatible safe naming, authorization propagation, pre-request project validation, error-body cancellation, symlink refusal, safe replacement, cleanup, and on-prem/cloud URL shapes are deterministic unit tests. +- [x] Corrupt/missing symbols, identity/version mismatch, compressed and inflated size bounds, adjustable timeout bounds, identity-derived safe naming, authorization propagation, pre-request project validation, error-body cancellation, symlink/Windows-junction refusal, backup-swap success/restoration, cleanup, and on-prem/cloud URL shapes are deterministic unit tests. - [x] No Production call is made. ## Break-on-record-write triage (SaaS Sandbox) diff --git a/skills/bc-al-source-symbols/SKILL.md b/skills/bc-al-source-symbols/SKILL.md index 06886bd..edd6204 100644 --- a/skills/bc-al-source-symbols/SKILL.md +++ b/skills/bc-al-source-symbols/SKILL.md @@ -40,10 +40,16 @@ for the same returned identity/version, and `"unchanged"` retained a byte-identi - The output directory is fixed to `.alpackages`; there is no arbitrary destination. Packages are bounded, validated through `SymbolReference.json`, hashed, and installed only after the complete response passes validation. +- Package requests default to 120 seconds and 256 MiB. If a legitimate package exceeds a + default, set `timeoutMs` or `maxBytes` deliberately; the hard limits are 300 seconds and + 512 MiB, and inflated `SymbolReference.json` output has its own 512 MiB cap. - The selected project must already be an AL project with an `app.json`. A missing or mistyped project path is rejected before authentication or download and is never created. -- For the special Microsoft `Application` concept package, omit `appId`; the official - package request selects it by publisher, name, and minimum version. +- For the special Microsoft `Application` concept package, omit `appId`; the validated + package request selects it by publisher, name, and minimum version. Third-party apps named + `Application` remain ordinary app-ID selectors. +- `NOT_FOUND` cannot distinguish a missing package from an older server that does not expose + `dev/packages`; confirm server capability when a known installed package returns that code. - A downloaded package supplies compiler symbols and may contain source according to the publisher's resource-exposure policy. The tool does not extract or claim source that Business Central did not expose through `bcdev_source`. diff --git a/src/core/package-download.ts b/src/core/package-download.ts index 36f516c..26c14d0 100644 --- a/src/core/package-download.ts +++ b/src/core/package-download.ts @@ -7,8 +7,12 @@ import { extractEntry } from "./snapshot/zip"; import type { ConnectionConfig } from "./types"; import { baseClientUrl } from "./urls"; -const DEFAULT_TIMEOUT_MS = 120_000; -const DEFAULT_MAX_BYTES = 256 * 1024 * 1024; +export const DEFAULT_PACKAGE_DOWNLOAD_TIMEOUT_MS = 120_000; +export const MAX_PACKAGE_DOWNLOAD_TIMEOUT_MS = 300_000; +export const DEFAULT_PACKAGE_DOWNLOAD_BYTES = 256 * 1024 * 1024; +export const MAX_PACKAGE_DOWNLOAD_BYTES = 512 * 1024 * 1024; +export const MAX_PACKAGE_SELECTOR_LENGTH = 250; +const DEFAULT_SYMBOL_REFERENCE_BYTES = 512 * 1024 * 1024; const MAX_VERSION_PART = 2_147_483_647; const GUID = /^[0-9a-f]{8}(?:-[0-9a-f]{4}){3}-[0-9a-f]{12}$/i; const installTails = new Map>(); @@ -22,11 +26,20 @@ export interface PackageSelector { interface NormalizedPackageSelector extends PackageSelector { versionParts: readonly [number, number, number, number]; + requestedAppId?: string; +} + +export interface PackageInstallFileOps { + rename(source: string, destination: string): Promise; + remove(path: string): Promise; } export interface PackageDownloadOptions { timeoutMs?: number; maxBytes?: number; + // Test/integration seam for proving the DEFLATE allocation bound without a huge fixture. + maxSymbolBytes?: number; + installFileOps?: PackageInstallFileOps; } export interface PackageDownloadResult { @@ -57,6 +70,9 @@ function nonblank(value: unknown, field: "publisher" | "appName"): string { if (typeof value !== "string" || value.trim() === "") throw invalid(`${field} must be a nonblank string`); const trimmed = value.trim(); if (/[\r\n]/.test(trimmed)) throw invalid(`${field} must not contain newline characters`); + if (trimmed.length > MAX_PACKAGE_SELECTOR_LENGTH) { + throw invalid(`${field} must not exceed ${MAX_PACKAGE_SELECTOR_LENGTH} characters`); + } return trimmed; } @@ -75,20 +91,35 @@ function parseVersion(value: unknown, field = "version"): { return { text: parts.join("."), parts }; } +function isApplicationConcept(publisher: string, appName: string): boolean { + // Only the Microsoft/Application identity was validated as the special concept selector. + // An ordinary third-party app named Application retains its app ID and identity check. + return publisher.toLowerCase() === "microsoft" && appName.toLowerCase() === "application"; +} + function normalizeSelector(selector: PackageSelector): NormalizedPackageSelector { const publisher = nonblank(selector.publisher, "publisher"); const appName = nonblank(selector.appName, "appName"); const version = parseVersion(selector.version); let appId: string | undefined; + let requestedAppId: string | undefined; if (selector.appId !== undefined) { if (typeof selector.appId !== "string" || !GUID.test(selector.appId.trim())) { throw invalid("appId must be a GUID"); } - // WIRE: Application is a concept reference rather than an app-ID reference, so the - // official AL client omits appId even when its specification carries one. - if (appName.toLowerCase() !== "application") appId = selector.appId.trim().toLowerCase(); + requestedAppId = selector.appId.trim().toLowerCase(); + // WIRE: BC28 SaaS resolves the case-insensitive Microsoft/Application concept without appId and + // ignores a deliberately supplied ID. See scripts/e2e-on-demand-source-symbols-2026-07-30.md. + if (!isApplicationConcept(publisher, appName)) appId = requestedAppId; } - return { publisher, appName, version: version.text, versionParts: version.parts, ...(appId ? { appId } : {}) }; + return { + publisher, + appName, + version: version.text, + versionParts: version.parts, + ...(appId ? { appId } : {}), + ...(requestedAppId ? { requestedAppId } : {}), + }; } function packageUrl(c: ConnectionConfig, selector: NormalizedPackageSelector): string { @@ -97,16 +128,14 @@ function packageUrl(c: ConnectionConfig, selector: NormalizedPackageSelector): s appName: selector.appName, versionText: selector.version, }); - // WIRE: PackagesApiClient.DownloadPackage sends appId except for the case-insensitive - // "Application" concept reference (dep-decomp PackagesApiClient.cs and - // al-codeanalysis-decomp SymbolReferenceSpecification.IsApplicationConceptReference). - if (selector.appId && selector.appName.toLowerCase() !== "application") { + // WIRE: BC28 SaaS accepts appId for ordinary packages and omits it for the case-insensitive + // Microsoft/Application concept. See scripts/e2e-on-demand-source-symbols-2026-07-30.md. + if (selector.appId) { params.set("appId", selector.appId); } if (c.tenant) params.set("tenant", c.tenant); - // WIRE: the official AL client downloads one package with GET dev/packages and the - // publisher/appName/versionText selector above (dep-decomp PackagesApiClient.cs). - // Validated live on BC28 on-prem 2026-07-04 and SaaS Sandbox 2026-07-30. + // WIRE: BC28 SaaS downloads one package with GET dev/packages and the + // publisher/appName/versionText selector above. Validated 2026-07-30; see the dated evidence. return `${baseClientUrl(c)}dev/packages?${params.toString()}`; } @@ -160,10 +189,14 @@ function protocolError( ); } -function packageIdentity(bytes: Buffer, selector: NormalizedPackageSelector): PackageIdentity { +function packageIdentity( + bytes: Buffer, + selector: NormalizedPackageSelector, + maxSymbolBytes: number, +): PackageIdentity { let symbolBytes: Buffer | null; try { - symbolBytes = extractEntry(bytes, "SymbolReference.json"); + symbolBytes = extractEntry(bytes, "SymbolReference.json", maxSymbolBytes); } catch (error) { throw protocolError("Business Central returned an invalid application package archive", {}, error); } @@ -230,9 +263,8 @@ function packageIdentity(bytes: Buffer, selector: NormalizedPackageSelector): Pa } function packageFilename(identity: PackageIdentity): string { - // WIRE: NavAppManifest.DefaultPackageName builds Publisher_Name_Version, then - // FileNameUtilities.SanitizeFilename removes invalid filename/path characters - // (al-codeanalysis-decomp NavAppManifest.cs and FileNameUtilities.cs). + // SAFETY: derive the destination from the validated package identity instead of trusting a + // response filename, and remove the Windows invalid/control character set on every platform. const defaultName = `${identity.publisher}_${identity.appName}_${identity.version}`; const safe = defaultName.replace(/[\u0000-\u001f<>:"/\\|?*]/g, ""); if (safe === "") throw protocolError("Business Central returned package identity that cannot form a safe filename"); @@ -330,9 +362,19 @@ async function existingFile(path: string): Promise { } } -async function replaceValidatedFile(tempPath: string, destination: string, hadExisting: boolean): Promise { +const defaultInstallFileOps: PackageInstallFileOps = { + rename: async (source, destination) => rename(source, destination), + remove: async (path) => rm(path, { force: true }), +}; + +async function replaceValidatedFile( + tempPath: string, + destination: string, + hadExisting: boolean, + fileOps: PackageInstallFileOps, +): Promise { try { - await rename(tempPath, destination); + await fileOps.rename(tempPath, destination); return; } catch (error) { // POSIX rename replaces an existing file atomically. Windows can reject that with @@ -341,14 +383,16 @@ async function replaceValidatedFile(tempPath: string, destination: string, hadEx } const backup = `${destination}.${randomUUID()}.backup`; - await rename(destination, backup); + await fileOps.rename(destination, backup); + // A process crash after this rename can leave only the ignored .backup. A later validated + // download recreates the canonical destination; the backup remains evidence, never a partial app. try { - await rename(tempPath, destination); + await fileOps.rename(tempPath, destination); } catch (error) { - await rename(backup, destination).catch(() => undefined); + await fileOps.rename(backup, destination).catch(() => undefined); throw error; } - await rm(backup, { force: true }); + await fileOps.remove(backup); } async function withInstallLock(destination: string, operation: () => Promise): Promise { @@ -368,20 +412,37 @@ async function withInstallLock(destination: string, operation: () => Promise< } } -async function installPackage(projectRoot: string, filename: string, bytes: Buffer, sha256: string): Promise<{ +async function installPackage( + projectRoot: string, + filename: string, + bytes: Buffer, + sha256: string, + fileOps: PackageInstallFileOps, +): Promise<{ status: PackageDownloadResult["status"]; packagePath: string; }> { const packageDir = join(projectRoot, ".alpackages"); try { await mkdir(packageDir, { recursive: true }); - const packageDirStat = await lstat(packageDir); - if (!packageDirStat.isDirectory() || packageDirStat.isSymbolicLink()) { - throw new Error(".alpackages is not a direct directory"); - } } catch (error) { throw filesystemError("Unable to create the AL package directory", packageDir, error); } + let packageDirStat; + try { + packageDirStat = await lstat(packageDir); + } catch (error) { + throw filesystemError("Unable to inspect the AL package directory", packageDir, error); + } + if (!packageDirStat.isDirectory() || packageDirStat.isSymbolicLink()) { + throw new BcDevError( + "CONFIGURATION_ERROR", + "AL package directory must be a direct directory, not a file, symlink, or junction", + "configuration", + false, + { path: packageDir }, + ); + } const packagePath = join(packageDir, filename); return withInstallLock(packagePath, async () => { @@ -393,11 +454,11 @@ async function installPackage(projectRoot: string, filename: string, bytes: Buff const tempPath = join(packageDir, `.${filename}.${randomUUID()}.tmp`); try { await writeFile(tempPath, bytes, { flag: "wx" }); - await replaceValidatedFile(tempPath, packagePath, existing !== null); + await replaceValidatedFile(tempPath, packagePath, existing !== null, fileOps); } catch (error) { throw filesystemError("Unable to install the downloaded AL package", packagePath, error); } finally { - await rm(tempPath, { force: true }).catch(() => undefined); + await fileOps.remove(tempPath).catch(() => undefined); } return { status: existing === null ? "downloaded" : "replaced", packagePath }; }); @@ -421,10 +482,20 @@ export async function downloadPackage( options: PackageDownloadOptions = {}, ): Promise { const selector = normalizeSelector(requested); - const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; - const maxBytes = options.maxBytes ?? DEFAULT_MAX_BYTES; + const timeoutMs = options.timeoutMs ?? DEFAULT_PACKAGE_DOWNLOAD_TIMEOUT_MS; + const maxBytes = options.maxBytes ?? DEFAULT_PACKAGE_DOWNLOAD_BYTES; + const maxSymbolBytes = options.maxSymbolBytes ?? DEFAULT_SYMBOL_REFERENCE_BYTES; if (!Number.isSafeInteger(timeoutMs) || timeoutMs <= 0) throw invalid("package download timeout must be positive"); if (!Number.isSafeInteger(maxBytes) || maxBytes <= 0) throw invalid("package download size limit must be positive"); + if (timeoutMs > MAX_PACKAGE_DOWNLOAD_TIMEOUT_MS) { + throw invalid(`package download timeout cannot exceed ${MAX_PACKAGE_DOWNLOAD_TIMEOUT_MS} ms`); + } + if (maxBytes > MAX_PACKAGE_DOWNLOAD_BYTES) { + throw invalid(`package download size limit cannot exceed ${MAX_PACKAGE_DOWNLOAD_BYTES} bytes`); + } + if (!Number.isSafeInteger(maxSymbolBytes) || maxSymbolBytes <= 0 || maxSymbolBytes > DEFAULT_SYMBOL_REFERENCE_BYTES) { + throw invalid(`package symbol size limit must be between 1 and ${DEFAULT_SYMBOL_REFERENCE_BYTES} bytes`); + } const projectRoot = await requireAlProject(project); const authorizationHeader = await authorization.getAuthorizationHeader(); @@ -453,14 +524,15 @@ export async function downloadPackage( await cancelUnusedBody(response); throw new BcDevError( "NOT_FOUND", - `No installed Business Central package matched ${selector.publisher}/${selector.appName} at version ${selector.version} or newer`, + `No installed Business Central package matched ${selector.publisher}/${selector.appName} at version ${selector.version} or newer, or this server does not expose dev/packages`, "server", false, { publisher: selector.publisher, appName: selector.appName, version: selector.version, - appId: selector.appId ?? null, + appId: selector.requestedAppId ?? null, + appIdSent: selector.appId !== undefined, }, ); } @@ -499,9 +571,15 @@ export async function downloadPackage( clearTimeout(timeout); } - const identity = packageIdentity(bytes, selector); + const identity = packageIdentity(bytes, selector, maxSymbolBytes); const sha256 = createHash("sha256").update(bytes).digest("hex"); - const installed = await installPackage(projectRoot, packageFilename(identity), bytes, sha256); + const installed = await installPackage( + projectRoot, + packageFilename(identity), + bytes, + sha256, + options.installFileOps ?? defaultInstallFileOps, + ); return { ...installed, publisher: identity.publisher, diff --git a/src/core/snapshot/zip.ts b/src/core/snapshot/zip.ts index 47f6989..666a506 100644 --- a/src/core/snapshot/zip.ts +++ b/src/core/snapshot/zip.ts @@ -10,6 +10,7 @@ interface CentralEntry { name: string; method: number; compressedSize: number; + uncompressedSize: number; localHeaderOffset: number; } @@ -37,12 +38,13 @@ function readCentralDirectory(buf: Buffer): CentralEntry[] { if (buf.readUInt32LE(off) !== CD_SIG) throw new Error("corrupt zip central directory"); const method = buf.readUInt16LE(off + 10); const compressedSize = buf.readUInt32LE(off + 20); + const uncompressedSize = buf.readUInt32LE(off + 24); const nameLen = buf.readUInt16LE(off + 28); const extraLen = buf.readUInt16LE(off + 30); const commentLen = buf.readUInt16LE(off + 32); const localHeaderOffset = archiveBase + buf.readUInt32LE(off + 42); const name = buf.toString("utf8", off + 46, off + 46 + nameLen); - entries.push({ name, method, compressedSize, localHeaderOffset }); + entries.push({ name, method, compressedSize, uncompressedSize, localHeaderOffset }); off += 46 + nameLen + extraLen + commentLen; } return entries; @@ -52,10 +54,16 @@ export function listEntryNames(zip: Uint8Array): string[] { return readCentralDirectory(Buffer.from(zip)).map((e) => e.name); } -export function extractEntry(zip: Uint8Array, name: string): Buffer | null { +export function extractEntry(zip: Uint8Array, name: string, maxOutputLength?: number): Buffer | null { + if (maxOutputLength !== undefined && (!Number.isSafeInteger(maxOutputLength) || maxOutputLength <= 0)) { + throw new RangeError("zip entry output limit must be a positive safe integer"); + } const buf = Buffer.from(zip); const entry = readCentralDirectory(buf).find((e) => e.name === name); if (!entry) return null; + if (maxOutputLength !== undefined && entry.uncompressedSize > maxOutputLength) { + throw new Error(`zip entry exceeds the ${maxOutputLength}-byte output limit`); + } const lh = entry.localHeaderOffset; if (buf.readUInt32LE(lh) !== LOCAL_SIG) throw new Error("corrupt zip local header"); const nameLen = buf.readUInt16LE(lh + 26); @@ -65,7 +73,18 @@ export function extractEntry(zip: Uint8Array, name: string): Buffer | null { throw new Error("corrupt zip: entry data exceeds archive"); // subarray would silently truncate } const data = buf.subarray(dataStart, dataStart + entry.compressedSize); - if (entry.method === 0) return Buffer.from(data); // STORE - if (entry.method === 8) return inflateRawSync(data); // DEFLATE + if (entry.method === 0) { + if (maxOutputLength !== undefined && data.length > maxOutputLength) { + throw new Error(`zip entry exceeds the ${maxOutputLength}-byte output limit`); + } + return Buffer.from(data); // STORE + } + if (entry.method === 8) { + // The central-directory size is untrusted. maxOutputLength remains the authoritative + // allocation bound when a hostile archive understates its uncompressed size. + return maxOutputLength === undefined + ? inflateRawSync(data) + : inflateRawSync(data, { maxOutputLength }); + } throw new Error(`unsupported zip compression method ${entry.method}`); } diff --git a/src/mcp/skills.generated.ts b/src/mcp/skills.generated.ts index a7e23f6..f9f23f3 100644 --- a/src/mcp/skills.generated.ts +++ b/src/mcp/skills.generated.ts @@ -21,7 +21,7 @@ export const skills: EmbeddedSkill[] = [ "description": "Retrieve deployed AL source or download one installed Business Central dependency symbol package without leaving the agent. Use when a debugger or coverage frame has no local file, or an AL project is missing one .app package in .alpackages.", "uri": "skill://bc-al-source-symbols/SKILL.md", "mimeType": "text/markdown", - "text": "---\nname: bc-al-source-symbols\ndescription: Retrieve deployed AL source or download one installed Business Central dependency symbol package without leaving the agent. Use when a debugger or coverage frame has no local file, or an AL project is missing one .app package in .alpackages.\n---\n\n# Retrieve deployed AL source and package symbols\n\n## Read one server object\n\nCall `bcdev_source { objectType, objectId }` with the numeric identity reported by a\ndebugger frame or coverage result. A nonempty `content` value is the server's deployed\nsource. `source: \"hub\"` means the REST endpoint had no answer and an active debugger\nprovided the source instead.\n\n`isAlContent: false` with empty content means Business Central did not return deployed AL\nsource. The application may be compiled-only or its source-exposure policy may disable\nretrieval; do not invent source from local files or treat an empty result as a successful\nsource fetch.\n\n## Download one symbol package\n\n1. Read the missing package identity from `app.json`, an AL compiler diagnostic, or other\n trusted project metadata.\n2. Call `bcdev_package_download` with `publisher`, `appName`, the four-part minimum\n `version`, and `appId` when known.\n3. Use `resolvedVersion` rather than assuming the requested minimum was returned.\n `packagePath` is the validated package under `/.alpackages`.\n4. Rerun the compile, source mapping, or coverage operation that needed the symbols.\n\n`status: \"downloaded\"` created the local package, `\"replaced\"` updated different bytes\nfor the same returned identity/version, and `\"unchanged\"` retained a byte-identical file.\n\n## Guardrails\n\n- The package tool downloads exactly one installed `.app`; it does not enumerate apps,\n resolve transitive dependencies, or reproduce the AL extension's full Download Symbols\n command. Repeat it only for package identities the project actually needs.\n- `version` is a minimum selector. Business Central can return a higher installed version,\n and the tool rejects a lower one or a mismatched name, publisher, or requested app ID.\n- The output directory is fixed to `.alpackages`; there is no arbitrary destination.\n Packages are bounded, validated through `SymbolReference.json`, hashed, and installed\n only after the complete response passes validation.\n- The selected project must already be an AL project with an `app.json`. A missing or\n mistyped project path is rejected before authentication or download and is never created.\n- For the special Microsoft `Application` concept package, omit `appId`; the official\n package request selects it by publisher, name, and minimum version.\n- A downloaded package supplies compiler symbols and may contain source according to the\n publisher's resource-exposure policy. The tool does not extract or claim source that\n Business Central did not expose through `bcdev_source`.\n- Connection defaults come from `.vscode/launch.json`. SaaS uses the current Azure CLI\n identity; on-premises uses `BC_DEV_USER` and `BC_DEV_PASSWORD`.\n" + "text": "---\nname: bc-al-source-symbols\ndescription: Retrieve deployed AL source or download one installed Business Central dependency symbol package without leaving the agent. Use when a debugger or coverage frame has no local file, or an AL project is missing one .app package in .alpackages.\n---\n\n# Retrieve deployed AL source and package symbols\n\n## Read one server object\n\nCall `bcdev_source { objectType, objectId }` with the numeric identity reported by a\ndebugger frame or coverage result. A nonempty `content` value is the server's deployed\nsource. `source: \"hub\"` means the REST endpoint had no answer and an active debugger\nprovided the source instead.\n\n`isAlContent: false` with empty content means Business Central did not return deployed AL\nsource. The application may be compiled-only or its source-exposure policy may disable\nretrieval; do not invent source from local files or treat an empty result as a successful\nsource fetch.\n\n## Download one symbol package\n\n1. Read the missing package identity from `app.json`, an AL compiler diagnostic, or other\n trusted project metadata.\n2. Call `bcdev_package_download` with `publisher`, `appName`, the four-part minimum\n `version`, and `appId` when known.\n3. Use `resolvedVersion` rather than assuming the requested minimum was returned.\n `packagePath` is the validated package under `/.alpackages`.\n4. Rerun the compile, source mapping, or coverage operation that needed the symbols.\n\n`status: \"downloaded\"` created the local package, `\"replaced\"` updated different bytes\nfor the same returned identity/version, and `\"unchanged\"` retained a byte-identical file.\n\n## Guardrails\n\n- The package tool downloads exactly one installed `.app`; it does not enumerate apps,\n resolve transitive dependencies, or reproduce the AL extension's full Download Symbols\n command. Repeat it only for package identities the project actually needs.\n- `version` is a minimum selector. Business Central can return a higher installed version,\n and the tool rejects a lower one or a mismatched name, publisher, or requested app ID.\n- The output directory is fixed to `.alpackages`; there is no arbitrary destination.\n Packages are bounded, validated through `SymbolReference.json`, hashed, and installed\n only after the complete response passes validation.\n- Package requests default to 120 seconds and 256 MiB. If a legitimate package exceeds a\n default, set `timeoutMs` or `maxBytes` deliberately; the hard limits are 300 seconds and\n 512 MiB, and inflated `SymbolReference.json` output has its own 512 MiB cap.\n- The selected project must already be an AL project with an `app.json`. A missing or\n mistyped project path is rejected before authentication or download and is never created.\n- For the special Microsoft `Application` concept package, omit `appId`; the validated\n package request selects it by publisher, name, and minimum version. Third-party apps named\n `Application` remain ordinary app-ID selectors.\n- `NOT_FOUND` cannot distinguish a missing package from an older server that does not expose\n `dev/packages`; confirm server capability when a known installed package returns that code.\n- A downloaded package supplies compiler symbols and may contain source according to the\n publisher's resource-exposure policy. The tool does not extract or claim source that\n Business Central did not expose through `bcdev_source`.\n- Connection defaults come from `.vscode/launch.json`. SaaS uses the current Azure CLI\n identity; on-premises uses `BC_DEV_USER` and `BC_DEV_PASSWORD`.\n" }, { "name": "bc-al-testing", diff --git a/src/mcp/tools/source-tools.ts b/src/mcp/tools/source-tools.ts index 7738718..4b8309b 100644 --- a/src/mcp/tools/source-tools.ts +++ b/src/mcp/tools/source-tools.ts @@ -1,5 +1,12 @@ import { z } from "zod"; -import { downloadPackage } from "../../core/package-download"; +import { + DEFAULT_PACKAGE_DOWNLOAD_BYTES, + DEFAULT_PACKAGE_DOWNLOAD_TIMEOUT_MS, + downloadPackage, + MAX_PACKAGE_DOWNLOAD_BYTES, + MAX_PACKAGE_DOWNLOAD_TIMEOUT_MS, + MAX_PACKAGE_SELECTOR_LENGTH, +} from "../../core/package-download"; import { fetchSourceContent, type SourceContentResult } from "../../core/source-content"; import type { ServerState } from "../state"; import { connectionShape, resolve, type ToolDefinition, type ToolDeps } from "./shared"; @@ -57,7 +64,7 @@ export function createSourceTools(state: ServerState, deps: ToolDeps): ToolDefin name: "bcdev_package_download", title: "Download one AL dependency package", description: - "Download one explicitly identified installed Business Central .app dependency/symbol package into /.alpackages. The version is a minimum; this does not enumerate or synchronize dependencies.", + "Download one explicitly identified installed Business Central .app dependency/symbol package into /.alpackages. The version is a minimum; a 404 can also mean the server lacks dev/packages. This does not enumerate or synchronize dependencies.", annotations: { readOnlyHint: false, destructiveHint: false, @@ -66,10 +73,10 @@ export function createSourceTools(state: ServerState, deps: ToolDeps): ToolDefin }, schema: { ...connectionShape, - publisher: z.string().trim().min(1) + publisher: z.string().trim().min(1).max(MAX_PACKAGE_SELECTOR_LENGTH) .regex(/^[^\r\n]+$/, "publisher must not contain newline characters") .describe("Installed package publisher"), - appName: z.string().trim().min(1) + appName: z.string().trim().min(1).max(MAX_PACKAGE_SELECTOR_LENGTH) .regex(/^[^\r\n]+$/, "appName must not contain newline characters") .describe("Installed package name"), version: z.string().trim() @@ -79,6 +86,12 @@ export function createSourceTools(state: ServerState, deps: ToolDeps): ToolDefin .regex(/^[0-9a-f]{8}(?:-[0-9a-f]{4}){3}-[0-9a-f]{12}$/i, "appId must be a GUID") .optional() .describe("Package app ID when known; omitted for publisher/name selection and the special Application package"), + timeoutMs: z.number().int().min(1).max(MAX_PACKAGE_DOWNLOAD_TIMEOUT_MS).optional().describe( + `Whole-request timeout in milliseconds (default ${DEFAULT_PACKAGE_DOWNLOAD_TIMEOUT_MS}; maximum ${MAX_PACKAGE_DOWNLOAD_TIMEOUT_MS})`, + ), + maxBytes: z.number().int().min(1).max(MAX_PACKAGE_DOWNLOAD_BYTES).optional().describe( + `Maximum compressed package download size in bytes (default ${DEFAULT_PACKAGE_DOWNLOAD_BYTES}; maximum ${MAX_PACKAGE_DOWNLOAD_BYTES})`, + ), }, outputSchema: z.object({ status: z.enum(["downloaded", "replaced", "unchanged"]) @@ -111,6 +124,10 @@ export function createSourceTools(state: ServerState, deps: ToolDeps): ToolDefin appId: params["appId"] as string | undefined, }, deps.fetchFn, + { + timeoutMs: params["timeoutMs"] as number | undefined, + maxBytes: params["maxBytes"] as number | undefined, + }, ); }, }, diff --git a/tests/core/package-download.test.ts b/tests/core/package-download.test.ts index 35d3dec..7eddf16 100644 --- a/tests/core/package-download.test.ts +++ b/tests/core/package-download.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "bun:test"; import { createHash } from "node:crypto"; import { existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, symlinkSync, writeFileSync } from "node:fs"; +import { rename as renameFile, rm as removeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { basename, join } from "node:path"; import { BasicAuthorizationProvider } from "../../src/core/authorization"; @@ -8,10 +9,11 @@ import { BcDevError } from "../../src/core/agent-errors"; import { downloadPackage, packageDownloadUrl, + type PackageInstallFileOps, type PackageSelector, } from "../../src/core/package-download"; import type { ConnectionConfig } from "../../src/core/types"; -import { buildAppPackage, buildStoredZip } from "../fixtures/app-package"; +import { buildAppPackage, buildDeflatedZip, buildStoredZip } from "../fixtures/app-package"; const APP_ID = "63ca2fa4-4f03-4f2b-a480-172fef340d3f"; const OTHER_ID = "00000000-0000-0000-0000-000000000001"; @@ -68,7 +70,7 @@ function responseFetch( } describe("packageDownloadUrl", () => { - test("builds the official on-prem selector and tenant query", () => { + test("builds the on-prem selector and tenant query", () => { expect(packageDownloadUrl(config, selector)).toBe( "http://localhost:7049/BC/dev/packages?publisher=Microsoft&appName=System+Application&versionText=28.0.0.0&appId=63ca2fa4-4f03-4f2b-a480-172fef340d3f&tenant=default", ); @@ -89,12 +91,19 @@ describe("packageDownloadUrl", () => { })).toBe( "https://api.businesscentral.dynamics.com/v2.0/My%20Sandbox/dev/packages?publisher=Microsoft&appName=application&versionText=28.0.0.0&tenant=tenant-id", ); + expect(packageDownloadUrl(cloud, { + publisher: "Third Party", + appName: "Application", + version: "28.0.0.0", + appId: APP_ID, + })).toContain(`appId=${APP_ID}`); }); test("rejects malformed selectors before making a request", () => { expect(() => packageDownloadUrl(config, { ...selector, publisher: " \n" })).toThrow(/publisher/); expect(() => packageDownloadUrl(config, { ...selector, version: "28.0" })).toThrow(/four-part/); expect(() => packageDownloadUrl(config, { ...selector, appId: "not-a-guid" })).toThrow(/GUID/); + expect(() => packageDownloadUrl(config, { ...selector, appName: "x".repeat(251) })).toThrow(/250 characters/); }); }); @@ -158,6 +167,69 @@ describe("downloadPackage", () => { expect(readFileSync(replaced.packagePath).equals(secondBytes)).toBe(true); }); + test("exercises the Windows backup swap and removes its backup after success", async () => { + const dir = project(); + const firstBytes = appPackage({}, "first"); + const secondBytes = appPackage({}, "second"); + const first = await downloadPackage(config, auth, dir, selector, responseFetch(firstBytes)); + let destinationAttempts = 0; + const fileOps: PackageInstallFileOps = { + rename: async (source, destination) => { + if (source.endsWith(".tmp") && destination === first.packagePath && ++destinationAttempts === 1) { + throw Object.assign(new Error("destination locked"), { code: "EPERM" }); + } + await renameFile(source, destination); + }, + remove: async (path) => removeFile(path, { force: true }), + }; + + const replaced = await downloadPackage( + config, + auth, + dir, + selector, + responseFetch(secondBytes), + { installFileOps: fileOps }, + ); + expect(replaced.status).toBe("replaced"); + expect(destinationAttempts).toBe(2); + expect(readFileSync(first.packagePath).equals(secondBytes)).toBe(true); + expect(readdirSync(join(dir, ".alpackages")).filter((name) => name.endsWith(".backup"))).toEqual([]); + }); + + test("restores the original package when the Windows backup swap cannot install the new file", async () => { + const dir = project(); + const firstBytes = appPackage({}, "first"); + const secondBytes = appPackage({}, "second"); + const first = await downloadPackage(config, auth, dir, selector, responseFetch(firstBytes)); + let destinationAttempts = 0; + const fileOps: PackageInstallFileOps = { + rename: async (source, destination) => { + if (source.endsWith(".tmp") && destination === first.packagePath) { + destinationAttempts++; + throw Object.assign(new Error("destination remains locked"), { + code: destinationAttempts === 1 ? "EPERM" : "EACCES", + }); + } + await renameFile(source, destination); + }, + remove: async (path) => removeFile(path, { force: true }), + }; + + await expect(downloadPackage( + config, + auth, + dir, + selector, + responseFetch(secondBytes), + { installFileOps: fileOps }, + )).rejects.toMatchObject({ code: "CONFIGURATION_ERROR" }); + expect(destinationAttempts).toBe(2); + expect(readFileSync(first.packagePath).equals(firstBytes)).toBe(true); + expect(readdirSync(join(dir, ".alpackages")).filter((name) => + name.endsWith(".backup") || name.endsWith(".tmp"))).toEqual([]); + }); + test("serializes simultaneous identical installs into downloaded then unchanged", async () => { const dir = project(); const bytes = appPackage(); @@ -241,6 +313,25 @@ describe("downloadPackage", () => { } }); + test("bounds inflated SymbolReference output before JSON parsing", async () => { + const symbols = Buffer.alloc(4096, 0x41); + const bytes = Buffer.concat([ + Buffer.from("NAVX"), + Buffer.alloc(36), + buildDeflatedZip([{ name: "SymbolReference.json", content: symbols }]), + ]); + const dir = project(); + await expect(downloadPackage( + config, + auth, + dir, + selector, + responseFetch(bytes), + { maxSymbolBytes: 128 }, + )).rejects.toMatchObject({ code: "PROTOCOL_ERROR" }); + expect(readdirSync(dir)).toEqual(["app.json"]); + }); + test("does not replace an existing package when the new response fails validation", async () => { const dir = project(); const good = appPackage(); @@ -258,7 +349,17 @@ describe("downloadPackage", () => { }); await expect(downloadPackage(config, auth, project(), selector, responseFetch(null, 404))).rejects.toMatchObject({ code: "NOT_FOUND", - details: { appId: APP_ID }, + details: { appId: APP_ID, appIdSent: true }, + }); + await expect(downloadPackage( + config, + auth, + project(), + { ...selector, appName: "Application" }, + responseFetch(null, 404), + )).rejects.toMatchObject({ + code: "NOT_FOUND", + details: { appId: APP_ID, appIdSent: false }, }); await expect(downloadPackage(config, auth, project(), selector, responseFetch(null, 503))).rejects.toMatchObject({ code: "SERVER_REJECTED", @@ -400,16 +501,16 @@ describe("downloadPackage", () => { expect(fetches).toBe(0); }); - if (process.platform !== "win32") { - test("rejects a symlinked .alpackages directory instead of writing outside the project", async () => { - const dir = project(); - const outside = project(); - mkdirSync(outside, { recursive: true }); - symlinkSync(outside, join(dir, ".alpackages"), "dir"); - await expect(downloadPackage(config, auth, dir, selector, responseFetch(appPackage()))).rejects.toMatchObject({ - code: "CONFIGURATION_ERROR", - }); - expect(readdirSync(outside).filter((name) => name.endsWith(".app"))).toEqual([]); + test("rejects a symlink or Windows junction instead of writing outside the project", async () => { + const dir = project(); + const outside = project(); + mkdirSync(outside, { recursive: true }); + symlinkSync(outside, join(dir, ".alpackages"), process.platform === "win32" ? "junction" : "dir"); + const error = await downloadPackage(config, auth, dir, selector, responseFetch(appPackage())).catch((caught) => caught); + expect(error).toMatchObject({ + code: "CONFIGURATION_ERROR", + message: expect.stringContaining("direct directory"), }); - } + expect(readdirSync(outside).filter((name) => name.endsWith(".app"))).toEqual([]); + }); }); diff --git a/tests/core/snapshot/zip.test.ts b/tests/core/snapshot/zip.test.ts index 641d993..f7302d1 100644 --- a/tests/core/snapshot/zip.test.ts +++ b/tests/core/snapshot/zip.test.ts @@ -134,6 +134,17 @@ describe("zip reader", () => { expect(() => extractEntry(Uint8Array.from(buf), "trunc.bin")).toThrow(/corrupt zip/); }); + test("bounds DEFLATE output even when the archive understates its uncompressed size", () => { + const zip = Buffer.from(buildZip([{ + name: "large.txt", + content: Buffer.alloc(4096, 0x41), + method: 8, + }])); + const cdOff = zip.readUInt32LE(zip.length - 22 + 16); + zip.writeUInt32LE(1, cdOff + 24); + expect(() => extractEntry(zip, "large.txt", 128)).toThrow(); + }); + test("extracts each member of a genuine multi-entry archive by name", () => { const a = Buffer.from('{"which":"a"}'); const b = Buffer.from('{"which":"b","pad":"xxxxxxxxxxxxxxxx"}'); diff --git a/tests/fixtures/app-package.ts b/tests/fixtures/app-package.ts index b9464d0..ce7ee05 100644 --- a/tests/fixtures/app-package.ts +++ b/tests/fixtures/app-package.ts @@ -1,3 +1,5 @@ +import { deflateRawSync } from "node:zlib"; + interface ZipEntry { name: string; content: Buffer; @@ -13,34 +15,35 @@ export interface AppPackageFixture { preamble?: boolean; } -export function buildStoredZip(entries: ZipEntry[]): Buffer { +function buildZip(entries: ZipEntry[], method: 0 | 8): Buffer { const locals: Buffer[] = []; const central: Buffer[] = []; let offset = 0; for (const entry of entries) { const name = Buffer.from(entry.name); + const compressed = method === 8 ? deflateRawSync(entry.content) : entry.content; const local = Buffer.alloc(30); local.writeUInt32LE(0x04034b50, 0); local.writeUInt16LE(20, 4); local.writeUInt16LE(0, 6); - local.writeUInt16LE(0, 8); - local.writeUInt32LE(entry.content.length, 18); + local.writeUInt16LE(method, 8); + local.writeUInt32LE(compressed.length, 18); local.writeUInt32LE(entry.content.length, 22); local.writeUInt16LE(name.length, 26); - locals.push(local, name, entry.content); + locals.push(local, name, compressed); const cd = Buffer.alloc(46); cd.writeUInt32LE(0x02014b50, 0); cd.writeUInt16LE(20, 4); cd.writeUInt16LE(20, 6); cd.writeUInt16LE(0, 8); - cd.writeUInt16LE(0, 10); - cd.writeUInt32LE(entry.content.length, 20); + cd.writeUInt16LE(method, 10); + cd.writeUInt32LE(compressed.length, 20); cd.writeUInt32LE(entry.content.length, 24); cd.writeUInt16LE(name.length, 28); cd.writeUInt32LE(offset, 42); central.push(cd, name); - offset += local.length + name.length + entry.content.length; + offset += local.length + name.length + compressed.length; } const localBytes = Buffer.concat(locals); @@ -54,6 +57,14 @@ export function buildStoredZip(entries: ZipEntry[]): Buffer { return Buffer.concat([localBytes, centralBytes, eocd]); } +export function buildStoredZip(entries: ZipEntry[]): Buffer { + return buildZip(entries, 0); +} + +export function buildDeflatedZip(entries: ZipEntry[]): Buffer { + return buildZip(entries, 8); +} + export function buildAppPackage(fixture: AppPackageFixture): Buffer { const symbols = { RuntimeVersion: "17.0", diff --git a/tests/mcp/tools.test.ts b/tests/mcp/tools.test.ts index 6878f55..d770398 100644 --- a/tests/mcp/tools.test.ts +++ b/tests/mcp/tools.test.ts @@ -729,6 +729,8 @@ describe("tools", () => { appName: "Demo Symbols", version: "1.0.0.0", appId: "2ca1721e-459e-4745-b96a-803517331326", + timeoutMs: 1_000, + maxBytes: 1_000_000, }) as Record; expect(result).toMatchObject({ @@ -743,6 +745,8 @@ describe("tools", () => { expect.stringContaining("rerun the AL compile"), ]); expect(requestUrl).toContain("dev/packages?"); + expect(JSON.stringify(tool.schema["timeoutMs"])).toContain("300000"); + expect(JSON.stringify(tool.schema["maxBytes"])).toContain("536870912"); expect(() => tool.outputSchema.parse(result)).not.toThrow(); }); From c053e9fc720021cb3aad3d6c5c46199075aea617 Mon Sep 17 00:00:00 2001 From: ianray <193674599+ianrayianray@users.noreply.github.com> Date: Fri, 31 Jul 2026 10:18:21 -0700 Subject: [PATCH 4/4] fix: integrate source symbols with merged roadmap --- README.md | 6 +++--- scripts/e2e-on-demand-source-symbols-2026-07-30.md | 2 +- src/core/package-download.ts | 11 +---------- tests/mcp/server.test.ts | 4 ++-- tests/mcp/tools.test.ts | 2 +- 5 files changed, 8 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 614d863..4a43155 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ MCP server for Business Central AL development: run tests (with code coverage) a [![Bun](https://img.shields.io/badge/bun-1.x-black)](https://bun.sh) [![TypeScript](https://img.shields.io/badge/typescript-strict-blue)](https://typescriptlang.org) [![BC dev API](https://img.shields.io/badge/BC%20dev%20API-%E2%89%A57.0-purple)]() -[![Tests](https://img.shields.io/badge/tests-649%20passing-green)]() +[![Tests](https://img.shields.io/badge/tests-668%20passing-green)]() ## Overview @@ -121,7 +121,7 @@ Azure access tokens are acquired with `az account get-access-token`, cached only MCP client (agent) | v (stdio) -src/mcp/server.ts ── tools/ (23 bcdev_* tools) ── state.ts (debug/test/profile ownership) +src/mcp/server.ts ── tools/ (24 bcdev_* tools) ── state.ts (debug/test/profile ownership) | v src/core/ (pure library — typed returns, injected deps) @@ -373,7 +373,7 @@ seconds, preventing a stale or hung preflight from holding that slot indefinitel |------|---------| | `src/mcp/index.ts` | stdio entry: builds deps, calls buildServer, connects transport | | `src/mcp/server.ts` | buildServer: registerTool/registerResource wiring (testable over InMemoryTransport) | -| `src/mcp/tools/` | The 23 bcdev_* tool definitions (zod schemas + metadata + handlers) | +| `src/mcp/tools/` | The 24 bcdev_* tool definitions (zod schemas + metadata + handlers) | | `src/mcp/state.ts` | Debug session singleton, event queue, run lock | | `src/core/hubs/test-runner-hub.ts` | TestRunnerHub client (Initialize/RunTests, coverage) | | `src/core/test-orchestration.ts` | Pure repeat-run identity, stability classification, and adjacent pass/fail diff analysis | diff --git a/scripts/e2e-on-demand-source-symbols-2026-07-30.md b/scripts/e2e-on-demand-source-symbols-2026-07-30.md index 64c77a5..8450055 100644 --- a/scripts/e2e-on-demand-source-symbols-2026-07-30.md +++ b/scripts/e2e-on-demand-source-symbols-2026-07-30.md @@ -54,7 +54,7 @@ Business Central operation in this run was an HTTP GET. ## Deterministic gates -- `bun test`: 649 passed, 0 failed after the deterministic review corrections. +- `bun test`: 668 passed, 0 failed after rebasing onto the merged test-orchestration branch. - `bun run typecheck`: passed. - `bun run build`: passed; four embedded skills generated. - Package URL/query, Application app-ID omission, typed authentication, diff --git a/src/core/package-download.ts b/src/core/package-download.ts index 26c14d0..4469ffe 100644 --- a/src/core/package-download.ts +++ b/src/core/package-download.ts @@ -3,6 +3,7 @@ import { lstat, mkdir, readFile, rename, rm, stat, writeFile } from "node:fs/pro import { join, resolve } from "node:path"; import type { AuthorizationProvider } from "./authorization"; import { BcDevError } from "./agent-errors"; +import { upperInvariantUtf16 } from "./al-identifiers"; import { extractEntry } from "./snapshot/zip"; import type { ConnectionConfig } from "./types"; import { baseClientUrl } from "./urls"; @@ -143,16 +144,6 @@ export function packageDownloadUrl(c: ConnectionConfig, selector: PackageSelecto return packageUrl(c, normalizeSelector(selector)); } -function upperInvariantUtf16(value: string): string { - let result = ""; - for (let index = 0; index < value.length; index++) { - const unit = value[index]!; - const upper = unit.toUpperCase(); - result += upper.length === 1 ? upper : unit; - } - return result; -} - function sameText(left: string, right: string): boolean { return upperInvariantUtf16(left) === upperInvariantUtf16(right); } diff --git a/tests/mcp/server.test.ts b/tests/mcp/server.test.ts index ba889e8..4b38f01 100644 --- a/tests/mcp/server.test.ts +++ b/tests/mcp/server.test.ts @@ -48,7 +48,7 @@ describe("server wiring", () => { test("tools/list exposes names, titles, annotations, schemas", async () => { const client = await connect(); const { tools } = await client.listTools(); - expect(tools).toHaveLength(23); + expect(tools).toHaveLength(24); const status = tools.find((t) => t.name === "bcdev_status")!; expect(status.title).toBe("BC server status"); expect(status.annotations?.readOnlyHint).toBe(true); @@ -196,7 +196,7 @@ describe("server wiring", () => { test("start exposes the kind:instrumentation option", async () => { const client = await connect(); const { tools } = await client.listTools(); - expect(tools).toHaveLength(23); + expect(tools).toHaveLength(24); const start = tools.find((t) => t.name === "bcdev_profile_start")!; expect(JSON.stringify(start.inputSchema)).toContain("instrumentation"); }); diff --git a/tests/mcp/tools.test.ts b/tests/mcp/tools.test.ts index d770398..93c8d93 100644 --- a/tests/mcp/tools.test.ts +++ b/tests/mcp/tools.test.ts @@ -67,7 +67,7 @@ describe("tools", () => { hub = new FakeHub(); }); - test("registers all 23 tools", () => { + test("registers all 24 tools", () => { const { tools } = setup(hub); expect([...tools.keys()].sort()).toEqual([ "bcdev_debug_attach",