diff --git a/CLAUDE.md b/CLAUDE.md index c79e15e..9a3c60d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -44,7 +44,7 @@ src/core/ # pure library — typed returns, injected deps, never prin types.ts, urls.ts src/mcp/ # thin frontend server.ts # buildServer: registerTool/registerResource wiring (testable over InMemoryTransport) - tools/ # the 11 bcdev_* tool definitions (zod schemas + metadata + handlers) + tools/ # the 23 bcdev_* tool definitions (zod schemas + metadata + handlers) skills.generated.ts # GENERATED from skills/ by scripts/embed-skills.ts — do not edit state.ts # debug session singleton, event queue, run lock index.ts # stdio entry: builds deps, calls buildServer, connects transport diff --git a/README.md b/README.md index 1310c45..4bfbb0c 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-622%20passing-green)]() +[![Tests](https://img.shields.io/badge/tests-641%20passing-green)]() ## Overview @@ -24,6 +24,7 @@ MCP server for Business Central AL development: run tests (with code coverage) a |---------|-------------| | **Agent-grade responses** | Every success returns schema-validated `structuredContent` plus contextual `nextSteps`; failures use stable, redacted JSON error bodies | | **Structured test runs** | Run-level pass/fail/abort counts, per-method duration, parsed AL call stacks, and local source mappings while retaining raw server output | +| **Test orchestration** | Repeat one test selection, diff adjacent passed/failed sets, retain every run, and classify stable, flaky, inconsistent, or incomplete methods | | **Code coverage** | `procedure` mode, mapped back to local source files (`line` mode exists but is unproven against real BC) | | **Coverage gap analysis** | Cross a Git diff with exact procedure coverage to report which changed procedures the selected tests did and did not exercise | | **Multi-codeunit plans** | Sequential codeunit groups over one hub connection | @@ -74,9 +75,10 @@ Running from source instead: `git clone` → `bun install && bun run build` → 1. `bcdev_status` — verify reachability, auth, and that test running is supported. 2. `bcdev_test_discover` — list test codeunits and `[Test]` methods from local `.al` files. 3. `bcdev_test_run { codeunits: [{ id: 50100 }], coverage: "procedure" }` — structured results. Add `coverageAgainst: "origin/main"` for changed-procedure analysis. After publishing the current changed objects to the target, also pass `changesDeployed: true` to permit `covered`/`uncovered` classifications; without that explicit assertion, changed procedures remain `unknown`. -4. `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 }] }`. -5. 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. -6. 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. +4. `bcdev_test_orchestrate { codeunits: [{ id: 50100 }], runs: 3 }` — repeat the same selection, retain every run, diff adjacent pass/fail sets, and flag flaky methods. +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. 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. @@ -117,7 +119,7 @@ Azure access tokens are acquired with `az account get-access-token`, cached only MCP client (agent) | v (stdio) -src/mcp/server.ts ── tools/ (22 bcdev_* tools) ── state.ts (debug/test/profile ownership) +src/mcp/server.ts ── tools/ (23 bcdev_* tools) ── state.ts (debug/test/profile ownership) | v src/core/ (pure library — typed returns, injected deps) @@ -137,6 +139,7 @@ src/core/ (pure library — typed returns, injected deps) | `bcdev_status` | Preflight: reachability, auth, dev API version, feature gates | | `bcdev_test_discover` | List test codeunits and `[Test]` methods from local `.al` files | | `bcdev_test_run` | Run tests with a summary, parsed/source-mapped failures, optional coverage, and Git-aware changed-procedure gaps | +| `bcdev_test_orchestrate` | Repeat one selection 2–20 times, retain every run, diff adjacent pass/fail sets, and flag flaky/incomplete methods | | `bcdev_debug_attach` | Arm next-session/user-filtered attach or target an existing NST session; optionally set breakpoints | | `bcdev_debug_run_tests` | Run tests with breakpoints live | | `bcdev_debug_wait` | Long-poll for session-bound / break / run-finished lifecycle events | @@ -188,7 +191,8 @@ requires an exact `company` and one explicit context: - `business` lists the dynamic `bc_actions_*` catalog. Pass `configurationName` only when selecting a named Business Central MCP configuration. - `runtime` lists the BC28 AL runtime catalog. Native runtime calls share the same singleton - test-run lock as `bcdev_test_run` and `bcdev_debug_run_tests`. + test-run lock as `bcdev_test_run`, `bcdev_test_orchestrate`, and + `bcdev_debug_run_tests`. - `debugging` lists troubleshooting tools for the active manual debugger. Attach, trigger the workload, and wait for a `break` first; a merely bound or resumed session is rejected. @@ -261,10 +265,27 @@ Test runs add `summary`; failed method rows add `failure.message`, `failure.pars `failure.callStack` frames. The original `output` and each frame's `raw` text are retained so an unrecognized or localized server format never loses evidence. Local source mapping is lazy and best-effort: passing runs without coverage do not scan the AL tree, indexes are reused per MCP -server and project, and an unreadable/missing project returns complete server results plus a +server and project, and an unreadable/missing project preserves the server evidence plus a nonfatal `sourceMappingWarning` that identifies whether call-stack or coverage file mapping was unavailable. +Use `bcdev_test_orchestrate` when repeatability is the question. It claims the same singleton +test-run slot for the entire sequence, executes 2–20 ordinary TestRunnerHub runs sequentially, +retains each enriched attempt, and returns exact adjacent additions/removals for the passed and failed +sets. A method is `flaky` only with positive pass-and-fail evidence. Missing or duplicate +observations, an aborted run, or a run with no real methods forces `complete: false`; mixed +passed/skipped or failed/skipped observations are `inconsistent`, not mislabeled as flaky. +If an attempt cannot start, earlier evidence and a final aborted attempt remain in `runs[]`. +After an aborted attempt, later requested runs are left as missing rather than started while +the prior server-side run may still be active. Client cancellation is observed between attempts; +it does not cancel an active Business Central run. `tests[].method` keeps the first requested or +observed spelling, so its casing can differ from raw server rows. An all-skipped selection retains +the existing `passed` outcome convention but warns that it contains no passing execution evidence. +Overlapping codeunit/method selections are rejected, while disjoint method groups for one codeunit +remain valid. Non-rolled-back test side effects repeat on every attempt. +Orchestration intentionally does not aggregate coverage — use `bcdev_test_run` for coverage and +changed-procedure analysis. + Pass `coverageAgainst` to `bcdev_test_run` for roadmap item 5's coverage-gap analysis. The tool resolves the ref's merge base with `HEAD`, compares that commit with the current working tree (committed branch changes, staged and unstaged edits, plus nonignored untracked AL files), and @@ -325,9 +346,10 @@ 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 22 bcdev_* tool definitions (zod schemas + metadata + handlers) | +| `src/mcp/tools/` | The 23 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 | | `src/core/hubs/debugger-hub.ts` | DebuggerHub client (attach, breakpoints, stepping, inspection) | | `src/core/hubs/signalr-base.ts` | Hub seam: auth query params, key normalization, `HubProxy` | | `src/core/authorization.ts` | Shared Basic/Azure CLI authorization provider and token cache | @@ -347,7 +369,7 @@ Ordered by intent, not commitment: 3. **CPU profiling** (`bcdev_profile_*`) — **shipped.** Capture a sampling CPU profile (`.alcpuprofile`, V8 format) of a live session and get back a ranked AL-hotspot summary, not just a blob. Four tools (status/start/poll/finish), validated end-to-end against a live BC28 — a real profile with AL call frames captured through the tools (`scripts/e2e-profile-results-2026-07-04.md`). (Full snapshot recording for VS Code replay shares the same core and stays deferred.) 4. **Entra ID auth** — **shipped.** Azure CLI-backed cloud sandbox/production access alongside explicit on-prem UserPassword. 5. **Coverage gap analysis** — **shipped.** Pass `coverageAgainst` to `bcdev_test_run` to cross the merge-base-to-working-tree Git diff with exact procedure coverage and identify covered, uncovered, or unresolved changed procedures. -6. **Test orchestration** — repeat runs, diff pass/fail sets, flag flaky tests. +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. diff --git a/scripts/e2e-test-orchestration-2026-07-29.md b/scripts/e2e-test-orchestration-2026-07-29.md new file mode 100644 index 0000000..93f50a8 --- /dev/null +++ b/scripts/e2e-test-orchestration-2026-07-29.md @@ -0,0 +1,126 @@ +# Test orchestration — SaaS evidence + +Validated on 2026-07-29 against a Business Central 28 SaaS Sandbox through the +real `bcdev_test_orchestrate` handler. The harness accepted only +`environmentType: "Sandbox"` and made no Production or on-premises call. + +Tenant, environment, company, user, token, authorization, authenticated URL, +raw server payloads, failure text, call-stack text, durations for individual +methods, and source paths were not retained. The fixture object and method +names are also omitted from this evidence; stable role labels identify the two +selections. + +## Stable passing selection + +- The default repeat count requested and completed three runs. +- The retained run numbers were exactly 1, 2, and 3. +- Every raw run summary reported `passed`, and every run retained at least one + real result row. +- The aggregate returned `complete: true`, `outcome: "passed"`, + `tests: 1`, `stablePassed: 1`, and zero stable failures, flakes, + inconsistencies, or incomplete identities. +- Both adjacent diffs had no passed/failed additions or removals and no changed + observation. + +## Stable failing selection + +- An explicit repeat count requested and completed two runs. +- The retained run numbers were exactly 1 and 2. +- Every raw run summary reported `failed`. +- Every raw run retained nonempty failure output plus a parsed call stack whose + frames retained raw evidence and used only null or string local file fields. + No output, frame text, or path was written to this evidence. +- The aggregate returned `complete: true`, `outcome: "failed"`, + `tests: 1`, `stableFailed: 1`, and zero stable passes, flakes, + inconsistencies, or incomplete identities. +- The adjacent diff had no passed/failed additions or removals and no changed + observation. + +## Lifecycle and exclusions + +- The shared singleton test-run slot was released after both orchestration + calls. +- Runs were issued through the existing TestRunnerHub client; no new wire + format or endpoint was introduced. +- No deliberately flaky server test was added or claimed live. Pass/fail flaky + classification, skipped inconsistency, missing/duplicate observations, + safe stop after an aborted attempt, missing later observations, mixed + flaky/inconsistent guidance, and exact adjacent set diffs are covered by + deterministic core and fake-hub tests. The abort path was not induced during + this initial run; see the expanded rerun below. +- No coverage, debugger, native runtime, profiling, record-write, Production, + or on-premises call was made during this acceptance run. + +## Expanded post-review rerun + +The corrected branch was exercised again through a real MCP `Client` and +`buildServer`, with the production SignalR and Azure CLI authorization +implementations. The harness refused Production-like environment names and +printed only role labels, counts, classifications, and stable error codes. + +### Surface and retained evidence + +- `tools/list` exposed the orchestration annotations, the 2–20 run bounds, all + stability states, ambiguous observations, and additive `nextSteps`. +- Four invalid inputs (below-minimum, above-maximum, fractional runs, and an + empty selection) returned `INVALID_ARGUMENT`, no `structuredContent`, and + input-schema recovery guidance without claiming the test-run slot. +- The default stable-pass case completed three attempts. Deterministic failure, + mixed pass/fail, whole-codeunit, case-insensitive method, and two-codeunit + selections each completed two attempts with the expected stable + classifications and empty adjacent diffs. +- The maximum accepted count completed all 20 attempts, retained 20 passing + observations, produced exactly 19 empty adjacent diffs, and returned no + warning. +- Every returned text payload matched its `structuredContent`. Aggregate + duration equalled the retained real-row durations. Orchestration returned no + coverage payload or coverage-gap analysis. +- Deterministic failures retained nonempty raw output, parsed frames, raw frame + evidence, and at least one local source mapping. Stable failure guidance + named the `stableFailed` classification; completed passing runs returned no + unnecessary next step. + +### Live fail-closed paths + +- A nonexistent selected method completed two server attempts. Each retained + the server's synthetic row, while the explicitly requested method received + two `missing` observations and the aggregate returned `complete: false`, + `outcome: "incomplete"`, warnings, and cautious guidance. +- A nonexistent codeunit returned three non-aborted, empty attempt envelopes. + The tool invented no method identity and returned an incomplete aggregate + with warnings. +- Repeating the same codeunit group twice in one plan produced a useful live + abort: the Sandbox returned one passing result, then aborted that attempt + before a duplicate row arrived. The orchestration retained that attempted + run, did not start the second requested attempt, represented the identity as + `passed, missing`, returned `complete: false`, and warned that server-side + cancellation could not be confirmed. A clean orchestration immediately + afterward succeeded, confirming lock release. + +This live duplicate-group result remains historical server evidence. The +post-review correction now rejects overlapping codeunit/method selections at +the MCP boundary while continuing to accept disjoint method groups for one +codeunit; deterministic tests cover both shapes. No additional server call was +needed for that input-only correction. + +The post-review correction also retains a synthetic aborted attempt when a +later setup or authorization call rejects, observes client cancellation between +attempts without claiming to cancel the active server run, aggregates duplicate +row warnings per run, and warns on an all-skipped selection. These lifecycle +and analysis paths are deterministic tests; they were not induced against the +Sandbox. + +### Live singleton contention + +- While one five-attempt orchestration held the shared slot, one direct + `bcdev_test_run` and a second `bcdev_test_orchestrate` both returned + `TEST_RUN_ACTIVE` plus wait guidance. +- The owning orchestration completed all five attempts, released the slot, and + a subsequent two-attempt orchestration succeeded. + +No deliberately flaky or skipped Sandbox fixture was introduced. Their +classifications, mixed flaky/inconsistent guidance, duplicate-row ambiguity, +and the debug-bound/native-runtime contention variants remain deterministic +tests. No Production, on-premises, debugger, native MCP, profiling, +record-write, source-download, package-download, or coverage call was made in +this rerun. diff --git a/scripts/e2e.md b/scripts/e2e.md index 2e75800..57c6a98 100644 --- a/scripts/e2e.md +++ b/scripts/e2e.md @@ -43,7 +43,32 @@ 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 20 tools; resources/list shows the three skill:// resources. +- [ ] tools/list shows title/annotations/outputSchema for all 23 tools; resources/list shows the three skill:// resources. + +## Test orchestration (SaaS Sandbox, BC28) + +Run only against the existing disposable SaaS Sandbox test fixture. Never retain tenant, +environment, company, user, token, authorization, authenticated URL, or raw server payloads. +Evidence belongs in `scripts/e2e-test-orchestration-2026-07-29.md`. + +- [x] `bcdev_test_orchestrate` repeats a known passing method three times sequentially, + returns three enriched raw runs, empty adjacent diffs, `stablePassed:1`, and + `complete:true`. +- [x] Repeating a known deterministic failure returns `stableFailed:1`, `flaky:0`, + `outcome:"failed"`, and retains parsed failure evidence in every raw run. + +- [x] A pass/fail sequence is classified `flaky`, while pass/skip is `inconsistent`; exact + passed/failed additions and removals are reported between adjacent runs. +- [x] Missing, duplicate, synthetic-only, and aborted observations fail the aggregate closed; + an aborted attempt stops the sequence, retains its evidence, and marks later requested + observations missing rather than risking an overlapping server-side run. +- [x] A later setup/auth rejection retains earlier runs plus an aborted attempt, client + cancellation stops only between attempts, disjoint same-codeunit groups remain valid, and + overlapping method coverage is rejected. +- [x] One orchestration holds the shared singleton slot against direct, debug-bound, + native-runtime, and second orchestration calls. +- [x] No Production or on-premises call is made during live acceptance. + ## Break-on-record-write triage (SaaS Sandbox) diff --git a/skills/bc-al-testing/SKILL.md b/skills/bc-al-testing/SKILL.md index 687a16a..b783bc9 100644 --- a/skills/bc-al-testing/SKILL.md +++ b/skills/bc-al-testing/SKILL.md @@ -16,7 +16,11 @@ description: Run AL tests against a Business Central dev endpoint with the bc-de 3. `bcdev_test_run { codeunits: [{ id: }] }` — runs on the server, returns a run `summary` plus per-method `passed | failed | skipped`, duration, and failure output. Restrict with `methods: ["Name"]`; pick the company with `company`. -4. To check changed-code coverage, publish the current changed objects, then add +4. To investigate repeatability, call + `bcdev_test_orchestrate { codeunits: [...], runs: 3 }`. It executes the same selection + sequentially, retains every enriched run, diffs adjacent passed/failed sets, and classifies + each method. +5. To check changed-code coverage, publish the current changed objects, then add `coverageAgainst: "origin/main"` and `changesDeployed: true`. The latter is your explicit confirmation that this working tree is deployed. This implies `coverage: "procedure"` and adds `coverageGaps` to the same result. @@ -24,7 +28,29 @@ description: Run AL tests against a Business Central dev endpoint with the bc-de ## Facts that save you time - **One run at a time.** A second `bcdev_test_run` while one is active fails with - "already running" — wait and retry. + "already running" — wait and retry. One orchestration owns that same slot for its entire + repeat sequence, so direct, debug-bound, native-runtime, and other orchestration runs cannot + overlap it. +- **Orchestration is bounded and evidence-preserving.** `runs` defaults to 3 and accepts 2–20. + `runs[]` retains every enriched per-method attempt and raw failure output. `diffs[]` reports + exact adjacent passed/failed set additions and removals plus every changed observation. +- **Flaky means observed pass and fail.** A method is `flaky` only when at least one run passed + and another failed. Passed/skipped or failed/skipped mixtures are `inconsistent`. A missing + result, duplicate result, aborted run, or synthetic-only run forces the aggregate + `complete: false`; inspect `warnings` and `tests[].observations` before trusting stability. + After an aborted attempt, orchestration retains it but does not start later attempts while + the prior server-side run may still be active; their observations are `missing`. A thrown setup + or authorization failure becomes a final aborted attempt instead of discarding earlier evidence. + Client cancellation is observed between attempts and never cancels an active Business Central + run. Overlapping codeunit/method selections are rejected, but disjoint method groups for the same + codeunit remain valid. Non-rolled-back test side effects repeat on every attempted run. +- **Read orchestration identities and skips conservatively.** `tests[].method` keeps the first + requested or observed spelling, so casing can differ from raw server rows. If every selected test + is skipped, the existing outcome convention remains `passed`, but `warnings` states that there was + no passing execution evidence. +- **Orchestration does not collect coverage.** Use `bcdev_test_run` separately for procedure + coverage or `coverageAgainst`; repeated coverage is deliberately not merged into a stability + result. - **Coverage:** pass `coverage: "procedure"` (validated against real BC). `"line"` is unproven — do not trust it without independent verification. Covered procedures map back to local source files when the object IDs exist in the project. @@ -59,7 +85,7 @@ description: Run AL tests against a Business Central dev endpoint with the bc-de localized — use the unchanged `output` text. Every frame also retains `raw`. - **Local mapping is best-effort.** Passing runs without coverage do not scan the AL tree. When mapping is needed, the project index is reused within that MCP server; if the directory is - missing or unreadable, the complete server result still returns with `sourceMappingWarning`. + missing or unreadable, the server evidence still returns with `sourceMappingWarning`. The warning identifies whether call-stack files remain `null`, coverage files remain unset, or both mappings were unavailable. - **Follow the response, not a memorized script.** Every success includes `nextSteps`; an empty diff --git a/skills/bc-native-mcp/SKILL.md b/skills/bc-native-mcp/SKILL.md index c095708..76d7540 100644 --- a/skills/bc-native-mcp/SKILL.md +++ b/skills/bc-native-mcp/SKILL.md @@ -51,8 +51,8 @@ passthrough. derives them from the configured cloud target and Azure CLI identity. - Native passthrough supports cloud Sandbox and Production. It does not expose a profiling context; the separate `bcdev_profile_*` tools remain the supported BC28 profiling path. -- A native `runtime` call shares the one-test-run lock with direct and debug-bound test runs. - Wait for the active run to finish instead of retrying concurrently. +- A native `runtime` call shares the one-test-run lock with direct, orchestrated, and + debug-bound test runs. Wait for the active run to finish instead of retrying concurrently. - A timeout after a native runtime call begins does not cancel Business Central's server-side test run. When the error reports `upstreamRunCancelled: false`, do not retry or start another test run until you have confirmed that the upstream run finished. diff --git a/src/core/al-identifiers.ts b/src/core/al-identifiers.ts new file mode 100644 index 0000000..c2b02b9 --- /dev/null +++ b/src/core/al-identifiers.ts @@ -0,0 +1,12 @@ +// AL identifiers are case-insensitive. Keep the transformation length-preserving so legal +// quoted identifiers such as "Größe" are not silently expanded. This matches the compiler-facing +// normalization used by the procedure identity engine. +export 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; +} diff --git a/src/core/al-procedures.ts b/src/core/al-procedures.ts index 165a18b..f9f86e0 100644 --- a/src/core/al-procedures.ts +++ b/src/core/al-procedures.ts @@ -1,6 +1,7 @@ import { readFile, readdir, stat } from "node:fs/promises"; import { join, relative, resolve } from "node:path"; import { AL_OBJECT_TYPE } from "./al-objects"; +import { upperInvariantUtf16 } from "./al-identifiers"; import { BcDevError } from "./agent-errors"; import { extractEntry } from "./snapshot/zip"; @@ -985,16 +986,6 @@ function parseProcedures( return { procedures, unsupportedExecutables, unattributedCode, complete: warnings.length === 0, warnings }; } -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 fnvUtf16(value: string): number { let hash = -2128831035; const bytes = Buffer.from(value, "utf16le"); diff --git a/src/core/test-orchestration.ts b/src/core/test-orchestration.ts new file mode 100644 index 0000000..9bc1208 --- /dev/null +++ b/src/core/test-orchestration.ts @@ -0,0 +1,326 @@ +import type { + CodeunitTestGroup, + RunTestsResult, + TestMethodResult, + TestStatus, +} from "./types"; +import { upperInvariantUtf16 } from "./al-identifiers"; + +export type TestObservationStatus = TestStatus | "missing" | "ambiguous"; +export type TestStabilityClassification = + | "stablePassed" + | "stableFailed" + | "stableSkipped" + | "flaky" + | "inconsistent" + | "incomplete"; +export type TestOrchestrationOutcome = "passed" | "failed" | "unstable" | "incomplete"; + +export interface OrchestratedTestIdentity { + codeunitId: number; + method: string; +} + +export interface TestRunObservation { + run: number; + status: TestObservationStatus; + durationMs: number | null; +} + +export interface OrchestratedTestCase extends OrchestratedTestIdentity { + classification: TestStabilityClassification; + complete: boolean; + passCount: number; + failCount: number; + skipCount: number; + missingCount: number; + ambiguousCount: number; + observations: TestRunObservation[]; +} + +export interface TestStatusSetDiff { + added: OrchestratedTestIdentity[]; + removed: OrchestratedTestIdentity[]; +} + +export interface TestObservationChange extends OrchestratedTestIdentity { + from: TestObservationStatus; + to: TestObservationStatus; +} + +export interface TestRunDiff { + fromRun: number; + toRun: number; + passed: TestStatusSetDiff; + failed: TestStatusSetDiff; + changed: TestObservationChange[]; +} + +export interface TestOrchestrationSummary { + tests: number; + stablePassed: number; + stableFailed: number; + stableSkipped: number; + flaky: number; + inconsistent: number; + incomplete: number; + totalDurationMs: number; +} + +export interface TestOrchestrationAnalysis { + runsRequested: number; + runsAttempted: number; + complete: boolean; + outcome: TestOrchestrationOutcome; + summary: TestOrchestrationSummary; + tests: OrchestratedTestCase[]; + diffs: TestRunDiff[]; + warnings: string[]; +} + +interface IdentityRecord extends OrchestratedTestIdentity { + key: string; +} + +function identityKey(codeunitId: number, method: string): string { + return `${codeunitId}\u0000${upperInvariantUtf16(method.trim())}`; +} + +function compareIdentity(left: OrchestratedTestIdentity, right: OrchestratedTestIdentity): number { + if (left.codeunitId !== right.codeunitId) return left.codeunitId - right.codeunitId; + if (left.method === right.method) return 0; + return left.method < right.method ? -1 : 1; +} + +function realRows(run: RunTestsResult): TestMethodResult[] { + return run.results.filter((row) => row.method.trim() !== ""); +} + +function classify(observations: TestRunObservation[]): TestStabilityClassification { + const statuses = new Set(observations.map((entry) => entry.status)); + if (statuses.has("passed") && statuses.has("failed")) return "flaky"; + if (statuses.has("missing") || statuses.has("ambiguous")) return "incomplete"; + if (statuses.size === 1) { + if (statuses.has("passed")) return "stablePassed"; + if (statuses.has("failed")) return "stableFailed"; + if (statuses.has("skipped")) return "stableSkipped"; + } + return "inconsistent"; +} + +function identity(record: IdentityRecord): OrchestratedTestIdentity { + return { codeunitId: record.codeunitId, method: record.method }; +} + +function statusSet( + tests: OrchestratedTestCase[], + observationIndex: number, + status: TestStatus, +): Set { + return new Set( + tests + .filter((test) => test.observations[observationIndex]?.status === status) + .map((test) => identityKey(test.codeunitId, test.method)), + ); +} + +function setDifference( + left: Set, + right: Set, + identities: Map, +): OrchestratedTestIdentity[] { + return [...left] + .filter((key) => !right.has(key)) + .map((key) => identity(identities.get(key)!)) + .sort(compareIdentity); +} + +export function analyzeTestOrchestration( + plan: CodeunitTestGroup[], + runs: RunTestsResult[], + runsRequested: number, +): TestOrchestrationAnalysis { + if (!Number.isInteger(runsRequested) || runsRequested < 1) { + throw new RangeError("runsRequested must be a positive integer"); + } + if (runs.length > runsRequested) { + throw new RangeError("The number of attempted runs cannot exceed runsRequested"); + } + const warnings: string[] = []; + const identities = new Map(); + + const remember = (codeunitId: number, method: string) => { + const normalized = method.trim(); + if (normalized === "") return; + const key = identityKey(codeunitId, normalized); + if (!identities.has(key)) identities.set(key, { key, codeunitId, method: normalized }); + }; + + for (const group of plan) { + if ((group.methods?.length ?? 0) > 0) { + for (const method of group.methods ?? []) remember(group.id, method); + } + } + + const rowsByRun = runs.map((run) => { + const rows = new Map(); + for (const row of realRows(run)) { + remember(row.codeunitId, row.method); + const key = identityKey(row.codeunitId, row.method); + const existing = rows.get(key); + if (existing) existing.push(row); + else rows.set(key, [row]); + } + return rows; + }); + + rowsByRun.forEach((rows, index) => { + const duplicateIdentities = [...rows.values()].filter((matches) => matches.length > 1).length; + if (duplicateIdentities > 0) { + warnings.push( + `Run ${index + 1} reported duplicate rows for ${duplicateIdentities} test ` + + `${duplicateIdentities === 1 ? "identity" : "identities"}; inspect tests[].observations.`, + ); + } + }); + + if (runs.length !== runsRequested) { + warnings.push(`Only ${runs.length} of ${runsRequested} requested runs returned a result.`); + } + runs.forEach((run, index) => { + if (run.runAborted === true) { + const stoppedEarly = index === runs.length - 1 && runs.length < runsRequested; + warnings.push( + stoppedEarly + ? `Run ${index + 1} aborted; later requested runs were not started because server-side cancellation could not be confirmed safely.` + : `Run ${index + 1} aborted; its observations cannot establish complete stability.`, + ); + } + if (realRows(run).length === 0) { + warnings.push(`Run ${index + 1} reported no real test methods.`); + } + }); + + const tests: OrchestratedTestCase[] = [...identities.values()] + .sort(compareIdentity) + .map((testIdentity) => { + const observations = Array.from({ length: runsRequested }, (_, index): TestRunObservation => { + const rows = rowsByRun[index]; + if (!rows) { + return { run: index + 1, status: "missing", durationMs: null }; + } + const matches = rows.get(testIdentity.key) ?? []; + if (matches.length === 0) { + return { run: index + 1, status: "missing", durationMs: null }; + } + if (matches.length > 1) { + return { run: index + 1, status: "ambiguous", durationMs: null }; + } + const row = matches[0]!; + return { run: index + 1, status: row.status, durationMs: row.durationMs }; + }); + const classification = classify(observations); + return { + codeunitId: testIdentity.codeunitId, + method: testIdentity.method, + classification, + complete: observations.every((entry) => + entry.status !== "missing" && entry.status !== "ambiguous"), + passCount: observations.filter((entry) => entry.status === "passed").length, + failCount: observations.filter((entry) => entry.status === "failed").length, + skipCount: observations.filter((entry) => entry.status === "skipped").length, + missingCount: observations.filter((entry) => entry.status === "missing").length, + ambiguousCount: observations.filter((entry) => entry.status === "ambiguous").length, + observations, + }; + }); + + const incompleteTests = tests.filter((test) => !test.complete).length; + if (incompleteTests > 0) { + warnings.push( + `${incompleteTests} test ${incompleteTests === 1 ? "identity has" : "identities have"} ` + + "a missing or ambiguous observation; inspect tests[].observations.", + ); + } + if (tests.length === 0) { + warnings.push("No real or explicitly requested test method identity was available for comparison."); + } + + const diffs: TestRunDiff[] = []; + for (let index = 1; index < runs.length; index++) { + const previousPassed = statusSet(tests, index - 1, "passed"); + const currentPassed = statusSet(tests, index, "passed"); + const previousFailed = statusSet(tests, index - 1, "failed"); + const currentFailed = statusSet(tests, index, "failed"); + const changed: TestObservationChange[] = []; + for (const test of tests) { + const from = test.observations[index - 1]; + const to = test.observations[index]; + if (!from || !to) { + throw new RangeError("Run observations did not match the requested orchestration bounds"); + } + if (from.status !== to.status) { + changed.push({ + codeunitId: test.codeunitId, + method: test.method, + from: from.status, + to: to.status, + }); + } + } + changed.sort(compareIdentity); + diffs.push({ + fromRun: index, + toRun: index + 1, + passed: { + added: setDifference(currentPassed, previousPassed, identities), + removed: setDifference(previousPassed, currentPassed, identities), + }, + failed: { + added: setDifference(currentFailed, previousFailed, identities), + removed: setDifference(previousFailed, currentFailed, identities), + }, + changed, + }); + } + + const summary: TestOrchestrationSummary = { + tests: tests.length, + stablePassed: tests.filter((test) => test.classification === "stablePassed").length, + stableFailed: tests.filter((test) => test.classification === "stableFailed").length, + stableSkipped: tests.filter((test) => test.classification === "stableSkipped").length, + flaky: tests.filter((test) => test.classification === "flaky").length, + inconsistent: tests.filter((test) => test.classification === "inconsistent").length, + incomplete: tests.filter((test) => test.classification === "incomplete").length, + totalDurationMs: runs + .flatMap(realRows) + .reduce((total, row) => total + row.durationMs, 0), + }; + const complete = runs.length === runsRequested + && runs.every((run) => run.runAborted !== true) + && tests.length > 0 + && tests.every((test) => test.complete); + if (tests.length > 0 && summary.stableSkipped === tests.length) { + warnings.push( + "Every selected test was skipped in every attempted run; outcome passed contains no passing execution evidence.", + ); + } + const outcome: TestOrchestrationOutcome = !complete + ? "incomplete" + : summary.flaky > 0 || summary.inconsistent > 0 + ? "unstable" + : summary.stableFailed > 0 + ? "failed" + : "passed"; + + return { + runsRequested, + runsAttempted: runs.length, + complete, + outcome, + summary, + tests, + diffs, + warnings, + }; +} diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 156e236..3f8d28e 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -92,9 +92,9 @@ export function buildServer(state: ServerState, deps: ToolDeps): McpServer { outputSchema: tool.outputSchema, annotations: tool.annotations, }, - async (params: Record) => { + async (params: Record, extra) => { try { - return toToolResponse(await tool.handler(params)); + return toToolResponse(await tool.handler(params, { signal: extra.signal })); } catch (err) { return toAgentToolError(tool.name, err); } diff --git a/src/mcp/skills.generated.ts b/src/mcp/skills.generated.ts index 8e8d718..4c19baf 100644 --- a/src/mcp/skills.generated.ts +++ b/src/mcp/skills.generated.ts @@ -21,14 +21,14 @@ export const skills: EmbeddedSkill[] = [ "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.", "uri": "skill://bc-al-testing/SKILL.md", "mimeType": "text/markdown", - "text": "---\nname: bc-al-testing\ndescription: 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---\n\n# Running AL tests against Business Central\n\n## Workflow\n\n1. `bcdev_status` — preflight. Confirms the dev endpoint is reachable, credentials work,\n and `supportsTestRunning` is true (requires dev API `WebApiVersion >= 7.0`, i.e. BC28+).\n Call this first whenever anything else fails.\n2. `bcdev_test_discover` — pure filesystem scan of the AL project for test codeunits\n (`Subtype = Test`) and their `[Test]` methods. No server involved. Use the returned\n `codeunitId` values in the next step.\n3. `bcdev_test_run { codeunits: [{ id: }] }` — runs on the server, returns\n a run `summary` plus per-method `passed | failed | skipped`, duration, and failure output.\n Restrict with `methods: [\"Name\"]`; pick the company with `company`.\n4. To check changed-code coverage, publish the current changed objects, then add\n `coverageAgainst: \"origin/main\"` and `changesDeployed: true`. The latter is your explicit\n confirmation that this working tree is deployed. This implies `coverage: \"procedure\"` and\n adds `coverageGaps` to the same result.\n\n## Facts that save you time\n\n- **One run at a time.** A second `bcdev_test_run` while one is active fails with\n \"already running\" — wait and retry.\n- **Coverage:** pass `coverage: \"procedure\"` (validated against real BC). `\"line\"` is\n unproven — do not trust it without independent verification. Covered procedures map\n back to local source files when the object IDs exist in the project.\n- **Changed-procedure gaps:** `coverageAgainst` resolves the ref's merge base with `HEAD` and\n compares it with the working tree, including committed branch changes, staged/unstaged edits,\n and nonignored untracked `.al` files. Each current executable procedure intersecting changed\n lines is `covered`, `uncovered`, or `unknown` in `coverageGaps`.\n- **Deployment is not inferred.** TestRunnerHub provides method identities but no artifact hash.\n Without `changesDeployed: true`, changed procedures remain `unknown` and `complete` is false even\n when server coverage contains the same method ID. Set it only after publishing the current objects;\n inspect `coverageGaps.deployment` to distinguish the assertion from tool verification.\n- **Read gap states conservatively.** `uncovered` means an exact compiler method identity was\n absent from procedure coverage returned for every requested test group. An unresolved signature,\n missing coverage payload, or aborted run is `unknown`, never a proven gap. Changed trigger spans\n are detected and force `complete: false` because local discovery does not yet classify trigger\n identities, including nested trigger scopes. Follow `warnings` before using the result as a gate.\n Broaden the selected tests and rerun with the same `coverageAgainst` ref to close gaps.\n- **Changed lines without a procedure identity block the gate.** Properties, field and control\n declarations, global variables, object headers, and `namespace`/`using` lines are counted in\n `coverageGaps.summary.unattributedChanges`, as are semantic preprocessor directives outside\n method spans. They are listed with their exact lines in `warnings` and set `complete: false`.\n Review them yourself; procedure coverage cannot speak about them.\n- **`coverageAgainst` requires an `app.json` with an explicit `runtime`** in the AL project — the\n runtime selects the method-ID hash variant, so the analysis refuses to guess.\n- **Tests run in codeunit declaration order**, not the order of your `methods` array.\n- **Synthetic results:** the run summary can contain an extra entry with an empty\n method name (a server-side quirk, e.g. after watch evaluations). `summary` excludes\n these from totals and reports their count as `syntheticResults`; raw rows remain available.\n- **Failed rows are parsed without losing evidence.** Read `failure.message` and\n `failure.callStack`; parsed frames use 1-based `line` and include local `file` when the\n object maps to this project. `failure.parsed: false` means the server format was opaque or\n localized — use the unchanged `output` text. Every frame also retains `raw`.\n- **Local mapping is best-effort.** Passing runs without coverage do not scan the AL tree. When\n mapping is needed, the project index is reused within that MCP server; if the directory is\n missing or unreadable, the complete server result still returns with `sourceMappingWarning`.\n The warning identifies whether call-stack files remain `null`, coverage files remain unset, or\n both mappings were unavailable.\n- **Follow the response, not a memorized script.** Every success includes `nextSteps`; an empty\n array is valid when the run is complete. An MCP error's text is JSON with stable\n `error.code`, `retryable`, and recovery `nextSteps` (and no `structuredContent`).\n- The tests must already be published to the server (publish via the dev endpoint or\n VS Code AL extension); this server runs tests, it does not publish apps.\n- Connection defaults come from the project's `.vscode/launch.json`. On-premises UserPassword\n credentials come from `BC_DEV_USER` / `BC_DEV_PASSWORD`; SaaS uses the current Azure CLI login.\n Auth failures name the relevant env vars or Azure CLI/tenant settings to check.\n\n## Debugging a failing test\n\nSwitch to the bc-al-debugging skill (skill://bc-al-debugging/SKILL.md): attach the\ndebugger with break-on-error, run the same codeunit via `bcdev_debug_run_tests`, and\ninspect live state at the failure line.\n" + "text": "---\nname: bc-al-testing\ndescription: 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---\n\n# Running AL tests against Business Central\n\n## Workflow\n\n1. `bcdev_status` — preflight. Confirms the dev endpoint is reachable, credentials work,\n and `supportsTestRunning` is true (requires dev API `WebApiVersion >= 7.0`, i.e. BC28+).\n Call this first whenever anything else fails.\n2. `bcdev_test_discover` — pure filesystem scan of the AL project for test codeunits\n (`Subtype = Test`) and their `[Test]` methods. No server involved. Use the returned\n `codeunitId` values in the next step.\n3. `bcdev_test_run { codeunits: [{ id: }] }` — runs on the server, returns\n a run `summary` plus per-method `passed | failed | skipped`, duration, and failure output.\n Restrict with `methods: [\"Name\"]`; pick the company with `company`.\n4. To investigate repeatability, call\n `bcdev_test_orchestrate { codeunits: [...], runs: 3 }`. It executes the same selection\n sequentially, retains every enriched run, diffs adjacent passed/failed sets, and classifies\n each method.\n5. To check changed-code coverage, publish the current changed objects, then add\n `coverageAgainst: \"origin/main\"` and `changesDeployed: true`. The latter is your explicit\n confirmation that this working tree is deployed. This implies `coverage: \"procedure\"` and\n adds `coverageGaps` to the same result.\n\n## Facts that save you time\n\n- **One run at a time.** A second `bcdev_test_run` while one is active fails with\n \"already running\" — wait and retry. One orchestration owns that same slot for its entire\n repeat sequence, so direct, debug-bound, native-runtime, and other orchestration runs cannot\n overlap it.\n- **Orchestration is bounded and evidence-preserving.** `runs` defaults to 3 and accepts 2–20.\n `runs[]` retains every enriched per-method attempt and raw failure output. `diffs[]` reports\n exact adjacent passed/failed set additions and removals plus every changed observation.\n- **Flaky means observed pass and fail.** A method is `flaky` only when at least one run passed\n and another failed. Passed/skipped or failed/skipped mixtures are `inconsistent`. A missing\n result, duplicate result, aborted run, or synthetic-only run forces the aggregate\n `complete: false`; inspect `warnings` and `tests[].observations` before trusting stability.\n After an aborted attempt, orchestration retains it but does not start later attempts while\n the prior server-side run may still be active; their observations are `missing`. A thrown setup\n or authorization failure becomes a final aborted attempt instead of discarding earlier evidence.\n Client cancellation is observed between attempts and never cancels an active Business Central\n run. Overlapping codeunit/method selections are rejected, but disjoint method groups for the same\n codeunit remain valid. Non-rolled-back test side effects repeat on every attempted run.\n- **Read orchestration identities and skips conservatively.** `tests[].method` keeps the first\n requested or observed spelling, so casing can differ from raw server rows. If every selected test\n is skipped, the existing outcome convention remains `passed`, but `warnings` states that there was\n no passing execution evidence.\n- **Orchestration does not collect coverage.** Use `bcdev_test_run` separately for procedure\n coverage or `coverageAgainst`; repeated coverage is deliberately not merged into a stability\n result.\n- **Coverage:** pass `coverage: \"procedure\"` (validated against real BC). `\"line\"` is\n unproven — do not trust it without independent verification. Covered procedures map\n back to local source files when the object IDs exist in the project.\n- **Changed-procedure gaps:** `coverageAgainst` resolves the ref's merge base with `HEAD` and\n compares it with the working tree, including committed branch changes, staged/unstaged edits,\n and nonignored untracked `.al` files. Each current executable procedure intersecting changed\n lines is `covered`, `uncovered`, or `unknown` in `coverageGaps`.\n- **Deployment is not inferred.** TestRunnerHub provides method identities but no artifact hash.\n Without `changesDeployed: true`, changed procedures remain `unknown` and `complete` is false even\n when server coverage contains the same method ID. Set it only after publishing the current objects;\n inspect `coverageGaps.deployment` to distinguish the assertion from tool verification.\n- **Read gap states conservatively.** `uncovered` means an exact compiler method identity was\n absent from procedure coverage returned for every requested test group. An unresolved signature,\n missing coverage payload, or aborted run is `unknown`, never a proven gap. Changed trigger spans\n are detected and force `complete: false` because local discovery does not yet classify trigger\n identities, including nested trigger scopes. Follow `warnings` before using the result as a gate.\n Broaden the selected tests and rerun with the same `coverageAgainst` ref to close gaps.\n- **Changed lines without a procedure identity block the gate.** Properties, field and control\n declarations, global variables, object headers, and `namespace`/`using` lines are counted in\n `coverageGaps.summary.unattributedChanges`, as are semantic preprocessor directives outside\n method spans. They are listed with their exact lines in `warnings` and set `complete: false`.\n Review them yourself; procedure coverage cannot speak about them.\n- **`coverageAgainst` requires an `app.json` with an explicit `runtime`** in the AL project — the\n runtime selects the method-ID hash variant, so the analysis refuses to guess.\n- **Tests run in codeunit declaration order**, not the order of your `methods` array.\n- **Synthetic results:** the run summary can contain an extra entry with an empty\n method name (a server-side quirk, e.g. after watch evaluations). `summary` excludes\n these from totals and reports their count as `syntheticResults`; raw rows remain available.\n- **Failed rows are parsed without losing evidence.** Read `failure.message` and\n `failure.callStack`; parsed frames use 1-based `line` and include local `file` when the\n object maps to this project. `failure.parsed: false` means the server format was opaque or\n localized — use the unchanged `output` text. Every frame also retains `raw`.\n- **Local mapping is best-effort.** Passing runs without coverage do not scan the AL tree. When\n mapping is needed, the project index is reused within that MCP server; if the directory is\n missing or unreadable, the server evidence still returns with `sourceMappingWarning`.\n The warning identifies whether call-stack files remain `null`, coverage files remain unset, or\n both mappings were unavailable.\n- **Follow the response, not a memorized script.** Every success includes `nextSteps`; an empty\n array is valid when the run is complete. An MCP error's text is JSON with stable\n `error.code`, `retryable`, and recovery `nextSteps` (and no `structuredContent`).\n- The tests must already be published to the server (publish via the dev endpoint or\n VS Code AL extension); this server runs tests, it does not publish apps.\n- Connection defaults come from the project's `.vscode/launch.json`. On-premises UserPassword\n credentials come from `BC_DEV_USER` / `BC_DEV_PASSWORD`; SaaS uses the current Azure CLI login.\n Auth failures name the relevant env vars or Azure CLI/tenant settings to check.\n\n## Debugging a failing test\n\nSwitch to the bc-al-debugging skill (skill://bc-al-debugging/SKILL.md): attach the\ndebugger with break-on-error, run the same codeunit via `bcdev_debug_run_tests`, and\ninspect live state at the failure line.\n" }, { "name": "bc-native-mcp", "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.", "uri": "skill://bc-native-mcp/SKILL.md", "mimeType": "text/markdown", - "text": "---\nname: bc-native-mcp\ndescription: 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---\n\n# Use Business Central native MCP tools\n\n## Workflow\n\n1. Select one context and call `bcdev_native_list` with the exact `company`:\n - `business` for `bc_actions_*`;\n - `runtime` for native AL runtime tools;\n - `debugging` for troubleshooting an active paused debugger.\n2. Read the returned `catalog.tools`. Use the exact `name`, `inputSchema`, and annotations;\n never invent a native tool name or argument shape.\n3. Call `bcdev_native_call` with the same target, context, and company. Put only the\n upstream tool arguments in `arguments`.\n4. Read `result` as the unchanged native `CallToolResult`. `result.isError: true` is an\n upstream Business Central tool failure retained as evidence, even though the bridge call\n itself completed.\n\nUse `cursor` from `catalog.nextCursor` to continue a paged listing. A named\n`configurationName` is valid only for `business`; omitting it uses Business Central's\ndefault dynamic configuration.\n\n## Debugging context\n\nFirst use `bcdev_debug_attach`, trigger the workload, and call `bcdev_debug_wait` until it\nreturns a `break`. Native debugging is deliberately unavailable while merely attached,\nbinding, running, or after `bcdev_debug_continue`. At the break, list the `debugging`\ncatalog and call its tools. Finish by using `bcdev_debug_continue` or\n`bcdev_debug_detach`.\n\nRecord-write triage owns its debugger internally and cannot be inspected through this\npassthrough.\n\n## Guardrails\n\n- Prefer `bcdev_test_run` for structured summaries, source mapping, and coverage. Prefer the\n first-class `bcdev_debug_*` tools for the normal inspect/step loop. Use native passthrough\n when its catalog or raw result is specifically useful.\n- Every native list or call opens, initializes, and closes a fresh upstream MCP session. For\n repeated debugger inspection, the first-class `bcdev_debug_*` tools avoid those extra\n round trips and should remain the default.\n- Debugging identity is a point-in-time snapshot taken when the native call starts. Do not\n issue `bcdev_debug_continue` concurrently with a native debugging call; await the native\n result first, then resume.\n- Treat every `bcdev_native_call` as potentially destructive. Inspect the upstream tool's\n annotations and arguments before calling it, especially against Production.\n- Do not pass tokens, authorization headers, endpoint URLs, or routing headers. The bridge\n derives them from the configured cloud target and Azure CLI identity.\n- Native passthrough supports cloud Sandbox and Production. It does not expose a profiling\n context; the separate `bcdev_profile_*` tools remain the supported BC28 profiling path.\n- A native `runtime` call shares the one-test-run lock with direct and debug-bound test runs.\n Wait for the active run to finish instead of retrying concurrently.\n- A timeout after a native runtime call begins does not cancel Business Central's server-side\n test run. When the error reports `upstreamRunCancelled: false`, do not retry or start another\n test run until you have confirmed that the upstream run finished.\n" + "text": "---\nname: bc-native-mcp\ndescription: 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---\n\n# Use Business Central native MCP tools\n\n## Workflow\n\n1. Select one context and call `bcdev_native_list` with the exact `company`:\n - `business` for `bc_actions_*`;\n - `runtime` for native AL runtime tools;\n - `debugging` for troubleshooting an active paused debugger.\n2. Read the returned `catalog.tools`. Use the exact `name`, `inputSchema`, and annotations;\n never invent a native tool name or argument shape.\n3. Call `bcdev_native_call` with the same target, context, and company. Put only the\n upstream tool arguments in `arguments`.\n4. Read `result` as the unchanged native `CallToolResult`. `result.isError: true` is an\n upstream Business Central tool failure retained as evidence, even though the bridge call\n itself completed.\n\nUse `cursor` from `catalog.nextCursor` to continue a paged listing. A named\n`configurationName` is valid only for `business`; omitting it uses Business Central's\ndefault dynamic configuration.\n\n## Debugging context\n\nFirst use `bcdev_debug_attach`, trigger the workload, and call `bcdev_debug_wait` until it\nreturns a `break`. Native debugging is deliberately unavailable while merely attached,\nbinding, running, or after `bcdev_debug_continue`. At the break, list the `debugging`\ncatalog and call its tools. Finish by using `bcdev_debug_continue` or\n`bcdev_debug_detach`.\n\nRecord-write triage owns its debugger internally and cannot be inspected through this\npassthrough.\n\n## Guardrails\n\n- Prefer `bcdev_test_run` for structured summaries, source mapping, and coverage. Prefer the\n first-class `bcdev_debug_*` tools for the normal inspect/step loop. Use native passthrough\n when its catalog or raw result is specifically useful.\n- Every native list or call opens, initializes, and closes a fresh upstream MCP session. For\n repeated debugger inspection, the first-class `bcdev_debug_*` tools avoid those extra\n round trips and should remain the default.\n- Debugging identity is a point-in-time snapshot taken when the native call starts. Do not\n issue `bcdev_debug_continue` concurrently with a native debugging call; await the native\n result first, then resume.\n- Treat every `bcdev_native_call` as potentially destructive. Inspect the upstream tool's\n annotations and arguments before calling it, especially against Production.\n- Do not pass tokens, authorization headers, endpoint URLs, or routing headers. The bridge\n derives them from the configured cloud target and Azure CLI identity.\n- Native passthrough supports cloud Sandbox and Production. It does not expose a profiling\n context; the separate `bcdev_profile_*` tools remain the supported BC28 profiling path.\n- A native `runtime` call shares the one-test-run lock with direct, orchestrated, and\n debug-bound test runs. Wait for the active run to finish instead of retrying concurrently.\n- A timeout after a native runtime call begins does not cancel Business Central's server-side\n test run. When the error reports `upstreamRunCancelled: false`, do not retry or start another\n test run until you have confirmed that the upstream run finished.\n" } ]; diff --git a/src/mcp/tools/agent-response.ts b/src/mcp/tools/agent-response.ts index f20f77e..cf5325d 100644 --- a/src/mcp/tools/agent-response.ts +++ b/src/mcp/tools/agent-response.ts @@ -12,7 +12,7 @@ function generatedNextSteps(name: string, result: Record, param : ["Use a Business Central server with developer API 7.0 or newer for test and debugger tools."]; case "bcdev_test_discover": return Array.isArray(result["tests"]) && result["tests"].length > 0 - ? ["Call bcdev_test_run with the returned codeunit IDs."] + ? ["Call bcdev_test_run with the returned codeunit IDs, or bcdev_test_orchestrate when repeatability is the question."] : []; case "bcdev_test_run": { const summary = result["summary"] as Record | undefined; @@ -41,6 +41,24 @@ function generatedNextSteps(name: string, result: Record, param } return steps; } + case "bcdev_test_orchestrate": { + if (result["outcome"] === "incomplete") { + return [ + "Review orchestration warnings and aborted raw runs, correct the server or test selection, then rerun bcdev_test_orchestrate before making a stability claim.", + ]; + } + if (result["outcome"] === "unstable") { + return [ + "Rerun only the methods classified flaky or inconsistent to reproduce each transition, then use bcdev_debug_attach and bcdev_debug_run_tests on the smallest unstable selection.", + ]; + } + if (result["outcome"] === "failed") { + return [ + "Use bcdev_debug_attach and bcdev_debug_run_tests for the methods classified stableFailed.", + ]; + } + return []; + } case "bcdev_debug_attach": return params["sessionId"] === undefined ? ["Create or trigger the matching session, then call bcdev_debug_wait."] @@ -152,8 +170,8 @@ export function withAgentResponses(tool: ToolDefinition): ToolDefinition { return { ...tool, outputSchema, - handler: async (params) => { - const value = await handler(params); + handler: async (params, context) => { + const value = await handler(params, context); if (typeof value !== "object" || value === null || Array.isArray(value)) { throw new Error(`Tool ${tool.name} returned a non-object success result`); } diff --git a/src/mcp/tools/shared.ts b/src/mcp/tools/shared.ts index 806ecf4..a753290 100644 --- a/src/mcp/tools/shared.ts +++ b/src/mcp/tools/shared.ts @@ -11,6 +11,7 @@ import type { ConnectionConfig } from "../../core/types"; import { DEFAULT_DEV_PORT } from "../../core/urls"; import type { GitChangeSet } from "../../core/git-changes"; import type { NativeMcpGateway } from "../../core/native-mcp"; +import { upperInvariantUtf16 } from "../../core/al-identifiers"; import { DebugSession, ServerState } from "../state"; export interface ToolDeps { @@ -33,7 +34,11 @@ export interface ToolDefinition { schema: z.ZodRawShape; outputSchema: z.ZodTypeAny; annotations: ToolAnnotations; - handler(params: Record): Promise; + handler(params: Record, context?: ToolExecutionContext): Promise; +} + +export interface ToolExecutionContext { + signal?: AbortSignal; } export interface DebugTarget { @@ -164,7 +169,9 @@ export const runTestsOutputSchema = z.looseObject({ syntheticResults: z.number(), failedTests: z.array(z.object({ codeunitId: z.number(), method: z.string() })), }), - sourceMappingWarning: z.string().optional().describe("Nonfatal warning when local AL files could not be indexed; server test results remain complete"), + sourceMappingWarning: z.string().optional().describe( + "Nonfatal warning when local AL files could not be indexed; server test evidence remains available", + ), coverage: z .array( z.looseObject({ @@ -298,6 +305,47 @@ export const codeunitsShape = z }), ) .min(1) + .superRefine((groups, context) => { + const selections = new Map }>(); + groups.forEach((group, groupIndex) => { + const existing = selections.get(group.id); + const methods = group.methods ?? []; + const selectsAll = methods.length === 0; + if (existing?.all === true || (existing !== undefined && selectsAll)) { + context.addIssue({ + code: "custom", + path: [groupIndex, "methods"], + message: `Test codeunit ${group.id} is selected more than once with overlapping method coverage`, + }); + } else if (existing) { + for (const method of methods) { + if (existing.methods.has(upperInvariantUtf16(method.trim()))) { + context.addIssue({ + code: "custom", + path: [groupIndex, "methods"], + message: `Test codeunit ${group.id} method ${method} is selected more than once`, + }); + } + } + } + const methodsInGroup = new Set(); + for (const method of methods) { + const key = upperInvariantUtf16(method.trim()); + if (methodsInGroup.has(key)) { + context.addIssue({ + code: "custom", + path: [groupIndex, "methods"], + message: `Test codeunit ${group.id} method ${method} is selected more than once`, + }); + } + methodsInGroup.add(key); + } + const next = existing ?? { all: false, methods: new Set() }; + next.all ||= selectsAll; + for (const method of methods) next.methods.add(upperInvariantUtf16(method.trim())); + selections.set(group.id, next); + }); + }) .describe("Test codeunits to run, optionally restricted to named methods"); export const breakpointShape = z.object({ diff --git a/src/mcp/tools/test-tools.ts b/src/mcp/tools/test-tools.ts index b631cf5..0aa32cb 100644 --- a/src/mcp/tools/test-tools.ts +++ b/src/mcp/tools/test-tools.ts @@ -3,11 +3,13 @@ import { resolve as resolvePath } from "node:path"; import { AlObjectIndex, discoverTests } from "../../core/al-objects"; import { TestRunnerClient } from "../../core/hubs/test-runner-hub"; import { fetchServerInfo } from "../../core/server-info"; -import type { CoverageMode } from "../../core/types"; +import type { CoverageMode, RunTestsResult } from "../../core/types"; import { enrichTestRun, mapTestRunSources, testRunNeedsSourceMapping } from "../../core/agent-results"; import { AlProcedureDiscoveryCache, discoverAlProcedureIdentities } from "../../core/al-procedures"; import { analyzeCoverageGaps } from "../../core/coverage-gaps"; import { BcDevError } from "../../core/agent-errors"; +import { analyzeTestOrchestration } from "../../core/test-orchestration"; +import { redactAuthorization } from "../../core/redaction"; import type { ServerState } from "../state"; import { claimTestRun, codeunitsShape, connectionShape, requireTestRunningSupport, resolve, runTestsOutputSchema, type ToolDefinition, type ToolDeps } from "./shared"; @@ -44,6 +46,96 @@ async function cachedObjectIndex(project: string, deps: ToolDeps): Promise { + const needingMapping = runs.filter(testRunNeedsSourceMapping); + if (needingMapping.length === 0) return; + let index: AlObjectIndex; + try { + index = await cachedObjectIndex(project, deps); + } catch { + for (const run of needingMapping) { + run.sourceMappingWarning = + "Local AL source mapping was unavailable; server test evidence is retained and call-stack file fields remain null."; + } + return; + } + for (const run of needingMapping) mapTestRunSources(run, index); +} + export function createTestTools(state: ServerState, deps: ToolDeps): ToolDefinition[] { return [ { @@ -188,5 +280,81 @@ export function createTestTools(state: ServerState, deps: ToolDeps): ToolDefinit } }, }, + { + name: "bcdev_test_orchestrate", + title: "Repeat and compare AL tests", + description: + "Run one AL test selection sequentially 2–20 times, retain every enriched attempt, diff adjacent passed/failed sets, and flag flaky or incomplete method outcomes. Non-rolled-back test side effects repeat on every attempt.", + // destructiveHint:false assumes BC test isolation (TestIsolation=Codeunit) rolls back data + // changes. Suites with disabled isolation or external side effects can repeat those effects. + annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: false, openWorldHint: true }, + outputSchema: testOrchestrationOutputSchema, + schema: { + ...connectionShape, + codeunits: codeunitsShape, + company: z.string().optional().describe("BC company name (default: the server's default company)"), + runs: z + .number() + .int() + .min(2) + .max(20) + .optional() + .describe("Number of sequential runs to compare (default 3; minimum 2; maximum 20)"), + }, + handler: async (params, context) => { + claimTestRun(state); + try { + const { config, authorization, project } = resolve(params, deps); + const plan = params["codeunits"] as Array<{ id: number; methods?: string[] }>; + const runsRequested = (params["runs"] as number | undefined) ?? 3; + await requireTestRunningSupport(config, authorization, deps); + const runs: RunTestsResult[] = []; + let cancellationObserved = false; + const clientCancelled = () => context?.signal?.aborted === true; + for (let index = 0; index < runsRequested; index++) { + if (clientCancelled()) { + cancellationObserved = true; + break; + } + let result: RunTestsResult; + try { + result = await new TestRunnerClient(deps.hubFactory).run( + config, + authorization, + plan, + { company: params["company"] as string | undefined }, + ); + } catch (error) { + const detail = redactAuthorization(error instanceof Error ? error.message : String(error)); + result = { + results: [], + runAborted: true, + abortReason: `Attempt ${index + 1} failed before a complete server result: ${detail}`, + }; + } + enrichTestRun(result); + runs.push(result); + if (result.runAborted === true) break; + if (clientCancelled() && index + 1 < runsRequested) { + cancellationObserved = true; + break; + } + } + await mapOrchestrationSources(runs, project, deps); + const analysis = analyzeTestOrchestration(plan, runs, runsRequested); + if (cancellationObserved) { + analysis.warnings.push( + "Client cancellation was observed between attempts; no later attempt was started. An active server run is never cancelled by this check.", + ); + } + return { + ...analysis, + runs: runs.map((run, index) => ({ ...run, run: index + 1 })), + }; + } finally { + state.testRunActive = false; + } + }, + }, ]; } diff --git a/tests/core/test-orchestration.test.ts b/tests/core/test-orchestration.test.ts new file mode 100644 index 0000000..778ed74 --- /dev/null +++ b/tests/core/test-orchestration.test.ts @@ -0,0 +1,314 @@ +import { describe, expect, test } from "bun:test"; +import { analyzeTestOrchestration } from "../../src/core/test-orchestration"; +import type { RunTestsResult, TestMethodResult } from "../../src/core/types"; + +function run( + results: TestMethodResult[], + options: { aborted?: boolean } = {}, +): RunTestsResult { + return { + results, + ...(options.aborted + ? { runAborted: true, abortReason: "synthetic dropped connection" } + : {}), + }; +} + +const row = ( + method: string, + status: "passed" | "failed" | "skipped", + durationMs = 1, + codeunitId = 50100, +): TestMethodResult => ({ + codeunitId, + method, + status, + durationMs, + output: status === "failed" ? "boom" : "", +}); + +describe("test orchestration analysis", () => { + test("classifies stable pass, fail, and skip while excluding synthetic rows", () => { + const runs = [1, 2, 3].map(() => run([ + row("Passes", "passed", 2), + row("Fails", "failed", 3), + row("Skipped", "skipped", 1), + row("", "failed", 100), + ])); + const result = analyzeTestOrchestration([{ id: 50100 }], runs, 3); + + expect(result).toMatchObject({ + complete: true, + outcome: "failed", + runsRequested: 3, + runsAttempted: 3, + summary: { + tests: 3, + stablePassed: 1, + stableFailed: 1, + stableSkipped: 1, + flaky: 0, + inconsistent: 0, + incomplete: 0, + totalDurationMs: 18, + }, + warnings: [], + }); + expect(result.tests.map((entry) => [entry.method, entry.classification])).toEqual([ + ["Fails", "stableFailed"], + ["Passes", "stablePassed"], + ["Skipped", "stableSkipped"], + ]); + expect(result.diffs).toHaveLength(2); + for (const diff of result.diffs) { + expect(diff.passed).toEqual({ added: [], removed: [] }); + expect(diff.failed).toEqual({ added: [], removed: [] }); + expect(diff.changed).toEqual([]); + } + }); + + test("flags pass/fail flakes and returns exact adjacent passed and failed set diffs", () => { + const result = analyzeTestOrchestration([{ id: 50100 }], [ + run([row("A", "passed"), row("B", "failed")]), + run([row("A", "failed"), row("B", "passed")]), + run([row("A", "passed"), row("B", "passed")]), + ], 3); + + expect(result.complete).toBe(true); + expect(result.outcome).toBe("unstable"); + expect(result.summary.flaky).toBe(2); + expect(result.tests.map((entry) => ({ + method: entry.method, + classification: entry.classification, + passCount: entry.passCount, + failCount: entry.failCount, + }))).toEqual([ + { method: "A", classification: "flaky", passCount: 2, failCount: 1 }, + { method: "B", classification: "flaky", passCount: 2, failCount: 1 }, + ]); + expect(result.diffs[0]).toEqual({ + fromRun: 1, + toRun: 2, + passed: { + added: [{ codeunitId: 50100, method: "B" }], + removed: [{ codeunitId: 50100, method: "A" }], + }, + failed: { + added: [{ codeunitId: 50100, method: "A" }], + removed: [{ codeunitId: 50100, method: "B" }], + }, + changed: [ + { codeunitId: 50100, method: "A", from: "passed", to: "failed" }, + { codeunitId: 50100, method: "B", from: "failed", to: "passed" }, + ], + }); + expect(result.diffs[1]).toEqual({ + fromRun: 2, + toRun: 3, + passed: { + added: [{ codeunitId: 50100, method: "A" }], + removed: [], + }, + failed: { + added: [], + removed: [{ codeunitId: 50100, method: "A" }], + }, + changed: [ + { codeunitId: 50100, method: "A", from: "failed", to: "passed" }, + ], + }); + }); + + test("distinguishes skipped-status inconsistency from pass/fail flakiness", () => { + const result = analyzeTestOrchestration([{ id: 50100 }], [ + run([row("SometimesSkipped", "passed")]), + run([row("SometimesSkipped", "skipped")]), + run([row("SometimesSkipped", "passed")]), + ], 3); + + expect(result).toMatchObject({ + complete: true, + outcome: "unstable", + summary: { flaky: 0, inconsistent: 1, incomplete: 0 }, + }); + expect(result.tests[0]).toMatchObject({ + classification: "inconsistent", + passCount: 2, + skipCount: 1, + }); + }); + + test("fails missing, duplicate, empty, and aborted evidence closed", () => { + const result = analyzeTestOrchestration( + [{ id: 50100, methods: ["NeverReported", "Duplicate"] }], + [ + run([ + row("Duplicate", "passed"), + row("duplicate", "failed"), + row("", "passed"), + ]), + run([row("", "failed")], { aborted: true }), + ], + 2, + ); + + expect(result.complete).toBe(false); + expect(result.outcome).toBe("incomplete"); + expect(result.tests).toEqual([ + { + codeunitId: 50100, + method: "Duplicate", + classification: "incomplete", + complete: false, + passCount: 0, + failCount: 0, + skipCount: 0, + missingCount: 1, + ambiguousCount: 1, + observations: [ + { run: 1, status: "ambiguous", durationMs: null }, + { run: 2, status: "missing", durationMs: null }, + ], + }, + { + codeunitId: 50100, + method: "NeverReported", + classification: "incomplete", + complete: false, + passCount: 0, + failCount: 0, + skipCount: 0, + missingCount: 2, + ambiguousCount: 0, + observations: [ + { run: 1, status: "missing", durationMs: null }, + { run: 2, status: "missing", durationMs: null }, + ], + }, + ]); + expect(result.warnings.join("\n")).toContain("Run 2 aborted"); + expect(result.warnings.join("\n")).toContain("Run 2 reported no real test methods"); + expect(result.warnings.join("\n")).toContain("duplicate rows for 1 test identity"); + expect(result.warnings.join("\n")).toContain("2 test identities"); + }); + + test("matches requested and reported method spellings case-insensitively", () => { + const result = analyzeTestOrchestration( + [{ id: 50100, methods: ["mIxEd"] }], + [ + run([row("Mixed", "passed", 1)]), + run([row("MIXED", "passed", 2)]), + ], + 2, + ); + + expect(result.complete).toBe(true); + expect(result.tests).toEqual([{ + codeunitId: 50100, + method: "mIxEd", + classification: "stablePassed", + complete: true, + passCount: 2, + failCount: 0, + skipCount: 0, + missingCount: 0, + ambiguousCount: 0, + observations: [ + { run: 1, status: "passed", durationMs: 1 }, + { run: 2, status: "passed", durationMs: 2 }, + ], + }]); + }); + + test("retains positive flaky evidence while marking a missing run incomplete", () => { + const result = analyzeTestOrchestration( + [{ id: 50100, methods: ["Intermittent"] }], + [ + run([row("Intermittent", "passed")]), + run([]), + run([row("Intermittent", "failed")]), + ], + 3, + ); + + expect(result.complete).toBe(false); + expect(result.outcome).toBe("incomplete"); + expect(result.tests[0]).toMatchObject({ + classification: "flaky", + complete: false, + passCount: 1, + failCount: 1, + missingCount: 1, + }); + }); + + test("marks unreturned requested runs as missing per-method evidence", () => { + const result = analyzeTestOrchestration( + [{ id: 50100, methods: ["A"] }], + [run([row("A", "passed")])], + 3, + ); + expect(result).toMatchObject({ + runsRequested: 3, + runsAttempted: 1, + complete: false, + outcome: "incomplete", + }); + expect(result.tests[0]).toEqual({ + codeunitId: 50100, + method: "A", + classification: "incomplete", + complete: false, + passCount: 1, + failCount: 0, + skipCount: 0, + missingCount: 2, + ambiguousCount: 0, + observations: [ + { run: 1, status: "passed", durationMs: 1 }, + { run: 2, status: "missing", durationMs: null }, + { run: 3, status: "missing", durationMs: null }, + ], + }); + expect(result.warnings[0]).toContain("Only 1 of 3"); + + const noRuns = analyzeTestOrchestration( + [{ id: 50100, methods: ["A"] }], + [], + 2, + ); + expect(noRuns.tests[0]).toMatchObject({ + classification: "incomplete", + complete: false, + missingCount: 2, + observations: [ + { run: 1, status: "missing" }, + { run: 2, status: "missing" }, + ], + }); + }); + + test("rejects impossible run bounds instead of indexing missing observations", () => { + expect(() => analyzeTestOrchestration( + [{ id: 50100, methods: ["A"] }], + [run([row("A", "passed")]), run([row("A", "passed")])], + 1, + )).toThrow(/cannot exceed runsRequested/); + expect(() => analyzeTestOrchestration([], [], 0)).toThrow(/positive integer/); + }); + + test("warns when every selected test is consistently skipped", () => { + const result = analyzeTestOrchestration( + [{ id: 50100, methods: ["Skipped"] }], + [run([row("Skipped", "skipped")]), run([row("Skipped", "skipped")])], + 2, + ); + + expect(result).toMatchObject({ + complete: true, + outcome: "passed", + summary: { stablePassed: 0, stableSkipped: 1 }, + }); + expect(result.warnings.join(" ")).toContain("contains no passing execution evidence"); + }); +}); diff --git a/tests/mcp/agent-response.test.ts b/tests/mcp/agent-response.test.ts index 6dc663f..cdaf254 100644 --- a/tests/mcp/agent-response.test.ts +++ b/tests/mcp/agent-response.test.ts @@ -63,4 +63,29 @@ describe("withAgentResponses", () => { })); expect((await incomplete.handler({}) as { nextSteps: string[] }).nextSteps.join(" ")).toContain("unresolved"); }); + + test("test-orchestration guidance follows incomplete, unstable, failed, and passed states", async () => { + const wrapped = (outcome: string, flaky = 0, inconsistent = 0) => withAgentResponses(tool({ + name: "bcdev_test_orchestrate", + handler: async () => ({ outcome, summary: { flaky, inconsistent } }), + })); + + expect( + (await wrapped("incomplete").handler({}) as { nextSteps: string[] }).nextSteps.join(" "), + ).toContain("stability claim"); + expect( + (await wrapped("unstable", 1).handler({}) as { nextSteps: string[] }).nextSteps.join(" "), + ).toContain("classified flaky"); + expect( + (await wrapped("unstable").handler({}) as { nextSteps: string[] }).nextSteps.join(" "), + ).toContain("inconsistent"); + const mixed = (await wrapped("unstable", 1, 1).handler({}) as { nextSteps: string[] }).nextSteps.join(" "); + expect(mixed).toContain("flaky or inconsistent"); + expect( + (await wrapped("failed").handler({}) as { nextSteps: string[] }).nextSteps.join(" "), + ).toContain("stableFailed"); + expect( + (await wrapped("passed").handler({}) as { nextSteps: string[] }).nextSteps, + ).toEqual([]); + }); }); diff --git a/tests/mcp/server.test.ts b/tests/mcp/server.test.ts index 230ccdc..3b64996 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(22); + expect(tools).toHaveLength(23); const status = tools.find((t) => t.name === "bcdev_status")!; expect(status.title).toBe("BC server status"); expect(status.annotations?.readOnlyHint).toBe(true); @@ -67,6 +67,16 @@ describe("server wiring", () => { expect(JSON.stringify(attachProperties["breakOnRecordWrite"])).toContain("nonTemporary"); const wait = tools.find((t) => t.name === "bcdev_debug_wait")!; expect(JSON.stringify(wait.outputSchema)).toContain("sessionBound"); + const orchestration = tools.find((t) => t.name === "bcdev_test_orchestrate")!; + expect(orchestration.annotations).toMatchObject({ + readOnlyHint: false, + destructiveHint: false, + idempotentHint: false, + openWorldHint: true, + }); + expect(JSON.stringify(orchestration.inputSchema)).toContain("maximum"); + expect(JSON.stringify(orchestration.outputSchema)).toContain("stablePassed"); + expect(JSON.stringify(orchestration.outputSchema)).toContain("ambiguous"); const recordWrites = tools.find((t) => t.name === "bcdev_record_writes_start")!; expect(JSON.stringify(recordWrites.inputSchema)).toContain("maxObservedWrites"); expect(JSON.stringify(recordWrites.inputSchema)).toContain("changesDeployed"); @@ -185,7 +195,7 @@ describe("server wiring", () => { test("start exposes the kind:instrumentation option", async () => { const client = await connect(); const { tools } = await client.listTools(); - expect(tools).toHaveLength(22); + expect(tools).toHaveLength(23); const start = tools.find((t) => t.name === "bcdev_profile_start")!; expect(JSON.stringify(start.inputSchema)).toContain("instrumentation"); }); diff --git a/tests/mcp/test-orchestration-tools.test.ts b/tests/mcp/test-orchestration-tools.test.ts new file mode 100644 index 0000000..08734da --- /dev/null +++ b/tests/mcp/test-orchestration-tools.test.ts @@ -0,0 +1,471 @@ +import { describe, expect, test } from "bun:test"; +import { mkdtempSync, mkdirSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { createAuthorizationProviderFactory } from "../../src/core/authorization"; +import type { AuthorizationProviderFactory } from "../../src/core/authorization"; +import type { HubFactory } from "../../src/core/hubs/signalr-base"; +import type { GitChangeSet } from "../../src/core/git-changes"; +import { ServerState } from "../../src/mcp/state"; +import { createTools, type ToolDeps } from "../../src/mcp/tools"; +import { FakeHub } from "../fakes/fake-hub"; +import { FakeNativeMcpGateway } from "../fakes/fake-native-mcp"; + +function makeProject(): string { + const dir = mkdtempSync(join(tmpdir(), "bcmcp-orchestration-")); + mkdirSync(join(dir, ".vscode")); + writeFileSync( + join(dir, ".vscode", "launch.json"), + JSON.stringify({ + configurations: [{ + type: "al", + request: "launch", + server: "http://localhost", + serverInstance: "BC", + }], + }), + ); + writeFileSync(join(dir, "app.json"), JSON.stringify({ runtime: "16.0" })); + writeFileSync( + join(dir, "T.Codeunit.al"), + 'codeunit 50100 "T"\n{\n Subtype = Test;\n\n [Test]\n procedure A()\n begin\n end;\n}\n', + ); + return dir; +} + +interface SetupOptions { + configureHub?: (hub: FakeHub, index: number) => void; + fetchFn?: typeof fetch; + authorizationFactory?: AuthorizationProviderFactory; +} + +function setup(options: SetupOptions = {}) { + const project = makeProject(); + const hubs: FakeHub[] = []; + let active = 0; + let maxActive = 0; + const hubFactory: HubFactory = (url, connectOptions) => { + const hub = new FakeHub(); + const index = hubs.length; + hub.url = url; + hub.opts = connectOptions; + const start = hub.start.bind(hub); + const stop = hub.stop.bind(hub); + hub.start = async () => { + active++; + maxActive = Math.max(maxActive, active); + await start(); + }; + hub.stop = async () => { + if (hub.started && !hub.stopped) active--; + await stop(); + }; + options.configureHub?.(hub, index); + hubs.push(hub); + return hub; + }; + const state = new ServerState(); + const gateway = new FakeNativeMcpGateway(); + const deps: ToolDeps = { + hubFactory, + authorizationFactory: options.authorizationFactory ?? createAuthorizationProviderFactory(), + fetchFn: options.fetchFn + ?? ((async () => new Response(JSON.stringify({ WebApiVersion: "7.0" }))) as unknown as typeof fetch), + env: { BC_DEV_USER: "u", BC_DEV_PASSWORD: "p" }, + cwd: project, + gitChanges: async (_project, baseRef): Promise => ({ + baseRef, + mergeBase: "a".repeat(40), + head: "workingTree", + files: [], + }), + nativeMcpGateway: gateway, + }; + const tools = new Map(createTools(state, deps).map((tool) => [tool.name, tool])); + return { + project, + state, + gateway, + hubs, + tools, + maxActive: () => maxActive, + }; +} + +async function waitFor(predicate: () => boolean): Promise { + for (let attempt = 0; attempt < 100; attempt++) { + if (predicate()) return; + await Bun.sleep(0); + } + throw new Error("condition did not become true"); +} + +describe("bcdev_test_orchestrate", () => { + test("runs sequentially, retains enriched evidence, maps failures, and flags a flake", async () => { + const statuses = [0, 1, 0]; + const { project, state, hubs, tools, maxActive } = setup({ + configureHub: (hub, index) => { + hub.onInvoke = (method) => { + if (method === "RunTests") { + queueMicrotask(() => { + const failed = statuses[index] === 1; + hub.emit( + "TestCompleted", + 50100, + "A", + statuses[index], + failed + ? 'boom\nCallStack:\n"T"(CodeUnit 50100).A() line 7' + : "", + index + 1, + ); + hub.emit("TestRunCompleted", { Tests: [] }); + }); + } + return undefined; + }; + }, + }); + + const tool = tools.get("bcdev_test_orchestrate")!; + const result = await tool.handler({ + codeunits: [{ id: 50100, methods: ["A"] }], + }) as { + outcome: string; + complete: boolean; + summary: { flaky: number; totalDurationMs: number }; + runs: Array<{ + run: number; + summary: { outcome: string }; + results: Array<{ failure?: { callStack: Array<{ file: string | null }> } }>; + }>; + diffs: Array<{ failed: { added: unknown[]; removed: unknown[] } }>; + tests: Array<{ classification: string }>; + nextSteps: string[]; + }; + + expect(result).toMatchObject({ + outcome: "unstable", + complete: true, + summary: { flaky: 1, totalDurationMs: 6 }, + tests: [{ classification: "flaky" }], + }); + expect(result.runs.map((run) => [run.run, run.summary.outcome])).toEqual([ + [1, "passed"], + [2, "failed"], + [3, "passed"], + ]); + expect(result.runs[1]?.results[0]?.failure?.callStack[0]?.file).toBe( + join(project, "T.Codeunit.al"), + ); + expect(result.diffs[0]?.failed.added).toEqual([{ codeunitId: 50100, method: "A" }]); + expect(result.diffs[1]?.failed.removed).toEqual([{ codeunitId: 50100, method: "A" }]); + expect(result.nextSteps.join(" ")).toContain("classified flaky"); + expect(hubs).toHaveLength(3); + expect(hubs.every((hub) => hub.stopped)).toBe(true); + expect(maxActive()).toBe(1); + expect(state.testRunActive).toBe(false); + expect(() => tool.outputSchema.parse(result)).not.toThrow(); + }); + + test("holds the shared singleton lock against every other test execution path", async () => { + const { state, gateway, hubs, tools } = setup({ + configureHub: (hub, index) => { + if (index > 0) { + hub.onInvoke = (method) => { + if (method === "RunTests") { + queueMicrotask(() => { + hub.emit("TestCompleted", 50100, "A", 0, "", 1); + hub.emit("TestRunCompleted", { Tests: [] }); + }); + } + return undefined; + }; + } + }, + }); + + const orchestration = tools.get("bcdev_test_orchestrate")!.handler({ + codeunits: [{ id: 50100, methods: ["A"] }], + runs: 2, + }); + expect(state.testRunActive).toBe(true); + + // requireSession only needs an owned manual-session value before claimTestRun rejects. + state.debug = {} as never; + const errors = await Promise.all([ + tools.get("bcdev_test_run")!.handler({ codeunits: [{ id: 50100 }] }).catch((error) => error), + tools.get("bcdev_test_orchestrate")!.handler({ + codeunits: [{ id: 50100 }], + runs: 2, + }).catch((error) => error), + tools.get("bcdev_debug_run_tests")!.handler({ + codeunits: [{ id: 50100 }], + }).catch((error) => error), + tools.get("bcdev_native_call")!.handler({ + company: "CRONUS", + context: "runtime", + toolName: "run_tests", + arguments: {}, + }).catch((error) => error), + ]); + for (const error of errors) { + expect(error).toMatchObject({ code: "TEST_RUN_ACTIVE", category: "state" }); + } + expect(gateway.toolCalls).toHaveLength(0); + + await waitFor(() => hubs.length === 1 && hubs[0]!.handlers.has("TestRunCompleted")); + hubs[0]!.emit("TestCompleted", 50100, "A", 0, "", 1); + hubs[0]!.emit("TestRunCompleted", { Tests: [] }); + await orchestration; + + expect(hubs).toHaveLength(2); + expect(state.testRunActive).toBe(false); + }); + + test("stops after an aborted run and marks later requested observations missing", async () => { + const { hubs, state, tools } = setup({ + configureHub: (hub, index) => { + hub.onInvoke = (method) => { + if (method === "RunTests") { + queueMicrotask(() => { + hub.emit("TestCompleted", 50100, "A", 0, "", 1); + if (index === 0) hub.close(new Error("socket dropped")); + else hub.emit("TestRunCompleted", { Tests: [] }); + }); + } + return undefined; + }; + }, + }); + + const tool = tools.get("bcdev_test_orchestrate")!; + const result = await tool.handler({ + codeunits: [{ id: 50100, methods: ["A"] }], + runs: 3, + }) as { + runsAttempted: number; + complete: boolean; + outcome: string; + runs: Array<{ runAborted?: boolean }>; + tests: Array<{ + classification: string; + complete: boolean; + missingCount: number; + observations: Array<{ status: string }>; + }>; + warnings: string[]; + nextSteps: string[]; + }; + + expect(result).toMatchObject({ + runsAttempted: 1, + complete: false, + outcome: "incomplete", + }); + expect(result.runs.map((run) => run.runAborted === true)).toEqual([true]); + expect(result.tests[0]).toMatchObject({ + classification: "incomplete", + complete: false, + missingCount: 2, + observations: [ + { status: "passed" }, + { status: "missing" }, + { status: "missing" }, + ], + }); + expect(result.warnings.join(" ")).toContain("server-side cancellation could not be confirmed"); + expect(result.nextSteps.join(" ")).toContain("before making a stability claim"); + expect(hubs).toHaveLength(1); + expect(state.testRunActive).toBe(false); + expect(() => tool.outputSchema.parse(result)).not.toThrow(); + }); + + test("retains earlier attempts when authorization rejects between runs", async () => { + let authorizationCalls = 0; + const { hubs, state, tools } = setup({ + authorizationFactory: () => ({ + getAuthorizationHeader: async () => { + authorizationCalls++; + if (authorizationCalls === 3) { + throw new Error("Authorization: Bearer must-not-leak"); + } + return "Basic dTpw"; + }, + }), + configureHub: (hub) => { + hub.onInvoke = (method) => { + if (method === "RunTests") { + queueMicrotask(() => { + hub.emit("TestCompleted", 50100, "A", 0, "", 1); + hub.emit("TestRunCompleted", { Tests: [] }); + }); + } + return undefined; + }; + }, + }); + + const tool = tools.get("bcdev_test_orchestrate")!; + const result = await tool.handler({ + codeunits: [{ id: 50100, methods: ["A"] }], + runs: 3, + }) as { + runsAttempted: number; + complete: boolean; + outcome: string; + runs: Array<{ run: number; runAborted?: boolean; abortReason?: string }>; + tests: Array<{ observations: Array<{ status: string }> }>; + }; + + expect(result).toMatchObject({ + runsAttempted: 2, + complete: false, + outcome: "incomplete", + runs: [ + { run: 1 }, + { run: 2, runAborted: true }, + ], + }); + expect(result.runs[1]?.abortReason).toContain("Attempt 2 failed"); + expect(result.runs[1]?.abortReason).toContain("[REDACTED]"); + expect(result.runs[1]?.abortReason).not.toContain("must-not-leak"); + expect(result.tests[0]?.observations.map((entry) => entry.status)).toEqual([ + "passed", + "missing", + "missing", + ]); + expect(hubs).toHaveLength(1); + expect(state.testRunActive).toBe(false); + expect(() => tool.outputSchema.parse(result)).not.toThrow(); + }); + + test("observes client cancellation between attempts without cancelling an active server run", async () => { + const controller = new AbortController(); + const { hubs, state, tools } = setup({ + configureHub: (hub, index) => { + hub.onInvoke = (method) => { + if (method === "RunTests") { + queueMicrotask(() => { + hub.emit("TestCompleted", 50100, "A", 0, "", 1); + if (index === 0) controller.abort(); + hub.emit("TestRunCompleted", { Tests: [] }); + }); + } + return undefined; + }; + }, + }); + + const result = await tools.get("bcdev_test_orchestrate")!.handler({ + codeunits: [{ id: 50100, methods: ["A"] }], + runs: 3, + }, { signal: controller.signal }) as { + runsAttempted: number; + complete: boolean; + warnings: string[]; + }; + + expect(result.runsAttempted).toBe(1); + expect(result.complete).toBe(false); + expect(result.warnings.join(" ")).toContain("Client cancellation was observed between attempts"); + expect(hubs).toHaveLength(1); + expect(state.testRunActive).toBe(false); + }); + + test("keeps source mapping nonfatal when the explicit project is unreadable", async () => { + const { tools } = setup({ + configureHub: (hub) => { + hub.onInvoke = (method) => { + if (method === "RunTests") { + queueMicrotask(() => { + hub.emit( + "TestCompleted", + 50100, + "A", + 1, + 'boom\nCallStack:\n"T"(CodeUnit 50100).A() line 7', + 1, + ); + hub.emit("TestRunCompleted", { Tests: [] }); + }); + } + return undefined; + }; + }, + }); + + const result = await tools.get("bcdev_test_orchestrate")!.handler({ + project: join(tmpdir(), "definitely-missing-orchestration-project"), + environmentType: "OnPrem", + server: "http://localhost", + serverInstance: "BC", + codeunits: [{ id: 50100, methods: ["A"] }], + runs: 2, + }) as { + outcome: string; + runs: Array<{ + sourceMappingWarning?: string; + results: Array<{ failure?: { callStack: Array<{ file: string | null }> } }>; + }>; + }; + + expect(result.outcome).toBe("failed"); + for (const run of result.runs) { + expect(run.sourceMappingWarning).toContain("call-stack file fields remain null"); + expect(run.results[0]?.failure?.callStack[0]?.file).toBeNull(); + } + }); + + test("claims before unsupported-server preflight and releases on failure", async () => { + const { state, hubs, tools } = setup({ + fetchFn: (async () => + new Response(JSON.stringify({ WebApiVersion: "6.0" }))) as unknown as typeof fetch, + }); + + const pending = tools.get("bcdev_test_orchestrate")!.handler({ + codeunits: [{ id: 50100 }], + runs: 2, + }); + expect(state.testRunActive).toBe(true); + const error = await pending.catch((caught) => caught); + expect(error).toMatchObject({ code: "UNSUPPORTED_SERVER", category: "server" }); + expect(state.testRunActive).toBe(false); + expect(hubs).toHaveLength(0); + }); + + test("publishes bounded repeat-count validation", () => { + const { tools } = setup(); + const schema = tools.get("bcdev_test_orchestrate")!.schema["runs"] as unknown as { + safeParse(value: unknown): { success: boolean }; + }; + expect(schema.safeParse(1).success).toBe(false); + expect(schema.safeParse(2).success).toBe(true); + expect(schema.safeParse(20).success).toBe(true); + expect(schema.safeParse(21).success).toBe(false); + }); + + test("rejects overlapping codeunit selections while allowing disjoint split groups", () => { + const { tools } = setup(); + const orchestrationSchema = tools.get("bcdev_test_orchestrate")!.schema["codeunits"] as unknown as { + safeParse(value: unknown): { success: boolean }; + }; + const directSchema = tools.get("bcdev_test_run")!.schema["codeunits"] as unknown as { + safeParse(value: unknown): { success: boolean }; + }; + const overlapping = [ + [{ id: 50100 }, { id: 50100, methods: ["A"] }], + [{ id: 50100, methods: ["A"] }, { id: 50100, methods: ["a"] }], + [{ id: 50100, methods: ["A", "A"] }], + ]; + for (const plan of overlapping) { + expect(orchestrationSchema.safeParse(plan).success).toBe(false); + expect(directSchema.safeParse(plan).success).toBe(false); + } + expect(orchestrationSchema.safeParse([ + { id: 50100, methods: ["A"] }, + { id: 50100, methods: ["B"] }, + { id: 50101 }, + ]).success).toBe(true); + }); +}); diff --git a/tests/mcp/tools.test.ts b/tests/mcp/tools.test.ts index b951fd1..3338bf8 100644 --- a/tests/mcp/tools.test.ts +++ b/tests/mcp/tools.test.ts @@ -66,7 +66,7 @@ describe("tools", () => { hub = new FakeHub(); }); - test("registers all 22 tools", () => { + test("registers all 23 tools", () => { const { tools } = setup(hub); expect([...tools.keys()].sort()).toEqual([ "bcdev_debug_attach", @@ -90,6 +90,7 @@ describe("tools", () => { "bcdev_source", "bcdev_status", "bcdev_test_discover", + "bcdev_test_orchestrate", "bcdev_test_run", ]); }); @@ -179,6 +180,7 @@ describe("tools", () => { await check("bcdev_status", {}); await check("bcdev_test_discover", {}); await check("bcdev_test_run", { codeunits: [{ id: 50100 }] }); + await check("bcdev_test_orchestrate", { codeunits: [{ id: 50100 }], runs: 2 }); await check("bcdev_debug_attach", { breakpoints: [] }); await check("bcdev_debug_variables", { frameId: 0 }); await check("bcdev_debug_eval", { frameId: 0, expression: "X" });