diff --git a/.github/workflows/review-fixtures.yml b/.github/workflows/review-fixtures.yml index 0de9a022..fff80e83 100644 --- a/.github/workflows/review-fixtures.yml +++ b/.github/workflows/review-fixtures.yml @@ -35,3 +35,7 @@ jobs: -ChangedPathsFile "$env:RUNNER_TEMP/changed-paths.txt" ` -CoverageReportPath "$env:RUNNER_TEMP/review-coverage.json" Get-Content -LiteralPath "$env:RUNNER_TEMP/review-coverage.json" + + - name: Check context-dependent fixture regressions + shell: pwsh + run: ./tools/Test-ReviewFixtureContexts.ps1 -Root . diff --git a/evaluation/README.md b/evaluation/README.md index 55323235..a96956af 100644 --- a/evaluation/README.md +++ b/evaluation/README.md @@ -15,6 +15,32 @@ is mandatory. Model-facing preparation hashes case IDs, neutralizes `Good`/`Bad` object-name tokens, and removes full-line sample comments so neither the article slug, domain, nor expected outcome reveals the answer. +When applicability depends on deployment facts that AL cannot establish, use +`articleOverrides` within the domain override, keyed by a selected article's +exact slug (without `.md`). Each entry requires a non-empty `context` string. +It replaces the domain-wide context for that article's positive and clean pair +and survives in all prepared request formats. Include all necessary deployment +facts, but no expected finding, article path, fix, or assessment of code quality. +Other selected articles retain their existing domain context, if any. + +An article entry can also supply `cleanContexts`, an object mapping lowercase +hyphenated control names to non-empty context strings. Each control reuses the +article's `.bad.al` source with that context and expects no findings. Its stable +manifest ID is `--clean-`; model-facing IDs remain opaque. +Use this only when changing the deployment prerequisite makes the same source +legitimate. The MCP description article supplies Business Central online 28 and +MCP exposure to both normal cases, then adds a REST-only, non-MCP clean context. +This controls applicability without treating every API lacking `AboutText` as +a defect. Invalid or unselected article overrides fail validation. + +The context regression check runs the real preparer and scorer, verifies all +three request formats, context isolation, unchanged neutralization, mandatory +clean controls, and rejection of findings on the non-MCP input: + +```powershell +pwsh ./tools/Test-ReviewFixtureContexts.ps1 -Root . +``` + The core AL samples for BLOB loading, isolated shipment events, expense-page scope filters, and stream encoding include their minimal demo tables in each companion independently. BLOB field types and shipment status members therefore @@ -72,7 +98,7 @@ pwsh ./tools/Test-ReviewFixtures.ps1 -Root . -ChangedPathsFile .changed-paths.tx 2. For a fast/small model, use one fresh invocation per `request-case-*.json`. Each request embeds the exact leaf instructions, that domain's candidate index rows with authoritative paths, and one opaque case. The model opens only matching articles and copies finding IDs from `candidateArticles[].path`. Save each response with the matching `result-case-*.json` name in the same directory. - `request-.json` files provide optional leaf batches containing every selected case for that domain and identify the selected layer-owned skill path; save those as `result-.json`. A normal convention-selected domain has one bad/good pair, while an `articles` override contributes one pair per listed article. Directory scoring prefers `result-case-*.json` when present and otherwise falls back to `result-*.json`. `review-request.json` is an optional all-domains stress test for larger models. Neither batch form is the preferred fast-model profile. + `request-.json` files provide optional leaf batches containing every selected case for that domain and identify the selected layer-owned skill path; save those as `result-.json`. A normal convention-selected domain has one bad/good pair, while an `articles` override contributes one pair per listed article plus any registered context controls. Directory scoring prefers `result-case-*.json` when present and otherwise falls back to `result-*.json`. `review-request.json` is an optional all-domains stress test for larger models. Neither batch form is the preferred fast-model profile. 3. Save only this result shape: diff --git a/evaluation/review-fixtures.json b/evaluation/review-fixtures.json index 5fc0cfbb..b2b4346d 100644 --- a/evaluation/review-fixtures.json +++ b/evaluation/review-fixtures.json @@ -230,8 +230,17 @@ "check-json-null-before-converting-values", "format-exchanged-values-with-standard-format-9", "api-page-least-privilege-write-access", - "set-text-encoding-on-streams-exchanged-externally" - ] + "set-text-encoding-on-streams-exchanged-externally", + "describe-api-pages-for-mcp-with-abouttext" + ], + "articleOverrides": { + "describe-api-pages-for-mcp-with-abouttext": { + "context": "Target Business Central online version 28. The API page and API query in this input are exposed as tools through Business Central's built-in MCP server.", + "cleanContexts": { + "non-mcp": "Target Business Central online version 28. The API page and API query in this input serve a REST integration only and are not exposed through MCP." + } + } + } } } } diff --git a/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.bad.al b/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.bad.al new file mode 100644 index 00000000..3eedb62d --- /dev/null +++ b/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.bad.al @@ -0,0 +1,71 @@ +// Intended for exposure through the Business Central online MCP server. +page 50381 "WS Margin Line API Bad" +{ + PageType = API; + Caption = 'marginLine'; + APIPublisher = 'contoso'; + APIGroup = 'salesMargin'; + APIVersion = 'v1.0'; + EntityName = 'marginLine'; + EntitySetName = 'marginLines'; + EntityCaption = 'Margin Line'; + EntitySetCaption = 'Margin Lines'; + ODataKeyFields = SystemId; + SourceTable = "Sales Line"; + SourceTableView = where("Document Type" = filter(Quote | Order | Invoice), Type = const(Item)); + Editable = false; + InsertAllowed = false; + ModifyAllowed = false; + DeleteAllowed = false; + + layout + { + area(content) + { + repeater(records) + { + field(id; Rec.SystemId) + { + Caption = 'id'; + Editable = false; + } + field(documentNumber; Rec."Document No.") + { + Caption = 'documentNumber'; + } + field(itemNumber; Rec."No.") + { + Caption = 'itemNumber'; + } + field(lineAmount; Rec."Line Amount") + { + Caption = 'lineAmount'; + } + } + } + } +} + +// This API query is also intended for exposure as an MCP tool. +query 50383 "WS Item Lines Query Bad" +{ + QueryType = API; + APIPublisher = 'contoso'; + APIGroup = 'salesMargin'; + APIVersion = 'v1.0'; + EntityName = 'itemSalesLine'; + EntitySetName = 'itemSalesLines'; + EntityCaption = 'Item Sales Line'; + EntitySetCaption = 'Item Sales Lines'; + // The entity caption omits the material document-type scope. + elements + { + dataitem(SalesLine; "Sales Line") + { + DataItemTableFilter = "Document Type" = filter(Quote | Order | Invoice), Type = const(Item); + column(id; SystemId) { } + column(documentNumber; "Document No.") { } + column(itemNumber; "No.") { } + } + } +} diff --git a/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.good.al b/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.good.al new file mode 100644 index 00000000..ad719a38 --- /dev/null +++ b/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.good.al @@ -0,0 +1,72 @@ +// Intended for exposure through the Business Central online MCP server. +page 50380 "WS Margin Line API Good" +{ + PageType = API; + Caption = 'marginLine'; + APIPublisher = 'contoso'; + APIGroup = 'salesMargin'; + APIVersion = 'v1.0'; + EntityName = 'marginLine'; + EntitySetName = 'marginLines'; + EntityCaption = 'Margin Line'; + EntitySetCaption = 'Margin Lines'; + AboutText = 'One item line of an unposted sales quote, order or invoice, with its document number, item and line amount. Read-only; credit memos and non-item lines are not included.'; + ODataKeyFields = SystemId; + SourceTable = "Sales Line"; + SourceTableView = where("Document Type" = filter(Quote | Order | Invoice), Type = const(Item)); + Editable = false; + InsertAllowed = false; + ModifyAllowed = false; + DeleteAllowed = false; + + layout + { + area(content) + { + repeater(records) + { + field(id; Rec.SystemId) + { + Caption = 'id'; + Editable = false; + } + field(documentNumber; Rec."Document No.") + { + Caption = 'documentNumber'; + } + field(itemNumber; Rec."No.") + { + Caption = 'itemNumber'; + } + field(lineAmount; Rec."Line Amount") + { + Caption = 'lineAmount'; + } + } + } + } +} + +// This API query is also intended for exposure as an MCP tool. +query 50382 "WS Item Lines Query Good" +{ + QueryType = API; + APIPublisher = 'contoso'; + APIGroup = 'salesMargin'; + APIVersion = 'v1.0'; + EntityName = 'itemSalesLine'; + EntitySetName = 'itemSalesLines'; + EntityCaption = 'Item Sales Line'; + EntitySetCaption = 'Item Sales Lines'; + AboutText = 'One item line of an unposted sales quote, order or invoice. Credit memos and non-item lines are excluded; returns the line ID, document number and item number.'; + elements + { + dataitem(SalesLine; "Sales Line") + { + DataItemTableFilter = "Document Type" = filter(Quote | Order | Invoice), Type = const(Item); + column(id; SystemId) { } + column(documentNumber; "Document No.") { } + column(itemNumber; "No.") { } + } + } +} diff --git a/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.md b/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.md new file mode 100644 index 00000000..d0b20c74 --- /dev/null +++ b/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.md @@ -0,0 +1,37 @@ +--- +bc-version: [28..] +domain: web-services +keywords: [api-page, api-query, abouttext, mcp, tool-description, entitysetcaption, metadata] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Describe API objects intended for Business Central MCP tools + +## Description + +For API pages and API queries exposed through the Business Central online MCP server, the documented tool-description precedence is `AboutText`, then `EntitySetCaption`, then a camel-cased `EntityName`. Treating `AboutText` only as a UI teaching-tip property can leave an agent with a caption that omits essential scope or computed-value semantics. This guidance covers BC 28 and later online environments with the relevant MCP API-object support; the earlier MCP preview is outside its scope. + +## Best Practice + +For an API object intended for MCP use, provide a tool description that explains what a row represents, material filters and any computed values that affect interpretation. Prefer `AboutText` when captions alone cannot convey those facts. Keep the description consistent with the actual filters, fields and allowed operations. Verify the description exposed by the configured MCP tool; metadata text does not enforce filters or permissions. + +The documented extraction order does not use `AboutTitle`. Do not require it solely for MCP: Microsoft ships API queries with `AboutText` and no `AboutTitle`, and the compiler accepts that form. Teaching tips have separate pairing requirements. Do not flag an absent `AboutText` when the effective fallback already describes the tool adequately, or when the object has no established MCP use. Other MCP servers and connectors can choose a different description source; check their contract before applying this rule. + +See sample: [`describe-api-pages-for-mcp-with-abouttext.good.al`](describe-api-pages-for-mcp-with-abouttext.good.al). + +## Anti Pattern + +An API object intended for an MCP tool exposes a description that omits material endpoint semantics, such as excluding credit memos or including only item lines, because it relies on a bare entity caption. Another defect is an `AboutText` that promises restrictions the endpoint does not implement. Missing `AboutText` alone is a discovery signal, not sufficient evidence of a finding; establish the intended MCP use and the deficient effective description. + +See sample: [`describe-api-pages-for-mcp-with-abouttext.bad.al`](describe-api-pages-for-mcp-with-abouttext.bad.al). + +## References + +- [API page type and MCP description precedence](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/devenv-api-pagetype#describe-the-api-for-mcp-tools) +- [API query type and MCP description precedence](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/devenv-api-querytype#describe-the-api-for-mcp-tools) +- [AboutText property and teaching-tip requirements](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/properties/devenv-abouttext-property) +- [BC 28 release and generally available enhanced MCP server](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/whatsnew/whatsnew-update-28-0) +- [API query support in the 2026 release wave 1 MCP enhancements](https://learn.microsoft.com/dynamics365/release-plan/2026wave1/smb/dynamics365-business-central/unlock-more-business-central-data-mcp-server-enhancements) +- [Microsoft API Finance query with AboutText and no AboutTitle](https://github.com/microsoft/BCApps/blob/e51c552efbeb6821db450b199aae6b015371866c/src/Apps/W1/APIReportsFinance/App/src/APIFinanceGLEntry.Query.al) diff --git a/microsoft/skills/review/al-web-services-review.md b/microsoft/skills/review/al-web-services-review.md index 310c604d..15295468 100644 --- a/microsoft/skills/review/al-web-services-review.md +++ b/microsoft/skills/review/al-web-services-review.md @@ -38,15 +38,16 @@ Discard files that are not applicable. Retain conditionally applicable files (an Narrow the relevant files to the subset that applies to the changes under review. For each relevant file, compute overlap against: - The changed AL object names and types — especially pages declared with `PageType = API`, API page `part` controls, queries declared with `QueryType = API`, and procedures that expose bound actions. -- The changed properties and triggers, weighted toward API page metadata (`APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `ODataKeyFields`, `SourceTable`, `SourceTableTemporary`), navigation metadata (`SubPageLink`, `Multiplicity`, and visible singleton or collection semantics), CRUD guards (`InsertAllowed`, `ModifyAllowed`, `DeleteAllowed`, `Editable`), the `OnOpenPage` trigger, and `OnValidate` triggers on exposed fields. +- The changed properties and triggers, weighted toward API page metadata (`APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `AboutText`, `EntitySetCaption`, `ODataKeyFields`, `SourceTable`, `SourceTableTemporary`), navigation metadata (`SubPageLink`, `Multiplicity`, and visible singleton or collection semantics), CRUD guards (`InsertAllowed`, `ModifyAllowed`, `DeleteAllowed`, `Editable`), the `OnOpenPage` trigger, and `OnValidate` triggers on exposed fields. - Outbound HTTP integration code that constructs or sends requests, captures a client method's optional Boolean result, checks an `HttpResponseMessage`, or reads and parses response content. - Integration code that converts values to or from exchanged text: `Format` or `Evaluate` on a request URL, body, or file line, and `JsonToken`/`JsonValue` conversions of payload properties. - Integration code that converts exchanged text to or from `Blob` bytes: trace encoding-sensitive operations such as `WriteText`/`ReadText` to `CreateOutStream`/`CreateInStream` with a missing or incompatible `TextEncoding`, relative to the counterpart's expected encoding or the incoming bytes — `set-text-encoding-on-streams-exchanged-externally`. Do not flag mere stream creation, binary content, compatible internal text round-trips, or byte-preserving download/HTTP/upload/`CopyStream` transfers of already encoded data. In particular, an explicit UTF-8 writer followed by a default-encoding InStream used only for download preserves the original bytes. - Webhook subscriber handlers and subscription lifecycle code, especially code that creates or renews subscriptions, handles `validationToken`, schedules from `expirationDateTime`, or targets resources whose eligibility is visible in the diff. +- An API page or API query has established MCP use and its description metadata is added, changed, or omitted — `describe-api-pages-for-mcp-with-abouttext.md`. Read the article before evaluating the effective tool description. - An API page (`PageType = API`) is added or changed and its `InsertAllowed`/`ModifyAllowed`/`DeleteAllowed` properties are left at their default `true` (or explicitly set `true`) for an operation the endpoint's stated purpose does not need — `api-page-least-privilege-write-access.md`. The signal is an operation left enabled beyond what the endpoint's own described purpose requires, not the mere presence of these properties; a page that genuinely needs full CRUD and grants it deliberately is not a violation. - An API page (`PageType = API`) with `InsertAllowed = true` lists a field in `ODataKeyFields` and that same field control sets `Editable = false` — `api-page-key-fields-must-be-editable-on-insert.md`. A system-generated key such as `SystemId` marked read-only is not this anti-pattern. - An API page exposes a field via `Rec` directly, and that field is a stored value computed from other fields inside an `OnValidate` trigger rather than a FlowField recalculated in `OnAfterGetRecord` — `stored-derived-fields-must-not-be-exposed-directly.md`. Exposing a genuine FlowField, or a value already recalculated in `OnAfterGetRecord`, is not this anti-pattern. -- Tokens extracted from the diff that relate to API surface and behaviour (`PageType`, `QueryType`, `API`, `api-page`, `page-part`, `APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `ODataKeyFields`, `SystemId`, `SubPageLink`, `subpagelink`, `Multiplicity`, `multiplicity`, `Many`, `ZeroOrOne`, `SourceTableTemporary`, `Job Queue Entry`, `webhook`, `webhookSupportedResources`, `webhook-supported-resources`, `subscriptions`, `notificationUrl`, `validationToken`, `validationtoken`, `expirationDateTime`, `expirationdatetime`, `ServiceEnabled`, `WebServiceActionContext`, `SetActionResponse`, `ReadIsolation`, `IsolationLevel`, `ReadCommitted`, `InsertAllowed`, `ModifyAllowed`, `DeleteAllowed`, `Editable`, `SourceTable`, `HttpClient`, `HttpRequestMessage`, `HttpResponseMessage`, `Get`, `Post`, `Put`, `Delete`, `Send`, `IsSuccessStatusCode`, `HttpStatusCode`, `Content`, `ReadAs`, `JsonObject`, `JsonToken`, `JsonValue`, `AsValue`, `IsNull`, `SelectToken`, `Format`, `Evaluate`, `XmlDocument`, `TextEncoding`, `CreateOutStream`, `CreateInStream`, `DownloadFromStream`, `UploadIntoStream`). +- Tokens extracted from the diff that relate to API surface and behaviour (`PageType`, `QueryType`, `API`, `api-page`, `page-part`, `APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `AboutText`, `EntitySetCaption`, `ODataKeyFields`, `SystemId`, `SubPageLink`, `subpagelink`, `Multiplicity`, `multiplicity`, `Many`, `ZeroOrOne`, `SourceTableTemporary`, `Job Queue Entry`, `webhook`, `webhookSupportedResources`, `webhook-supported-resources`, `subscriptions`, `notificationUrl`, `validationToken`, `validationtoken`, `expirationDateTime`, `expirationdatetime`, `ServiceEnabled`, `WebServiceActionContext`, `SetActionResponse`, `ReadIsolation`, `IsolationLevel`, `ReadCommitted`, `InsertAllowed`, `ModifyAllowed`, `DeleteAllowed`, `Editable`, `SourceTable`, `HttpClient`, `HttpRequestMessage`, `HttpResponseMessage`, `Get`, `Post`, `Put`, `Delete`, `Send`, `IsSuccessStatusCode`, `HttpStatusCode`, `Content`, `ReadAs`, `JsonObject`, `JsonToken`, `JsonValue`, `AsValue`, `IsNull`, `SelectToken`, `Format`, `Evaluate`, `XmlDocument`, `TextEncoding`, `CreateOutStream`, `CreateInStream`, `DownloadFromStream`, `UploadIntoStream`). A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone. diff --git a/tools/Test-ReviewFixtureContexts.ps1 b/tools/Test-ReviewFixtureContexts.ps1 new file mode 100644 index 00000000..df5c7812 --- /dev/null +++ b/tools/Test-ReviewFixtureContexts.ps1 @@ -0,0 +1,101 @@ +<# +.SYNOPSIS + Regression checks for context-dependent review fixtures after neutral preparation. +#> +[CmdletBinding()] +param( + [string] $Root = (Resolve-Path (Join-Path $PSScriptRoot '..')), + [string] $WorkDirectory = (Join-Path ([IO.Path]::GetTempPath()) "bcquality-context-tests-$([guid]::NewGuid().ToString('N'))") +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' +$Root = (Resolve-Path -LiteralPath $Root).Path +New-Item -ItemType Directory -Path $WorkDirectory -ErrorAction Stop | Out-Null +$runner = (Get-Process -Id $PID).Path +$harness = Join-Path $Root 'tools/Test-ReviewFixtures.ps1' +$prepared = Join-Path $WorkDirectory 'prepared' +$log = @(& $runner -NoProfile -File $harness -Root $Root -PrepareDirectory $prepared 2>&1) +if ($LASTEXITCODE -ne 0) { throw "Fixture preparation failed: $($log -join [Environment]::NewLine)" } + +function Assert-True { + param([bool] $Condition, [string] $Message) + if (-not $Condition) { throw $Message } +} + +function Get-OpaqueId { + param([string] $ManifestId) + $hash = [Security.Cryptography.SHA256]::HashData([Text.Encoding]::UTF8.GetBytes($ManifestId)) + return 'case-' + [Convert]::ToHexString($hash).Substring(0, 8).ToLowerInvariant() +} + +$slug = 'describe-api-pages-for-mcp-with-abouttext' +$prefix = "web-services-$slug" +$mcpContext = "Target Business Central online version 28. The API page and API query in this input are exposed as tools through Business Central's built-in MCP server." +$restContext = 'Target Business Central online version 28. The API page and API query in this input serve a REST integration only and are not exposed through MCP.' +$ids = @{ + bad = Get-OpaqueId "$prefix-bad" + good = Get-OpaqueId "$prefix-good" + rest = Get-OpaqueId "$prefix-clean-non-mcp" +} +$all = Get-Content -LiteralPath (Join-Path $prepared 'review-request.json') -Raw | ConvertFrom-Json +$domain = Get-Content -LiteralPath (Join-Path $prepared 'request-web-services.json') -Raw | ConvertFrom-Json +foreach ($kind in 'bad', 'good', 'rest') { + $id = $ids[$kind] + $expectedContext = if ($kind -eq 'rest') { $restContext } else { $mcpContext } + $singlePath = Join-Path $prepared "request-$id.json" + Assert-True (Test-Path -LiteralPath $singlePath) "Missing prepared context case: $kind" + $single = Get-Content -LiteralPath $singlePath -Raw | ConvertFrom-Json + foreach ($request in $all, $domain, $single) { + $case = @($request.cases | Where-Object id -eq $id) + Assert-True ($case.Count -eq 1) "Context case $kind must occur once in each request format." + Assert-True ($case[0].PSObject.Properties.Name -contains 'context') "Prepared $kind case lost its deployment context." + Assert-True ($case[0].context -ceq $expectedContext) "Prepared $kind case has the wrong context." + $keys = @($case[0].PSObject.Properties.Name | Sort-Object) -join ',' + Assert-True ($keys -ceq 'context,file,id') "Prepared $kind case reveals internal fixture metadata." + } +} +$badCode = Get-Content -LiteralPath (Join-Path $prepared "$($ids.bad).al") -Raw +$restCode = Get-Content -LiteralPath (Join-Path $prepared "$($ids.rest).al") -Raw +Assert-True ($badCode -ceq $restCode) 'The non-MCP control must use exactly the positive case code; only deployment context changes.' +Assert-True ($badCode -notmatch '(?m)^\s*//') 'Full-line comments must still be removed.' +foreach ($case in @($domain.cases | Where-Object { $_.id -notin @($ids.Values) })) { + Assert-True ($case.PSObject.Properties.Name -notcontains 'context') 'MCP context leaked to another web-services article.' +} +$finance = Get-Content -LiteralPath (Join-Path $prepared 'request-finance.json') -Raw | ConvertFrom-Json +foreach ($case in $finance.cases) { + Assert-True ($case.context -ceq 'Target Business Central 28, worldwide standard application (w1). Review each file as an independent source input.') 'Existing domain-wide context changed.' +} + +# Exercise scoring through the public CLI. Other positive cases intentionally +# have no findings; only the context control's specific diagnostics are asserted. +$results = [pscustomobject]@{ cases = @($all.cases | ForEach-Object { [pscustomobject]@{ id = $_.id; findings = @() } }) } +$resultsPath = Join-Path $WorkDirectory 'results.json' +$results | ConvertTo-Json -Depth 8 | Set-Content -LiteralPath $resultsPath -Encoding utf8 +$score = @(& $runner -NoProfile -File $harness -Root $Root -ResultsPath $resultsPath 2>&1) -join "`n" +Assert-True ($LASTEXITCODE -eq 1) 'Empty findings must fail the positive cases.' +Assert-True (-not $score.Contains("${prefix}-clean-non-mcp:")) 'An empty non-MCP control should be accepted.' +($results.cases | Where-Object id -eq $ids.rest).findings = @('microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.md') +$results | ConvertTo-Json -Depth 8 | Set-Content -LiteralPath $resultsPath -Encoding utf8 +$score = @(& $runner -NoProfile -File $harness -Root $Root -ResultsPath $resultsPath 2>&1) -join "`n" +Assert-True ($LASTEXITCODE -eq 1 -and $score.Contains("${prefix}-clean-non-mcp: clean control produced finding(s):")) 'Scoring must reject a finding on the non-MCP control.' +$results.cases = @($results.cases | Where-Object id -ne $ids.rest) +$results | ConvertTo-Json -Depth 8 | Set-Content -LiteralPath $resultsPath -Encoding utf8 +$score = @(& $runner -NoProfile -File $harness -Root $Root -ResultsPath $resultsPath 2>&1) -join "`n" +Assert-True ($LASTEXITCODE -eq 1 -and $score.Contains("Results missing case: ${prefix}-clean-non-mcp")) 'Scoring must require the non-MCP control.' + +foreach ($invalid in 'unselected-article', 'empty-context', 'empty-clean-context', 'invalid-control-name') { + $manifest = Get-Content -LiteralPath (Join-Path $Root 'evaluation/review-fixtures.json') -Raw | ConvertFrom-Json + $entry = $manifest.overrides.'web-services'.articleOverrides.$slug + switch ($invalid) { + 'unselected-article' { $manifest.overrides.'web-services'.articleOverrides | Add-Member -NotePropertyName 'not-selected' -NotePropertyValue $entry } + 'empty-context' { $entry.context = ' ' } + 'empty-clean-context' { $entry.cleanContexts.'non-mcp' = ' ' } + 'invalid-control-name' { $entry.cleanContexts | Add-Member -NotePropertyName '../escape' -NotePropertyValue $restContext } + } + $manifestPath = Join-Path $WorkDirectory "$invalid.json" + $manifest | ConvertTo-Json -Depth 12 | Set-Content -LiteralPath $manifestPath -Encoding utf8 + $validation = @(& $runner -NoProfile -File $harness -Root $Root -ManifestPath $manifestPath 2>&1) -join "`n" + Assert-True ($LASTEXITCODE -eq 1 -and $validation.Contains('articleOverrides')) "Invalid article override was not diagnosed: $invalid" +} +Write-Host 'Review fixture context regression checks PASSED: request formats, isolation, neutralization, clean-control scoring, and invalid overrides.' -ForegroundColor Green diff --git a/tools/Test-ReviewFixtures.ps1 b/tools/Test-ReviewFixtures.ps1 index f9be5127..b6b599a5 100644 --- a/tools/Test-ReviewFixtures.ps1 +++ b/tools/Test-ReviewFixtures.ps1 @@ -281,10 +281,53 @@ foreach ($domain in $leafDomains) { } else { $null } + $articleOverrides = @{} + if ($override -and ($override.PSObject.Properties.Name -contains 'articleOverrides')) { + if ($override.articleOverrides -isnot [pscustomobject]) { + $problems.Add("${domain}: articleOverrides must be an object keyed by selected article slug.") | Out-Null + } else { + foreach ($property in $override.articleOverrides.PSObject.Properties) { + $slug = $property.Name + $entry = $property.Value + $problemCount = $problems.Count + if ($slug -cnotin @($selectedArticles | ForEach-Object BaseName)) { + $problems.Add("${domain}: articleOverrides names an unselected article: $slug") | Out-Null + } + if ($entry -isnot [pscustomobject]) { + $problems.Add("${domain}: articleOverrides.$slug must be an object.") | Out-Null + continue + } + if (($entry.PSObject.Properties.Name -notcontains 'context') -or + ($entry.context -isnot [string]) -or [string]::IsNullOrWhiteSpace($entry.context)) { + $problems.Add("${domain}: articleOverrides.$slug.context must be a non-empty string.") | Out-Null + } + foreach ($name in $entry.PSObject.Properties.Name) { + if ($name -notin 'context', 'cleanContexts') { + $problems.Add("${domain}: unknown articleOverrides.$slug property: $name") | Out-Null + } + } + if ($entry.PSObject.Properties.Name -contains 'cleanContexts') { + if ($entry.cleanContexts -isnot [pscustomobject]) { + $problems.Add("${domain}: articleOverrides.$slug.cleanContexts must be an object keyed by control name.") | Out-Null + } else { + foreach ($control in $entry.cleanContexts.PSObject.Properties) { + if ($control.Name -cnotmatch '^[a-z0-9]+(?:-[a-z0-9]+)*$' -or + $control.Value -isnot [string] -or [string]::IsNullOrWhiteSpace($control.Value)) { + $problems.Add("${domain}: articleOverrides.$slug.cleanContexts requires lowercase hyphenated names and non-empty context strings.") | Out-Null + } + } + } + } + if ($problems.Count -eq $problemCount) { $articleOverrides[$slug] = $entry } + } + } + } for ($articleIndex = 0; $articleIndex -lt $selectedArticles.Count; $articleIndex++) { $selectedArticle = $selectedArticles[$articleIndex] $articlePath = [string]$selectedArticle.ArticlePath $sampleDirectory = (Split-Path -Parent $articlePath).Replace('\', '/') + $articleOverride = $articleOverrides[[string]$selectedArticle.BaseName] + $caseContext = if ($articleOverride) { [string]$articleOverride.context } else { $context } foreach ($kind in 'bad', 'good') { $caseId = if ($articleIndex -eq 0) { "$domain-$kind" @@ -297,11 +340,25 @@ foreach ($domain in $leafDomains) { input = "$sampleDirectory/$($selectedArticle.BaseName).$kind.al" expected = if ($kind -eq 'bad') { @($articlePath) } else { @() } } - if ($context) { - $case | Add-Member -NotePropertyName context -NotePropertyValue $context + if ($caseContext) { + $case | Add-Member -NotePropertyName context -NotePropertyValue $caseContext } $caseList.Add($case) | Out-Null } + if ($articleOverride -and ($articleOverride.PSObject.Properties.Name -contains 'cleanContexts')) { + foreach ($control in $articleOverride.cleanContexts.PSObject.Properties) { + # Reuse the positive source: the changed deployment context alone + # makes the rule inapplicable. Never expose this marker to models. + $caseList.Add([pscustomobject]@{ + id = "$domain-$($selectedArticle.BaseName)-clean-$($control.Name)" + domain = $domain + input = "$sampleDirectory/$($selectedArticle.BaseName).bad.al" + expected = @() + context = [string]$control.Value + contextControl = $true + }) | Out-Null + } + } } } @@ -330,8 +387,10 @@ foreach ($case in $cases) { if ($expected.Count -and $inputRelativePath -notmatch '\.bad\.[^.]+$') { $problems.Add("${id}: positive case must use a .bad sample: $inputRelativePath") | Out-Null } - if (-not $expected.Count -and $inputRelativePath -notmatch '\.good\.[^.]+$') { - $problems.Add("${id}: clean case must use a .good sample: $inputRelativePath") | Out-Null + $isContextControl = ($case.PSObject.Properties.Name -contains 'contextControl') -and $case.contextControl + if (-not $expected.Count -and $inputRelativePath -notmatch '\.good\.[^.]+$' -and + -not ($isContextControl -and $inputRelativePath -match '\.bad\.al$')) { + $problems.Add("${id}: clean case must use a .good sample or a registered context control: $inputRelativePath") | Out-Null } foreach ($reference in $expected) {