diff --git a/.github/workflows/bake-dataquery-gold.yml b/.github/workflows/bake-dataquery-gold.yml new file mode 100644 index 000000000..b9bed2786 --- /dev/null +++ b/.github/workflows/bake-dataquery-gold.yml @@ -0,0 +1,76 @@ +name: Bake Data-Query Gold Rows + +# Runs each data-query gold query once against a BC container and stores the resulting rows as +# gold_rows in dataset/dataquery.jsonl, then commits them back to the branch. Evaluation then compares +# the agent's answer to these baked rows instead of recompiling/running the gold query live per run. + +on: + workflow_dispatch: + inputs: + git-ref: + description: "Branch to bake and commit to" + required: false + default: "" + +permissions: + contents: write + +env: + CATEGORY: data-query + +jobs: + get-entries: + uses: ./.github/workflows/get-entries.yml + with: + category: data-query + + bake: + needs: get-entries + runs-on: ${{ needs.get-entries.outputs.runner }} + if: needs.get-entries.outputs.entries != '[]' + timeout-minutes: 120 + environment: + name: ado-read + deployment: false + permissions: + contents: write + id-token: write + steps: + - name: Checkout repository + uses: actions/checkout@v5 + with: + ref: ${{ inputs.git-ref || github.ref_name }} + + # Provision a single BC container (all data-query entries share the same version + dataset). + - name: Setup BC Container + id: setup-env + timeout-minutes: 40 + uses: ./.github/actions/setup-bc-container-repo + with: + instance-id: ${{ fromJson(needs.get-entries.outputs.entries)[0] }} + category: data-query + azure-client-id: ${{ secrets.AZURE_CLIENT_ID }} + azure-tenant-id: ${{ secrets.AZURE_TENANT_ID }} + github-token: ${{ secrets.GITHUB_TOKEN }} + skip-container: "false" + skip-repo: "true" + + - name: Setup Python with UV + uses: ./.github/actions/setup-python-uv + + - name: Bake gold rows + run: uv run bcbench dataset bake-dataquery-gold --repo-path "${{ steps.setup-env.outputs.repo_path }}" + + - name: Commit baked dataset + run: | + git config user.name "github-actions[bot]" + git config user.email "github-actions[bot]@users.noreply.github.com" + if (git diff --quiet -- dataset/dataquery.jsonl) { + Write-Output "No gold_rows changes to commit." + } + else { + git add dataset/dataquery.jsonl + git commit -m "Bake data-query gold rows (run ${{ github.run_id }})" + git push origin HEAD:${{ inputs.git-ref || github.ref_name }} + } + shell: pwsh diff --git a/.github/workflows/claude-evaluation.yml b/.github/workflows/claude-evaluation.yml index b61b64816..02cfd71f9 100644 --- a/.github/workflows/claude-evaluation.yml +++ b/.github/workflows/claude-evaluation.yml @@ -24,6 +24,7 @@ on: - "bug-fix" - "test-generation" - "code-review" + - "data-query" - "extensibility-request-implement" - "extensibility-request-triage" test-run: @@ -41,6 +42,21 @@ on: required: false default: false type: boolean + bc-mcp: + description: "Enable the Business Central MCP server" + required: false + default: false + type: boolean + ms-learn-mcp: + description: "Enable the Microsoft Learn MCP server" + required: false + default: false + type: boolean + skills: + description: "Enable agent skills" + required: false + default: false + type: boolean repeat: description: "Number of times to run sequentially (ignored for test runs)" required: false @@ -151,14 +167,28 @@ jobs: --repo-path "${{ steps.setup-env.outputs.repo_path }}" ` --output-dir "${{ env.EVALUATION_RESULTS_DIR }}" ` ${{ inputs.al-mcp && '--al-mcp' || '' }} ` - ${{ inputs.al-lsp && '--al-lsp' || '' }} + ${{ inputs.al-lsp && '--al-lsp' || '' }} ` + ${{ inputs.bc-mcp && '--bc-mcp' || '' }} ` + ${{ inputs.ms-learn-mcp && '--ms-learn-mcp' || '' }} ` + ${{ inputs.skills && '--skills' || '' }} + + - name: Capture BC NST logs + if: ${{ always() && inputs.bc-mcp }} + shell: pwsh + run: | + # TEMPORARY (diagnostic): pull NST event log + server config + MCP Configuration row to + # investigate why the BC MCP endpoint exposes zero Data Query tools. + .\scripts\Capture-NstLogs.ps1 -ContainerName "$env:BC_CONTAINER_NAME" -OutputDir "${{ env.EVALUATION_RESULTS_DIR }}" - name: Upload evaluation results uses: actions/upload-artifact@v6 if: always() with: name: evaluation-results-${{ github.run_id }}-${{ matrix.entry }} - path: ${{ env.EVALUATION_RESULTS_DIR }}/**/*.jsonl + # TEMPORARY (remove before merge): include agent *.log to diagnose BC MCP tool use. + path: | + ${{ env.EVALUATION_RESULTS_DIR }}/**/*.jsonl + ${{ env.EVALUATION_RESULTS_DIR }}/**/*.log retention-days: ${{ inputs.test-run && 1 || 30 }} summarize-results: @@ -188,4 +218,4 @@ jobs: repeat: ${{ inputs.repeat }} existing-tag: ${{ needs.pin-commit.outputs.tag-name }} workflow-inputs: | - {"model": "${{ inputs.model }}", "category": "${{ inputs.category }}", "test-run": "${{ inputs.test-run }}", "al-mcp": "${{ inputs.al-mcp }}", "al-lsp": "${{ inputs.al-lsp }}", "git-ref": "${{ inputs.git-ref || github.ref_name }}"} + {"model": "${{ inputs.model }}", "category": "${{ inputs.category }}", "test-run": "${{ inputs.test-run }}", "al-mcp": "${{ inputs.al-mcp }}", "al-lsp": "${{ inputs.al-lsp }}", "bc-mcp": "${{ inputs.bc-mcp }}", "ms-learn-mcp": "${{ inputs.ms-learn-mcp }}", "skills": "${{ inputs.skills }}", "git-ref": "${{ inputs.git-ref || github.ref_name }}"} diff --git a/.github/workflows/copilot-evaluation.yml b/.github/workflows/copilot-evaluation.yml index 5eeeeda84..081f0d0d8 100644 --- a/.github/workflows/copilot-evaluation.yml +++ b/.github/workflows/copilot-evaluation.yml @@ -29,6 +29,7 @@ on: - "bug-fix" - "test-generation" - "code-review" + - "data-query" - "extensibility-request-implement" - "extensibility-request-triage" test-run: @@ -46,6 +47,21 @@ on: required: false default: false type: boolean + bc-mcp: + description: "Enable the Business Central MCP server" + required: false + default: false + type: boolean + ms-learn-mcp: + description: "Enable the Microsoft Learn MCP server" + required: false + default: false + type: boolean + skills: + description: "Enable agent skills" + required: false + default: false + type: boolean repeat: description: "Number of times to run sequentially (ignored for test runs)" required: false @@ -143,7 +159,12 @@ jobs: timeout-minutes: 120 shell: pwsh env: - COPILOT_GITHUB_TOKEN: ${{ github.token }} + # Copilot CLI must authenticate MCP with a USER token: the Actions github.token (a ghs_ + # installation token) gets 403 from GET /copilot/mcp_registry and fails closed, blocking ALL + # custom MCP servers (github/copilot-cli#4346). The org policy is already allow_all, so a + # Copilot-licensed user PAT lets the registry fetch succeed and the BC/MS-Learn MCP load. + # Falls back to github.token when the secret is absent (MCP off, but completions still work). + COPILOT_GITHUB_TOKEN: ${{ secrets.COPILOT_CLI_TOKEN || github.token }} GH_TOKEN: ${{ github.token }} run: | Write-Output "::add-mask::$env:COPILOT_GITHUB_TOKEN" @@ -154,14 +175,20 @@ jobs: --repo-path "${{ steps.setup-env.outputs.repo_path }}" ` --output-dir "${{ env.EVALUATION_RESULTS_DIR }}" ` ${{ inputs.al-mcp && '--al-mcp' || '' }} ` - ${{ inputs.al-lsp && '--al-lsp' || '' }} + ${{ inputs.al-lsp && '--al-lsp' || '' }} ` + ${{ inputs.bc-mcp && '--bc-mcp' || '' }} ` + ${{ inputs.ms-learn-mcp && '--ms-learn-mcp' || '' }} ` + ${{ inputs.skills && '--skills' || '' }} - name: Upload evaluation results uses: actions/upload-artifact@v6 if: always() with: name: evaluation-results-${{ github.run_id }}-${{ matrix.entry }} - path: ${{ env.EVALUATION_RESULTS_DIR }}/**/*.jsonl + # TEMPORARY (remove before merge): include agent *.log to diagnose BC MCP tool use. + path: | + ${{ env.EVALUATION_RESULTS_DIR }}/**/*.jsonl + ${{ env.EVALUATION_RESULTS_DIR }}/**/*.log retention-days: ${{ inputs.test-run && 1 || 30 }} summarize-results: @@ -191,4 +218,4 @@ jobs: repeat: ${{ inputs.repeat }} existing-tag: ${{ needs.pin-commit.outputs.tag-name }} workflow-inputs: | - {"model": "${{ inputs.model }}", "category": "${{ inputs.category }}", "test-run": "${{ inputs.test-run }}", "al-mcp": "${{ inputs.al-mcp }}", "al-lsp": "${{ inputs.al-lsp }}", "git-ref": "${{ inputs.git-ref || github.ref_name }}"} + {"model": "${{ inputs.model }}", "category": "${{ inputs.category }}", "test-run": "${{ inputs.test-run }}", "al-mcp": "${{ inputs.al-mcp }}", "al-lsp": "${{ inputs.al-lsp }}", "bc-mcp": "${{ inputs.bc-mcp }}", "ms-learn-mcp": "${{ inputs.ms-learn-mcp }}", "skills": "${{ inputs.skills }}", "git-ref": "${{ inputs.git-ref || github.ref_name }}"} diff --git a/.gitignore b/.gitignore index 550b1da5d..67f6b383c 100644 --- a/.gitignore +++ b/.gitignore @@ -283,3 +283,6 @@ docs/Gemfile docs/Gemfile.lock *.orig + +# Local diagnostic artifact download folders (temp, leading underscore) +_*/ diff --git a/dataset/dataquery.jsonl b/dataset/dataquery.jsonl new file mode 100644 index 000000000..9d624cba2 --- /dev/null +++ b/dataset/dataquery.jsonl @@ -0,0 +1,11 @@ +{"instance_id": "dataquery__outstanding-sales-value-by-customer-1", "environment_setup_version": "29.0", "created_at": "2026-07-10", "metadata": {"area": "sales"}, "nl_prompt": "For each customer that has at least one open sales order line, return the customer's number, the customer's name, and their total outstanding amount. Use open sales order lines (Sales Line records whose document type is Order) and sum the line 'Outstanding Amount' field (which is net of VAT). Customers with no open sales order lines must not appear.", "ordered": false, "gold_query": "query 50100 OutstandingSalesByCustomer\n{\n QueryType = Normal;\n\n elements\n {\n dataitem(Customer; Customer)\n {\n column(No; \"No.\") { }\n column(Name; Name) { }\n dataitem(SalesLine; \"Sales Line\")\n {\n DataItemLink = \"Sell-to Customer No.\" = Customer.\"No.\";\n DataItemTableFilter = \"Document Type\" = const(Order);\n column(TotalOutstanding; \"Outstanding Amount\") { Method = Sum; }\n }\n }\n }\n}"} +{"instance_id": "dataquery__total-sold-quantity-by-item-1", "environment_setup_version": "29.0", "created_at": "2026-07-10", "metadata": {"area": "inventory"}, "nl_prompt": "Across all posted sales invoice lines whose type is Item, return each item's number together with the total quantity sold (the sum of the line Quantity). Produce one row per item number.", "ordered": false, "gold_query": "query 50100 SoldQuantityByItem\n{\n QueryType = Normal;\n\n elements\n {\n dataitem(SalesInvoiceLine; \"Sales Invoice Line\")\n {\n DataItemTableFilter = Type = const(Item);\n column(ItemNo; \"No.\") { }\n column(TotalQuantity; Quantity) { Method = Sum; }\n }\n }\n}"} +{"instance_id": "dataquery__avg-invoice-amount-by-country-1", "environment_setup_version": "29.0", "created_at": "2026-07-10", "metadata": {"area": "finance"}, "nl_prompt": "For each country/region, return the country/region code and the average posted sales invoice line amount. Average the 'Amount' field (net of VAT) over all posted sales invoice lines, grouping the lines by their bill-to customer's Country/Region Code.", "ordered": false, "gold_query": "query 50100 AvgInvoiceAmountByCountry\n{\n QueryType = Normal;\n\n elements\n {\n dataitem(Customer; Customer)\n {\n column(CountryRegionCode; \"Country/Region Code\") { }\n dataitem(SalesInvoiceHeader; \"Sales Invoice Header\")\n {\n DataItemLink = \"Bill-to Customer No.\" = Customer.\"No.\";\n dataitem(SalesInvoiceLine; \"Sales Invoice Line\")\n {\n DataItemLink = \"Document No.\" = SalesInvoiceHeader.\"No.\";\n column(AvgAmount; Amount) { Method = Average; }\n }\n }\n }\n }\n}"} +{"instance_id": "dataquery__total-purchase-amount-by-vendor-1", "environment_setup_version": "29.0", "created_at": "2026-07-10", "metadata": {"area": "purchasing"}, "nl_prompt": "For each vendor that has at least one posted purchase invoice line, return the vendor's number, the vendor's name, and their total posted purchase amount. Sum the 'Amount' field (net of VAT) from posted purchase invoice lines. Vendors with no posted purchase invoice lines must not appear.", "ordered": false, "gold_query": "query 50100 PurchaseAmountByVendor\n{\n QueryType = Normal;\n\n elements\n {\n dataitem(Vendor; Vendor)\n {\n column(No; \"No.\") { }\n column(Name; Name) { }\n dataitem(PurchInvLine; \"Purch. Inv. Line\")\n {\n DataItemLink = \"Buy-from Vendor No.\" = Vendor.\"No.\";\n column(TotalAmount; Amount) { Method = Sum; }\n }\n }\n }\n}"} +{"instance_id": "dataquery__open-sales-order-count-by-customer-1", "environment_setup_version": "29.0", "created_at": "2026-07-13", "metadata": {"area": "sales"}, "nl_prompt": "For each customer that has at least one open sales order, return the customer's number, the customer's name, and the number of open sales orders they have. An open sales order is a sales document whose document type is Order; count the order documents (headers), not the order lines. Customers with no open sales orders must not appear.", "ordered": false, "gold_query": "query 50100 OpenSalesOrdersByCustomer\n{\n QueryType = Normal;\n\n elements\n {\n dataitem(Customer; Customer)\n {\n column(No; \"No.\") { }\n column(Name; Name) { }\n dataitem(SalesHeader; \"Sales Header\")\n {\n DataItemLink = \"Sell-to Customer No.\" = Customer.\"No.\";\n DataItemTableFilter = \"Document Type\" = const(Order);\n column(OrderCount) { Method = Count; }\n }\n }\n }\n}"} +{"instance_id": "dataquery__opportunity-count-by-status-1", "environment_setup_version": "29.0", "created_at": "2026-07-10", "metadata": {"area": "crm"}, "nl_prompt": "Return the number of CRM opportunities in each status. Group the opportunities by their Status field and, for each status value that occurs, output the status and the count of opportunities with that status.", "ordered": false, "gold_query": "query 50100 OpportunityCountByStatus\n{\n QueryType = Normal;\n\n elements\n {\n dataitem(Opportunity; Opportunity)\n {\n column(Status; Status) { }\n column(OpportunityCount) { Method = Count; }\n }\n }\n}"} +{"instance_id": "dataquery__customer-count-by-country-1", "environment_setup_version": "29.0", "created_at": "2026-07-28", "metadata": {"area": "sales"}, "nl_prompt": "Group all customers by their Country/Region Code and return, for each distinct code, the Country/Region Code and the number of customers that have it. Include customers whose Country/Region Code is blank as their own group. Count the Customer records (one row per distinct Country/Region Code).", "ordered": false, "gold_query": "query 50100 CustomerCountByCountry\n{\n QueryType = Normal;\n\n elements\n {\n dataitem(Customer; Customer)\n {\n column(CountryRegionCode; \"Country/Region Code\") { }\n column(CustomerCount) { Method = Count; }\n }\n }\n}"} +{"instance_id": "dataquery__outstanding-purchase-value-by-vendor-1", "environment_setup_version": "29.0", "created_at": "2026-07-28", "metadata": {"area": "purchase"}, "nl_prompt": "For each vendor that has at least one open purchase order line, return the vendor's number, the vendor's name, and their total outstanding amount. Use open purchase order lines (Purchase Line records whose Document Type is Order) and sum the line 'Outstanding Amount' field (net of VAT). Vendors with no open purchase order lines must not appear.", "ordered": false, "gold_query": "query 50100 OutstandingPurchaseByVendor\n{\n QueryType = Normal;\n\n elements\n {\n dataitem(Vendor; Vendor)\n {\n column(No; \"No.\") { }\n column(Name; Name) { }\n dataitem(PurchaseLine; \"Purchase Line\")\n {\n DataItemLink = \"Buy-from Vendor No.\" = Vendor.\"No.\";\n DataItemTableFilter = \"Document Type\" = const(Order);\n column(TotalOutstanding; \"Outstanding Amount\") { Method = Sum; }\n }\n }\n }\n}"} +{"instance_id": "dataquery__total-posted-sales-amount-by-customer-1", "environment_setup_version": "29.0", "created_at": "2026-07-28", "metadata": {"area": "sales"}, "nl_prompt": "For each customer that has at least one posted sales invoice line, return the customer's number, the customer's name, and their total posted sales amount. Join posted sales invoice headers to their lines via Document No., group by the header's Bill-to Customer No., and sum the line 'Amount' field (net of VAT). Customers with no posted sales invoice lines must not appear.", "ordered": false, "gold_query": "query 50100 PostedSalesAmountByCustomer\n{\n QueryType = Normal;\n\n elements\n {\n dataitem(Customer; Customer)\n {\n column(No; \"No.\") { }\n column(Name; Name) { }\n dataitem(SalesInvoiceHeader; \"Sales Invoice Header\")\n {\n DataItemLink = \"Bill-to Customer No.\" = Customer.\"No.\";\n dataitem(SalesInvoiceLine; \"Sales Invoice Line\")\n {\n DataItemLink = \"Document No.\" = SalesInvoiceHeader.\"No.\";\n column(TotalAmount; Amount) { Method = Sum; }\n }\n }\n }\n }\n}"} +{"instance_id": "dataquery__line-count-per-open-sales-order-1", "environment_setup_version": "29.0", "created_at": "2026-07-28", "metadata": {"area": "sales"}, "nl_prompt": "For each open sales order, return the order's document number and the number of lines it has. Use Sales Line records whose Document Type is Order, group them by Document No., and count the lines per order (one row per order document number).", "ordered": false, "gold_query": "query 50100 LineCountPerOpenSalesOrder\n{\n QueryType = Normal;\n\n elements\n {\n dataitem(SalesLine; \"Sales Line\")\n {\n DataItemTableFilter = \"Document Type\" = const(Order);\n column(DocumentNo; \"Document No.\") { }\n column(LineCount) { Method = Count; }\n }\n }\n}"} +{"instance_id": "dataquery__total-purchased-quantity-by-item-1", "environment_setup_version": "29.0", "created_at": "2026-07-28", "metadata": {"area": "purchase"}, "nl_prompt": "Across all posted purchase invoice lines whose Type is Item, return each item's number together with the total purchased quantity (the sum of the line Quantity). Produce one row per item number.", "ordered": false, "gold_query": "query 50100 PurchasedQuantityByItem\n{\n QueryType = Normal;\n\n elements\n {\n dataitem(PurchInvLine; \"Purch. Inv. Line\")\n {\n DataItemTableFilter = Type = const(Item);\n column(ItemNo; \"No.\") { }\n column(TotalQuantity; Quantity) { Method = Sum; }\n }\n }\n}"} diff --git a/docs/data-query.md b/docs/data-query.md new file mode 100644 index 000000000..dc8a19c3a --- /dev/null +++ b/docs/data-query.md @@ -0,0 +1,46 @@ +--- +layout: default +title: Data Query - BC-Bench +--- + +# Data Query + +This category benchmarks an agent's ability to **generate Business Central AL queries** from a natural-language data question — an offline query-generation benchmark. There is **no MCP server and no live server in the loop**: the agent writes an AL query, and the query is evaluated deterministically. + +Given a question, the agent authors a single AL `query` object and writes it to `query.al`. The harness then **compiles and runs both the generated query and a gold reference query** against a fixed dataset (the BC container's Contoso demo data) and compares the result sets. + +## How it is scored + +Data Query is **execution-based** (like bug-fix), with no LLM judge: + +- **build** — the generated query compiled and ran. +- **resolved** (the headline `ResolutionRate`) — the generated query's result set **matches the gold query's**. Rows are compared by value (numbers normalized, column names/order ignored); row order is ignored unless the entry marks the question as `ordered`. + +To execute a query, the harness wraps it as an API query (injecting `APIPublisher`/`APIGroup`/`APIVersion`/`EntitySetName`), publishes a throwaway app to the container, and reads the query's OData endpoint. This runs on the `GitHub-BCBench` self-hosted runner (`requires_container = True`). + +> This complements the AI Test Toolkit evals in the BC platform repo: those test the **MCP server** end-to-end, while BC-Bench benchmarks **models/agents** on query generation. + +## Dataset + +Each entry has an `nl_prompt` (the question) and a `gold_query` (the reference AL query whose result set defines "correct"), plus `environment_setup_version` (the BC artifact) and `ordered`. See `dataset/dataquery.jsonl`. + +## Running it (no local containers) + +Trigger it from the GitHub **Actions** tab — the self-hosted `GitHub-BCBench` runner provisions the BC container for you (a stock **sandbox artifact with Cronus/Contoso demo data** — no special build is needed, since the query is just compiled and run): + +1. Actions → **Evaluation with GitHub Copilot** (or **Evaluation with Claude Code**) → **Run workflow**. +2. Set **category** = `data-query`, pick a **model**, leave **test-run** = `true` for a quick 2-entry run. +3. The run: provisions the container → the agent writes `query.al` → the harness compiles + runs the generated and gold queries → compares result sets → `summarize-results` reports `ResolutionRate` / `BuildRate`. + +`data-query` sets `requires_container = True`; its container setup **skips the repo clone** (there is no repo — the agent generates from scratch) and just stands up the sandbox container. + +### Local (optional) + +```bash +uv run bcbench evaluate copilot dataquery__outstanding-sales-value-by-customer-1 \ + --category data-query --container-name --username admin --password +``` + +The optional `al-mcp` / `al-lsp` levers give the agent AL compiler/language-server feedback while it authors the query. + +[← Back to Home](index.md) diff --git a/docs/index.md b/docs/index.md index a34539ef0..d49dfc990 100644 --- a/docs/index.md +++ b/docs/index.md @@ -12,6 +12,7 @@ A benchmark for evaluating AI coding agents on real-world **Business Central (AL | [Bug Fixing](bug-fix.md) | Follows [SWE-Bench](https://www.swebench.com/) methodology to evaluate bug fixing in AL code | | [Test Generation](test-generation.md) | "Reverses" SWE-Bench: Generates reproduction tests (TDD) instead of fixes | | [Code Review](code-review.md) | Reviews AL pull requests; scored with Precision / Recall / F1 against gold findings | +| [Data Query](data-query.md) | Generates AL queries from natural-language data questions; scored deterministically by running the query and comparing its result set to a gold query | ## Diagnostics diff --git a/scripts/BCBenchUtils.psm1 b/scripts/BCBenchUtils.psm1 index e3baf0b2d..9b0f23436 100644 --- a/scripts/BCBenchUtils.psm1 +++ b/scripts/BCBenchUtils.psm1 @@ -490,7 +490,7 @@ function Get-BCBenchDatasetPath { param( [Parameter(Mandatory = $true)] # Category validation lives only here: every caller resolves the dataset path through this function, so there's no need to duplicate ValidateSet on each caller. - [ValidateSet("bug-fix", "test-generation", "code-review", "nl2al", "extensibility-request-implement", "extensibility-request-triage")] + [ValidateSet("bug-fix", "test-generation", "code-review", "nl2al", "data-query", "extensibility-request-implement", "extensibility-request-triage")] [string] $Category ) @@ -499,6 +499,7 @@ function Get-BCBenchDatasetPath { "test-generation" { $DatasetName = "bcbench.jsonl" } "code-review" { $DatasetName = "codereview.jsonl" } "nl2al" { $DatasetName = "nl2al.jsonl" } + "data-query" { $DatasetName = "dataquery.jsonl" } "extensibility-request-implement" { $DatasetName = "extensibility_request_implement.jsonl" } "extensibility-request-triage" { $DatasetName = "extensibility_request_triage.jsonl" } } diff --git a/scripts/BCContainerManagement.psm1 b/scripts/BCContainerManagement.psm1 index ef8aaad51..a59bf5892 100644 --- a/scripts/BCContainerManagement.psm1 +++ b/scripts/BCContainerManagement.psm1 @@ -302,6 +302,8 @@ function New-BCContainerSync { shortcuts = 'None' memoryLimit = "16G" isolation = "hyperv" + # TEMPORARY (remove before merge): required to build a container from an insider (BC 29) artifact. + accept_insiderEula = $true } if ($AcceptEula) { @@ -349,4 +351,85 @@ function New-BCCompilerFolderSync { Write-Log "Compiler folder created at: $compilerFolder" -Level Success } -Export-ModuleMember -Function Test-Database, Set-AppVersion, Move-AppIntoDevScope, Initialize-ContainerForDevelopment, Test-ContainerExists, New-BCContainerSync, New-BCCompilerFolderSync +function Publish-MCPConfigApp { + [CmdletBinding()] + param( + [Parameter(Mandatory = $true)] + [string]$ContainerName, + + [Parameter(Mandatory = $true)] + [string]$Version, + + [Parameter(Mandatory = $true)] + [PSCredential]$Credential, + + [Parameter(Mandatory = $true)] + [string]$BuildRoot + ) + + Import-Module "$PSScriptRoot\AppUtils.psm1" -Force -DisableNameChecking + + [int]$major = ([System.Version]$Version).Major + [string]$sourceFolder = Join-Path $PSScriptRoot "al\mcp-config-setup" + # Build inside BuildRoot: Compile-AppInBcContainer only accepts a project folder that is shared with + # the container, and BuildRoot ($RepoPath) is the folder mounted into it. Cleaned up after publish so + # nothing leaks into the agent's workspace. + [string]$buildFolder = Join-Path $BuildRoot ".bcbench-mcp-config-app" + + if (Test-Path $buildFolder) { + Remove-Item -Path $buildFolder -Recurse -Force + } + Copy-Item -Path $sourceFolder -Destination $buildFolder -Recurse -Force + + # The source keeps a placeholder so the same app builds against any evaluated BC version; + # pin the app/platform dependency to the container's major version at publish time. + [string]$appJsonPath = Join-Path $buildFolder "app.json" + (Get-Content -Path $appJsonPath -Raw).Replace("__APP_VERSION__", "$major.0.0.0") | Set-Content -Path $appJsonPath -Encoding UTF8 + + try { + Write-Log "Publishing BC-Bench MCP config app to provision the MCP configuration..." -Level Info + Invoke-AppBuildAndPublish -containerName $ContainerName -appProjectFolder $buildFolder -credential $Credential -skipVerification -useDevEndpoint + Write-Log "BC MCP configuration 'BCBench' provisioned and activated." -Level Success + } + finally { + Remove-Item -Path $buildFolder -Recurse -Force -ErrorAction SilentlyContinue + } +} + +function Get-BCMCPConnectionInfo { + [CmdletBinding()] + param( + [Parameter(Mandatory = $true)] + [string]$ContainerName + ) + + # The evaluated agent runs on the host, not inside the container, and the runner does not update + # its hosts file -- so reach the BC web listener by the container's IP address. + [string]$ip = Get-BcContainerIpAddress -containerName $ContainerName + if (-not $ip) { + throw "Could not resolve IP address for container $ContainerName; BC MCP endpoint is unreachable from the host." + } + + # BcContainerHelper containers always serve the 'BC' instance on port 7048; the MCP endpoint hangs + # off the same base as the API endpoint the query harness already uses (base + '/mcp'). + [string]$baseUrl = "http://${ip}:7048/BC" + + $companies = @(Get-CompanyInBcContainer -containerName $ContainerName) + $evaluationCompany = $companies | Where-Object { $_.EvaluationCompany } | Select-Object -First 1 + if (-not $evaluationCompany) { + $evaluationCompany = $companies | Select-Object -First 1 + } + + # BcContainerHelper has exposed the company name as either CompanyName or Name across versions. + [string]$company = $evaluationCompany.CompanyName + if (-not $company) { + $company = $evaluationCompany.Name + } + + return [PSCustomObject]@{ + BaseUrl = $baseUrl + Company = $company + } +} + +Export-ModuleMember -Function Test-Database, Set-AppVersion, Move-AppIntoDevScope, Initialize-ContainerForDevelopment, Test-ContainerExists, New-BCContainerSync, New-BCCompilerFolderSync, Publish-MCPConfigApp, Get-BCMCPConnectionInfo diff --git a/scripts/Capture-NstLogs.ps1 b/scripts/Capture-NstLogs.ps1 new file mode 100644 index 000000000..cfa0827f0 --- /dev/null +++ b/scripts/Capture-NstLogs.ps1 @@ -0,0 +1,105 @@ +<# +.SYNOPSIS + Capture BC Server (NST) diagnostics from a running BcContainerHelper container. + + Best-effort and non-fatal: pulls the container's Windows Application event log (NAV/MCP entries), + the NST server configuration, and the persisted "MCP Configuration" row so we can see why the BC + MCP endpoint exposes zero Data Query tools. Writes .log files into -OutputDir for artifact upload. +#> +param( + [Parameter(Mandatory)][string]$ContainerName, + [Parameter(Mandatory)][string]$OutputDir +) + +$ErrorActionPreference = 'Continue' + +function Write-Diag($message) { Write-Host "[Capture-NstLogs] $message" } + +if (-not $ContainerName) { + Write-Diag 'No container name provided; skipping NST log capture.' + return +} + +New-Item -ItemType Directory -Force -Path $OutputDir | Out-Null + +# 1. NAV/MCP-related Application event log entries from inside the container. +try { + Write-Diag "Capturing NST Application event log from '$ContainerName'..." + $events = Invoke-ScriptInBcContainer -containerName $ContainerName -scriptblock { + Get-WinEvent -LogName Application -MaxEvents 3000 -ErrorAction SilentlyContinue | + Where-Object { $_.ProviderName -match 'NAV|Dynamics' -or $_.Message -match 'MCP|AlQuery|Data.?Query|query tool|tools/list' } | + Sort-Object TimeCreated | + ForEach-Object { '{0:o} [{1}] {2} (Id {3}): {4}' -f $_.TimeCreated, $_.LevelDisplayName, $_.ProviderName, $_.Id, ($_.Message -replace '\r?\n', ' ') } + } + ($events | Out-String) | Out-File -FilePath (Join-Path $OutputDir 'nst-eventlog.log') -Encoding utf8 + Write-Diag "Captured $(@($events).Count) NAV/MCP event(s)." +} +catch { + "Event log capture failed: $_" | Out-File -FilePath (Join-Path $OutputDir 'nst-eventlog.log') -Encoding utf8 + Write-Diag "Event log capture failed: $_" +} + +# 2. Discover any dedicated NAV/MCP event channels (in case MCP logs somewhere other than Application). +try { + $channels = Invoke-ScriptInBcContainer -containerName $ContainerName -scriptblock { + Get-WinEvent -ListLog * -ErrorAction SilentlyContinue | + Where-Object { $_.LogName -match 'NAV|Dynamics|MCP' } | + ForEach-Object { '{0} (records: {1})' -f $_.LogName, $_.RecordCount } + } + ($channels | Out-String) | Out-File -FilePath (Join-Path $OutputDir 'nst-eventchannels.log') -Encoding utf8 +} +catch { + Write-Diag "Event channel discovery failed: $_" +} + +# 3. NST server configuration (feature keys / MCP-related settings). +try { + Write-Diag 'Capturing NST server configuration...' + $config = Get-BcContainerServerConfiguration -containerName $ContainerName + ($config | Format-List * | Out-String) | Out-File -FilePath (Join-Path $OutputDir 'nst-serverconfig.log') -Encoding utf8 +} +catch { + Write-Diag "Server configuration capture failed: $_" +} + +# 4. Persisted "MCP Configuration" row(s) - confirms whether EnableAlQueryTools actually got set. +try { + Write-Diag 'Querying the MCP Configuration table...' + $config = Get-BcContainerServerConfiguration -containerName $ContainerName + $dbServer = $config.DatabaseServer + $dbInstance = $config.DatabaseInstance + $dbName = $config.DatabaseName + $server = if ($dbInstance) { "$dbServer\$dbInstance" } else { $dbServer } + + $rows = Invoke-ScriptInBcContainer -containerName $ContainerName -scriptblock { + param($sqlServer, $database) + $table = (sqlcmd -S $sqlServer -d $database -h -1 -W -Q "SET NOCOUNT ON; SELECT TOP 1 name FROM sys.tables WHERE name LIKE '%MCP Configuration%'" 2>&1 | ForEach-Object { "$_".Trim() } | Where-Object { $_ -and $_ -notmatch 'rows affected' } | Select-Object -First 1) + if ($table) { + "Table: $table" + '--- columns ---' + sqlcmd -S $sqlServer -d $database -h -1 -Q "SET NOCOUNT ON; SELECT c.name FROM sys.columns c JOIN sys.tables t ON c.object_id = t.object_id WHERE t.name = '$table' ORDER BY c.column_id" 2>&1 + '--- row(s) (pipe-separated) ---' + sqlcmd -S $sqlServer -d $database -s "|" -W -Q "SELECT * FROM [dbo].[$table]" 2>&1 + } + else { + 'No table matching %MCP Configuration% found in the tenant database.' + } + } -argumentList $server, $dbName + ($rows | Out-String) | Out-File -FilePath (Join-Path $OutputDir 'mcp-configuration.log') -Encoding utf8 +} +catch { + Write-Diag "MCP Configuration query failed: $_" +} + +# 5. Install status of the MCP Config Setup app (published != installed; OnInstall must have run). +try { + Write-Diag 'Checking MCP Config Setup app install status...' + $apps = Get-BcContainerAppInfo -containerName $ContainerName -tenant default -tenantSpecificProperties | + Where-Object { $_.Name -match 'MCP' } + ($apps | Format-List Name, Publisher, Version, IsInstalled, IsPublished, SyncState | Out-String) | Out-File -FilePath (Join-Path $OutputDir 'mcp-app-status.log') -Encoding utf8 +} +catch { + Write-Diag "App install status check failed: $_" +} + +Write-Diag 'NST diagnostics capture complete.' diff --git a/scripts/Setup-ContainerAndRepository.ps1 b/scripts/Setup-ContainerAndRepository.ps1 index b6efd0179..b05203cbf 100644 --- a/scripts/Setup-ContainerAndRepository.ps1 +++ b/scripts/Setup-ContainerAndRepository.ps1 @@ -72,7 +72,10 @@ if (-not $SkipRepo) { Invoke-GitCloneWithRetry -RepoUrl $cloneInfo.Url -Token $cloneInfo.Token -ClonePath $RepoPath -CommitSha $commitSha -SparseCheckoutPaths $cloneInfo.SparseCheckoutPaths } else { - Write-Log "Skipping repository clone (SkipRepo flag set)" -Level Info + # Categories that scaffold their own workspace still need the folder to exist: it is shared into + # the container below, and Compile-AppInBcContainer throws for any path not shared with it. + Write-Log "Skipping repository clone (SkipRepo flag set); creating empty workspace at $RepoPath" -Level Info + New-Item -ItemType Directory -Path $RepoPath -Force | Out-Null } if (-not $SkipContainer) { @@ -88,8 +91,9 @@ if (-not $SkipContainer) { Write-Log "Creating container $ContainerName for version $Version..." -Level Info - # Get BC artifact URL - [string] $url = Get-BCArtifactUrl -version $Version -Country $Country + # TEMPORARY (remove before merge): Data Query tools only exist in BC 29, which is not GA on the + # public feed yet, so pull the sandbox artifact from the insider feed. + [string] $url = Get-BCArtifactUrl -version $Version -Country $Country -select Latest -storageAccount bcinsider -accept_insiderEula Write-Log "Retrieved artifact URL: $url" -Level Info # Create container synchronously with NAV folder shared @@ -99,6 +103,21 @@ if (-not $SkipContainer) { New-BCCompilerFolderSync -ContainerName $ContainerName -ArtifactUrl $url Initialize-ContainerForDevelopment -ContainerName $ContainerName -RepoVersion ([System.Version]$Version) + + # data-query benchmarks the agent WITH the BC MCP server as a feedback loop. Publish the install + # app that provisions and activates the 'BCBench' MCP configuration, then expose the endpoint and + # company to the agent step so it can point its MCP client at the container. + if ($Category -eq 'data-query') { + Publish-MCPConfigApp -ContainerName $ContainerName -Version $Version -Credential $credential -BuildRoot $RepoPath + + $mcpInfo = Get-BCMCPConnectionInfo -ContainerName $ContainerName + Write-Log "BC MCP base URL: $($mcpInfo.BaseUrl) (company '$($mcpInfo.Company)')" -Level Info + + if ($env:GITHUB_ENV) { + "BC_MCP_URL=$($mcpInfo.BaseUrl)" | Out-File -FilePath $env:GITHUB_ENV -Append + "BC_MCP_COMPANY=$($mcpInfo.Company)" | Out-File -FilePath $env:GITHUB_ENV -Append + } + } } else { Write-Log "Skipping BC container setup (SkipContainer flag set)" -Level Info diff --git a/scripts/al/mcp-config-setup/MCPConfigSetup.Codeunit.al b/scripts/al/mcp-config-setup/MCPConfigSetup.Codeunit.al new file mode 100644 index 000000000..66226140b --- /dev/null +++ b/scripts/al/mcp-config-setup/MCPConfigSetup.Codeunit.al @@ -0,0 +1,34 @@ +namespace BCBench.MCP; + +using System.MCP; + +// Installed at container-setup time (not part of the benchmarked workspace). Provisions and activates +// the MCP configuration the evaluated agent connects to over the BC MCP server; this app is the single +// place that decides which server capabilities the eval exposes. Idempotent: re-installs reuse the +// existing configuration by name. +codeunit 50150 "BCBench MCP Config Setup" +{ + Subtype = Install; + + trigger OnInstallAppPerCompany() + begin + EnsureConfiguration(); + end; + + local procedure EnsureConfiguration() + var + MCPConfig: Codeunit "MCP Config"; + ConfigId: Guid; + begin + ConfigId := MCPConfig.GetConfigurationIdByName(ConfigNameTok); + if IsNullGuid(ConfigId) then + ConfigId := MCPConfig.CreateConfiguration(ConfigNameTok, ConfigDescriptionTok); + + MCPConfig.EnableDataQueryTools(ConfigId, true); + MCPConfig.ActivateConfiguration(ConfigId, true); + end; + + var + ConfigNameTok: Label 'BCBench', Locked = true; + ConfigDescriptionTok: Label 'BC-Bench evaluation', Locked = true; +} diff --git a/scripts/al/mcp-config-setup/app.json b/scripts/al/mcp-config-setup/app.json new file mode 100644 index 000000000..f8ad27ed0 --- /dev/null +++ b/scripts/al/mcp-config-setup/app.json @@ -0,0 +1,19 @@ +{ + "id": "9f3d6b6e-4c2a-4d3f-9b7a-2f1e8c5a7d10", + "name": "BC-Bench MCP Config Setup", + "publisher": "BC-Bench", + "version": "1.0.0.0", + "brief": "Provisions the BC MCP configuration for BC-Bench evaluation.", + "description": "Installed at container-setup time to provision and activate the 'BCBench' Business Central MCP configuration the evaluated agent connects to.", + "application": "__APP_VERSION__", + "platform": "__APP_VERSION__", + "idRanges": [ + { + "from": 50150, + "to": 50159 + } + ], + "runtime": "13.0", + "target": "OnPrem", + "dependencies": [] +} diff --git a/src/bcbench/agent/claude/agent.py b/src/bcbench/agent/claude/agent.py index 1bc7bceb2..892bf4ce4 100644 --- a/src/bcbench/agent/claude/agent.py +++ b/src/bcbench/agent/claude/agent.py @@ -1,13 +1,11 @@ -import json -import os import shutil import subprocess from pathlib import Path import yaml -from bcbench.agent.claude.metrics import parse_metrics -from bcbench.agent.shared import build_al_lsp_plugin, build_mcp_config, build_prompt, parse_tool_usage_from_hooks, resolve_config_plugins +from bcbench.agent.claude.metrics import parse_stream_output +from bcbench.agent.shared import agent_subprocess_env, build_al_lsp_plugin, build_mcp_config, build_prompt, parse_tool_usage_from_hooks, resolve_config_plugins, start_bc_mcp_gateway from bcbench.config import get_config from bcbench.dataset import BaseDatasetEntry from bcbench.exceptions import AgentError, AgentTimeoutError @@ -27,6 +25,9 @@ def run_claude_code( output_dir: Path, al_mcp: bool = False, al_lsp: bool = False, + bc_mcp: bool = False, + ms_learn_mcp: bool = False, + skills: bool = False, container_name: str = "bcbench", ) -> tuple[AgentMetrics | None, ExperimentConfiguration]: """Run Claude Code on a single dataset entry. @@ -44,10 +45,20 @@ def run_claude_code( logger.info(f"Running Claude Code on: {entry.instance_id}") prompt: str = build_prompt(entry, repo_path, claude_config, category, al_mcp=al_mcp) - mcp_config_json, mcp_server_names = build_mcp_config(claude_config, entry, repo_path, al_mcp=al_mcp, container_name=container_name) + bc_gateway = start_bc_mcp_gateway(bc_mcp) + mcp_config_json, mcp_server_names = build_mcp_config( + claude_config, + entry, + repo_path, + al_mcp=al_mcp, + bc_mcp=bc_mcp, + ms_learn_mcp=ms_learn_mcp, + container_name=container_name, + bc_mcp_gateway_url=bc_gateway.base_url if bc_gateway else None, + ) lsp_plugin_dir: Path | None = build_al_lsp_plugin(entry, category, repo_path, AgentHarness.CLAUDE, al_lsp=al_lsp, container_name=container_name) instructions_enabled: bool = setup_instructions_from_config(claude_config, entry, repo_path, harness=AgentHarness.CLAUDE) - skills_enabled: bool = setup_agent_skills(claude_config, entry, repo_path, harness=AgentHarness.CLAUDE) + skills_enabled: bool = setup_agent_skills(claude_config, entry, repo_path, harness=AgentHarness.CLAUDE, skills_enabled_override=skills) custom_agent: str | None = setup_custom_agent(claude_config, entry, repo_path, harness=AgentHarness.CLAUDE) tool_log_path: Path = setup_hooks(repo_path, AgentHarness.CLAUDE, output_dir) plugins: list[tuple[PluginConfig, Path]] = resolve_config_plugins(claude_config, allow_copilot_manifest=False) @@ -67,7 +78,8 @@ def run_claude_code( try: cmd_args = [ claude_cmd, - "--output-format=json", + "--output-format=stream-json", # emit every event (incl. tool_use, session init) as JSONL + "--verbose", # required for stream-json in --print mode "--strict-mcp-config", # Only use MCP servers from --mcp-config, ignoring all other MCP configurations "--setting-sources=project,local", f"--model={model}", @@ -101,10 +113,17 @@ def run_claude_code( result = subprocess.run( cmd_args, cwd=str(repo_path), - env={ - **os.environ, - "CLAUDE_CODE_DISABLE_AUTO_MEMORY": "1", - }, + env=agent_subprocess_env( + { + "CLAUDE_CODE_DISABLE_AUTO_MEMORY": "1", + # BC MCP's first tools/list compiles the tool catalog and can take ~45s on a cold + # container, well past Claude's 30s default MCP startup timeout -> the server is + # marked "failed" and its tools never register. Raise both the connection and tool + # execution timeouts so the slow first response is tolerated. + "MCP_TIMEOUT": "180000", + "MCP_TOOL_TIMEOUT": "180000", + } + ), timeout=_config.timeout.agent_execution, check=True, capture_output=True, @@ -113,21 +132,18 @@ def run_claude_code( stdout: str = result.stdout.decode("utf-8", errors="replace") if result.stdout else "" logger.debug(f"Claude Code raw output: {stdout}") - metrics = None - for line in stdout.splitlines(): - striped_line: str = line.strip() - if striped_line: - try: - data = json.loads(striped_line) - if "result" in data: - logger.info(data["result"]) - metrics = parse_metrics(data) - except json.JSONDecodeError: - logger.warning(f"Skipping non-JSON line: {striped_line}") - - tool_usage: dict[str, int] | None = parse_tool_usage_from_hooks(tool_log_path) - if metrics and tool_usage: - metrics = metrics.model_copy(update={"tool_usage": tool_usage}) + # Persist the full event stream for transcript analysis (the workflow uploads *.log artifacts). + transcript_path = output_dir / f"claude-transcript-{entry.instance_id}.log" + transcript_path.write_text(stdout, encoding="utf-8") + + metrics, final_response = parse_stream_output(stdout.splitlines()) + if final_response: + logger.info(final_response) + + # The stream's tool_use events capture sub-agent and MCP tool calls; fall back to the pre-tool-use + # hook only when the stream carried none. + if metrics and not metrics.tool_usage and (hook_tool_usage := parse_tool_usage_from_hooks(tool_log_path)): + metrics = metrics.model_copy(update={"tool_usage": hook_tool_usage}) except subprocess.TimeoutExpired: logger.exception(f"Claude Code timed out after {_config.timeout.agent_execution} seconds") metrics = AgentMetrics(execution_time=_config.timeout.agent_execution) @@ -140,3 +156,6 @@ def run_claude_code( raise else: return metrics, config + finally: + if bc_gateway is not None: + bc_gateway.stop() diff --git a/src/bcbench/agent/claude/metrics.py b/src/bcbench/agent/claude/metrics.py index f4ee6a3d7..bd4f1321e 100644 --- a/src/bcbench/agent/claude/metrics.py +++ b/src/bcbench/agent/claude/metrics.py @@ -1,3 +1,7 @@ +import json +from collections import Counter +from collections.abc import Sequence + from bcbench.logger import get_logger from bcbench.types import AgentMetrics @@ -41,3 +45,60 @@ def parse_metrics(data: dict) -> AgentMetrics | None: logger.warning("No metrics found in Claude Code output") return None + + +def parse_stream_output(output_lines: Sequence[str]) -> tuple[AgentMetrics | None, str | None]: + """Parse metrics + final response from `claude --output-format=stream-json --verbose` (JSONL) stdout. + + Event shapes (Claude Code): + system/init: lists the `tools` and `mcp_servers` registered for the session; logged so a run + shows whether the BC MCP tools actually connected in-session. + assistant: ``message.content`` holds ``tool_use`` blocks whose ``name`` (e.g. + ``mcp__bcmcp__bc_data_query``) captures sub-agent and MCP tool calls the pre-tool-use hook + never sees. + result: terminal event carrying duration/turns/usage and the final ``result`` text. + + Returns: + The parsed metrics (with tool usage from the stream) and the agent's final response text. + """ + tool_usage: Counter[str] = Counter() + final_response: str | None = None + metrics: AgentMetrics | None = None + + for line_number, line in enumerate(output_lines, start=1): + if not line.strip(): + continue + + try: + event = json.loads(line) + except json.JSONDecodeError as error: + logger.warning(f"Skipping invalid JSON from Claude Code output at line {line_number}: {error}") + continue + + if not isinstance(event, dict): + continue + + match event.get("type"): + case "system" if event.get("subtype") == "init": + servers = event.get("mcp_servers") + tools = event.get("tools") + tool_count = len(tools) if isinstance(tools, list) else "?" + logger.info(f"Claude session init: mcp_servers={servers}, {tool_count} tools registered") + case "assistant": + message = event.get("message") + if isinstance(message, dict): + for block in message.get("content", []): + if isinstance(block, dict) and block.get("type") == "tool_use": + name = block.get("name") + if isinstance(name, str) and name: + tool_usage[name] += 1 + case "result": + metrics = parse_metrics(event) + result_text = event.get("result") + if isinstance(result_text, str) and result_text: + final_response = result_text + + if tool_usage: + metrics = (metrics or AgentMetrics()).model_copy(update={"tool_usage": dict(tool_usage)}) + + return metrics, final_response diff --git a/src/bcbench/agent/copilot/agent.py b/src/bcbench/agent/copilot/agent.py index e7c63be85..78a68aac8 100644 --- a/src/bcbench/agent/copilot/agent.py +++ b/src/bcbench/agent/copilot/agent.py @@ -1,6 +1,5 @@ """GitHub Copilot CLI Agent implementation.""" -import os import subprocess import sys from pathlib import Path @@ -8,7 +7,7 @@ import yaml from bcbench.agent.copilot.metrics import parse_output -from bcbench.agent.shared import build_al_lsp_plugin, build_mcp_config, build_prompt, parse_tool_usage_from_hooks, resolve_config_plugins +from bcbench.agent.shared import agent_subprocess_env, build_al_lsp_plugin, build_mcp_config, build_prompt, parse_tool_usage_from_hooks, resolve_config_plugins, start_bc_mcp_gateway from bcbench.config import get_config from bcbench.copilot_cli import find_copilot from bcbench.dataset import BaseDatasetEntry @@ -29,6 +28,9 @@ def run_copilot_agent( output_dir: Path, al_mcp: bool = False, al_lsp: bool = False, + bc_mcp: bool = False, + ms_learn_mcp: bool = False, + skills: bool = False, container_name: str = "bcbench", ) -> tuple[AgentMetrics | None, ExperimentConfiguration]: """Run GitHub Copilot CLI agent on a single dataset entry. @@ -48,10 +50,20 @@ def run_copilot_agent( logger.info(f"Running GitHub Copilot CLI on: {entry.instance_id}") prompt: str = build_prompt(entry, repo_path, copilot_config, category, al_mcp=al_mcp) - mcp_config_json, mcp_server_names = build_mcp_config(copilot_config, entry, repo_path, al_mcp=al_mcp, container_name=container_name) + bc_gateway = start_bc_mcp_gateway(bc_mcp) + mcp_config_json, mcp_server_names = build_mcp_config( + copilot_config, + entry, + repo_path, + al_mcp=al_mcp, + bc_mcp=bc_mcp, + ms_learn_mcp=ms_learn_mcp, + container_name=container_name, + bc_mcp_gateway_url=bc_gateway.base_url if bc_gateway else None, + ) lsp_plugin_dir: Path | None = build_al_lsp_plugin(entry, category, repo_path, AgentHarness.COPILOT, al_lsp=al_lsp, container_name=container_name) instructions_enabled: bool = setup_instructions_from_config(copilot_config, entry, repo_path, harness=AgentHarness.COPILOT) - skills_enabled: bool = setup_agent_skills(copilot_config, entry, repo_path, harness=AgentHarness.COPILOT) + skills_enabled: bool = setup_agent_skills(copilot_config, entry, repo_path, harness=AgentHarness.COPILOT, skills_enabled_override=skills) custom_agent: str | None = setup_custom_agent(copilot_config, entry, repo_path, harness=AgentHarness.COPILOT) tool_log_path: Path = setup_hooks(repo_path, AgentHarness.COPILOT, output_dir) plugins: list[tuple[PluginConfig, Path]] = resolve_config_plugins(copilot_config, allow_copilot_manifest=True) @@ -99,11 +111,12 @@ def run_copilot_agent( result = subprocess.run( cmd_args, cwd=str(repo_path), - env={ - **os.environ, - "GITHUB_COPILOT_PROMPT_MODE_REPO_HOOKS": "true", - "GITHUB_COPILOT_PROMPT_MODE_WORKSPACE_MCP": "true", - }, + env=agent_subprocess_env( + { + "GITHUB_COPILOT_PROMPT_MODE_REPO_HOOKS": "true", + "GITHUB_COPILOT_PROMPT_MODE_WORKSPACE_MCP": "true", + } + ), capture_output=True, timeout=_config.timeout.agent_execution, check=True, @@ -119,9 +132,11 @@ def run_copilot_agent( if final_response: logger.info(final_response) - tool_usage: dict[str, int] | None = parse_tool_usage_from_hooks(tool_log_path) - if metrics and tool_usage: - metrics = metrics.model_copy(update={"tool_usage": tool_usage}) + # Tool usage now comes from the JSON event stream (tool.execution_start), which — unlike the + # pre-tool-use hook — also captures sub-agent and MCP tool calls. Fall back to the hook only if + # the stream carried none. + if metrics and not metrics.tool_usage and (hook_tool_usage := parse_tool_usage_from_hooks(tool_log_path)): + metrics = metrics.model_copy(update={"tool_usage": hook_tool_usage}) except subprocess.TimeoutExpired: logger.exception(f"Copilot CLI timed out after {_config.timeout.agent_execution} seconds") metrics = AgentMetrics(execution_time=_config.timeout.agent_execution) @@ -134,3 +149,6 @@ def run_copilot_agent( raise else: return metrics, config + finally: + if bc_gateway is not None: + bc_gateway.stop() diff --git a/src/bcbench/agent/copilot/metrics.py b/src/bcbench/agent/copilot/metrics.py index e268335c8..677cb0bb6 100644 --- a/src/bcbench/agent/copilot/metrics.py +++ b/src/bcbench/agent/copilot/metrics.py @@ -1,4 +1,5 @@ import json +from collections import Counter from collections.abc import Sequence from bcbench.logger import get_logger @@ -21,11 +22,25 @@ def _milliseconds_to_seconds(value: object) -> float | None: return None if milliseconds is None else milliseconds / 1000.0 +def _tool_label(data: dict) -> str | None: + """Tool name for a tool.execution_start event, sub-labelling LSP ops (lsp:) like the hook did.""" + tool_name = data.get("toolName") + if not isinstance(tool_name, str) or not tool_name: + return None + if tool_name == "lsp": + arguments = data.get("arguments") + if isinstance(arguments, dict) and isinstance(arguments.get("operation"), str): + return f"lsp:{arguments['operation']}" + return tool_name + + def parse_output(output_lines: Sequence[str]) -> tuple[AgentMetrics | None, str | None]: """Parse metrics and the agent's final response from `copilot --output-format=json` (JSONL) stdout. Relevant events (CLI 1.0.80): model.call_start: one per request sent to the model, so counting them yields the turn count. + tool.execution_start: one per tool invocation (including sub-agent and MCP tool calls, which + the pre-tool-use hook never sees), so counting them by ``toolName`` yields tool usage. session.usage_checkpoint: `data.totalNanoAiu` is cumulative for the session, so the last one wins. result: terminal event whose `usage` sits at the event root rather than under `data`. @@ -36,6 +51,7 @@ def parse_output(output_lines: Sequence[str]) -> tuple[AgentMetrics | None, str llm_duration: float | None = None ai_credits: float | None = None turn_count = 0 + tool_usage: Counter[str] = Counter() response: str | None = None final_response: str | None = None @@ -56,6 +72,10 @@ def parse_output(output_lines: Sequence[str]) -> tuple[AgentMetrics | None, str match event.get("type"): case "model.call_start": turn_count += 1 + case "tool.execution_start": + data = event.get("data") + if isinstance(data, dict) and (label := _tool_label(data)): + tool_usage[label] += 1 case "assistant.message": data = event.get("data") if not isinstance(data, dict): @@ -89,6 +109,7 @@ def parse_output(output_lines: Sequence[str]) -> tuple[AgentMetrics | None, str llm_duration=llm_duration, ai_credits=ai_credits, turn_count=turn_count or None, + tool_usage=dict(tool_usage) or None, ) else: logger.warning("No metrics found in Copilot JSON output") diff --git a/src/bcbench/agent/shared/__init__.py b/src/bcbench/agent/shared/__init__.py index 50581fef4..37607c960 100644 --- a/src/bcbench/agent/shared/__init__.py +++ b/src/bcbench/agent/shared/__init__.py @@ -1,9 +1,11 @@ """Shared code for CLI-based agents (Claude, Copilot).""" +from bcbench.agent.shared.env import agent_subprocess_env from bcbench.agent.shared.hooks_parser import parse_tool_usage_from_hooks from bcbench.agent.shared.lsp import build_al_lsp_plugin from bcbench.agent.shared.mcp import build_mcp_config +from bcbench.agent.shared.mcp_gateway import start_bc_mcp_gateway from bcbench.agent.shared.plugin import resolve_config_plugins from bcbench.agent.shared.prompt import build_prompt -__all__ = ["build_al_lsp_plugin", "build_mcp_config", "build_prompt", "parse_tool_usage_from_hooks", "resolve_config_plugins"] +__all__ = ["agent_subprocess_env", "build_al_lsp_plugin", "build_mcp_config", "build_prompt", "parse_tool_usage_from_hooks", "resolve_config_plugins", "start_bc_mcp_gateway"] diff --git a/src/bcbench/agent/shared/config.yaml b/src/bcbench/agent/shared/config.yaml index a6dd44d9e..7caa2ea7f 100644 --- a/src/bcbench/agent/shared/config.yaml +++ b/src/bcbench/agent/shared/config.yaml @@ -103,6 +103,21 @@ prompt: If there are no findings, write an empty array. Write only valid JSON to `review.json`, with no surrounding object, Markdown fence, or commentary. + data-query-template: | + Answer the following Business Central data question using the ACTUAL data from the connected + environment. You cannot answer from general knowledge — you must retrieve the real data with the + available Business Central data tools and report exactly what they return. If the data tools are + not immediately listed, discover them first (they may be provided as deferred/searchable tools). + + Write two files in {{repo_path}}: + - `answer.json`: a JSON array of the result rows that answer the question, one JSON object per row. + - `query.al`: the single AL query object you used to obtain the data. + + Question: + {{task}} + + You MUST write answer.json before finishing; if you do not, there is no output to evaluate. + # controls: # 1. whether to copy custom instructions from `src/bcbench/agent/shared/instructions//` # - Copilot: copies to repo/.github/ and renames AGENTS.md to copilot-instructions.md @@ -177,9 +192,19 @@ mcp: "{{package_cache_path}}", ] - # - name: "mslearn" - # type: "http" - # url: "https://learn.microsoft.com/api/mcp" + # Business Central MCP server (toggled via --bc-mcp CLI flag). The url is filled in + # programmatically by mcp.py to point at a localhost gateway (mcp_gateway.py) that injects auth + # upstream, so no credentials appear here. Which tools the server exposes is decided server-side by + # the MCP configuration the setup-time AL app provisions. + - name: "bcmcp" + type: "http" + url: "" + + # Microsoft Learn MCP (toggled via --ms-learn-mcp CLI flag): official AL docs + code samples the + # bc-al-query-mcp skill grounds its AL syntax in. Public server, no auth. + - name: "mslearn" + type: "http" + url: "https://learn.microsoft.com/api/mcp" # - name: "filesystem" # type: "stdio" diff --git a/src/bcbench/agent/shared/env.py b/src/bcbench/agent/shared/env.py new file mode 100644 index 000000000..6989664c7 --- /dev/null +++ b/src/bcbench/agent/shared/env.py @@ -0,0 +1,18 @@ +import os + +# BC container connection details/credentials the harness uses to build the MCP config and to reach the +# container. They must NOT leak into a launched agent's own process environment: otherwise the agent can +# read the credentials and query BC's API directly from a shell, bypassing the MCP server the benchmark +# is meant to exercise. MCP servers still receive what they need through the MCP configuration (an +# embedded env block for altool, an auth header for the BC MCP server), so withholding these from the +# agent process closes the direct-API side-door without breaking MCP connectivity. +_WITHHELD_ENV_PREFIXES = ("BC_SERVER_", "BC_MCP_") +_WITHHELD_ENV_VARS = frozenset({"BC_CONTAINER_NAME"}) + + +def agent_subprocess_env(overrides: dict[str, str] | None = None) -> dict[str, str]: + """``os.environ`` for a launched agent, with the BC container connection vars removed.""" + env = {k: v for k, v in os.environ.items() if not k.startswith(_WITHHELD_ENV_PREFIXES) and k not in _WITHHELD_ENV_VARS} + if overrides: + env.update(overrides) + return env diff --git a/src/bcbench/agent/shared/instructions/dataquery/skills/bc-al-query-mcp/SKILL.md b/src/bcbench/agent/shared/instructions/dataquery/skills/bc-al-query-mcp/SKILL.md new file mode 100644 index 000000000..e1037e6ea --- /dev/null +++ b/src/bcbench/agent/shared/instructions/dataquery/skills/bc-al-query-mcp/SKILL.md @@ -0,0 +1,168 @@ +--- +name: bc-al-query-mcp +description: "Use when: writing, fixing, validating, or running Business Central AL query objects with bc_data MCP tools and Microsoft Learn MCP docs. Covers AL query syntax, table discovery, schemas, relations, joins, filters, FlowFields, pagination, and read-only data retrieval." +argument-hint: "[business question, data need, or AL query to fix]" +user-invocable: true +disable-model-invocation: false +--- + +# Business Central AL Query MCP + +Use this skill when the user asks to query Business Central data, write or fix an AL `query` object, join BC tables, discover BC fields, validate an AL query, or use the `bc_data` MCP tools together with Microsoft Learn. + +## Required Capabilities + +The environment should expose these MCP tool groups: + +- Microsoft Learn MCP: docs search, docs fetch, and code sample search for official AL query syntax and examples. +- Business Central data MCP: table search, table schema, table relations, and AL query compile/run. + +If tools are deferred, load them with tool search before use. Do not assume a tool is available until it has been loaded or returned by discovery. + +The MCP tool names may appear with a server-name prefix that differs by host — for example `bcmcp-bc_data_query` (Copilot CLI) or `mcp__bcmcp__bc_data_query` (Claude Code). The bare names used below (`bc_data_find_tables`, `bc_data_get_table_schema`, `bc_data_get_table_relations`, `bc_data_query`) refer to these tools regardless of prefix; always invoke the exact name shown in your available tool list or returned by tool search. + +## Core Rule + +Microsoft Learn provides general AL query authoring knowledge. The BC data MCP tools provide live tenant-specific metadata and execution. Use both. Never write a tenant query from memory alone. + +## Non-Negotiables + +- Always use `bc_data_get_table_schema` for every table before writing the query. +- Always verify joins with `bc_data_get_table_relations` before writing a multi-table query. +- Prefer `bc_data_find_tables` with `searchMode: keyword` for known BC entity names. Use semantic search only as a supplement and verify results. +- Compile with `bc_data_query` and `returnData: false` before running with `returnData: true`. +- Keep queries read-only, narrow, and paged. Use only the columns needed for the user's question. +- Do not dump sensitive raw business data unless the user explicitly asks for rows. Prefer summaries, counts, and representative samples. +- If a compile or execution error occurs, use the diagnostic location plus schema/relations to repair the same query. Do not blindly rewrite from scratch. +- If permissions, missing tables, or unavailable MCP tools block the task, report the exact blocker and the next viable option. + +## Workflow + +1. Identify the user's actual data question. + - Determine the business entity, date range, filters, measures, and whether raw rows or an aggregate answer is needed. + - Ask a concise clarification only if the query cannot be scoped safely. + +2. Ground AL syntax in Microsoft Learn when needed. + - Search/fetch docs for `Business Central AL query object`, `DataItemLink`, `SqlJoinType`, `DataItemTableFilter`, `ColumnFilter`, `Filtering in Query objects`, and `Aggregating data in Query objects`. + - Use code sample search with `language: al` when examples are useful. + +3. Discover the live tables. + - Use keyword search for likely names, for example `customer`, `sales invoice`, `item ledger entry`, `vendor ledger entry`. + - If the user describes a business concept instead of table names, try semantic search, but validate with keyword search and schemas. + +4. Inspect schemas. + - Call schema for every candidate table. + - Use `nameContains` to narrow large schemas, for example `['no', 'posting date', 'amount', 'customer']`. + - Note primary keys, field names, field classes, field types, FlowFields, FlowFilters, and relation hints. + +5. Discover joins. + - For each multi-table query, call relations in the useful direction. + - Use `relatedToTableIds` when checking a specific join path. + - Remember that `DataItemLink` is set on the lower/nested dataitem. + +6. Compose the AL query. + - Use a normal query object unless the user specifically needs an API query. + - Quote table and field names that contain spaces, punctuation, or reserved words. + - Use stable column aliases without spaces, usually underscores. + - Put parent tables higher and child/detail tables nested beneath them. + - Set `SqlJoinType = InnerJoin;` when only matching child rows should appear. If omitted, AL query dataitems default to `LeftOuterJoin`. + - Use `DataItemTableFilter` for static filters. + - For date filters in the BC data MCP execution path, prefer quoted ISO date strings, for example `filter('2025-01-01'..'2025-01-31')`. + - Use FlowFields only when needed; they can be convenient but may add subqueries and cost. + +7. Validate before execution. + - First call `bc_data_query` with `returnData: false`. + - Confirm the returned columns, types, and dataitems match the intended shape. + - If validation fails, fix field names, aliases, links, filter syntax, or query structure using the exact diagnostic. + +8. Run safely. + - Use `returnData: true`, `top` no larger than needed, and `skip` for paging. + - Use `resultFormat: resource` for larger results or when downstream analysis is needed. + - On page 0, use `totalCount` when present. Continue paging only when the user needs more data. + +9. Present the result. + - Include the final AL query when the user asked for a query or when it helps reproducibility. + - State which tables, fields, joins, and filters were used. + - Summarize results without overexposing tenant data. + - Mention validation status: compiled only, compiled and ran, or blocked with reason. + +## Query Patterns + +### Single Table + +```al +query 50100 CustomerOverview +{ + QueryType = Normal; + + elements + { + dataitem(Customer; Customer) + { + column(No_; "No.") { } + column(Name; Name) { } + column(Blocked; Blocked) { } + column(Balance_LCY; "Balance (LCY)") { } + } + } +} +``` + +### Header And Lines + +```al +query 50101 PostedSalesInvoiceLines +{ + QueryType = Normal; + + elements + { + dataitem(SalesInvoiceHeader; "Sales Invoice Header") + { + column(Invoice_No_; "No.") { } + column(Sell_to_Customer_No_; "Sell-to Customer No.") { } + column(Posting_Date; "Posting Date") { } + + dataitem(SalesInvoiceLine; "Sales Invoice Line") + { + DataItemLink = "Document No." = SalesInvoiceHeader."No."; + SqlJoinType = InnerJoin; + + column(Line_No_; "Line No.") { } + column(Item_No_; "No.") { } + column(Description; Description) { } + column(Quantity; Quantity) { } + column(Line_Amount; Amount) { } + } + } + } +} +``` + +### Static Date Filter + +```al +DataItemTableFilter = "Posting Date" = filter('2025-01-01'..'2025-01-31'); +``` + +### Aggregate Column + +```al +column(Total_Quantity; Quantity) +{ + Method = Sum; +} +``` + +## Common Recovery Moves + +- `AL0345` or invalid column source: re-check schema for the exact field name on the parent dataitem's table. +- Join returns too many rows: verify `DataItemLink` field direction and add `SqlJoinType = InnerJoin;` when appropriate. +- No rows returned: validate filters first, then run a smaller unfiltered query with identifying columns. +- Date filter errors: use quoted ISO date strings in the filter expression. +- Semantic table search returns nothing: retry with keyword fragments and inspect schemas. +- Large result sets: reduce columns, add filters, page with `top`/`skip`, or use `resultFormat: resource`. + +## Quality Bar + +A good answer from this skill includes enough evidence that the query is grounded in the live environment: discovered tables, checked fields, verified relations, compile status, and a safe execution or clear blocker. The agent should not merely produce plausible AL code; it should validate the query against the connected Business Central instance whenever the tools are available. diff --git a/src/bcbench/agent/shared/mcp.py b/src/bcbench/agent/shared/mcp.py index a8881ee4e..a309214bc 100644 --- a/src/bcbench/agent/shared/mcp.py +++ b/src/bcbench/agent/shared/mcp.py @@ -15,6 +15,22 @@ _jinja = SandboxedEnvironment(autoescape=False) +# Server names for the independently-toggled MCP servers. +_BC_MCP_SERVER_NAME = "bcmcp" +_MS_LEARN_MCP_SERVER_NAME = "mslearn" + + +def _redact_mcp_config(mcp_config: dict[str, Any]) -> dict[str, Any]: + """Deep copy with any Authorization header masked, so DEBUG logs never leak container credentials.""" + import copy + + redacted = copy.deepcopy(mcp_config) + for server in redacted.get("mcpServers", {}).values(): + headers = server.get("headers") + if isinstance(headers, dict) and "Authorization" in headers: + headers["Authorization"] = "Basic ***REDACTED***" + return redacted + def _build_server_entry(server: dict[str, Any], template_context: dict[str, Any]) -> tuple[str, dict[str, Any]]: server_type: str = server["type"] @@ -22,39 +38,76 @@ def _build_server_entry(server: dict[str, Any], template_context: dict[str, Any] match server_type: case "http": - return server_name, { + entry: dict[str, Any] = { "type": server_type, "url": server["url"], } + headers: dict[str, str] = server.get("headers", {}) + if headers: + entry["headers"] = headers + return server_name, entry case "stdio": args: list[str] = server["args"] rendered_args = [_jinja.from_string(arg).render(**template_context) for arg in args] command: str = shutil.which(server["command"]) or server["command"] - entry: dict[str, Any] = { + stdio_entry: dict[str, Any] = { "type": server_type, "command": command, "args": rendered_args, } env: dict[str, str] = server.get("env", {}) if env: - entry["env"] = env - return server_name, entry + stdio_entry["env"] = env + return server_name, stdio_entry case _: logger.error(f"Unsupported MCP server type: {server_type}, {server}") raise AgentError(f"Unsupported MCP server type: {server_type}") -def build_mcp_config(config: dict[str, Any], entry: BaseDatasetEntry, repo_path: Path, al_mcp: bool = False, container_name: str = "bcbench") -> tuple[str | None, list[str] | None]: +def _configure_bc_mcp_server(server: dict[str, Any], gateway_base_url: str | None) -> None: + """Point the BC MCP server at the local credential-free gateway. + + The gateway (``mcp_gateway.py``) fronts the real BC MCP endpoint: it injects the Basic auth / + Company / ConfigurationName headers upstream and rejects any non-``/mcp`` path. So the agent's MCP + config carries only a ``http://127.0.0.1:/.../mcp`` URL with no credentials -- nothing the + agent can replay against BC's ``/api`` or scrape from the launched process command line. + """ + if not gateway_base_url: + raise AgentError("BC MCP requested but the local MCP gateway URL is unavailable.") + + server["url"] = gateway_base_url.rstrip("/") + "/mcp" + server.pop("headers", None) + + +def build_mcp_config( + config: dict[str, Any], + entry: BaseDatasetEntry, + repo_path: Path, + al_mcp: bool = False, + bc_mcp: bool = False, + ms_learn_mcp: bool = False, + container_name: str = "bcbench", + bc_mcp_gateway_url: str | None = None, +) -> tuple[str | None, list[str] | None]: mcp_servers: list[dict[str, Any]] = config.get("mcp", {}).get("servers", []) if not al_mcp: mcp_servers = list(filter(lambda s: s.get("name") != "altool", mcp_servers)) + if not bc_mcp: + mcp_servers = list(filter(lambda s: s.get("name") != _BC_MCP_SERVER_NAME, mcp_servers)) + + if not ms_learn_mcp: + mcp_servers = list(filter(lambda s: s.get("name") != _MS_LEARN_MCP_SERVER_NAME, mcp_servers)) + if not mcp_servers: return None, None template_context: dict[str, str | Path] = {"repo_path": repo_path} + if bc_mcp: + _configure_bc_mcp_server(next(s for s in mcp_servers if s["name"] == _BC_MCP_SERVER_NAME), bc_mcp_gateway_url) + if al_mcp: compiler_folder, symbols_folder = compiler_symbol_folder_for_container(container_name) template_context["package_cache_path"] = str(symbols_folder) @@ -82,6 +135,6 @@ def build_mcp_config(config: dict[str, Any], entry: BaseDatasetEntry, repo_path: mcp_config = {"mcpServers": dict(map(lambda s: _build_server_entry(s, template_context), mcp_servers))} logger.info(f"Using MCP servers: {mcp_server_names}") - logger.debug(f"MCP configuration: {json.dumps(mcp_config, indent=2)}") + logger.debug(f"MCP configuration: {json.dumps(_redact_mcp_config(mcp_config), indent=2)}") return json.dumps(mcp_config, separators=(",", ":")), mcp_server_names diff --git a/src/bcbench/agent/shared/mcp_gateway.py b/src/bcbench/agent/shared/mcp_gateway.py new file mode 100644 index 000000000..cdf64c8d3 --- /dev/null +++ b/src/bcbench/agent/shared/mcp_gateway.py @@ -0,0 +1,418 @@ +"""A localhost MCP gateway that fronts the BC MCP endpoint for a benchmarked agent. + +Why this exists: the agent must reach BC *only* through the MCP server, never the raw OData ``/api`` +or the SQL database. The BC container serves ``/api`` and ``/mcp`` on the same port, so a plain +firewall cannot separate them, and putting the Basic credentials in the agent's MCP config leaks them +onto the agent process command line (recoverable via ``Get-CimInstance Win32_Process``), which the +agent could replay against ``/api``. + +This gateway closes both holes: it path-restricts to ``/mcp`` (everything else -> 403) and injects the +Basic auth / Company / ConfigurationName headers itself, so the agent's MCP config carries only a +credential-free ``http://127.0.0.1:/.../mcp`` URL. The upstream endpoint and credentials come +from the harness environment (``BC_MCP_URL`` / ``BC_SERVER_*`` / ``BC_MCP_COMPANY``), which is never +scrubbed for the harness itself. +""" + +import base64 +import json +import os +import threading +import time +from http.client import HTTPConnection +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from urllib.parse import urlsplit + +from bcbench.exceptions import AgentError +from bcbench.logger import get_logger + +logger = get_logger(__name__) + +# Must match the configuration name the setup-time AL app creates (scripts/al/mcp-config-setup). +_CONFIGURATION_NAME = "BCBench" + +# Connection-level headers that must not be forwarded across a proxy hop (RFC 7230 6.1), plus the +# framing/credential headers this gateway sets itself. +_HOP_BY_HOP = frozenset( + { + "connection", + "keep-alive", + "proxy-authenticate", + "proxy-authorization", + "te", + "trailer", + "transfer-encoding", + "upgrade", + } +) +_STRIPPED_REQUEST_HEADERS = _HOP_BY_HOP | {"host", "content-length", "accept-encoding", "authorization", "company", "configurationname"} + +_UPSTREAM_TIMEOUT_SECONDS = 600 +_STREAM_CHUNK_BYTES = 8192 +_PROBE_TIMEOUT_SECONDS = 180 + + +def _jsonrpc_method_and_id(body: bytes | None) -> tuple[str | None, object]: + if not body: + return None, None + try: + obj = json.loads(body) + except (json.JSONDecodeError, UnicodeDecodeError): + return None, None + if not isinstance(obj, dict): + return None, None + return obj.get("method"), obj.get("id") + + +def _read_jsonrpc(response, deadline: float) -> tuple[dict, str]: # noqa: ANN001 - http.client.HTTPResponse + """Parse a JSON-RPC result and return it plus a raw snippet of the response for diagnostics. + + For SSE, read line by line and return as soon as a JSON-RPC result/error arrives: the BC MCP + endpoint keeps the event stream open for later messages, so reading to EOF would block until the + socket times out even though the answer already arrived. + """ + content_type = response.getheader("Content-Type", "") or "" + if "text/event-stream" in content_type: + seen: list[str] = [] + while time.monotonic() < deadline: + raw_line = response.readline() + if not raw_line: + break + line = raw_line.decode("utf-8", errors="replace").strip() + if line: + seen.append(line) + if line.startswith("data:"): + try: + obj = json.loads(line[5:].strip()) + except json.JSONDecodeError: + continue + if isinstance(obj, dict) and ("result" in obj or "error" in obj): + return obj, line[:800] + return {}, " | ".join(seen)[:800] + text = response.read().decode("utf-8", errors="replace") + try: + return (json.loads(text) if text.strip() else {}), text[:800] + except json.JSONDecodeError: + return {}, text[:800] + + +class BcMcpGateway: + def __init__(self, upstream_url: str, username: str, password: str, company: str | None) -> None: + split = urlsplit(upstream_url) + if not split.hostname: + raise AgentError(f"BC MCP upstream URL is malformed: {upstream_url!r}") + + self._origin_host: str = split.hostname + self._origin_port: int = split.port or (443 if split.scheme == "https" else 80) + base_path: str = split.path.rstrip("/") + self._mcp_path: str = f"{base_path}/mcp" + self._base_path: str = base_path + + injected: dict[str, str] = { + "Authorization": f"Basic {base64.b64encode(f'{username}:{password}'.encode()).decode()}", + "ConfigurationName": _CONFIGURATION_NAME, + } + if company: + injected["Company"] = company + self._injected_headers: dict[str, str] = injected + + self._server: ThreadingHTTPServer | None = None + self._thread: threading.Thread | None = None + self._lock = threading.Lock() + self._forwarded_count = 0 + self.base_url: str | None = None + # tools/list "result" object captured during warm-up. BC composes the tool catalog per MCP + # session and the first tools/list is slow (~45s) and sometimes dropped by the server, which + # blows past the agent's MCP startup timeout so the server registers zero tools. The catalog is + # identical across sessions, so once warm-up has it the gateway answers tools/list from here, + # decoupling the agent from BC's cold per-session composition. + self._cached_tools_result: dict[str, object] | None = None + + @property + def forwarded_count(self) -> int: + with self._lock: + return self._forwarded_count + + def _note_forwarded(self) -> None: + with self._lock: + self._forwarded_count += 1 + + def start(self) -> "BcMcpGateway": + gateway = self + server = ThreadingHTTPServer(("127.0.0.1", 0), _build_handler(gateway)) + self._server = server + port = server.server_address[1] + self.base_url = f"http://127.0.0.1:{port}{self._base_path}" + self._thread = threading.Thread(target=server.serve_forever, name="bc-mcp-gateway", daemon=True) + self._thread.start() + return self + + def stop(self) -> None: + if self._server is not None: + self._server.shutdown() + self._server.server_close() + self._server = None + if self._thread is not None: + self._thread.join(timeout=5) + self._thread = None + logger.info(f"BC MCP gateway forwarded {self.forwarded_count} request(s) to the BC MCP endpoint") + + def _rpc(self, host: str, port: int, extra_headers: dict[str, str], method: str, params: dict | None, request_id: int | None = None, session_id: str | None = None) -> tuple[str | None, dict, str]: + connection = HTTPConnection(host, port, timeout=_PROBE_TIMEOUT_SECONDS) + try: + payload: dict[str, object] = {"jsonrpc": "2.0", "method": method} + if request_id is not None: + payload["id"] = request_id + if params is not None: + payload["params"] = params + headers = {"Content-Type": "application/json", "Accept": "application/json, text/event-stream", **extra_headers} + if session_id: + headers["Mcp-Session-Id"] = session_id + connection.request("POST", self._mcp_path, body=json.dumps(payload).encode(), headers=headers) + response = connection.getresponse() + returned_session = response.getheader("Mcp-Session-Id") + result, raw = _read_jsonrpc(response, deadline=time.monotonic() + _PROBE_TIMEOUT_SECONDS) + return returned_session, result, f"HTTP {response.status}, Content-Type={response.getheader('Content-Type', '')!r}, body={raw!r}" + finally: + connection.close() + + def _handshake_tools(self, host: str, port: int, extra_headers: dict[str, str]) -> tuple[list[str], float, str]: + """initialize -> notifications/initialized -> tools/list against one target; returns (tools, tools_list_seconds, diag).""" + session_id, init_result, init_diag = self._rpc(host, port, extra_headers, "initialize", {"protocolVersion": "2024-11-05", "capabilities": {}, "clientInfo": {"name": "bcbench-probe", "version": "1.0"}}, request_id=1) + logger.info(f"BC MCP initialize result: session={session_id!r} result={json.dumps(init_result)[:400]} diag={init_diag[:200]}") + if session_id: + self._rpc(host, port, extra_headers, "notifications/initialized", None, session_id=session_id) + before_list = time.monotonic() + _, listed, diag = self._rpc(host, port, extra_headers, "tools/list", {}, request_id=2, session_id=session_id) + result = listed.get("result") + tools = [t.get("name") for t in (result or {}).get("tools", []) if isinstance(t, dict)] + if tools and isinstance(result, dict): + with self._lock: + self._cached_tools_result = result + return tools, time.monotonic() - before_list, diag + + def probe_tools(self) -> list[str]: + """Warm up BC MCP and log its exposed tools, probing BOTH through the gateway and directly. + + Best-effort (never raises): warms the cold endpoint before the agent connects, and comparing the + via-gateway result against a direct-to-BC result isolates a gateway relay problem from a genuine + server-side one. The direct probe carries the injected auth/Company/ConfigurationName headers. + """ + gateway_tools: list[str] = [] + # Via the gateway (credential-free; the gateway injects auth upstream) - the agent's exact path. + if self.base_url is not None: + split = urlsplit(self.base_url) + try: + gateway_tools, secs, diag = self._handshake_tools(split.hostname or "127.0.0.1", split.port or 80, {}) + logger.info(f"BC MCP warm-up (via gateway): exposes {len(gateway_tools)} tool(s): {gateway_tools} (tools/list {secs:.1f}s)") + if not gateway_tools: + logger.info(f"BC MCP warm-up (via gateway) empty tools/list -> {diag}") + except Exception as exc: # noqa: BLE001 - a warm-up diagnostic must never break a run + logger.warning(f"BC MCP warm-up (via gateway) failed (non-fatal): {exc}") + + # Directly to BC (bypassing the gateway) with the real headers - isolates gateway vs server. + try: + direct_tools, secs, diag = self._handshake_tools(self._origin_host, self._origin_port, self._injected_headers) + logger.info(f"BC MCP warm-up (direct to BC): exposes {len(direct_tools)} tool(s): {direct_tools} (tools/list {secs:.1f}s)") + if not direct_tools: + logger.info(f"BC MCP warm-up (direct to BC) empty tools/list -> {diag}") + except Exception as exc: # noqa: BLE001 - a warm-up diagnostic must never break a run + logger.warning(f"BC MCP warm-up (direct to BC) failed (non-fatal): {exc}") + + return gateway_tools + + +def _build_handler(gateway: BcMcpGateway) -> type[BaseHTTPRequestHandler]: + class _ProxyHandler(BaseHTTPRequestHandler): + protocol_version = "HTTP/1.1" + + def log_message(self, format: str, *args: object) -> None: # match stdlib signature; silence access log + pass + + def _path_allowed(self) -> bool: + path_only = self.path.split("?", 1)[0] + return path_only == gateway._mcp_path or path_only.startswith(gateway._mcp_path + "/") + + def _handle(self) -> None: + if not self._path_allowed(): + logger.info(f"BC MCP gateway BLOCKED {self.command} {self.path} -> 403") + self.send_error(403, "Forbidden") + return + + length = self.headers.get("Content-Length") + body: bytes | None = self.rfile.read(int(length)) if length else None + rpc_method, rpc_id = _jsonrpc_method_and_id(body) + self._response_started = False + + if rpc_method == "tools/list" and self._serve_cached_tools(rpc_id): + return + + request_headers: dict[str, str] = {k: v for k, v in self.headers.items() if k.lower() not in _STRIPPED_REQUEST_HEADERS} + request_headers["Host"] = f"{gateway._origin_host}:{gateway._origin_port}" + request_headers.update(gateway._injected_headers) + + connection = HTTPConnection(gateway._origin_host, gateway._origin_port, timeout=_UPSTREAM_TIMEOUT_SECONDS) + started = time.monotonic() + try: + connection.request(self.command, self.path, body=body, headers=request_headers) + response = connection.getresponse() + gateway._note_forwarded() + logger.info(f"BC MCP gateway {self.command} {rpc_method or self.path} -> HTTP {response.status} {response.getheader('Content-Type', '')} ({time.monotonic() - started:.1f}s)") + # Relay faithfully, byte-for-byte, holding streams open exactly as BC does (its MCP + # server keeps SSE streams open as the client's event channel). The one exception is the + # initialize reply: BC advertises capabilities.experimental = {"x-ms-headerless": true}, + # which makes Claude's MCP client fail the connection; strip it so the client sees a + # standard server (BC still works over the normal header-based session the probe uses). + if rpc_method == "initialize" and response.status == 200 and "text/event-stream" in (response.getheader("Content-Type", "") or ""): + self._relay_initialize(response) + else: + self._relay(response) + except (ConnectionError, OSError) as error: + # The client (agent) closing its side mid-stream is normal; don't misreport it as an + # upstream failure, and don't try to send an error once the response has begun. + if self._response_started: + logger.debug(f"BC MCP gateway client disconnected during {self.command} {rpc_method or self.path}: {error}") + self.close_connection = True + else: + logger.exception(f"BC MCP gateway failed to reach upstream for {self.command} {rpc_method or self.path} after {time.monotonic() - started:.1f}s") + self.send_error(502, "Bad Gateway") + except Exception: + logger.exception(f"BC MCP gateway error handling {self.command} {rpc_method or self.path} after {time.monotonic() - started:.1f}s") + if not self._response_started: + self.send_error(502, "Bad Gateway") + finally: + connection.close() + + def _serve_cached_tools(self, request_id: object) -> bool: + """Answer tools/list from the warm-up cache, bypassing BC's slow per-session composition. + + Framed as a single-event SSE stream (then closed) to mirror how BC replies to tools/list, so + the client sees the same transport it would from the real endpoint. + """ + with gateway._lock: + cached = gateway._cached_tools_result + if cached is None: + return False + event = ("event: message\ndata: " + json.dumps({"jsonrpc": "2.0", "id": request_id, "result": cached}) + "\n\n").encode() + self._response_started = True + self.send_response(200) + self.send_header("Content-Type", "text/event-stream") + self.send_header("Cache-Control", "no-cache") + self.send_header("Content-Length", str(len(event))) + self.end_headers() + self.wfile.write(event) + self.wfile.flush() + tools = cached.get("tools") if isinstance(cached, dict) else None + tool_count = len(tools) if isinstance(tools, list) else "?" + logger.info(f"BC MCP gateway tools/list -> served {tool_count} tool(s) from warm-up cache") + return True + + def _relay_initialize(self, response) -> None: # noqa: ANN001 - http.client.HTTPResponse + """Relay the initialize SSE reply but strip capabilities.experimental from the result. + + BC advertises ``capabilities.experimental = {"x-ms-headerless": true}``; Claude's MCP client + fails the connection when it sees it (bisected against a replica of BC's exact initialize + response). Rewrite just that first result event, then keep relaying faithfully so the stream + behaves exactly like BC's for everything else. + """ + self._response_started = True + forwarded_headers = [(k, v) for k, v in response.getheaders() if k.lower() not in _HOP_BY_HOP and k.lower() not in ("content-length", "content-type")] + self.send_response_only(200) + for key, value in forwarded_headers: + self.send_header(key, value) + self.send_header("Content-Type", "text/event-stream") + self.send_header("Transfer-Encoding", "chunked") + self.end_headers() + + deadline = time.monotonic() + _UPSTREAM_TIMEOUT_SECONDS + rewritten = False + while time.monotonic() < deadline: + raw_line = response.readline() + if not raw_line: + break + out_line = raw_line + stripped = raw_line.decode("utf-8", errors="replace").strip() + if not rewritten and stripped.startswith("data:"): + try: + obj = json.loads(stripped[5:].strip()) + except json.JSONDecodeError: + obj = None + if isinstance(obj, dict) and isinstance(obj.get("result"), dict): + capabilities = obj["result"].get("capabilities") + removed = isinstance(capabilities, dict) and capabilities.pop("experimental", None) is not None + out_line = ("data: " + json.dumps(obj) + "\n").encode() + rewritten = True + logger.info(f"BC MCP gateway rewrote initialize result (experimental stripped={removed})") + self.wfile.write(b"%X\r\n" % len(out_line)) + self.wfile.write(out_line) + self.wfile.write(b"\r\n") + self.wfile.flush() + # Keep relaying (holding the stream open) exactly like BC until the upstream or client closes. + self.wfile.write(b"0\r\n\r\n") + self.wfile.flush() + + def _relay(self, response) -> None: # noqa: ANN001 - http.client.HTTPResponse + self._response_started = True + self.send_response_only(response.status) + content_length: str | None = None + for key, value in response.getheaders(): + lowered = key.lower() + if lowered == "content-length": + content_length = value + continue + if lowered in _HOP_BY_HOP: + continue + self.send_header(key, value) + + if content_length is not None: + self.send_header("Content-Length", content_length) + self.end_headers() + remaining = int(content_length) + while remaining > 0: + chunk = response.read(min(_STREAM_CHUNK_BYTES, remaining)) + if not chunk: + break + self.wfile.write(chunk) + remaining -= len(chunk) + else: + # No content length -> stream (e.g. SSE) with our own chunked framing, flushing each + # block so server-sent events reach the agent as they arrive. Use read1(): plain read() + # blocks trying to fill the whole buffer, which stalls an SSE stream the server holds + # open after a small event (that stall is what made tools/list time out through here). + self.send_header("Transfer-Encoding", "chunked") + self.end_headers() + while True: + chunk = response.read1(_STREAM_CHUNK_BYTES) + if not chunk: + break + self.wfile.write(b"%X\r\n" % len(chunk)) + self.wfile.write(chunk) + self.wfile.write(b"\r\n") + self.wfile.flush() + self.wfile.write(b"0\r\n\r\n") + self.wfile.flush() + + do_GET = _handle + do_POST = _handle + do_DELETE = _handle + + return _ProxyHandler + + +def start_bc_mcp_gateway(enabled: bool) -> BcMcpGateway | None: + """Start a localhost MCP gateway in front of the BC container, or return None when disabled.""" + if not enabled: + return None + + upstream = os.environ.get("BC_MCP_URL") + if not upstream: + raise AgentError("BC MCP requested but BC_MCP_URL is not set; container setup must export it.") + + gateway = BcMcpGateway( + upstream_url=upstream, + username=os.environ.get("BC_SERVER_USERNAME", ""), + password=os.environ.get("BC_SERVER_PASSWORD", ""), + company=os.environ.get("BC_MCP_COMPANY"), + ).start() + logger.info(f"BC MCP gateway listening at {gateway.base_url}/mcp (credential-free; path-restricted to /mcp)") + gateway.probe_tools() + return gateway diff --git a/src/bcbench/commands/dataset.py b/src/bcbench/commands/dataset.py index 11a27e2b3..bdd9e60f3 100644 --- a/src/bcbench/commands/dataset.py +++ b/src/bcbench/commands/dataset.py @@ -1,11 +1,13 @@ """CLI commands for dataset operations.""" import json +from pathlib import Path from typing import Annotated import typer -from bcbench.cli_options import EvaluationCategoryOption +from bcbench.cli_options import ContainerName, ContainerPassword, ContainerUsername, EvaluationCategoryOption +from bcbench.config import get_config from bcbench.dataset import BaseDatasetEntry, CodeReviewEntry, RepoGroundedEntry from bcbench.dataset.dataset_entry import NL2ALEntry, _BugFixTestGenBase from bcbench.github_actions import write_step_outputs @@ -13,6 +15,7 @@ from bcbench.types import EvaluationCategory logger = get_logger(__name__) +_config = get_config() dataset_app = typer.Typer(help="Query and analyze dataset") @@ -182,6 +185,37 @@ def version( write_step_outputs({github_output: entry.environment_setup_version}) +@dataset_app.command("bake-dataquery-gold") +def bake_dataquery_gold( + container_name: ContainerName = "", + username: ContainerUsername = "", + password: ContainerPassword = "", + repo_path: Annotated[Path, typer.Option(help="Container-shared work folder for building the gold query app")] = _config.paths.testbed_path, + version: Annotated[str, typer.Option(help="BC version override; defaults to each entry's environment_setup_version")] = "", +) -> None: + """Run each data-query gold query once and store its result rows as gold_rows in the dataset. + + Populates the baked expected data so evaluation no longer compiles/runs the gold query live. + Requires a running BC container (repo_path must be shared with it). + """ + from bcbench.operations import execute_al_query + from bcbench.types import ContainerConfig + + dataset_path = EvaluationCategory.DATA_QUERY.dataset_path + entries = [json.loads(line) for line in dataset_path.read_text(encoding="utf-8").splitlines() if line.strip()] + container = ContainerConfig(container_name, username, password) + + for entry in entries: + entry_version = version or entry["environment_setup_version"] + logger.info(f"Baking gold rows for {entry['instance_id']} (v{entry_version})") + rows = execute_al_query(entry["gold_query"], container, entry_version, repo_path, "gold") + entry["gold_rows"] = rows + logger.info(f" -> {len(rows)} rows") + + dataset_path.write_text("\n".join(json.dumps(e, ensure_ascii=False) for e in entries) + "\n", encoding="utf-8") + logger.info(f"Baked gold rows for {len(entries)} entries into {dataset_path}") + + def _modified_instance_ids_from_diff(diff_output: str) -> list[str]: instance_ids = [] diff --git a/src/bcbench/commands/evaluate.py b/src/bcbench/commands/evaluate.py index 6033811b3..90ba5da2c 100644 --- a/src/bcbench/commands/evaluate.py +++ b/src/bcbench/commands/evaluate.py @@ -55,6 +55,9 @@ def evaluate_copilot( run_id: RunId = "copilot_test_run", al_mcp: Annotated[bool, typer.Option("--al-mcp", help="Enable AL MCP server")] = False, al_lsp: Annotated[bool, typer.Option("--al-lsp", help="Enable AL LSP server")] = False, + bc_mcp: Annotated[bool, typer.Option("--bc-mcp", help="Enable the Business Central MCP server")] = False, + ms_learn_mcp: Annotated[bool, typer.Option("--ms-learn-mcp", help="Enable the Microsoft Learn MCP server")] = False, + skills: Annotated[bool, typer.Option("--skills", help="Enable agent skills for the run")] = False, ) -> None: """ Evaluate GitHub Copilot CLI on single dataset entry. @@ -89,6 +92,9 @@ def evaluate_copilot( output_dir=ctx.result_dir, al_mcp=al_mcp if ctx.container else False, al_lsp=al_lsp, + bc_mcp=bc_mcp if ctx.container else False, + ms_learn_mcp=ms_learn_mcp, + skills=skills, container_name=ctx.get_container().name if ctx.container else "", ), ) @@ -110,6 +116,9 @@ def evaluate_claude_code( run_id: RunId = "claude_code_test_run", al_mcp: Annotated[bool, typer.Option("--al-mcp", help="Enable AL MCP server")] = False, al_lsp: Annotated[bool, typer.Option("--al-lsp", help="Enable AL LSP server")] = False, + bc_mcp: Annotated[bool, typer.Option("--bc-mcp", help="Enable the Business Central MCP server")] = False, + ms_learn_mcp: Annotated[bool, typer.Option("--ms-learn-mcp", help="Enable the Microsoft Learn MCP server")] = False, + skills: Annotated[bool, typer.Option("--skills", help="Enable agent skills for the run")] = False, ) -> None: """ Evaluate Claude Code on single dataset entry. @@ -144,6 +153,9 @@ def evaluate_claude_code( output_dir=ctx.result_dir, al_mcp=al_mcp if ctx.container else False, al_lsp=al_lsp, + bc_mcp=bc_mcp if ctx.container else False, + ms_learn_mcp=ms_learn_mcp, + skills=skills, container_name=ctx.get_container().name if ctx.container else "", ), ) @@ -370,7 +382,7 @@ def evaluate(self, context: EvaluationContext[BaseDatasetEntry]) -> None: logger.info("Mock pipeline: Generating random evaluation result") match context.category: - case EvaluationCategory.BUG_FIX | EvaluationCategory.TEST_GENERATION: + case EvaluationCategory.BUG_FIX | EvaluationCategory.TEST_GENERATION | EvaluationCategory.DATA_QUERY: scenarios = ["success", "build-fail"] case EvaluationCategory.CODE_REVIEW: scenarios = ["invalid", "valid"] diff --git a/src/bcbench/commands/run.py b/src/bcbench/commands/run.py index c9cce020b..6cf05a278 100644 --- a/src/bcbench/commands/run.py +++ b/src/bcbench/commands/run.py @@ -36,6 +36,9 @@ def run_copilot( output_dir: OutputDir = _config.paths.evaluation_results_path, al_mcp: Annotated[bool, typer.Option("--al-mcp", help="Enable AL MCP server")] = False, al_lsp: Annotated[bool, typer.Option("--al-lsp", help="Enable AL LSP server")] = False, + bc_mcp: Annotated[bool, typer.Option("--bc-mcp", help="Enable the Business Central MCP server")] = False, + ms_learn_mcp: Annotated[bool, typer.Option("--ms-learn-mcp", help="Enable the Microsoft Learn MCP server")] = False, + skills: Annotated[bool, typer.Option("--skills", help="Enable agent skills for the run")] = False, ) -> None: """ Run GitHub Copilot CLI on a single entry to generate the category output. @@ -56,6 +59,9 @@ def run_copilot( output_dir=output_dir, al_mcp=al_mcp if container_name else False, al_lsp=al_lsp, + bc_mcp=bc_mcp if container_name else False, + ms_learn_mcp=ms_learn_mcp, + skills=skills, container_name=container_name, ) @@ -70,6 +76,9 @@ def run_claude( output_dir: OutputDir = _config.paths.evaluation_results_path, al_mcp: Annotated[bool, typer.Option("--al-mcp", help="Enable AL MCP server")] = False, al_lsp: Annotated[bool, typer.Option("--al-lsp", help="Enable AL LSP server")] = False, + bc_mcp: Annotated[bool, typer.Option("--bc-mcp", help="Enable the Business Central MCP server")] = False, + ms_learn_mcp: Annotated[bool, typer.Option("--ms-learn-mcp", help="Enable the Microsoft Learn MCP server")] = False, + skills: Annotated[bool, typer.Option("--skills", help="Enable agent skills for the run")] = False, ) -> None: """ Run Claude Code on a single entry to generate the category output. @@ -90,6 +99,9 @@ def run_claude( output_dir=output_dir, al_mcp=al_mcp if container_name else False, al_lsp=al_lsp, + bc_mcp=bc_mcp if container_name else False, + ms_learn_mcp=ms_learn_mcp, + skills=skills, container_name=container_name, ) diff --git a/src/bcbench/config.py b/src/bcbench/config.py index c06cf4b77..24e6bf0b3 100644 --- a/src/bcbench/config.py +++ b/src/bcbench/config.py @@ -87,7 +87,7 @@ def default(cls) -> TimeoutConfig: """Get default timeout configuration.""" return cls( build_baseapp=30 * 60, # 30 minutes for BaseApp compilation - build_app=5 * 60, # 5 minutes for application compilation + build_app=900, # TEMPORARY (revert with baking): live gold compile/publish blows 500s on insider 29 test_execution=3 * 60, # 3 minutes for test execution agent_execution=60 * 60, # 60 minutes for coding agent (claude and copilot) execution # Total bcal CLI budget per instance. diff --git a/src/bcbench/dataset/__init__.py b/src/bcbench/dataset/__init__.py index 8b08e5679..fed14ee84 100644 --- a/src/bcbench/dataset/__init__.py +++ b/src/bcbench/dataset/__init__.py @@ -1,7 +1,7 @@ """Dataset module for querying, validating and analyzing dataset entries.""" from bcbench.dataset.codereview import ArticleId, CodeReviewEntry, CodeReviewEntryMetadata, ReviewComment, Severity -from bcbench.dataset.dataset_entry import BaseDatasetEntry, BugFixEntry, NL2ALEntry, RepoGroundedEntry, TestEntry, TestGenEntry +from bcbench.dataset.dataset_entry import BaseDatasetEntry, BugFixEntry, DataQueryEntry, NL2ALEntry, RepoGroundedEntry, TestEntry, TestGenEntry from bcbench.dataset.extensibility_request import ExtRequestImplementEntry, ExtRequestTriageEntry, ManagedLabel __all__ = [ @@ -10,6 +10,7 @@ "BugFixEntry", "CodeReviewEntry", "CodeReviewEntryMetadata", + "DataQueryEntry", "ExtRequestImplementEntry", "ExtRequestTriageEntry", "ManagedLabel", diff --git a/src/bcbench/dataset/dataset_entry.py b/src/bcbench/dataset/dataset_entry.py index 3c87b6fe5..256bdbde9 100644 --- a/src/bcbench/dataset/dataset_entry.py +++ b/src/bcbench/dataset/dataset_entry.py @@ -4,7 +4,7 @@ import re from abc import abstractmethod from pathlib import Path -from typing import Annotated, Literal, Self +from typing import Annotated, Any, Literal, Self from pydantic import BaseModel, ConfigDict, Field, model_validator @@ -14,7 +14,7 @@ _config = get_config() -__all__ = ["BaseDatasetEntry", "BugFixEntry", "NL2ALEntry", "RepoGroundedEntry", "TestEntry", "TestGenEntry"] +__all__ = ["BaseDatasetEntry", "BugFixEntry", "DataQueryEntry", "NL2ALEntry", "RepoGroundedEntry", "TestEntry", "TestGenEntry"] class TestEntry(BaseModel): @@ -192,3 +192,34 @@ def get_task(self) -> str: def get_expected_output(self) -> Checklist: return {"assertions": self.expected} + + +class DataQueryEntry(BaseDatasetEntry): + """Dataset entry for the data-query category — answer a BC data question using the data tools. + + Execution-based: the agent retrieves the actual data (writing the rows to answer.json, plus the + query it used to query.al); evaluation compares those rows to the entry's expected rows. The + expected rows are the baked ``gold_rows`` (or, if not baked, computed by running ``gold_query`` + against the fixed Contoso container). The workspace is scaffolded by the pipeline, so there is no + repo or commit. + """ + + nl_prompt: Annotated[str, Field(min_length=1, pattern=r"^[^\x00]*$")] + gold_query: Annotated[str, Field(min_length=1, pattern=r"^[^\x00]*$")] + # Precomputed expected result rows for gold_query, "baked" once against the fixed container dataset + # so evaluation compares the agent's data to these without recompiling/running the gold query live. + # Empty means not yet baked; evaluation then falls back to running the gold query. + gold_rows: list[dict[str, Any]] = Field(default_factory=list) + # Whether row order is significant when comparing result sets (e.g. the question asks for a + # specific ranking). Defaults to False: result sets are compared order-insensitively. + ordered: bool = False + + @property + def customization_profile(self) -> str: + return "dataquery" + + def get_task(self) -> str: + return self.nl_prompt + + def get_expected_output(self) -> str: + return self.gold_query diff --git a/src/bcbench/evaluate/__init__.py b/src/bcbench/evaluate/__init__.py index 458f2cfe5..f4be463bb 100644 --- a/src/bcbench/evaluate/__init__.py +++ b/src/bcbench/evaluate/__init__.py @@ -3,9 +3,19 @@ from bcbench.evaluate.base import EvaluationPipeline from bcbench.evaluate.bugfix import BugFixPipeline from bcbench.evaluate.codereview import CodeReviewPipeline +from bcbench.evaluate.dataquery import DataQueryPipeline from bcbench.evaluate.ext_request_implement import ExtRequestImplementPipeline from bcbench.evaluate.ext_request_triage import ExtRequestTriagePipeline from bcbench.evaluate.nl2al import NL2ALPipeline from bcbench.evaluate.testgeneration import TestGenerationPipeline -__all__ = ["BugFixPipeline", "CodeReviewPipeline", "EvaluationPipeline", "ExtRequestImplementPipeline", "ExtRequestTriagePipeline", "NL2ALPipeline", "TestGenerationPipeline"] +__all__ = [ + "BugFixPipeline", + "CodeReviewPipeline", + "DataQueryPipeline", + "EvaluationPipeline", + "ExtRequestImplementPipeline", + "ExtRequestTriagePipeline", + "NL2ALPipeline", + "TestGenerationPipeline", +] diff --git a/src/bcbench/evaluate/dataquery.py b/src/bcbench/evaluate/dataquery.py new file mode 100644 index 000000000..0b1ad63a2 --- /dev/null +++ b/src/bcbench/evaluate/dataquery.py @@ -0,0 +1,144 @@ +import json +from collections.abc import Callable, Mapping, Sequence +from decimal import Decimal, InvalidOperation +from pathlib import Path + +from bcbench.dataset import DataQueryEntry +from bcbench.evaluate.base import EvaluationPipeline +from bcbench.github_actions import github_log_group +from bcbench.logger import get_logger +from bcbench.operations import clear_directory +from bcbench.results.base import ExecutionBasedEvaluationResult +from bcbench.types import EvaluationContext + +logger = get_logger(__name__) + +__all__ = ["DataQueryPipeline", "result_sets_match"] + +GENERATED_QUERY_FILE = "query.al" +ANSWER_FILE = "answer.json" + + +def _load_answer_rows(answer_file: Path) -> list[Mapping[str, object]]: + """Parse the agent's answer.json into a list of row objects. + + Accepts a bare JSON array, a single object (one row), or an object wrapping the rows under a + common key (``value``/``rows``/``data``/``results``) so a copied OData payload still works. + """ + try: + data = json.loads(answer_file.read_text(encoding="utf-8-sig") or "[]") + except json.JSONDecodeError as e: + raise ValueError(f"{ANSWER_FILE} is not valid JSON: {e}") from None + + if isinstance(data, dict): + wrapped = next((data[k] for k in ("value", "rows", "data", "results") if isinstance(data.get(k), list)), None) + data = wrapped if wrapped is not None else [data] + + if not isinstance(data, list): + raise TypeError(f"{ANSWER_FILE} must be a JSON array of row objects") + + rows: list[Mapping[str, object]] = [] + for row in data: + if not isinstance(row, dict): + raise TypeError(f"{ANSWER_FILE} rows must be JSON objects, got {type(row).__name__}") + rows.append(row) + return rows + + +def _normalize_value(value: object) -> str: + if value is None: + return "" + if isinstance(value, bool): + return str(value).lower() + if isinstance(value, (int, float, Decimal)): + # Only values that arrived as numeric JSON types are canonicalized: scale/trailing-zero- + # insensitive (500 == 500.0) with full precision preserved (1.00001 != 1.00002) and no float + # rounding (Decimal built from the value's string form). Both gold and generated rows come + # through the same OData->JSON pipeline, so amounts are numbers on both sides. + try: + return str(Decimal(str(value)).normalize()) + except (InvalidOperation, ValueError): + return str(value) + # Strings (and anything else) are preserved verbatim apart from a whitespace trim. Business Central + # Code/No. fields are JSON strings even when digit-only, so "001" must NOT collapse to "1" — coercing + # them through Decimal would let a wrong result be scored as matching the gold. + return str(value).strip() + + +def _normalize_rows(rows: Sequence[Mapping[str, object]], ordered: bool) -> list[tuple[str, ...]]: + # Compare on values only: drop OData/system metadata keys ('@'-prefixed) and ignore column + # names/order so a correct query still matches the gold even if it names columns differently. + normalized = [tuple(sorted(_normalize_value(v) for k, v in row.items() if not k.startswith("@"))) for row in rows] + return normalized if ordered else sorted(normalized) + + +def result_sets_match(generated: Sequence[Mapping[str, object]], gold: Sequence[Mapping[str, object]], ordered: bool = False) -> bool: + """Compare two query result sets for equality. + + Values are compared (numbers normalized, column names/order ignored); row order is ignored + unless ``ordered`` is True (the question asks for a specific ranking). + """ + return _normalize_rows(generated, ordered) == _normalize_rows(gold, ordered) + + +class DataQueryPipeline(EvaluationPipeline[DataQueryEntry]): + """Pipeline for the data-query category — generate an AL query, evaluate deterministically. + + The agent answers a data question by retrieving the ACTUAL data with the BC data tools and writing + the rows to ``answer.json`` (plus the ``query.al`` it used, kept only for inspection). Evaluation + runs the entry's gold query against the container's fixed (Contoso) dataset and compares the gold + rows to the agent's rows: build = the agent produced a well-formed answer.json; resolved = its rows + match the gold query's. The data can't be answered from model knowledge, so a correct answer requires + genuinely querying the environment. + """ + + def setup_workspace(self, entry: DataQueryEntry, repo_path: Path) -> None: + # The workspace is shared into the running container, so its contents are cleared in place. + clear_directory(repo_path) + + def setup(self, context: EvaluationContext[DataQueryEntry]) -> None: + self.setup_workspace(context.entry, context.repo_path) + + def run_agent(self, context: EvaluationContext[DataQueryEntry], agent_runner: Callable) -> None: + with github_log_group(f"{context.agent_name} -- Entry: {context.entry.instance_id}"): + context.metrics, context.experiment = agent_runner(context) + + def evaluate(self, context: EvaluationContext[DataQueryEntry]) -> None: + query_file = context.repo_path / GENERATED_QUERY_FILE + # query.al is only an inspection artifact now; scoring is on the data the agent retrieved. + generated_query = query_file.read_text(encoding="utf-8").strip() if query_file.exists() else "" + answer_file = context.repo_path / ANSWER_FILE + + gold_rows = self._gold_rows(context) + + if not answer_file.exists(): + logger.warning(f"Agent produced no {ANSWER_FILE} for {context.entry.instance_id}") + self.save_result(context, ExecutionBasedEvaluationResult.create_build_failure(context, output=generated_query, error_message=f"No {ANSWER_FILE} produced")) + return + + try: + agent_rows = _load_answer_rows(answer_file) + except (ValueError, TypeError) as e: + logger.warning(f"Unusable {ANSWER_FILE} for {context.entry.instance_id}: {e}") + self.save_result(context, ExecutionBasedEvaluationResult.create_build_failure(context, output=generated_query, error_message=str(e))) + return + + resolved = result_sets_match(agent_rows, gold_rows, context.entry.ordered) + error_message = None if resolved else f"Result set mismatch: answer {len(agent_rows)} rows vs gold {len(gold_rows)} rows" + result = ExecutionBasedEvaluationResult.create_result(context, output=generated_query, build=True, resolved=resolved, error_message=error_message) + logger.info(f"{context.entry.instance_id}: build=True resolved={resolved}") + self.save_result(context, result) + + def _gold_rows(self, context: EvaluationContext[DataQueryEntry]) -> Sequence[Mapping[str, object]]: + """The expected rows: use the baked gold_rows if present, else compute them live (fail loud). + + A gold query that doesn't compile/run is a harness/dataset bug, not the agent's fault, so the + live fallback deliberately does NOT catch its failure — it must fail the run loudly. + """ + if context.entry.gold_rows: + return list(context.entry.gold_rows) + + from bcbench.operations import execute_al_query + + logger.info(f"No baked gold_rows for {context.entry.instance_id}; running gold query live") + return execute_al_query(context.entry.gold_query, context.get_container(), context.entry.environment_setup_version, context.repo_path, "gold") diff --git a/src/bcbench/operations/__init__.py b/src/bcbench/operations/__init__.py index 0b3d7ac58..652057c1d 100644 --- a/src/bcbench/operations/__init__.py +++ b/src/bcbench/operations/__init__.py @@ -6,10 +6,12 @@ build_ps_dataset_tests_script, build_ps_test_script, copy_symbol_apps, + execute_al_query, resolve_artifact_version_root, run_tests, + wrap_query_as_api, ) -from bcbench.operations.filesystem_operations import remove_tree +from bcbench.operations.filesystem_operations import clear_directory, remove_tree from bcbench.operations.git_operations import ( apply_patch, checkout_commit, @@ -40,10 +42,12 @@ "checkout_commit", "clean_project_paths", "clean_repo", + "clear_directory", "clone_repo_at_revision", "commit_changes", "copy_problem_statement_folder", "copy_symbol_apps", + "execute_al_query", "extract_tests_from_patch", "fetch_commit_if_missing", "has_changes", @@ -58,4 +62,5 @@ "setup_instructions_from_config", "setup_repo_prebuild", "stage_and_get_diff", + "wrap_query_as_api", ] diff --git a/src/bcbench/operations/bc_operations.py b/src/bcbench/operations/bc_operations.py index 88ea254d2..b9a047513 100644 --- a/src/bcbench/operations/bc_operations.py +++ b/src/bcbench/operations/bc_operations.py @@ -13,6 +13,8 @@ from bcbench.dataset.dataset_entry import _BugFixTestGenBase from bcbench.exceptions import BuildError, BuildTimeoutExpired, TestExecutionError, TestExecutionTimeoutExpired from bcbench.logger import get_logger +from bcbench.operations.filesystem_operations import remove_tree +from bcbench.operations.setup_operations import bootstrap_app_json from bcbench.types import ContainerConfig logger = get_logger(__name__) @@ -235,3 +237,182 @@ def run_test_suite(test_entries: list[TestEntry], expectation: Literal["Pass", " except subprocess.TimeoutExpired: logger.exception(f"Test execution timed out after {_config.timeout.test_execution} seconds") raise TestExecutionTimeoutExpired(test_entries_json, _config.timeout.test_execution) from None + + +# --- data-query category: compile + run an AL query and capture its rows via a wrapped API query --- + +# API metadata injected into a generated/gold query so it is exposed over OData and can be fetched. +_QUERY_API_PUBLISHER = "bcbench" +_QUERY_API_GROUP = "eval" +_QUERY_API_VERSION = "v1.0" + + +def _safe_object_name(object_id: int) -> str: + """A short, unique, always-valid query object name. + + The object name is irrelevant to a query's result set (we score by comparing data, not + identifiers), but AL requires it to be a valid identifier of <=30 characters and unique in + the tenant. Normalizing it keeps the benchmark focused on query logic instead of failing an + otherwise-correct query just because the agent chose a long/descriptive name (AL0305). + """ + return f"BCBenchQuery{object_id}" + + +def _entity_set_name(object_id: int) -> str: + """Per-object OData entity set so the generated and gold API queries don't collide on route.""" + return f"bcbenchResults{object_id}" + + +def _entity_name(object_id: int) -> str: + return f"bcbenchResult{object_id}" + + +def _query_api_properties(object_id: int) -> str: + return ( + "QueryType = API;\n" + f" APIPublisher = '{_QUERY_API_PUBLISHER}';\n" + f" APIGroup = '{_QUERY_API_GROUP}';\n" + f" APIVersion = '{_QUERY_API_VERSION}';\n" + f" EntityName = '{_entity_name(object_id)}';\n" + f" EntitySetName = '{_entity_set_name(object_id)}';" + ) + + +def wrap_query_as_api(query_text: str, object_id: int) -> str: + """Turn a plain AL query object into an API query the harness can fetch over OData. + + Reassigns the object id and normalizes the object name (so generated and gold apps don't + collide and long names don't cause AL0305), drops any existing ``QueryType`` line, and + injects the API properties right after the object's opening brace. Pure string transform so + it can be unit-tested without a container. + """ + import re + + safe_name = _safe_object_name(object_id) + # AL keywords are case-insensitive; match `query`/`QueryType` in any casing. + text, replaced = re.subn( + r'(\bquery\s+)\d+\s+("(?:[^"\\]|\\.)*"|\w+)', + rf"\g<1>{object_id} {safe_name}", + query_text, + count=1, + flags=re.IGNORECASE, + ) + if replaced == 0: + raise BuildError("query-wrap", f"No AL query object declaration found in generated output:\n{query_text}") + + text = re.sub(r"\bQueryType\s*=\s*\w+\s*;", "", text, count=1, flags=re.IGNORECASE) + + brace_index = text.find("{") + if brace_index == -1: + raise BuildError("query-wrap", f"Generated query has no object body ('{{' not found):\n{query_text}") + return f"{text[: brace_index + 1]}\n {_query_api_properties(object_id)}\n{text[brace_index + 1 :]}" + + +_QUERY_RUN_TEMPLATE = Template( + """ +Import-Module BcContainerHelper -Force -DisableNameChecking +Import-Module '$app_utils_path' -Force +$$ErrorActionPreference = 'Stop' + +$$password = ConvertTo-SecureString '$password' -AsPlainText -Force +$$credential = New-Object System.Management.Automation.PSCredential('$username', $$password) + +# Remove any app left installed by a previous run of the same suffix so re-running against the +# same container doesn't fail with an object-ID conflict on the fixed 50100/50101 range. +UnInstall-BcContainerApp -containerName '$container_name' -name '$app_name' -publisher '$app_publisher' -force -doNotSaveData -ErrorAction SilentlyContinue +UnPublish-BcContainerApp -containerName '$container_name' -name '$app_name' -publisher '$app_publisher' -ErrorAction SilentlyContinue + +# Compile + publish the wrapped API query with the same proven helper the other categories use +# (clears/sets an explicit .alpackages symbol folder, GenerateReportLayout=No, ForceSync, +# dependencyPublishingOption=ignore) so Base Application symbols resolve reliably. +Invoke-AppBuildAndPublish -containerName '$container_name' -appProjectFolder '$app_dir' -credential $$credential -skipVerification -useDevEndpoint + +try { + # Read the query's rows over the OData/API endpoint from *inside* the container, so we don't depend + # on host->container name resolution or published ports (the runner does not update its hosts file). + # Basic auth header is built by hand rather than via -Credential: PowerShell 7 (used inside the + # container) refuses -Credential over plain HTTP, and a manual header works on both 5.1 and 7. + $$json = Invoke-ScriptInBcContainer -containerName '$container_name' -argumentList $$credential, '$publisher', '$group', '$version', '$entity_set' -scriptblock { + param($$cred, $$pub, $$grp, $$ver, $$eset) + $$pair = "$$($$cred.UserName):$$($$cred.GetNetworkCredential().Password)" + $$headers = @{ Authorization = 'Basic ' + [Convert]::ToBase64String([Text.Encoding]::ASCII.GetBytes($$pair)) } + $$base = 'http://localhost:7048/BC/api' + $$companyId = (Invoke-RestMethod -Uri "$$base/v2.0/companies" -Headers $$headers).value[0].id + # Follow @odata.nextLink so large result sets aren't silently truncated to the first page. + $$rows = [System.Collections.Generic.List[object]]::new() + $$uri = "$$base/$$pub/$$grp/$$ver/companies($$companyId)/$$eset" + while ($$uri) { + $$page = Invoke-RestMethod -Uri $$uri -Headers $$headers + if ($$null -ne $$page.value) { foreach ($$row in $$page.value) { $$rows.Add($$row) } } + $$uri = $$page.'@odata.nextLink' + } + $$rows | ConvertTo-Json -Depth 10 -Compress + } + $$json | Out-File -FilePath '$result_file' -Encoding utf8 +} +finally { + # Best-effort teardown so the container doesn't accumulate throwaway apps between runs. + UnInstall-BcContainerApp -containerName '$container_name' -name '$app_name' -publisher '$app_publisher' -force -doNotSaveData -ErrorAction SilentlyContinue + UnPublish-BcContainerApp -containerName '$container_name' -name '$app_name' -publisher '$app_publisher' -ErrorAction SilentlyContinue +} +""".strip() +) + + +def execute_al_query(query_text: str, container: ContainerConfig, version: str, work_root: Path, suffix: str) -> list[dict]: + """Compile + publish an AL query (wrapped as an API query) to the container and return its rows. + + Builds a throwaway app under ``work_root/.bcbench-query-``, compiles + publishes it, + then reads the query's OData endpoint. Raises :class:`BuildError` if the query does not + compile or publish. + + NOTE: the container-side steps (compile/publish/OData fetch) require a running BC container + and have not been validated locally; the wrapping and comparison logic are unit-tested. + """ + import json + + object_id = 50100 if suffix == "generated" else 50101 + app_dir = work_root / f".bcbench-query-{suffix}" + if app_dir.exists(): + remove_tree(app_dir) + + app_name = f"BC-Bench Query {suffix}" + app_publisher = "BC-Bench" + bootstrap_app_json(app_dir, app_name, version, id_range=(object_id, object_id), publisher=app_publisher) + (app_dir / "query.al").write_text(wrap_query_as_api(query_text, object_id), encoding="utf-8") + # Symbols are downloaded into an explicit .alpackages folder by Invoke-AppBuildAndPublish (below). + + result_file = app_dir / "result.json" + app_utils_path = _config.paths.ps_script_path / "AppUtils.psm1" + ps_script = _QUERY_RUN_TEMPLATE.substitute( + app_utils_path=_escape_ps_string(str(app_utils_path)), + container_name=_escape_ps_string(container.name), + username=_escape_ps_string(container.username), + password=_escape_ps_string(container.password), + app_dir=_escape_ps_string(str(app_dir)), + app_name=_escape_ps_string(app_name), + app_publisher=_escape_ps_string(app_publisher), + publisher=_QUERY_API_PUBLISHER, + group=_QUERY_API_GROUP, + version=_QUERY_API_VERSION, + entity_set=_entity_set_name(object_id), + result_file=_escape_ps_string(str(result_file)), + ) + + try: + subprocess.run( + ["pwsh", "-NoProfile", "-NonInteractive", "-Command", ps_script], + cwd=work_root, + capture_output=True, + check=True, + text=True, + timeout=_config.timeout.build_app, + ) + except subprocess.CalledProcessError as e: + logger.debug(f"Query compile/publish/fetch failed ({suffix}): {e.stdout}\n{e.stderr}") + raise BuildError(f"query-{suffix}", (e.stdout or "") + (e.stderr or "")) from None + except subprocess.TimeoutExpired: + raise BuildTimeoutExpired(f"query-{suffix}", _config.timeout.build_app) from None + + rows = json.loads(result_file.read_text(encoding="utf-8-sig") or "[]") + return rows if isinstance(rows, list) else [rows] diff --git a/src/bcbench/operations/filesystem_operations.py b/src/bcbench/operations/filesystem_operations.py index 3a3b35fa1..4b745aad7 100644 --- a/src/bcbench/operations/filesystem_operations.py +++ b/src/bcbench/operations/filesystem_operations.py @@ -18,3 +18,18 @@ def remove_tree(path: Path) -> None: Use when `shutil.rmtree` fails due to read-only files, which usually occur in secondary runs on Windows. """ shutil.rmtree(path, onexc=_force_remove_readonly) + + +def clear_directory(path: Path) -> None: + """ + Remove everything inside a directory, leaving the directory itself in place. + + Use instead of `remove_tree` when the directory should survive. + """ + path.mkdir(parents=True, exist_ok=True) + for child in path.iterdir(): + if child.is_dir(): + remove_tree(child) + else: + child.chmod(stat.S_IWRITE) + child.unlink() diff --git a/src/bcbench/operations/skills_operations.py b/src/bcbench/operations/skills_operations.py index ba5fe9b34..b3bda008e 100644 --- a/src/bcbench/operations/skills_operations.py +++ b/src/bcbench/operations/skills_operations.py @@ -9,14 +9,24 @@ logger = get_logger(__name__) -def setup_agent_skills(agent_config: dict, entry: BaseDatasetEntry, repo_path: Path, harness: AgentHarness) -> bool: +def setup_agent_skills( + agent_config: dict, + entry: BaseDatasetEntry, + repo_path: Path, + harness: AgentHarness, + skills_enabled_override: bool | None = None, +) -> bool: """ Setup skills in the repository if available. + Args: + skills_enabled_override: When not None, takes precedence over ``config.yaml``'s + ``skills.enabled`` (used to toggle skills per run via the ``--skills`` CLI flag). + Returns: True if skills were copied, False if skills are disabled. """ - skills_enabled: bool = agent_config["skills"]["enabled"] + skills_enabled: bool = agent_config["skills"]["enabled"] if skills_enabled_override is None else skills_enabled_override if skills_enabled: source_skills: Path = _get_source_instructions_path(entry.customization_profile) diff --git a/src/bcbench/results/base.py b/src/bcbench/results/base.py index 15eda4c3f..aad37762a 100644 --- a/src/bcbench/results/base.py +++ b/src/bcbench/results/base.py @@ -115,6 +115,11 @@ def create_success(cls, context: "EvaluationContext", output: str) -> Self: def create_build_failure(cls, context: "EvaluationContext", output: str, error_message: str) -> Self: return cls(**cls._base_fields(context), output=output, error_message=error_message, resolved=False, build=False) + @classmethod + def create_result(cls, context: "EvaluationContext", output: str, *, build: bool, resolved: bool, error_message: str | None = None) -> Self: + """General factory for execution outcomes, e.g. compiled+ran but produced the wrong result (build=True, resolved=False).""" + return cls(**cls._base_fields(context), output=output, build=build, resolved=resolved, error_message=error_message) + @property def status_label(self) -> str: if self.timeout: diff --git a/src/bcbench/results/summary.py b/src/bcbench/results/summary.py index 826318dc9..f3d8da6ed 100644 --- a/src/bcbench/results/summary.py +++ b/src/bcbench/results/summary.py @@ -178,6 +178,7 @@ def from_results(cls, results: Sequence[BaseEvaluationResult], run_id: str) -> " return summary.model_copy( update={ + "total": total, "resolved": resolved, "failed": total - resolved, "build": build, diff --git a/src/bcbench/types.py b/src/bcbench/types.py index ec75a2896..884a541db 100644 --- a/src/bcbench/types.py +++ b/src/bcbench/types.py @@ -229,6 +229,7 @@ class EvaluationCategory(StrEnum): TEST_GENERATION = "test-generation" CODE_REVIEW = "code-review" NL2AL = "nl2al" + DATA_QUERY = "data-query" # Implement an approved extensibility request (add an event/extension point) as an AL code change. # The sibling ext-advisor category is planned but not yet implemented. EXT_REQUEST_IMPLEMENT = "extensibility-request-implement" @@ -248,6 +249,8 @@ def dataset_path(self) -> Path: return get_config().paths.dataset_dir / "codereview.jsonl" case EvaluationCategory.NL2AL: return get_config().paths.dataset_dir / "nl2al.jsonl" + case EvaluationCategory.DATA_QUERY: + return get_config().paths.dataset_dir / "dataquery.jsonl" case EvaluationCategory.EXT_REQUEST_IMPLEMENT: return get_config().paths.dataset_dir / "extensibility_request_implement.jsonl" case EvaluationCategory.EXT_REQUEST_TRIAGE: @@ -257,7 +260,7 @@ def dataset_path(self) -> Path: @property def entry_class(self) -> type[BaseDatasetEntry]: - from bcbench.dataset import BugFixEntry, CodeReviewEntry, ExtRequestImplementEntry, ExtRequestTriageEntry, NL2ALEntry, TestGenEntry + from bcbench.dataset import BugFixEntry, CodeReviewEntry, DataQueryEntry, ExtRequestImplementEntry, ExtRequestTriageEntry, NL2ALEntry, TestGenEntry match self: case EvaluationCategory.BUG_FIX: @@ -268,6 +271,8 @@ def entry_class(self) -> type[BaseDatasetEntry]: return CodeReviewEntry case EvaluationCategory.NL2AL: return NL2ALEntry + case EvaluationCategory.DATA_QUERY: + return DataQueryEntry case EvaluationCategory.EXT_REQUEST_IMPLEMENT: return ExtRequestImplementEntry case EvaluationCategory.EXT_REQUEST_TRIAGE: @@ -277,7 +282,7 @@ def entry_class(self) -> type[BaseDatasetEntry]: @property def result_class(self) -> type[BaseEvaluationResult]: - from bcbench.results.base import JudgeBasedEvaluationResult + from bcbench.results.base import ExecutionBasedEvaluationResult, JudgeBasedEvaluationResult from bcbench.results.bugfix import BugFixResult from bcbench.results.codereview import CodeReviewResult from bcbench.results.testgeneration import TestGenerationResult @@ -291,6 +296,8 @@ def result_class(self) -> type[BaseEvaluationResult]: return CodeReviewResult case EvaluationCategory.NL2AL: return JudgeBasedEvaluationResult + case EvaluationCategory.DATA_QUERY: + return ExecutionBasedEvaluationResult case EvaluationCategory.EXT_REQUEST_IMPLEMENT: return JudgeBasedEvaluationResult case EvaluationCategory.EXT_REQUEST_TRIAGE: @@ -313,6 +320,8 @@ def summary_class(self) -> type[EvaluationResultSummary]: return CodeReviewResultSummary case EvaluationCategory.NL2AL: return JudgeBasedEvaluationResultSummary + case EvaluationCategory.DATA_QUERY: + return ExecutionBasedEvaluationResultSummary case EvaluationCategory.EXT_REQUEST_IMPLEMENT: return JudgeBasedEvaluationResultSummary case EvaluationCategory.EXT_REQUEST_TRIAGE: @@ -334,6 +343,8 @@ def aggregate_class(self) -> type[LeaderboardAggregate]: return CodeReviewLeaderboardAggregate case EvaluationCategory.NL2AL: return JudgeBasedLeaderboardAggregate + case EvaluationCategory.DATA_QUERY: + return ExecutionBasedLeaderboardAggregate case EvaluationCategory.EXT_REQUEST_IMPLEMENT: return JudgeBasedLeaderboardAggregate case EvaluationCategory.EXT_REQUEST_TRIAGE: @@ -343,7 +354,7 @@ def aggregate_class(self) -> type[LeaderboardAggregate]: @property def pipeline(self) -> EvaluationPipeline: - from bcbench.evaluate import BugFixPipeline, CodeReviewPipeline, ExtRequestImplementPipeline, ExtRequestTriagePipeline, NL2ALPipeline, TestGenerationPipeline + from bcbench.evaluate import BugFixPipeline, CodeReviewPipeline, DataQueryPipeline, ExtRequestImplementPipeline, ExtRequestTriagePipeline, NL2ALPipeline, TestGenerationPipeline match self: case EvaluationCategory.BUG_FIX: @@ -354,6 +365,8 @@ def pipeline(self) -> EvaluationPipeline: return CodeReviewPipeline() case EvaluationCategory.NL2AL: return NL2ALPipeline() + case EvaluationCategory.DATA_QUERY: + return DataQueryPipeline() case EvaluationCategory.EXT_REQUEST_IMPLEMENT: return ExtRequestImplementPipeline() case EvaluationCategory.EXT_REQUEST_TRIAGE: @@ -368,7 +381,7 @@ def judge_model(self) -> str | None: judge = get_config().judge match self: - case EvaluationCategory.BUG_FIX | EvaluationCategory.TEST_GENERATION: + case EvaluationCategory.BUG_FIX | EvaluationCategory.TEST_GENERATION | EvaluationCategory.DATA_QUERY: return None case EvaluationCategory.CODE_REVIEW: return judge.code_review_model @@ -393,6 +406,8 @@ def evaluators(self) -> list[str]: return ["precision_score", "recall_score", "f1_score", "valid_review_output"] case EvaluationCategory.NL2AL: return ["lm_checklist"] + case EvaluationCategory.DATA_QUERY: + return ["resolution_rate", "build_rate"] case EvaluationCategory.EXT_REQUEST_IMPLEMENT: return ["lm_checklist"] case EvaluationCategory.EXT_REQUEST_TRIAGE: @@ -410,6 +425,8 @@ def core_score(self) -> str: return "F1Score" case EvaluationCategory.NL2AL | EvaluationCategory.EXT_REQUEST_IMPLEMENT | EvaluationCategory.EXT_REQUEST_TRIAGE: return "test_passed" + case EvaluationCategory.DATA_QUERY: + return "ResolutionRate" raise ValueError(f"Unknown evaluation category: {self}") @@ -417,7 +434,7 @@ def core_score(self) -> str: def requires_container(self) -> bool: """Whether evaluating this category builds/runs AL code and therefore needs a BC container.""" match self: - case EvaluationCategory.BUG_FIX | EvaluationCategory.TEST_GENERATION: + case EvaluationCategory.BUG_FIX | EvaluationCategory.TEST_GENERATION | EvaluationCategory.DATA_QUERY: return True case EvaluationCategory.CODE_REVIEW | EvaluationCategory.NL2AL | EvaluationCategory.EXT_REQUEST_IMPLEMENT | EvaluationCategory.EXT_REQUEST_TRIAGE: return False @@ -438,7 +455,7 @@ def runner(self) -> str: Only categories that require building BaseApp needs self-hosted runners. """ match self: - case EvaluationCategory.BUG_FIX | EvaluationCategory.TEST_GENERATION: + case EvaluationCategory.BUG_FIX | EvaluationCategory.TEST_GENERATION | EvaluationCategory.DATA_QUERY: return "GitHub-BCBench" case EvaluationCategory.CODE_REVIEW | EvaluationCategory.EXT_REQUEST_IMPLEMENT | EvaluationCategory.EXT_REQUEST_TRIAGE: return "ubuntu-latest" diff --git a/tests/conftest.py b/tests/conftest.py index 181aec512..0d82c9033 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -13,7 +13,7 @@ import pytest -from bcbench.dataset import BaseDatasetEntry, BugFixEntry, ExtRequestImplementEntry, ExtRequestTriageEntry, ManagedLabel, NL2ALEntry, TestEntry +from bcbench.dataset import BaseDatasetEntry, BugFixEntry, DataQueryEntry, ExtRequestImplementEntry, ExtRequestTriageEntry, ManagedLabel, NL2ALEntry, TestEntry from bcbench.dataset.codereview import CodeReviewEntry, CodeReviewEntryMetadata, ReviewComment, Severity from bcbench.dataset.dataset_entry import _BugFixTestGenBase from bcbench.evaluate.review_parsing import parse_review_output @@ -361,6 +361,35 @@ def sample_nl2al_entry() -> NL2ALEntry: return create_nl2al_entry() +VALID_DATA_QUERY_PROMPT = "Return the total sales amount per customer." +VALID_GOLD_QUERY = ( + 'query 50100 SalesByCustomer\n{\n QueryType = Normal;\n elements\n {\n dataitem(Customer; Customer)\n {\n column(No; "No.") { }\n }\n }\n}' +) + + +def create_data_query_entry( + instance_id: str = "dataquery__sales-by-customer-1", + environment_setup_version: str = VALID_ENVIRONMENT_VERSION, + nl_prompt: str = VALID_DATA_QUERY_PROMPT, + created_at: str = VALID_CREATED_AT, + gold_query: str = VALID_GOLD_QUERY, + ordered: bool = False, +) -> DataQueryEntry: + return DataQueryEntry( + instance_id=instance_id, + environment_setup_version=environment_setup_version, + nl_prompt=nl_prompt, + created_at=created_at, + gold_query=gold_query, + ordered=ordered, + ) + + +@pytest.fixture +def sample_data_query_entry() -> DataQueryEntry: + return create_data_query_entry() + + def create_ext_implement_entry( instance_id: str = "microsoftInternal__NAV-Ext_Request_Impl-30361", repo: str = "microsoftInternal/NAV", diff --git a/tests/test_agent_env.py b/tests/test_agent_env.py new file mode 100644 index 000000000..7846dd7ef --- /dev/null +++ b/tests/test_agent_env.py @@ -0,0 +1,34 @@ +from bcbench.agent.shared.env import agent_subprocess_env + + +def test_scrubs_bc_connection_vars(monkeypatch): + monkeypatch.setenv("BC_SERVER_URL", "http://bcbench-sales") + monkeypatch.setenv("BC_SERVER_USERNAME", "admin") + monkeypatch.setenv("BC_SERVER_PASSWORD", "secret") + monkeypatch.setenv("BC_MCP_URL", "http://172.17.0.2:7048/BC") + monkeypatch.setenv("BC_MCP_COMPANY", "CRONUS") + monkeypatch.setenv("BC_CONTAINER_NAME", "bcbench-sales") + + env = agent_subprocess_env() + + assert not any(k.startswith(("BC_SERVER_", "BC_MCP_")) for k in env) + assert "BC_CONTAINER_NAME" not in env + + +def test_preserves_other_vars(monkeypatch): + monkeypatch.setenv("PATH", "/usr/bin") + monkeypatch.setenv("BC_SERVER_PASSWORD", "secret") + + env = agent_subprocess_env() + + assert env["PATH"] == "/usr/bin" + assert "BC_SERVER_PASSWORD" not in env + + +def test_overrides_are_applied(monkeypatch): + monkeypatch.setenv("BC_SERVER_PASSWORD", "secret") + + env = agent_subprocess_env({"FLAG": "on"}) + + assert env["FLAG"] == "on" + assert "BC_SERVER_PASSWORD" not in env diff --git a/tests/test_agent_skills.py b/tests/test_agent_skills.py index 21aafc7e2..4fa918a03 100644 --- a/tests/test_agent_skills.py +++ b/tests/test_agent_skills.py @@ -8,7 +8,7 @@ import pytest -from bcbench.dataset import RepoGroundedEntry +from bcbench.dataset import BaseDatasetEntry, RepoGroundedEntry from bcbench.operations import setup_agent_skills from bcbench.operations.instruction_operations import _get_source_instructions_path from bcbench.types import AgentHarness @@ -160,3 +160,45 @@ def test_skills_disabled(): assert result is False assert not (repo_path / ".github" / "skills").exists() + + +def test_skills_override_enables_when_config_disabled(): + """--skills (override=True) enables skills even when config.yaml has them disabled.""" + with TemporaryDirectory() as tmpdir: + repo_path = Path(tmpdir) + entry = MagicMock(spec=BaseDatasetEntry) + entry.customization_profile = "microsoftInternal-NAV" + config = {"skills": {"enabled": False}} + + result = setup_agent_skills(config, entry, repo_path, harness=AgentHarness.COPILOT, skills_enabled_override=True) + + assert result is True + assert (repo_path / ".github" / "skills").exists() + + +def test_skills_override_disables_when_config_enabled(): + """override=False wins over an enabled config, so no skills are copied.""" + with TemporaryDirectory() as tmpdir: + repo_path = Path(tmpdir) + entry = MagicMock(spec=BaseDatasetEntry) + entry.customization_profile = "microsoftInternal-NAV" + config = {"skills": {"enabled": True}} + + result = setup_agent_skills(config, entry, repo_path, harness=AgentHarness.COPILOT, skills_enabled_override=False) + + assert result is False + assert not (repo_path / ".github" / "skills").exists() + + +def test_skills_override_none_falls_back_to_config(): + """override=None (default) preserves the config-driven behavior.""" + with TemporaryDirectory() as tmpdir: + repo_path = Path(tmpdir) + entry = MagicMock(spec=BaseDatasetEntry) + entry.customization_profile = "microsoftInternal-NAV" + config = {"skills": {"enabled": False}} + + result = setup_agent_skills(config, entry, repo_path, harness=AgentHarness.COPILOT, skills_enabled_override=None) + + assert result is False + assert not (repo_path / ".github" / "skills").exists() diff --git a/tests/test_claude_code_agent.py b/tests/test_claude_code_agent.py index b62c7766c..2141c4266 100644 --- a/tests/test_claude_code_agent.py +++ b/tests/test_claude_code_agent.py @@ -43,7 +43,8 @@ def test_claude_code_excludes_user_settings_and_auto_memory(tmp_path: Path, monk assert mock_run.call_args.args[0] == [ "claude", - "--output-format=json", + "--output-format=stream-json", + "--verbose", "--strict-mcp-config", "--setting-sources=project,local", "--model=claude-test-model", diff --git a/tests/test_claude_code_metrics.py b/tests/test_claude_code_metrics.py index 8a5608a3a..654b8df2f 100644 --- a/tests/test_claude_code_metrics.py +++ b/tests/test_claude_code_metrics.py @@ -1,8 +1,10 @@ """Tests for Claude Code metrics parsing.""" +import json + import pytest -from bcbench.agent.claude.metrics import parse_metrics +from bcbench.agent.claude.metrics import parse_metrics, parse_stream_output class TestClaudeCodeMetricsParsing: @@ -115,3 +117,51 @@ def test_parse_metrics_with_model_usage(self): assert metrics.turn_count == 14 assert metrics.prompt_tokens == 41 + 22439 + 246700 assert metrics.completion_tokens == 1909 + + +class TestClaudeStreamParsing: + def _lines(self, *events: dict) -> list[str]: + return [json.dumps(event) for event in events] + + def test_counts_mcp_tool_use_across_assistant_messages(self): + lines = self._lines( + {"type": "system", "subtype": "init", "mcp_servers": [{"name": "bcmcp", "status": "connected"}], "tools": ["Bash", "mcp__bcmcp__bc_data_query"]}, + {"type": "assistant", "message": {"content": [{"type": "text", "text": "Looking"}, {"type": "tool_use", "name": "mcp__bcmcp__bc_data_find_tables", "input": {}}]}}, + {"type": "assistant", "message": {"content": [{"type": "tool_use", "name": "mcp__bcmcp__bc_data_query", "input": {}}]}}, + {"type": "assistant", "message": {"content": [{"type": "tool_use", "name": "mcp__bcmcp__bc_data_query", "input": {}}]}}, + {"type": "result", "duration_ms": 5000, "num_turns": 3, "result": "Done"}, + ) + + metrics, final_response = parse_stream_output(lines) + + assert final_response == "Done" + assert metrics is not None + assert metrics.tool_usage == {"mcp__bcmcp__bc_data_find_tables": 1, "mcp__bcmcp__bc_data_query": 2} + assert metrics.execution_time == 5.0 + assert metrics.turn_count == 3 + + def test_tool_usage_without_result_event_still_returned(self): + lines = self._lines( + {"type": "assistant", "message": {"content": [{"type": "tool_use", "name": "Bash", "input": {}}]}}, + ) + + metrics, final_response = parse_stream_output(lines) + + assert final_response is None + assert metrics is not None + assert metrics.tool_usage == {"Bash": 1} + + def test_no_events_returns_none(self): + metrics, final_response = parse_stream_output(["", " "]) + + assert metrics is None + assert final_response is None + + def test_skips_malformed_lines(self): + lines = ["not json", json.dumps({"type": "result", "duration_ms": 1000, "result": "ok"})] + + metrics, final_response = parse_stream_output(lines) + + assert final_response == "ok" + assert metrics is not None + assert metrics.execution_time == 1.0 diff --git a/tests/test_copilot_metrics_parsing.py b/tests/test_copilot_metrics_parsing.py index 61fbd8977..204091289 100644 --- a/tests/test_copilot_metrics_parsing.py +++ b/tests/test_copilot_metrics_parsing.py @@ -78,3 +78,40 @@ def test_parse_output_without_metrics(): assert metrics is None assert response == "done" + + +def test_parse_output_counts_tool_usage_from_stream(): + output_lines = [ + _json_line({"type": "model.call_start", "data": {"turnId": "0"}}), + _json_line({"type": "tool.execution_start", "data": {"toolName": "bc_data_query", "arguments": {}}}), + _json_line({"type": "tool.execution_start", "data": {"toolName": "bc_data_query", "arguments": {}}}), + _json_line({"type": "tool.execution_start", "data": {"toolName": "task", "arguments": {}}}), + # A sub-agent's inner tool call surfaces in the same stream and must be counted too. + _json_line({"type": "tool.execution_start", "data": {"toolName": "view", "arguments": {"path": "x"}}}), + ] + + metrics, _ = parse_output(output_lines) + + assert metrics is not None + assert metrics.tool_usage == {"bc_data_query": 2, "task": 1, "view": 1} + + +def test_parse_output_sublabels_lsp_operations(): + output_lines = [ + _json_line({"type": "model.call_start", "data": {"turnId": "0"}}), + _json_line({"type": "tool.execution_start", "data": {"toolName": "lsp", "arguments": {"operation": "hover"}}}), + _json_line({"type": "tool.execution_start", "data": {"toolName": "lsp", "arguments": {"operation": "hover"}}}), + _json_line({"type": "tool.execution_start", "data": {"toolName": "lsp", "arguments": {"operation": "findReferences"}}}), + ] + + metrics, _ = parse_output(output_lines) + + assert metrics is not None + assert metrics.tool_usage == {"lsp:hover": 2, "lsp:findReferences": 1} + + +def test_parse_output_tool_usage_none_when_no_tools(): + metrics, _ = parse_output([_json_line({"type": "model.call_start", "data": {"turnId": "0"}})]) + + assert metrics is not None + assert metrics.tool_usage is None diff --git a/tests/test_dataquery_evaluation.py b/tests/test_dataquery_evaluation.py new file mode 100644 index 000000000..355a476f7 --- /dev/null +++ b/tests/test_dataquery_evaluation.py @@ -0,0 +1,221 @@ +import json + +import pytest + +from bcbench.evaluate.dataquery import _load_answer_rows, result_sets_match +from bcbench.exceptions import BuildError +from bcbench.operations import bc_operations, wrap_query_as_api +from bcbench.types import ContainerConfig + + +class TestResultSetsMatch: + def test_identical_rows_match(self): + rows = [{"No": "C1", "Total": 100}, {"No": "C2", "Total": 200}] + assert result_sets_match(rows, rows) + + def test_row_order_ignored_when_unordered(self): + generated = [{"No": "C2", "Total": 200}, {"No": "C1", "Total": 100}] + gold = [{"No": "C1", "Total": 100}, {"No": "C2", "Total": 200}] + assert result_sets_match(generated, gold, ordered=False) + + def test_row_order_enforced_when_ordered(self): + generated = [{"No": "C2", "Total": 200}, {"No": "C1", "Total": 100}] + gold = [{"No": "C1", "Total": 100}, {"No": "C2", "Total": 200}] + assert not result_sets_match(generated, gold, ordered=True) + + def test_numeric_normalization(self): + # Amounts arrive as numeric JSON types on both sides; scale differences must not matter. + assert result_sets_match([{"Total": 500}], [{"Total": 500.0}]) + + def test_column_names_ignored(self): + assert result_sets_match([{"ItemNo": "I1", "Qty": 5}], [{"No": "I1", "Total": 5}]) + + def test_odata_metadata_keys_ignored(self): + generated = [{"@odata.etag": "W/abc", "No": "C1", "Total": 100}] + gold = [{"No": "C1", "Total": 100}] + assert result_sets_match(generated, gold) + + def test_mismatch_detected(self): + assert not result_sets_match([{"No": "C1", "Total": 100}], [{"No": "C1", "Total": 999}]) + + def test_different_row_count_mismatch(self): + assert not result_sets_match([{"No": "C1"}], [{"No": "C1"}, {"No": "C2"}]) + + def test_close_but_distinct_values_do_not_match(self): + # Guards against numeric rounding collapsing distinct values into a false positive. + assert not result_sets_match([{"Total": 1.00001}], [{"Total": 1.00002}]) + + def test_high_precision_preserved(self): + assert result_sets_match([{"Total": 1.000000001}], [{"Total": 1.000000001}]) + assert not result_sets_match([{"Total": 1.000000001}], [{"Total": 1.000000002}]) + + def test_scale_insensitive(self): + assert result_sets_match([{"Total": 500}], [{"Total": 500.00}]) + + def test_digit_only_code_strings_not_collapsed(self): + # BC Code/No. fields are JSON strings even when digit-only: "001" and "1" are DISTINCT records + # and must never be scored as matching just because they are numerically equal. + assert not result_sets_match([{"No": "001"}], [{"No": "1"}]) + assert not result_sets_match([{"No": "0010"}], [{"No": "10"}]) + + def test_identical_code_strings_match(self): + assert result_sets_match([{"No": "001", "Name": "Acme"}], [{"No": "001", "Name": "Acme"}]) + + def test_numeric_string_not_coerced_to_number(self): + # A code that happens to look like a scaled number must not match the numeric value 1. + assert not result_sets_match([{"Key": "1.0"}], [{"Key": 1}]) + + +class TestLoadAnswerRows: + def _write(self, tmp_path, content: str): + p = tmp_path / "answer.json" + p.write_text(content, encoding="utf-8") + return p + + def test_bare_array(self, tmp_path): + rows = _load_answer_rows(self._write(tmp_path, '[{"No": "C1", "Total": 100}]')) + assert rows == [{"No": "C1", "Total": 100}] + + def test_single_object_becomes_one_row(self, tmp_path): + assert _load_answer_rows(self._write(tmp_path, '{"Total": 42}')) == [{"Total": 42}] + + def test_odata_value_wrapper_unwrapped(self, tmp_path): + rows = _load_answer_rows(self._write(tmp_path, '{"@odata.context": "x", "value": [{"No": "C1"}]}')) + assert rows == [{"No": "C1"}] + + def test_empty_file_is_empty_list(self, tmp_path): + assert _load_answer_rows(self._write(tmp_path, "")) == [] + + def test_invalid_json_raises(self, tmp_path): + with pytest.raises(ValueError, match="not valid JSON"): + _load_answer_rows(self._write(tmp_path, "{not json")) + + def test_non_object_rows_raise(self, tmp_path): + with pytest.raises(TypeError, match="must be JSON objects"): + _load_answer_rows(self._write(tmp_path, "[1, 2, 3]")) + + +class TestWrapQueryAsApi: + PLAIN_QUERY = 'query 50100 MyQuery\n{\n QueryType = Normal;\n\n elements\n {\n dataitem(Customer; Customer)\n {\n column(No; "No.") { }\n }\n }\n}' + LONG_NAME_QUERY = 'query 50100 "Items on Open Sales and Purchase Orders"\n{\n elements\n {\n dataitem(Item; Item)\n {\n column(No; "No.") { }\n }\n }\n}' + + def test_reassigns_object_id_and_name(self): + wrapped = wrap_query_as_api(self.PLAIN_QUERY, 50101) + assert "query 50101 BCBenchQuery50101" in wrapped + assert "query 50100" not in wrapped + assert "MyQuery" not in wrapped + + def test_normalizes_overlong_quoted_name(self): + # A descriptive >30-char name would trip AL0305; the harness normalizes it away. + wrapped = wrap_query_as_api(self.LONG_NAME_QUERY, 50100) + assert "query 50100 BCBenchQuery50100" in wrapped + assert "Items on Open Sales and Purchase Orders" not in wrapped + + def test_injects_api_properties(self): + wrapped = wrap_query_as_api(self.PLAIN_QUERY, 50100) + assert "QueryType = API;" in wrapped + assert "APIPublisher = 'bcbench';" in wrapped + assert "EntitySetName = 'bcbenchResults50100';" in wrapped + + def test_generated_and_gold_use_distinct_entity_sets(self): + # Both apps can be published to the same tenant; distinct entity sets avoid an OData route collision. + generated = wrap_query_as_api(self.PLAIN_QUERY, 50100) + gold = wrap_query_as_api(self.PLAIN_QUERY, 50101) + assert "EntitySetName = 'bcbenchResults50100';" in generated + assert "EntitySetName = 'bcbenchResults50101';" in gold + + def test_drops_existing_querytype(self): + wrapped = wrap_query_as_api(self.PLAIN_QUERY, 50100) + assert "QueryType = Normal;" not in wrapped + + def test_preserves_query_body(self): + wrapped = wrap_query_as_api(self.PLAIN_QUERY, 50100) + assert "dataitem(Customer; Customer)" in wrapped + assert 'column(No; "No.")' in wrapped + + def test_uppercase_query_keyword_reassigned(self): + wrapped = wrap_query_as_api('Query 50123 "My Q"\n{\n elements { }\n}', 50100) + assert "50100 BCBenchQuery50100" in wrapped + assert "50123" not in wrapped + + def test_compact_and_cased_querytype_removed(self): + # QueryType on the same line as the brace (no leading newline) and in any casing must still + # be stripped, else the injected QueryType = API duplicates the property. + wrapped = wrap_query_as_api("query 50100 Q\n{ querytype = Normal; elements { } }", 50100) + assert wrapped.count("QueryType") == 1 + assert "QueryType = API;" in wrapped + + def test_missing_brace_raises_builderror(self): + with pytest.raises(BuildError): + wrap_query_as_api("query 50100 MyQuery no body here", 50100) + + def test_no_query_declaration_raises_builderror(self): + with pytest.raises(BuildError): + wrap_query_as_api("codeunit 50100 NotAQuery { }", 50100) + + +def test_execute_al_query_bootstraps_app_manifest(tmp_path, monkeypatch): + app_dir = tmp_path / ".bcbench-query-generated" + + def write_empty_result(*args, **kwargs): + (app_dir / "result.json").write_text("[]", encoding="utf-8") + + monkeypatch.setattr(bc_operations.subprocess, "run", write_empty_result) + + rows = bc_operations.execute_al_query( + 'query 50100 MyQuery { elements { dataitem(Customer; Customer) { column(No; "No.") { } } } }', + ContainerConfig(name="bcserver", username="admin", password="password"), + "26.0.12345.0", + tmp_path, + "generated", + ) + + manifest = json.loads((app_dir / "app.json").read_text(encoding="utf-8")) + assert rows == [] + assert manifest["name"] == "BC-Bench Query generated" + assert manifest["idRanges"] == [{"from": 50100, "to": 50100}] + assert manifest["runtime"] == "15.0" + + +class TestQueryRunTemplate: + def _render(self): + return bc_operations._QUERY_RUN_TEMPLATE.substitute( + app_utils_path="AppUtils.psm1", + container_name="c", + username="u", + password="p", + app_dir="d", + app_name="BC-Bench Query generated", + app_publisher="BC-Bench", + publisher=bc_operations._QUERY_API_PUBLISHER, + group=bc_operations._QUERY_API_GROUP, + version=bc_operations._QUERY_API_VERSION, + entity_set=bc_operations._entity_set_name(50100), + result_file="r", + ) + + def test_uses_proven_build_helper(self): + assert "Invoke-AppBuildAndPublish" in self._render() + + def test_fetches_from_inside_container(self): + script = self._render() + assert "Invoke-ScriptInBcContainer" in script + assert "http://localhost:7048/BC/api" in script + + def test_does_not_use_credential_over_http(self): + # PowerShell 7 (inside the container) refuses -Credential over plain HTTP; we must build a + # Basic auth header by hand instead. + script = self._render() + assert "-Credential" not in script.split("Invoke-ScriptInBcContainer", 1)[1] + assert "Authorization" in script + assert "Basic " in script + + def test_follows_odata_nextlink(self): + # Result sets larger than one OData page must not be silently truncated. + assert "@odata.nextLink" in self._render() + + def test_uninstalls_throwaway_app(self): + # Re-running against the same container must not fail with an object-ID conflict. + script = self._render() + assert "UnPublish-BcContainerApp" in script + assert "UnInstall-BcContainerApp" in script diff --git a/tests/test_mcp_config.py b/tests/test_mcp_config.py index 35d623006..435a22093 100644 --- a/tests/test_mcp_config.py +++ b/tests/test_mcp_config.py @@ -6,6 +6,7 @@ from bcbench.agent.shared.altool_paths import build_assembly_probing_paths as _build_assembly_probing_paths from bcbench.agent.shared.mcp import build_mcp_config +from bcbench.exceptions import AgentError from tests.conftest import create_dataset_entry @@ -26,6 +27,20 @@ def _make_config(*servers: dict) -> dict: "url": "https://learn.microsoft.com/api/mcp", } +BCMCP_SERVER = { + "name": "bcmcp", + "type": "http", + "url": "", + "headers": {}, +} + +# A server not gated by any flag, used to assert generic pass-through behavior. +OTHER_HTTP_SERVER = { + "name": "docs", + "type": "http", + "url": "https://example.com/mcp", +} + @pytest.fixture def entry(): @@ -68,7 +83,7 @@ def test_altool_excluded_when_al_mcp_disabled(self, entry, repo_path): assert result == (None, None) def test_altool_excluded_but_other_servers_kept(self, entry, repo_path): - config = _make_config(ALTOOL_SERVER, MSLEARN_SERVER) + config = _make_config(ALTOOL_SERVER, OTHER_HTTP_SERVER) config_json, names = build_mcp_config(config, entry, repo_path, al_mcp=False) assert config_json is not None @@ -76,16 +91,70 @@ def test_altool_excluded_but_other_servers_kept(self, entry, repo_path): parsed = json.loads(config_json) assert "altool" not in parsed["mcpServers"] - assert "mslearn" in parsed["mcpServers"] - assert names == ["mslearn"] + assert "docs" in parsed["mcpServers"] + assert names == ["docs"] def test_returns_server_names(self, entry, repo_path): - config = _make_config(ALTOOL_SERVER, MSLEARN_SERVER) + config = _make_config(ALTOOL_SERVER, OTHER_HTTP_SERVER) _, names = build_mcp_config(config, entry, repo_path, al_mcp=True) assert names is not None - assert set(names) == {"altool", "mslearn"} + assert set(names) == {"altool", "docs"} + + +class TestBcMcp: + _GATEWAY_URL = "http://127.0.0.1:54321/BC" + + def test_bcmcp_excluded_when_disabled(self, entry, repo_path): + assert build_mcp_config(_make_config(BCMCP_SERVER), entry, repo_path, bc_mcp=False) == (None, None) + + def test_mslearn_excluded_when_disabled(self, entry, repo_path): + assert build_mcp_config(_make_config(MSLEARN_SERVER), entry, repo_path, ms_learn_mcp=False) == (None, None) + + def test_flags_are_independent(self, entry, repo_path): + config = _make_config(BCMCP_SERVER, MSLEARN_SERVER) + + # bc-mcp only -> no mslearn + _, bc_only = build_mcp_config(config, entry, repo_path, bc_mcp=True, ms_learn_mcp=False, bc_mcp_gateway_url=self._GATEWAY_URL) + assert bc_only == ["bcmcp"] + + # ms-learn only -> no bcmcp (and no gateway needed) + _, learn_only = build_mcp_config(config, entry, repo_path, bc_mcp=False, ms_learn_mcp=True) + assert learn_only == ["mslearn"] + + # both -> both + _, both = build_mcp_config(config, entry, repo_path, bc_mcp=True, ms_learn_mcp=True, bc_mcp_gateway_url=self._GATEWAY_URL) + assert set(both) == {"bcmcp", "mslearn"} + + def test_mslearn_url_passthrough(self, entry, repo_path): + config_json, _ = build_mcp_config(_make_config(MSLEARN_SERVER), entry, repo_path, ms_learn_mcp=True) + assert json.loads(config_json)["mcpServers"]["mslearn"]["url"] == "https://learn.microsoft.com/api/mcp" + + def test_bcmcp_points_at_gateway_without_credentials(self, entry, repo_path): + config_json, _ = build_mcp_config(_make_config(BCMCP_SERVER), entry, repo_path, bc_mcp=True, bc_mcp_gateway_url=self._GATEWAY_URL) + bcmcp = json.loads(config_json)["mcpServers"]["bcmcp"] + + assert bcmcp["url"] == "http://127.0.0.1:54321/BC/mcp" + # The gateway injects auth upstream, so the agent config carries no credentials or headers. + assert "headers" not in bcmcp + assert "Authorization" not in config_json + assert "Basic" not in config_json + + def test_raises_when_gateway_url_missing(self, entry, repo_path): + with pytest.raises(AgentError): + build_mcp_config(_make_config(BCMCP_SERVER), entry, repo_path, bc_mcp=True) + + def test_redaction_masks_authorization_header(self): + from bcbench.agent.shared.mcp import _redact_mcp_config + + config = {"mcpServers": {"bcmcp": {"type": "http", "url": "u", "headers": {"Authorization": "Basic sekret", "Company": "Contoso"}}}} + redacted = _redact_mcp_config(config) + + assert redacted["mcpServers"]["bcmcp"]["headers"]["Authorization"] == "Basic ***REDACTED***" + assert redacted["mcpServers"]["bcmcp"]["headers"]["Company"] == "Contoso" + # Original is untouched (deep copy). + assert config["mcpServers"]["bcmcp"]["headers"]["Authorization"] == "Basic sekret" class TestAltoolEnvForwarding: diff --git a/tests/test_mcp_gateway.py b/tests/test_mcp_gateway.py new file mode 100644 index 000000000..149463d84 --- /dev/null +++ b/tests/test_mcp_gateway.py @@ -0,0 +1,385 @@ +import base64 +import json +import threading +import time +from http.client import HTTPConnection +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from urllib.parse import urlsplit + +import pytest + +from bcbench.agent.shared.mcp_gateway import BcMcpGateway, start_bc_mcp_gateway +from bcbench.exceptions import AgentError + + +class _UpstreamHandler(BaseHTTPRequestHandler): + protocol_version = "HTTP/1.1" + + def log_message(self, format: str, *args: object) -> None: # match stdlib signature; silence access log + pass + + def _record(self) -> None: + self.server.last_headers = dict(self.headers.items()) + self.server.last_path = self.path + self.server.last_method = self.command + length = self.headers.get("Content-Length") + self.server.last_body = self.rfile.read(int(length)) if length else b"" + + def do_POST(self) -> None: + self._record() + payload = json.dumps({"jsonrpc": "2.0", "id": 1, "result": {"ok": True}}).encode() + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Mcp-Session-Id", "sess-123") + self.send_header("Content-Length", str(len(payload))) + self.end_headers() + self.wfile.write(payload) + + def do_GET(self) -> None: + self._record() + # Stream an SSE response with no Content-Length, ended by closing the connection. + self.send_response(200) + self.send_header("Content-Type", "text/event-stream") + self.send_header("Connection", "close") + self.end_headers() + self.wfile.write(b"event: message\ndata: one\n\n") + self.wfile.flush() + self.wfile.write(b"event: message\ndata: two\n\n") + self.wfile.flush() + + +@pytest.fixture +def upstream(): + server = ThreadingHTTPServer(("127.0.0.1", 0), _UpstreamHandler) + server.last_headers = {} + server.last_path = None + server.last_method = None + server.last_body = b"" + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + yield server + server.shutdown() + server.server_close() + thread.join(timeout=5) + + +@pytest.fixture +def gateway(upstream, monkeypatch): + port = upstream.server_address[1] + monkeypatch.setenv("BC_MCP_URL", f"http://127.0.0.1:{port}/BC") + monkeypatch.setenv("BC_SERVER_USERNAME", "admin") + monkeypatch.setenv("BC_SERVER_PASSWORD", "secret") + monkeypatch.setenv("BC_MCP_COMPANY", "CRONUS International Ltd.") + gw = start_bc_mcp_gateway(enabled=True) + yield gw + gw.stop() + + +def _request(base_url: str, method: str, path: str, body: bytes | None = None): + split = urlsplit(base_url) + conn = HTTPConnection(split.hostname, split.port, timeout=10) + try: + conn.request(method, path, body=body) + response = conn.getresponse() + return response.status, dict(response.getheaders()), response.read() + finally: + conn.close() + + +class TestBcMcpGateway: + def test_disabled_returns_none(self): + assert start_bc_mcp_gateway(enabled=False) is None + + def test_raises_without_upstream_url(self, monkeypatch): + monkeypatch.delenv("BC_MCP_URL", raising=False) + with pytest.raises(AgentError): + start_bc_mcp_gateway(enabled=True) + + def test_base_url_mirrors_upstream_path(self, gateway): + assert gateway.base_url.endswith("/BC") + assert gateway.base_url.startswith("http://127.0.0.1:") + + def test_forwards_mcp_post_and_injects_credentials(self, gateway, upstream): + status, _headers, body = _request(gateway.base_url, "POST", "/BC/mcp", body=b'{"jsonrpc":"2.0"}') + + assert status == 200 + assert json.loads(body)["result"] == {"ok": True} + # The upstream saw the injected credentials/headers, not the (credential-free) agent request. + expected_auth = "Basic " + base64.b64encode(b"admin:secret").decode() + assert upstream.last_headers["Authorization"] == expected_auth + assert upstream.last_headers["ConfigurationName"] == "BCBench" + assert upstream.last_headers["Company"] == "CRONUS International Ltd." + assert upstream.last_path == "/BC/mcp" + assert upstream.last_body == b'{"jsonrpc":"2.0"}' + + def test_passes_through_response_headers(self, gateway): + _status, headers, _body = _request(gateway.base_url, "POST", "/BC/mcp", body=b"{}") + assert headers.get("Mcp-Session-Id") == "sess-123" + + def test_streams_sse_response(self, gateway): + status, headers, body = _request(gateway.base_url, "GET", "/BC/mcp") + assert status == 200 + assert headers["Content-Type"] == "text/event-stream" + assert b"data: one" in body + assert b"data: two" in body + + def test_rejects_non_mcp_path(self, gateway, upstream): + upstream.last_path = None # clear traffic from the start-up warm-up probe + status, _headers, _body = _request(gateway.base_url, "GET", "/BC/api/v2.0/companies") + assert status == 403 + # A blocked request never reaches the upstream. + assert upstream.last_path is None + + def test_rejects_mcp_prefix_without_boundary(self, gateway): + status, _headers, _body = _request(gateway.base_url, "POST", "/BC/mcpsomething", body=b"{}") + assert status == 403 + + def test_counts_forwarded_requests(self, gateway): + baseline = gateway.forwarded_count # start_bc_mcp_gateway already ran a warm-up probe + _request(gateway.base_url, "POST", "/BC/mcp", body=b"{}") + _request(gateway.base_url, "GET", "/BC/api") # blocked, not counted + _request(gateway.base_url, "POST", "/BC/mcp", body=b"{}") + assert gateway.forwarded_count - baseline == 2 + + +class _McpHandler(BaseHTTPRequestHandler): + protocol_version = "HTTP/1.1" + + def log_message(self, format: str, *args: object) -> None: # match stdlib signature; silence access log + pass + + def do_POST(self) -> None: + import json as _json + + n = int(self.headers.get("Content-Length", 0)) + req = _json.loads(self.rfile.read(n)) if n else {} + method = req.get("method") + if method == "initialize": + self._json({"jsonrpc": "2.0", "id": req.get("id"), "result": {"protocolVersion": "2024-11-05", "capabilities": {"tools": {}}}}, {"Mcp-Session-Id": "sess-xyz"}) + elif method and method.startswith("notifications/"): + self.send_response(202) + self.send_header("Content-Length", "0") + self.end_headers() + elif method == "tools/list": + tools = [{"name": "bc_data_find_tables"}, {"name": "bc_data_query"}] + # Answer as SSE to exercise the gateway's chunked relay + the probe's SSE parsing. + payload = "event: message\ndata: " + _json.dumps({"jsonrpc": "2.0", "id": req.get("id"), "result": {"tools": tools}}) + "\n\n" + self.send_response(200) + self.send_header("Content-Type", "text/event-stream") + self.send_header("Connection", "close") + self.end_headers() + self.wfile.write(payload.encode()) + else: + self._json({"jsonrpc": "2.0", "id": req.get("id"), "result": {}}) + + def _json(self, obj, extra_headers=None) -> None: + import json as _json + + body = _json.dumps(obj).encode() + self.send_response(200) + self.send_header("Content-Type", "application/json") + for k, v in (extra_headers or {}).items(): + self.send_header(k, v) + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + +class TestBcMcpProbe: + @pytest.fixture + def mcp_gateway(self, monkeypatch): + server = ThreadingHTTPServer(("127.0.0.1", 0), _McpHandler) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + port = server.server_address[1] + monkeypatch.setenv("BC_MCP_URL", f"http://127.0.0.1:{port}/BC") + monkeypatch.setenv("BC_SERVER_USERNAME", "admin") + monkeypatch.setenv("BC_SERVER_PASSWORD", "secret") + monkeypatch.delenv("BC_MCP_COMPANY", raising=False) + gw = start_bc_mcp_gateway(enabled=True) + yield gw + gw.stop() + server.shutdown() + server.server_close() + thread.join(timeout=5) + + def test_probe_returns_exposed_tool_names(self, mcp_gateway): + assert mcp_gateway.probe_tools() == ["bc_data_find_tables", "bc_data_query"] + + def test_tools_list_served_from_cache_after_warmup(self, mcp_gateway): + # start_bc_mcp_gateway already ran warm-up, populating the tools/list cache. + import json as _json + + status, headers, body = _request(mcp_gateway.base_url, "POST", "/BC/mcp", body=b'{"jsonrpc":"2.0","id":7,"method":"tools/list"}') + assert status == 200 + # Served as a single-event SSE stream, mirroring BC's tools/list framing. + assert headers["Content-Type"] == "text/event-stream" + data_line = next(line for line in body.decode().splitlines() if line.startswith("data:")) + payload = _json.loads(data_line[len("data:") :].strip()) + assert payload["id"] == 7 + assert [t["name"] for t in payload["result"]["tools"]] == ["bc_data_find_tables", "bc_data_query"] + + def test_probe_never_raises_on_bad_upstream(self, monkeypatch): + monkeypatch.setenv("BC_MCP_URL", "http://127.0.0.1:1/BC") # nothing listening + monkeypatch.setenv("BC_SERVER_USERNAME", "admin") + monkeypatch.setenv("BC_SERVER_PASSWORD", "secret") + gw = start_bc_mcp_gateway(enabled=True) + try: + assert gw.probe_tools() == [] + finally: + gw.stop() + + +class _HeldOpenSseHandler(BaseHTTPRequestHandler): + """Sends one small SSE event, flushes, then holds the stream open before closing.""" + + protocol_version = "HTTP/1.1" + + def log_message(self, format: str, *args: object) -> None: # match stdlib signature; silence access log + pass + + def do_GET(self) -> None: + self.send_response(200) + self.send_header("Content-Type", "text/event-stream") + self.send_header("Connection", "close") + self.end_headers() + self.wfile.write(b"event: message\ndata: early\n\n") + self.wfile.flush() + time.sleep(3.0) # keep the stream open after the event, as the BC MCP endpoint does + + +class _HeldOpenPostSseHandler(BaseHTTPRequestHandler): + """Answers a POST with an SSE event carrying a JSON-RPC result, then holds the stream open.""" + + protocol_version = "HTTP/1.1" + + def log_message(self, format: str, *args: object) -> None: # match stdlib signature; silence access log + pass + + def do_POST(self) -> None: + n = int(self.headers.get("Content-Length", 0)) + if n: + self.rfile.read(n) + self.send_response(200) + self.send_header("Content-Type", "text/event-stream") + self.send_header("Mcp-Session-Id", "sess-hold") + self.end_headers() + self.wfile.write(b'event: message\ndata: {"jsonrpc":"2.0","id":1,"result":{"ok":true}}\n\n') + self.wfile.flush() + time.sleep(30) # hold open like BC; the gateway relays faithfully without waiting for the end + + +def test_gateway_relays_post_sse_event_promptly_without_waiting_for_close(): + server = ThreadingHTTPServer(("127.0.0.1", 0), _HeldOpenPostSseHandler) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + port = server.server_address[1] + gateway = BcMcpGateway(f"http://127.0.0.1:{port}/BC", "admin", "secret", None).start() + split = urlsplit(gateway.base_url) + connection = HTTPConnection(split.hostname, split.port, timeout=10) + try: + start = time.monotonic() + connection.request("POST", "/BC/mcp", body=b'{"jsonrpc":"2.0","id":1,"method":"initialize"}') + response = connection.getresponse() + # The gateway relays BC's SSE bytes faithfully (holding the stream open); the response event + # reaches the client promptly even though the upstream keeps the stream open. + assert response.status == 200 + assert response.getheader("Content-Type") == "text/event-stream" + line = b"" + while b"data:" not in line: + line = response.readline() + if not line: + break + elapsed = time.monotonic() - start + assert b'"result"' in line + assert elapsed < 3.0 + finally: + connection.close() + gateway.stop() + server.shutdown() + server.server_close() + thread.join(timeout=5) + + +class _InitializeExperimentalHandler(BaseHTTPRequestHandler): + """Answers initialize over SSE with a capabilities.experimental block, held open like BC.""" + + protocol_version = "HTTP/1.1" + + def log_message(self, format: str, *args: object) -> None: # match stdlib signature; silence access log + pass + + def do_POST(self) -> None: + import json as _json + + n = int(self.headers.get("Content-Length", 0)) + req = _json.loads(self.rfile.read(n)) if n else {} + result = {"protocolVersion": "2024-11-05", "capabilities": {"experimental": {"x-ms-headerless": True}, "tools": {}}, "serverInfo": {"name": "BC"}} + self.send_response(200) + self.send_header("Content-Type", "text/event-stream") + self.send_header("Mcp-Session-Id", "sess-init") + self.end_headers() + self.wfile.write(("data: " + _json.dumps({"jsonrpc": "2.0", "id": req.get("id"), "result": result}) + "\n\n").encode()) + self.wfile.flush() + time.sleep(30) # hold the stream open like BC + + +def test_gateway_strips_experimental_from_initialize(): + import json as _json + + server = ThreadingHTTPServer(("127.0.0.1", 0), _InitializeExperimentalHandler) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + port = server.server_address[1] + gateway = BcMcpGateway(f"http://127.0.0.1:{port}/BC", "admin", "secret", None).start() + split = urlsplit(gateway.base_url) + connection = HTTPConnection(split.hostname, split.port, timeout=10) + try: + connection.request("POST", "/BC/mcp", body=b'{"jsonrpc":"2.0","id":1,"method":"initialize"}') + response = connection.getresponse() + assert response.status == 200 + line = b"" + while b"data:" not in line: + line = response.readline() + if not line: + break + payload = _json.loads(line.decode()[len("data:") :].strip()) + # The x-ms-headerless experimental capability (which breaks Claude) is stripped; the rest stays. + assert "experimental" not in payload["result"]["capabilities"] + assert "tools" in payload["result"]["capabilities"] + assert payload["result"]["protocolVersion"] == "2024-11-05" + finally: + connection.close() + gateway.stop() + server.shutdown() + server.server_close() + thread.join(timeout=5) + + +def test_gateway_relays_held_open_sse_event_promptly(): + server = ThreadingHTTPServer(("127.0.0.1", 0), _HeldOpenSseHandler) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + port = server.server_address[1] + gateway = BcMcpGateway(f"http://127.0.0.1:{port}/BC", "admin", "secret", None).start() + split = urlsplit(gateway.base_url) + connection = HTTPConnection(split.hostname, split.port, timeout=10) + try: + connection.request("GET", "/BC/mcp") + response = connection.getresponse() + start = time.monotonic() + line = b"" + while b"data:" not in line: + line = response.readline() + if not line: + break + elapsed = time.monotonic() - start + assert b"data: early" in line + # read1() flushes the event immediately; the old read() would stall until the upstream closes (~3s). + assert elapsed < 2.0 + finally: + connection.close() + gateway.stop() + server.shutdown() + server.server_close() + thread.join(timeout=5) diff --git a/tests/test_type_exhaustiveness.py b/tests/test_type_exhaustiveness.py index ef67eb9d6..7dbd77d04 100644 --- a/tests/test_type_exhaustiveness.py +++ b/tests/test_type_exhaustiveness.py @@ -2,7 +2,7 @@ import pytest -from bcbench.dataset import BugFixEntry, CodeReviewEntry, ExtRequestImplementEntry, ExtRequestTriageEntry, NL2ALEntry +from bcbench.dataset import BugFixEntry, CodeReviewEntry, DataQueryEntry, ExtRequestImplementEntry, ExtRequestTriageEntry, NL2ALEntry from bcbench.dataset.codereview import ReviewComment, Severity from bcbench.types import AgentHarness, AgentMetrics, EvaluationCategory @@ -67,7 +67,11 @@ def test_all_categories_have_aggregate_classes(): def test_all_categories_handled_in_get_expected_output( - sample_dataset_entry_with_problem_statement: BugFixEntry, sample_nl2al_entry: NL2ALEntry, sample_ext_implement_entry: "ExtRequestImplementEntry", sample_ext_triage_entry: "ExtRequestTriageEntry" + sample_dataset_entry_with_problem_statement: BugFixEntry, + sample_nl2al_entry: NL2ALEntry, + sample_data_query_entry: DataQueryEntry, + sample_ext_implement_entry: ExtRequestImplementEntry, + sample_ext_triage_entry: ExtRequestTriageEntry, ): for category in EvaluationCategory: entry_cls = category.entry_class @@ -84,6 +88,8 @@ def test_all_categories_handled_in_get_expected_output( ) elif entry_cls is NL2ALEntry: entry = sample_nl2al_entry + elif entry_cls is DataQueryEntry: + entry = sample_data_query_entry elif entry_cls is ExtRequestImplementEntry: entry = sample_ext_implement_entry elif entry_cls is ExtRequestTriageEntry: diff --git a/tools/run_gateway_local.py b/tools/run_gateway_local.py new file mode 100644 index 000000000..bb84ce3aa --- /dev/null +++ b/tools/run_gateway_local.py @@ -0,0 +1,36 @@ +import importlib.util +import os +import sys +import time + +_spec = importlib.util.spec_from_file_location( + "mcp_gateway", + os.path.join(os.path.dirname(__file__), "..", "src", "bcbench", "agent", "shared", "mcp_gateway.py"), +) +_mod = importlib.util.module_from_spec(_spec) +# Stub the two bcbench imports mcp_gateway needs so we don't pull the whole package. +import types # noqa: E402 + +_exc = types.ModuleType("bcbench.exceptions") +_exc.AgentError = type("AgentError", (Exception,), {}) +_log = types.ModuleType("bcbench.logger") +import logging # noqa: E402 + +logging.basicConfig(level=logging.INFO) +_log.get_logger = lambda _name: logging.getLogger("mcp_gateway") +sys.modules["bcbench.exceptions"] = _exc +sys.modules["bcbench.logger"] = _log +_spec.loader.exec_module(_mod) +start_bc_mcp_gateway = _mod.start_bc_mcp_gateway + +os.environ["BC_MCP_URL"] = sys.argv[1] # upstream mock, e.g. http://127.0.0.1:8765/BC +os.environ["BC_SERVER_USERNAME"] = "admin" +os.environ["BC_SERVER_PASSWORD"] = "secret" + +gw = start_bc_mcp_gateway(enabled=True) +print(f"GATEWAY_URL={gw.base_url}/mcp", flush=True) +try: + while True: + time.sleep(1) +except KeyboardInterrupt: + gw.stop()