From 11fbd227f4a90e962e3370e60753a71a76b6fc69 Mon Sep 17 00:00:00 2001 From: demiliani Date: Thu, 1 Oct 2026 11:05:00 +0200 Subject: [PATCH 1/3] Validate composed reviews against the expected leaf worklist --- docs/standalone-runner.md | 46 ++++- microsoft/skills/review/al-code-review.md | 14 +- skills/do.md | 41 ++++- tools/Test-ReviewContract.ps1 | 200 +++++++++++++++++++++- tools/Validate-FindingsReport.ps1 | 141 ++++++++++++++- 5 files changed, 424 insertions(+), 18 deletions(-) diff --git a/docs/standalone-runner.md b/docs/standalone-runner.md index e5e2a0c4..d3186ef7 100644 --- a/docs/standalone-runner.md +++ b/docs/standalone-runner.md @@ -53,7 +53,9 @@ only result. with `tools/Resolve-SkillWorklist.ps1`, passing the enabled layers and disabled skill paths from the task context. Execute every resolved leaf as a discrete invocation. Leaves are independent and may be scheduled serially - or concurrently. + or concurrently. Before dispatch, preserve the final ordered selection and + legitimate configuration/input-compatibility exclusions as a private expected + composition artifact; see [composition acceptance](#composition-acceptance). 5. Capture the exact Task return as the immutable raw audit payload and primary transport. Preserve it unchanged in private artifacts or host logs. Before the full DO acceptance gate, create a normalized candidate only for DO's @@ -61,7 +63,7 @@ only result. telemetry, and accept the candidate only if the entire copy passes the unchanged strict gate. Use `tools/Validate-FindingsReport.ps1`, passing the exact source paths and fully retrieved article paths; pass `-SkillKind super` - for the final rolled-up report. The accepted report contains no undeclared + and `-ExpectedCompositionPath` for the final rolled-up report. The accepted report contains no undeclared telemetry fields. 6. Collect each accepted findings-report into `sub-results` in the declared `sub-skills` order, not completion order. Run the super-skill self-review @@ -74,6 +76,44 @@ The runner must never inspect the diff to skip a review domain. A leaf decides its own task-level applicability and reports `not-applicable` or `no-knowledge`. +## Composition acceptance + +A report can be internally consistent while omitting a selected review. Bind +the final acceptance gate to the host's selection, not just the returned +reports. The private JSON input to `-ExpectedCompositionPath` follows the +[DO consumer acceptance contract](../skills/do.md#consumer-acceptance-gate). +For example, if style is selected and security was disabled: + +```json +{ + "superSkill": { "id": "al-code-review", "version": 1 }, + "subSkills": [{ "id": "al-style-review", "version": 1 }], + "skipped": [{ "id": "al-security-review", "version": 1, "reason": "configuration" }] +} +``` + +Build this artifact from `Resolve-SkillWorklist.ps1` and the input-compatibility +decision before dispatch. Resolver `skipped` entries have no version: enrich +them from the declared skill's indexed version. Move an input-incompatible +selected slot to `skipped` with its selected version and `not-applicable` +reason; never use source content or later model output to make that decision. +Keep paths, layers, and other resolver metadata if useful for private audit. +Do not add the artifact to the findings-report or overwrite it to hide an +unfinished invocation. Preserve it with the run's raw payloads. + +The gate rejects repeated or unexpected leaf IDs, wrong selected versions, +reordered results, and fabricated or missing exclusions. If selected leaves +remain unfinished, the report must be `partial` with a non-failed returned +report, otherwise `failed`; identify unfinished leaf IDs in `outcome-reason`. +Do not fabricate leaf reports or use configuration skips for budget exhaustion. +Wait for started invocations to finish; omit the self-review if the selected +composition remains incomplete. Coverage still sums the non-failed leaf +knowledge worklists, not the number of selected leaf slots. + +Calls without the expected artifact remain supported for structural/semantic +validation, but cannot certify that a composed review covered its selection. +They still reject duplicate leaf IDs and returned-and-skipped conflicts. + ## Runner-owned choices Keep these settings and behaviors outside BCQuality: @@ -113,6 +153,8 @@ A compatible runner: only part of the review is reliable; - orders `sub-results` by the declared worklist and orders rendered findings deterministically; +- validates composition against the host-owned selection, and reports selected + leaves left unfinished as incomplete rather than clean or configured away; - calculates top-level severity counts from deduplicated top-level findings, not by summing leaf counts; - preserves knowledge paths verbatim and verifies references before publishing; diff --git a/microsoft/skills/review/al-code-review.md b/microsoft/skills/review/al-code-review.md index 7a3d0c60..8699d7d7 100644 --- a/microsoft/skills/review/al-code-review.md +++ b/microsoft/skills/review/al-code-review.md @@ -71,6 +71,14 @@ Sub-skills that fail either check are not invoked and are recorded in `skipped-s The worklist is the list of sub-skills judged relevant by the previous step. Every sub-skill in the worklist will be invoked in the Action step. +Before dispatch, preserve that selection in a private host-owned expected +composition artifact using DO's input contract. Start from the resolver's +ordered selection; enrich configuration skips with the declared indexed +skill's version, and record any input-incompatible exclusions with +`reason: "not-applicable"`. Do not derive this artifact from leaf reports or +change it merely because execution later runs out of budget. Pass its path as +`-ExpectedCompositionPath` for final super-skill validation. + ## Action ### Execution discipline (mandatory) @@ -85,7 +93,7 @@ The Action step consists of **discrete leaf invocations**, not one combined gene - Do not collapse multiple sub-skills into one shared reasoning step. Each sub-skill has a distinct knowledge subset and a distinct evaluation procedure; sharing one rolled-up scan dilutes per-skill attention and causes leaves to silently underreport (this has been observed in production: leaf skills returned empty `findings[]` while their standalone runs against the same diff produced multiple matches). - The agent self-review pass is its own final iteration. Begin it only after every sub-skill in the worklist has completed and its sub-result is recorded. - Sub-skills are independent: re-walking the diff once per sub-skill is correct and expected. The output schema accommodates this — `sub-results` carries one entry per sub-skill, each a complete findings-report, in the frontmatter `sub-skills` order regardless of completion order. -- When isolated calls are unavailable and the current model cannot finish every leaf within its budget, return `partial` with completed `sub-results` and name the first unevaluated sub-skill in `outcome-reason`. Never silently mark the remaining leaves clean. +- When the execution budget prevents invoking every selected leaf, wait for started invocations to finish and preserve their accepted `sub-results`. Return `partial` if any returned report is non-`failed`, otherwise `failed`, and name the unfinished leaf IDs in `outcome-reason`. Do not run the self-review on incomplete composition, invent sub-results, or record budget exhaustion as a configured/input-incompatible skip. Never silently mark the remaining leaves clean. ### Roll up sub-skill findings @@ -138,7 +146,9 @@ Calculate `summary.counts` from the final top-level `findings[]`, after failed s Derive `outcome` using the DO rollup rules. `outcome-reason` is populated for `partial` and `failed` and SHOULD summarize per-sub-skill state, for example: *"al-security-review failed (tool timeout); al-performance-review completed."* Before emitting the rollup, apply DO's consumer acceptance gate to every nested -and top-level finding. A leaf's nested report is its accepted exact return or +and top-level finding, and validate the final report with +`-SkillKind super -ExpectedCompositionPath ` against the +selection preserved before dispatch. A leaf's nested report is its accepted exact return or its accepted normalized candidate copy; its exact Task return remains the separate immutable raw audit payload. Treat an invalid sub-result as failed and exclude all of its findings from the top-level rollup. Never reconstruct it diff --git a/skills/do.md b/skills/do.md index db0d6346..16fbdd9b 100644 --- a/skills/do.md +++ b/skills/do.md @@ -224,7 +224,21 @@ as a findings-report, a coordinator or host MUST validate it deterministically: Hosts SHOULD execute `tools/Validate-FindingsReport.ps1` with the exact source scope and the leaf's recorded set of fully retrieved article paths. Pass -`-SkillKind super` when validating a super-skill's rolled-up report. Pass +`-SkillKind super -ExpectedCompositionPath ` when validating +a super-skill's rolled-up report. Prepare that private artifact before leaf +dispatch, after layer resolution and input compatibility checks. It contains +`superSkill` (`id`, `version`), ordered selected `subSkills` (each with `id`, +`version`), and `skipped` (each with `id`, `version`, `reason`). Reasons are +`configuration` or `not-applicable`; budget exhaustion is not a skip reason. +Additional resolver metadata may be retained in the artifact, not the report. +The validator binds the super-skill and leaf identities and versions, checks +selected order, and requires exact agreement on exclusions. Every returned +leaf must be unique; a selected leaf cannot be reclassified as skipped by the +report. Without the artifact, validation remains structural and semantic but +cannot prove composition completeness, selected versions, order, or legitimate +exclusions. Duplicate or both returned-and-skipped leaf IDs are invalid even +without the artifact. Never derive the expected composition from model output. +Pass `-AllowBoundedNormalization` only when the host preserves the immutable raw payload and records `removedRanges` in private telemetry as required above. @@ -330,7 +344,7 @@ Omit `suggested-code` only when the appropriate fix depends on context the skill - `reference` — the suppressed file (same object shape as `findings[].references`). - `reason` — `layer-precedence` when another layer won under READ's precedence rules; `configuration` when the consumer disabled the file's layer. -**`sub-results`** — super-skills only. Array of complete findings-reports, one per sub-skill that was invoked (i.e., every sub-skill not listed in `skipped-sub-skills`). Each entry MUST itself conform to this output contract. Entries MUST appear in the worklist's declared order, regardless of invocation or completion order. Leaf skills MUST NOT emit `sub-results`. +**`sub-results`** — super-skills only. Array of complete findings-reports, one per invoked sub-skill, with no duplicate skill IDs. Each entry MUST itself conform to this output contract. Entries MUST appear in the worklist's declared order, regardless of invocation or completion order. A selected leaf left uninvoked by budget exhaustion has no fabricated sub-result and is not a configured or input-incompatible skip; its absence requires the incomplete-composition outcome below. Leaf skills MUST NOT emit `sub-results`. **`skipped-sub-skills`** — super-skills only. Array of sub-skills that were declared in frontmatter but not invoked. `reason` is `configuration` when the orchestrator disabled the sub-skill, or `not-applicable` when the super-skill's Relevance step ruled it out. @@ -356,10 +370,14 @@ orchestrator. Each leaf invocation MUST remain a discrete evaluation with its own complete findings-report. An orchestrator MAY execute independent leaves serially or -concurrently, but MUST invoke every worklisted leaf, preserve `sub-results` in -the declared worklist order, and wait for every invocation to finish before -performing any super-skill self-review or final rollup. Scheduling MUST NOT -change relevance, coverage, failure, reference-integrity, or output semantics. +concurrently, but MUST attempt every worklisted leaf, preserve `sub-results` +in the declared worklist order, and wait for every started invocation to +finish before final rollup. If its execution budget prevents dispatching +remaining leaves, preserve the unfinished selection in the host-owned expected +composition and use the incomplete-composition outcome below. Do not perform +the super-skill self-review until every selected leaf has returned. Scheduling +MUST NOT change relevance, coverage, failure, reference-integrity, or output +semantics. Orchestrators SHOULD generate `skill-index.json` with `tools/Build-SkillIndex.ps1` instead of parsing Markdown. Each declared @@ -390,7 +408,16 @@ The five required sections still apply. Their meaning shifts from knowledge file ### Outcome rollup -A super-skill's `outcome` is derived from its sub-skills' outcomes. Let S be the multiset of sub-skill outcomes for sub-skills in the worklist (skipped sub-skills do not contribute): +A super-skill's `outcome` is derived from its selected worklist and returned +sub-skills' outcomes. If selected leaves have no returned report, the outcome +is `partial` when at least one returned report is non-`failed`, or `failed` +when no non-`failed` report is available. It MUST NOT be `completed`, +`not-applicable`, or `no-knowledge`. Name unfinished leaf IDs in +`outcome-reason`, preserve the valid returned reports, and never invent +successful or failed invocations for leaves that were not invoked. + +When every selected leaf has returned, let S be the multiset of their outcomes +(skipped sub-skills do not contribute): - `failed` — every element of S is `failed`. - `partial` — S contains at least one `partial`, OR S contains at least one `failed` alongside at least one non-`failed` outcome. diff --git a/tools/Test-ReviewContract.ps1 b/tools/Test-ReviewContract.ps1 index adacb361..118fc3a9 100644 --- a/tools/Test-ReviewContract.ps1 +++ b/tools/Test-ReviewContract.ps1 @@ -269,6 +269,8 @@ try { & $validator -ReportPath $reportPath -BCQualityRoot $Root } + $completedSecurityLeaf = $completedLeaf | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $completedSecurityLeaf.skill.id = 'al-security-review' $validSuperReport = [ordered]@{ skill = [ordered]@{ id = 'al-code-review'; version = 1 } outcome = 'completed' @@ -278,12 +280,207 @@ try { } findings = @() suppressed = @() - 'sub-results' = @($completedLeaf, $completedLeaf) + 'sub-results' = @($completedLeaf, $completedSecurityLeaf) } Set-Content -LiteralPath $reportPath -Value ($validSuperReport | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM $acceptedSuper = & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super Assert-True (-not $acceptedSuper.normalized) 'valid super-skill report is accepted' + $duplicateLeafReport = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $duplicateLeafReport.'sub-results' = @($completedLeaf, $completedLeaf) + Set-Content -LiteralPath $reportPath -Value ($duplicateLeafReport | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + Assert-ThrowsLike -Pattern '*SUPER_DUPLICATE_SUB_RESULT*' -Action { + & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super + } + + $compositionPath = Join-Path $tmp 'composition.json' + $expectedComposition = [ordered]@{ + superSkill = @{ id = 'al-code-review'; version = 1 } + subSkills = @( + @{ id = 'al-style-review'; version = 1 } + @{ id = 'al-security-review'; version = 1 } + ) + skipped = @() + } + Set-Content -LiteralPath $compositionPath -Value ($expectedComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + Set-Content -LiteralPath $reportPath -Value ($validSuperReport | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + $acceptedBoundSuper = & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super ` + -ExpectedCompositionPath $compositionPath + Assert-True (-not $acceptedBoundSuper.normalized) 'complete composition matches the expected worklist' + + $missingLeafReport = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $missingLeafReport.'sub-results' = @($completedLeaf) + $missingLeafReport.summary.coverage.'worklist-size' = 1 + $missingLeafReport.summary.coverage.'items-evaluated' = 1 + Set-Content -LiteralPath $reportPath -Value ($missingLeafReport | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + Assert-ThrowsLike -Pattern '*SUPER_OUTCOME_MISMATCH*' -Action { + & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super -ExpectedCompositionPath $compositionPath + } + $missingLeafReport.outcome = 'partial' + $missingLeafReport | Add-Member -NotePropertyName 'outcome-reason' -NotePropertyValue 'al-security-review was not evaluated before the budget expired.' + Set-Content -LiteralPath $reportPath -Value ($missingLeafReport | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + $acceptedIncompleteSuper = & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super ` + -ExpectedCompositionPath $compositionPath + Assert-True ($acceptedIncompleteSuper.report.outcome -ceq 'partial') 'unfinished selected leaves require a truthful partial outcome' + + function Assert-CompositionReport { + param([object] $Candidate, [string] $ErrorPattern) + + Set-Content -LiteralPath $reportPath -Value ($Candidate | ConvertTo-Json -Depth 30) -Encoding utf8NoBOM + if ($ErrorPattern) { + Assert-ThrowsLike -Pattern $ErrorPattern -Action { + & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super ` + -ExpectedCompositionPath $compositionPath -AllowBoundedNormalization + } + } + else { + $accepted = & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super ` + -ExpectedCompositionPath $compositionPath + Assert-True (-not $accepted.normalized) 'valid expected composition is accepted without repairs' + } + } + + foreach ($case in @( + @{ Pattern = '*SUPER_IDENTITY_MISMATCH*'; Change = { param($candidate) $candidate.skill.id = 'al-other-review' } } + @{ Pattern = '*SUPER_IDENTITY_MISMATCH*'; Change = { param($candidate) $candidate.skill.version = 2 } } + @{ Pattern = '*SUPER_LEAF_VERSION_MISMATCH*'; Change = { param($candidate) $candidate.'sub-results'[0].skill.version = 2 } } + @{ Pattern = '*SUPER_UNEXPECTED_SUB_RESULT*'; Change = { param($candidate) $candidate.'sub-results'[0].skill.id = 'al-other-review' } } + @{ Pattern = '*SUPER_SUB_RESULT_ORDER*'; Change = { param($candidate) $candidate.'sub-results' = @($candidate.'sub-results'[1], $candidate.'sub-results'[0]) } } + @{ Pattern = '*SUPER_DUPLICATE_SUB_RESULT*'; Change = { param($candidate) $candidate.'sub-results' = @($candidate.'sub-results'[0], $candidate.'sub-results'[0]) } } + )) { + $candidate = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + & $case.Change $candidate + Assert-CompositionReport $candidate $case.Pattern + } + + $fabricatedSkip = $missingLeafReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $fabricatedSkip | Add-Member -NotePropertyName 'skipped-sub-skills' -NotePropertyValue @( + @{ skill = @{ id = 'al-security-review'; version = 1 }; reason = 'configuration' } + ) + Assert-CompositionReport $fabricatedSkip '*SUPER_UNEXPECTED_SKIP*' + + $noResults = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $noResults.'sub-results' = @() + $noResults.summary.coverage.'worklist-size' = 0 + $noResults.summary.coverage.'items-evaluated' = 0 + $noResults.outcome = 'not-applicable' + Assert-CompositionReport $noResults '*SUPER_OUTCOME_MISMATCH*' + $noResults.outcome = 'failed' + $noResults | Add-Member -NotePropertyName 'outcome-reason' -NotePropertyValue 'No selected leaf could be evaluated.' + Assert-CompositionReport $noResults + + $allFailed = $noResults | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $allFailed.'sub-results' = @($completedLeaf, $completedSecurityLeaf) | ConvertTo-Json -Depth 20 | ConvertFrom-Json + foreach ($leaf in $allFailed.'sub-results') { + $leaf.outcome = 'failed' + $leaf.summary.coverage.'items-evaluated' = 0 + $leaf | Add-Member -NotePropertyName 'outcome-reason' -NotePropertyValue 'Invocation failed.' + } + Assert-CompositionReport $allFailed + + $skipComposition = $expectedComposition | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $skipComposition.subSkills = @($skipComposition.subSkills[0]) + $skipComposition.skipped = @(@{ id = 'al-security-review'; version = 1; reason = 'not-applicable' }) + Set-Content -LiteralPath $compositionPath -Value ($skipComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + $validSkippedReport = $fabricatedSkip | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $validSkippedReport.outcome = 'completed' + $validSkippedReport.PSObject.Properties.Remove('outcome-reason') + $validSkippedReport.'skipped-sub-skills'[0].reason = 'not-applicable' + Assert-CompositionReport $validSkippedReport + Assert-CompositionReport $missingLeafReport '*SUPER_SKIP_MISSING*' + $wrongSkip = $validSkippedReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $wrongSkip.'skipped-sub-skills'[0].reason = 'configuration' + Assert-CompositionReport $wrongSkip '*SUPER_SKIP_MISMATCH*' + $wrongSkip.'skipped-sub-skills'[0].reason = 'not-applicable' + $wrongSkip.'skipped-sub-skills'[0].skill.version = 2 + Assert-CompositionReport $wrongSkip '*SUPER_SKIP_MISMATCH*' + $duplicateSkip = $validSkippedReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $duplicateSkip.'skipped-sub-skills' = @($duplicateSkip.'skipped-sub-skills'[0], $duplicateSkip.'skipped-sub-skills'[0]) + Assert-CompositionReport $duplicateSkip '*SUPER_SKIP_CONFLICT*' + $returnedAndSkipped = $validSkippedReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $returnedAndSkipped.'skipped-sub-skills'[0].skill.id = 'al-style-review' + Assert-CompositionReport $returnedAndSkipped '*SUPER_SKIP_CONFLICT*' + + $allSkippedComposition = $expectedComposition | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $allSkippedComposition.skipped = @($allSkippedComposition.subSkills | ForEach-Object { + @{ id = $_.id; version = $_.version; reason = 'configuration' } + }) + $allSkippedComposition.subSkills = @() + Set-Content -LiteralPath $compositionPath -Value ($allSkippedComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + $allSkippedReport = $noResults | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $allSkippedReport.outcome = 'not-applicable' + $allSkippedReport.PSObject.Properties.Remove('outcome-reason') + $allSkippedReport | Add-Member -NotePropertyName 'skipped-sub-skills' -NotePropertyValue @( + $allSkippedComposition.skipped | ForEach-Object { @{ skill = @{ id = $_.id; version = $_.version }; reason = $_.reason } } + ) + Assert-CompositionReport $allSkippedReport + + $invalidComposition = $expectedComposition | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $invalidComposition.skipped = @(@{ id = 'al-style-review'; version = 1; reason = 'configuration' }) + Set-Content -LiteralPath $compositionPath -Value ($invalidComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + Assert-CompositionReport $validSuperReport '*Invalid expected composition*' + $invalidComposition.skipped = @() + $invalidComposition.subSkills[0].version = '1' + Set-Content -LiteralPath $compositionPath -Value ($invalidComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + Assert-CompositionReport $validSuperReport '*Invalid expected composition*' + Set-Content -LiteralPath $compositionPath -Value ($expectedComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + + $layerFixtureRoot = Join-Path $tmp 'layered-skills' + foreach ($layer in 'microsoft', 'community', 'custom') { + $layerDirectory = Join-Path $layerFixtureRoot $layer + New-Item -ItemType Directory -Path $layerDirectory -Force | Out-Null + $sourceSkills = Join-Path $Root "$layer/skills" + if (Test-Path -LiteralPath $sourceSkills -PathType Container) { + Copy-Item -LiteralPath $sourceSkills -Destination (Join-Path $layerDirectory 'skills') -Recurse + } + } + $customSkillDirectory = Join-Path $layerFixtureRoot 'custom/skills/review' + New-Item -ItemType Directory -Path $customSkillDirectory -Force | Out-Null + $customSkillPath = 'custom/skills/review/company-style-review.md' + $styleSkillText = Get-Content -LiteralPath (Join-Path $Root 'microsoft/skills/review/al-style-review.md') -Raw + Set-Content -LiteralPath (Join-Path $layerFixtureRoot $customSkillPath) ` + -Value ($styleSkillText -replace '(?m)^version: 1\r?$', 'version: 7') -Encoding utf8NoBOM + $fixtureIndexPath = Join-Path $tmp 'layered-skill-index.json' + & (Join-Path $Root 'tools/Build-SkillIndex.ps1') -BCQualityRoot $layerFixtureRoot -IndexPath $fixtureIndexPath | Out-Null + $fixtureIndex = Get-Content -LiteralPath $fixtureIndexPath -Raw | ConvertFrom-Json + foreach ($selection in @( + @{ Disabled = @(); ExpectedLayer = 'custom'; ExpectedVersion = 7 } + @{ Disabled = @($customSkillPath); ExpectedLayer = 'microsoft'; ExpectedVersion = 1 } + @{ Disabled = @($customSkillPath, 'microsoft/skills/review/al-style-review.md'); ExpectedLayer = $null } + )) { + $resolved = & (Join-Path $Root 'tools/Resolve-SkillWorklist.ps1') -BCQualityRoot $layerFixtureRoot ` + -IndexPath $fixtureIndexPath -SuperSkillPath 'microsoft/skills/review/al-code-review.md' ` + -DisabledSkills $selection.Disabled + $styleSlots = @($resolved.subSkills | Where-Object id -CEQ 'al-style-review') + if ($selection.ExpectedLayer) { + Assert-True ($styleSlots.Count -eq 1 -and $styleSlots[0].layer -ceq $selection.ExpectedLayer -and + $styleSlots[0].version -eq $selection.ExpectedVersion) 'resolver-selected override or fallback is authoritative' + } + else { + Assert-True ($styleSlots.Count -eq 0) 'fully disabled slot is not selected' + } + $resolved.skipped = @($resolved.skipped | ForEach-Object { + $declaredPath = $_.declaredPath + $declaredSkill = @($fixtureIndex.skills | Where-Object path -CEQ $declaredPath)[0] + @{ id = $_.id; version = $declaredSkill.version; reason = $_.reason; declaredPath = $declaredPath } + }) + Set-Content -LiteralPath $compositionPath -Value ($resolved | ConvertTo-Json -Depth 30) -Encoding utf8NoBOM + $resolvedReport = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $resolvedReport.'sub-results' = @($resolved.subSkills | ForEach-Object { + $leafReport = $completedLeaf | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $leafReport.skill.id = $_.id + $leafReport.skill.version = $_.version + $leafReport + }) + $resolvedReport.summary.coverage.'worklist-size' = $resolved.subSkills.Count + $resolvedReport.summary.coverage.'items-evaluated' = $resolved.subSkills.Count + $resolvedReport | Add-Member -NotePropertyName 'skipped-sub-skills' -NotePropertyValue @( + $resolved.skipped | ForEach-Object { @{ skill = @{ id = $_.id; version = $_.version }; reason = $_.reason } } + ) + Assert-CompositionReport $resolvedReport + } + Set-Content -LiteralPath $compositionPath -Value ($expectedComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + $styleFindingLeaf = $validReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json $securityFindingLeaf = $validReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json $securityFindingLeaf.skill.id = 'al-security-review' @@ -446,6 +643,7 @@ try { Set-Content -LiteralPath $reportPath -Value ($partialSuperReport | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM $acceptedPartialSuper = & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super Assert-True (-not $acceptedPartialSuper.normalized) 'partial super-skill excludes failed coverage from its rollup' + Assert-CompositionReport $partialSuperReport $failedLeafLeakage = $partialSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json $failedLeafLeakage.summary.counts.minor = 1 diff --git a/tools/Validate-FindingsReport.ps1 b/tools/Validate-FindingsReport.ps1 index d3442d35..cf42c5b7 100644 --- a/tools/Validate-FindingsReport.ps1 +++ b/tools/Validate-FindingsReport.ps1 @@ -12,6 +12,7 @@ param( [string[]] $RetrievedArticlePaths = @(), [ValidateSet('leaf', 'super')] [string] $SkillKind = 'leaf', + [string] $ExpectedCompositionPath, [switch] $AllowBoundedNormalization ) @@ -34,6 +35,61 @@ catch { throw "Invalid findings-report JSON or schema: $($_.Exception.Message)" } +$expectedComposition = $null +$expectedLeaves = [Collections.Generic.Dictionary[string, object]]::new([StringComparer]::Ordinal) +$expectedSkips = [Collections.Generic.Dictionary[string, object]]::new([StringComparer]::Ordinal) +if ($ExpectedCompositionPath) { + if ($SkillKind -cne 'super') { + throw 'Expected composition is supported only for super-skill reports.' + } + $contractSchema = Get-Content -LiteralPath $schemaPath -Raw | ConvertFrom-Json -AsHashtable + $identitySchema = @{ + type = 'object' + required = @('id', 'version') + properties = $contractSchema.definitions.skillReference.properties + } + $skipProperties = @{ + id = $identitySchema.properties.id + version = $identitySchema.properties.version + reason = @{ enum = @('configuration', 'not-applicable') } + } + $compositionSchema = @{ + type = 'object' + required = @('superSkill', 'subSkills', 'skipped') + properties = @{ + superSkill = $identitySchema + subSkills = @{ type = 'array'; items = $identitySchema } + skipped = @{ + type = 'array' + items = @{ type = 'object'; required = @('id', 'version', 'reason'); properties = $skipProperties } + } + } + } | ConvertTo-Json -Depth 20 + try { + $compositionRaw = Get-Content -LiteralPath $ExpectedCompositionPath -Raw + if (-not ($compositionRaw | Test-Json -Schema $compositionSchema -ErrorAction Stop)) { + throw 'Expected composition does not satisfy its input contract.' + } + $expectedComposition = $compositionRaw | ConvertFrom-Json -Depth 100 + $expectedIds = [Collections.Generic.HashSet[string]]::new([StringComparer]::Ordinal) + foreach ($leaf in @($expectedComposition.subSkills)) { + if (-not $expectedIds.Add([string]$leaf.id)) { + throw "Duplicate expected skill id '$($leaf.id)'." + } + $expectedLeaves.Add([string]$leaf.id, $leaf) + } + foreach ($skip in @($expectedComposition.skipped)) { + if (-not $expectedIds.Add([string]$skip.id)) { + throw "Duplicate or selected-and-skipped expected skill id '$($skip.id)'." + } + $expectedSkips.Add([string]$skip.id, $skip) + } + } + catch { + throw "Invalid expected composition: $($_.Exception.Message)" + } +} + $retrieved = [Collections.Generic.HashSet[string]]::new([StringComparer]::Ordinal) foreach ($path in $RetrievedArticlePaths) { $retrieved.Add($path) | Out-Null @@ -79,8 +135,14 @@ function Get-SemanticErrors { } function Get-DerivedSuperOutcome { - param([object[]] $SubResults) + param([object[]] $SubResults, [int] $MissingResults = 0) + if ($MissingResults -gt 0) { + if (@($SubResults | Where-Object outcome -CNE 'failed').Count) { + return 'partial' + } + return 'failed' + } if (-not $SubResults.Count) { return 'not-applicable' } @@ -342,20 +404,87 @@ function Get-SemanticErrors { if ($CurrentSkillKind -ceq 'super' -and $hasSubResults) { $subResults = @($Current.'sub-results') + $producerIds = [Collections.Generic.HashSet[string]]::new([StringComparer]::Ordinal) for ($index = 0; $index -lt $subResults.Count; $index++) { + if (-not $producerIds.Add([string]$subResults[$index].skill.id)) { + Add-Error 'SUPER_DUPLICATE_SUB_RESULT' "$ReportPathPrefix.sub-results[$index].skill.id" ` + 'A leaf may appear only once in sub-results.' + } Test-Report $subResults[$index] "$ReportPathPrefix.sub-results[$index]" 'leaf' } - $expectedOutcome = Get-DerivedSuperOutcome $subResults + $skippedIds = [Collections.Generic.HashSet[string]]::new([StringComparer]::Ordinal) + $skips = if ($hasSkippedSubSkills) { @($Current.'skipped-sub-skills') } else { @() } + foreach ($skip in $skips) { + if (-not $skippedIds.Add([string]$skip.skill.id) -or $producerIds.Contains([string]$skip.skill.id)) { + Add-Error 'SUPER_SKIP_CONFLICT' "$ReportPathPrefix.skipped-sub-skills" ` + "Skill '$($skip.skill.id)' is duplicated or both returned and skipped." + } + } + + $missingResults = 0 + if ($null -ne $expectedComposition) { + if ($Current.skill.id -cne $expectedComposition.superSkill.id -or + $Current.skill.version -ne $expectedComposition.superSkill.version) { + Add-Error 'SUPER_IDENTITY_MISMATCH' "$ReportPathPrefix.skill" 'Super-skill identity differs from expected composition.' + } + $previousSlot = -1 + $orderedIds = @($expectedComposition.subSkills | ForEach-Object { $_.id }) + for ($index = 0; $index -lt $subResults.Count; $index++) { + $identity = $subResults[$index].skill + if (-not $expectedLeaves.ContainsKey([string]$identity.id)) { + Add-Error 'SUPER_UNEXPECTED_SUB_RESULT' "$ReportPathPrefix.sub-results[$index].skill" ` + "Skill '$($identity.id)' was not selected." + continue + } + if ($identity.version -ne $expectedLeaves[$identity.id].version) { + Add-Error 'SUPER_LEAF_VERSION_MISMATCH' "$ReportPathPrefix.sub-results[$index].skill.version" ` + "Unexpected version for '$($identity.id)'." + } + $slot = [Array]::IndexOf($orderedIds, $identity.id) + if ($slot -le $previousSlot) { + Add-Error 'SUPER_SUB_RESULT_ORDER' "$ReportPathPrefix.sub-results[$index].skill" ` + 'Sub-results must preserve the selected worklist order.' + } + $previousSlot = $slot + } + foreach ($leaf in @($expectedComposition.subSkills)) { + if (-not $producerIds.Contains([string]$leaf.id)) { + $missingResults++ + } + } + foreach ($skip in $skips) { + if (-not $expectedSkips.ContainsKey([string]$skip.skill.id)) { + Add-Error 'SUPER_UNEXPECTED_SKIP' "$ReportPathPrefix.skipped-sub-skills" ` + "Skill '$($skip.skill.id)' was not excluded by the coordinator." + continue + } + $expectedSkip = $expectedSkips[$skip.skill.id] + if ($skip.skill.version -ne $expectedSkip.version -or $skip.reason -cne $expectedSkip.reason) { + Add-Error 'SUPER_SKIP_MISMATCH' "$ReportPathPrefix.skipped-sub-skills" ` + "Skip identity or reason differs for '$($skip.skill.id)'." + } + } + foreach ($skip in @($expectedComposition.skipped)) { + if (-not $skippedIds.Contains([string]$skip.id)) { + Add-Error 'SUPER_SKIP_MISSING' "$ReportPathPrefix.skipped-sub-skills" ` + "Expected exclusion '$($skip.id)' is missing." + } + } + } + + $expectedOutcome = Get-DerivedSuperOutcome $subResults $missingResults if ($Current.outcome -cne $expectedOutcome) { Add-Error 'SUPER_OUTCOME_MISMATCH' "$ReportPathPrefix.outcome" "Expected '$expectedOutcome' from sub-results." } $includedSubResults = @($subResults | Where-Object outcome -CNE 'failed') - $expectedWorklistSize = ($includedSubResults | Measure-Object -Property { $_.summary.coverage.'worklist-size' } -Sum).Sum - $expectedItemsEvaluated = ($includedSubResults | Measure-Object -Property { $_.summary.coverage.'items-evaluated' } -Sum).Sum - if ($null -eq $expectedWorklistSize) { $expectedWorklistSize = 0 } - if ($null -eq $expectedItemsEvaluated) { $expectedItemsEvaluated = 0 } + $expectedWorklistSize = 0 + $expectedItemsEvaluated = 0 + foreach ($subResult in $includedSubResults) { + $expectedWorklistSize += $subResult.summary.coverage.'worklist-size' + $expectedItemsEvaluated += $subResult.summary.coverage.'items-evaluated' + } if ($worklistSize -ne $expectedWorklistSize -or $itemsEvaluated -ne $expectedItemsEvaluated) { Add-Error 'SUPER_COVERAGE_MISMATCH' "$ReportPathPrefix.summary.coverage" ` "Expected worklist-size $expectedWorklistSize and items-evaluated $expectedItemsEvaluated from non-failed sub-results." From aecd081778e911b4e24ef46e5ea4e72c9facbc89 Mon Sep 17 00:00:00 2001 From: demiliani Date: Thu, 1 Oct 2026 14:58:19 +0200 Subject: [PATCH 2/3] Bind composed leaf reports to host-accepted results --- docs/standalone-runner.md | 28 +++- microsoft/skills/review/al-code-review.md | 8 ++ skills/do.md | 29 +++- tools/Test-ReviewContract.ps1 | 157 +++++++++++++++++++++- tools/Validate-FindingsReport.ps1 | 95 ++++++++++++- 5 files changed, 303 insertions(+), 14 deletions(-) diff --git a/docs/standalone-runner.md b/docs/standalone-runner.md index d3186ef7..74b2f8ed 100644 --- a/docs/standalone-runner.md +++ b/docs/standalone-runner.md @@ -88,7 +88,8 @@ For example, if style is selected and security was disabled: { "superSkill": { "id": "al-code-review", "version": 1 }, "subSkills": [{ "id": "al-style-review", "version": 1 }], - "skipped": [{ "id": "al-security-review", "version": 1, "reason": "configuration" }] + "skipped": [{ "id": "al-security-review", "version": 1, "reason": "configuration" }], + "acceptedResults": [] } ``` @@ -101,10 +102,28 @@ Keep paths, layers, and other resolver metadata if useful for private audit. Do not add the artifact to the findings-report or overwrite it to hide an unfinished invocation. Preserve it with the run's raw payloads. +Initialize `acceptedResults` before dispatch. After accepting a leaf, save its +exact accepted copy (including any permitted normalization) in an immutable +host-owned file outside worker/composer write access. Append only its capture +to `acceptedResults`, for example: + +```json +{"id": "al-style-review", "version": 1, "reportPath": "accepted/style.json"} +``` + +`reportPath` may be absolute or relative to the composition artifact's directory. +Capture a host-created failed validation report the same way. Never construct +these captures from the composed `sub-results` or permit the composing model +to supply or alter them. Keep the original selection and exclusions unchanged. + The gate rejects repeated or unexpected leaf IDs, wrong selected versions, -reordered results, and fabricated or missing exclusions. If selected leaves -remain unfinished, the report must be `partial` with a non-failed returned -report, otherwise `failed`; identify unfinished leaf IDs in `outcome-reason`. +reordered results, fabricated or missing exclusions, and uncaptured or altered +leaf content. JSON property order is immaterial; array order, field presence, +types, and values must match. Every captured leaf must appear in `sub-results`. +If selected leaves remain unfinished, the report must be `partial` with a non-failed returned +report, otherwise `failed`; name every unfinished leaf ID exactly in +`outcome-reason`. Top-level `from-sub-skill: "agent"` findings are rejected +while selected leaf results are missing. Do not fabricate leaf reports or use configuration skips for budget exhaustion. Wait for started invocations to finish; omit the self-review if the selected composition remains incomplete. Coverage still sums the non-failed leaf @@ -112,6 +131,7 @@ knowledge worklists, not the number of selected leaf slots. Calls without the expected artifact remain supported for structural/semantic validation, but cannot certify that a composed review covered its selection. +They also cannot bind nested leaves to the host's accepted outputs. They still reject duplicate leaf IDs and returned-and-skipped conflicts. ## Runner-owned choices diff --git a/microsoft/skills/review/al-code-review.md b/microsoft/skills/review/al-code-review.md index 8699d7d7..30bf659c 100644 --- a/microsoft/skills/review/al-code-review.md +++ b/microsoft/skills/review/al-code-review.md @@ -78,6 +78,14 @@ skill's version, and record any input-incompatible exclusions with `reason: "not-applicable"`. Do not derive this artifact from leaf reports or change it merely because execution later runs out of budget. Pass its path as `-ExpectedCompositionPath` for final super-skill validation. +Initialize `acceptedResults` to `[]`. After each leaf's acceptance gate, the +host saves its exact accepted copy (or host-created failed validation result) +in an immutable private file and appends its `id`, `version`, and `reportPath` +to that array. The host alone owns these captures; neither workers nor the +composing model may write them. Never derive them from composed `sub-results`. +Keep the pre-dispatch selection and exclusions unchanged. Final validation +requires every nested leaf to match its captured JSON content exactly and +every captured result to be included. Property order is immaterial. ## Action diff --git a/skills/do.md b/skills/do.md index 16fbdd9b..e5aef5b6 100644 --- a/skills/do.md +++ b/skills/do.md @@ -228,16 +228,31 @@ scope and the leaf's recorded set of fully retrieved article paths. Pass a super-skill's rolled-up report. Prepare that private artifact before leaf dispatch, after layer resolution and input compatibility checks. It contains `superSkill` (`id`, `version`), ordered selected `subSkills` (each with `id`, -`version`), and `skipped` (each with `id`, `version`, `reason`). Reasons are -`configuration` or `not-applicable`; budget exhaustion is not a skip reason. +`version`), `skipped` (each with `id`, `version`, `reason`), and an initially +empty `acceptedResults` array. Reasons are `configuration` or `not-applicable`; +budget exhaustion is not a skip reason. Additional resolver metadata may be retained in the artifact, not the report. +After each leaf passes its acceptance gate, the host saves the exact accepted +copy in a private immutable file and appends an `acceptedResults` entry with +`id`, `version`, and `reportPath`. Capture host-created failed validation +results the same way. Paths may be absolute or relative to the composition +artifact's directory. Capture the normalized accepted copy when normalization +was permitted, not the invalid raw return. Do not expose these files or write +access to the artifact to leaf workers or the composing model. Only the host +may append captures; the pre-dispatch selection and exclusions remain fixed. The validator binds the super-skill and leaf identities and versions, checks -selected order, and requires exact agreement on exclusions. Every returned -leaf must be unique; a selected leaf cannot be reclassified as skipped by the -report. Without the artifact, validation remains structural and semantic but +selected order, and requires exact agreement on exclusions. Each nested leaf +must exactly match its host-captured accepted JSON content, ignoring object +property order but preserving array order, types, values, and field presence. +Every captured leaf must be included; uncaptured or altered leaves are invalid. +Every returned leaf must be unique; a selected leaf cannot be reclassified as skipped by the +report. When selected leaves are missing, `outcome-reason` must name every +missing ID exactly and top-level `from-sub-skill: "agent"` findings are forbidden. +Without the artifact, validation remains structural and semantic but cannot prove composition completeness, selected versions, order, or legitimate -exclusions. Duplicate or both returned-and-skipped leaf IDs are invalid even -without the artifact. Never derive the expected composition from model output. +exclusions, nor bind leaves to accepted host outputs. Duplicate or both +returned-and-skipped leaf IDs are invalid even without the artifact. Never +derive the expected composition from model output. Pass `-AllowBoundedNormalization` only when the host preserves the immutable raw payload and records `removedRanges` in private telemetry as required above. diff --git a/tools/Test-ReviewContract.ps1 b/tools/Test-ReviewContract.ps1 index 118fc3a9..a567e4ce 100644 --- a/tools/Test-ReviewContract.ps1 +++ b/tools/Test-ReviewContract.ps1 @@ -294,6 +294,18 @@ try { } $compositionPath = Join-Path $tmp 'composition.json' + function Save-AcceptedLeafReports { + param([object[]] $LeafReports) + + foreach ($leafReport in $LeafReports) { + $leafPath = Join-Path $tmp "$([guid]::NewGuid()).json" + Set-Content -LiteralPath $leafPath -Value ($leafReport | ConvertTo-Json -Depth 100) -Encoding utf8NoBOM + $accepted = & $validator -ReportPath $leafPath -BCQualityRoot $Root + Assert-True (-not $accepted.normalized) 'host captures a validated leaf report' + @{ id = $leafReport.skill.id; version = $leafReport.skill.version; reportPath = $leafPath } + } + } + $expectedComposition = [ordered]@{ superSkill = @{ id = 'al-code-review'; version = 1 } subSkills = @( @@ -301,6 +313,7 @@ try { @{ id = 'al-security-review'; version = 1 } ) skipped = @() + acceptedResults = @(Save-AcceptedLeafReports @($completedLeaf, $completedSecurityLeaf)) } Set-Content -LiteralPath $compositionPath -Value ($expectedComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM Set-Content -LiteralPath $reportPath -Value ($validSuperReport | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM @@ -308,6 +321,9 @@ try { -ExpectedCompositionPath $compositionPath Assert-True (-not $acceptedBoundSuper.normalized) 'complete composition matches the expected worklist' + $incompleteComposition = $expectedComposition | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $incompleteComposition.acceptedResults = @($incompleteComposition.acceptedResults[0]) + Set-Content -LiteralPath $compositionPath -Value ($incompleteComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM $missingLeafReport = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json $missingLeafReport.'sub-results' = @($completedLeaf) $missingLeafReport.summary.coverage.'worklist-size' = 1 @@ -340,6 +356,103 @@ try { } } + $genericMissingReason = $missingLeafReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $genericMissingReason.'outcome-reason' = 'Budget expired.' + Assert-CompositionReport $genericMissingReason '*SUPER_MISSING_LEAF_REASON*' + $genericMissingReason.'outcome-reason' = 'prefix-al-security-review-suffix was not evaluated.' + Assert-CompositionReport $genericMissingReason '*SUPER_MISSING_LEAF_REASON*' + + foreach ($fabricatedOutcome in 'completed', 'not-applicable', 'no-knowledge') { + $fabricatedLeafReport = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $fabricatedLeafReport.'sub-results'[1].outcome = $fabricatedOutcome + if ($fabricatedOutcome -cne 'completed') { + $fabricatedLeafReport.'sub-results'[1].summary.coverage.'worklist-size' = 0 + $fabricatedLeafReport.'sub-results'[1].summary.coverage.'items-evaluated' = 0 + $fabricatedLeafReport.summary.coverage.'worklist-size' = 1 + $fabricatedLeafReport.summary.coverage.'items-evaluated' = 1 + } + Assert-CompositionReport $fabricatedLeafReport '*SUPER_LEAF_NOT_ACCEPTED*' + } + Set-Content -LiteralPath $compositionPath -Value ($expectedComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + Assert-CompositionReport $missingLeafReport '*SUPER_ACCEPTED_LEAF_MISSING*' + $alteredLeafReport = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $alteredLeafReport.'sub-results'[1].summary.coverage.'worklist-size' = 2 + $alteredLeafReport.'sub-results'[1].summary.coverage.'items-evaluated' = 2 + $alteredLeafReport.summary.coverage.'worklist-size' = 3 + $alteredLeafReport.summary.coverage.'items-evaluated' = 3 + Assert-CompositionReport $alteredLeafReport '*SUPER_LEAF_CONTENT_MISMATCH*' + $reorderedProperties = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $reorderedProperties.'sub-results'[0].skill = [pscustomobject]@{ version = 1; id = 'al-style-review' } + Assert-CompositionReport $reorderedProperties + foreach ($alteredOutcome in 'not-applicable', 'no-knowledge') { + $alteredOutcomeReport = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $alteredOutcomeReport.'sub-results'[1].outcome = $alteredOutcome + $alteredOutcomeReport.'sub-results'[1].summary.coverage.'worklist-size' = 0 + $alteredOutcomeReport.'sub-results'[1].summary.coverage.'items-evaluated' = 0 + $alteredOutcomeReport.summary.coverage.'worklist-size' = 1 + $alteredOutcomeReport.summary.coverage.'items-evaluated' = 1 + Assert-CompositionReport $alteredOutcomeReport '*SUPER_LEAF_CONTENT_MISMATCH*' + } + $relativeCaptureComposition = $expectedComposition | ConvertTo-Json -Depth 20 | ConvertFrom-Json + foreach ($capture in $relativeCaptureComposition.acceptedResults) { + $capture.reportPath = Split-Path -Leaf $capture.reportPath + } + Set-Content -LiteralPath $compositionPath -Value ($relativeCaptureComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + Assert-CompositionReport $validSuperReport + $uncapturedComposition = $expectedComposition | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $uncapturedComposition.PSObject.Properties.Remove('acceptedResults') + Set-Content -LiteralPath $compositionPath -Value ($uncapturedComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + Assert-CompositionReport $validSuperReport '*Invalid expected composition*' + $duplicateCaptureComposition = $expectedComposition | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $duplicateCaptureComposition.acceptedResults = @($duplicateCaptureComposition.acceptedResults[0], $duplicateCaptureComposition.acceptedResults[0]) + Set-Content -LiteralPath $compositionPath -Value ($duplicateCaptureComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + Assert-CompositionReport $validSuperReport '*Invalid expected composition*' + + $capturedFindingLeaf = $completedLeaf | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $capturedFindingLeaf.findings = @(@{ + id = 'agent:leaf-issue' + severity = 'minor' + confidence = 'medium' + message = 'Preserve this accepted leaf finding.' + references = @() + }) + $secondCapturedFinding = $capturedFindingLeaf.findings[0] | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $secondCapturedFinding.id = 'agent:second-leaf-issue' + $capturedFindingLeaf.findings = @($capturedFindingLeaf.findings[0], $secondCapturedFinding) + $capturedFindingLeaf.summary.counts.minor = 2 + $findingComposition = $expectedComposition | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $findingComposition.acceptedResults = @(Save-AcceptedLeafReports @($capturedFindingLeaf, $completedSecurityLeaf)) + Set-Content -LiteralPath $compositionPath -Value ($findingComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + $capturedFindingReport = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $capturedFindingReport.'sub-results'[0] = $capturedFindingLeaf + $capturedFindingReport.findings = @($capturedFindingLeaf.findings | ConvertTo-Json -Depth 20 | ConvertFrom-Json) + foreach ($finding in $capturedFindingReport.findings) { + $finding.id = "al-style-review:$($finding.id)" + $finding | Add-Member -NotePropertyName 'from-sub-skill' -NotePropertyValue 'al-style-review' + } + $capturedFindingReport.summary.counts.minor = 2 + Assert-CompositionReport $capturedFindingReport + $reorderedFindingReport = $capturedFindingReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $reorderedFindingReport.'sub-results'[0].findings = @( + $reorderedFindingReport.'sub-results'[0].findings[1] + $reorderedFindingReport.'sub-results'[0].findings[0] + ) + Assert-CompositionReport $reorderedFindingReport '*SUPER_LEAF_CONTENT_MISMATCH*' + $addedLeafFieldReport = $capturedFindingReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $addedLeafFieldReport.'sub-results'[0] | Add-Member -NotePropertyName 'outcome-reason' -NotePropertyValue 'A composer-added field.' + Assert-CompositionReport $addedLeafFieldReport '*SUPER_LEAF_CONTENT_MISMATCH*' + $alteredFindingReport = $capturedFindingReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $alteredFindingReport.'sub-results'[0].findings[0].message = 'A fabricated replacement message.' + $alteredFindingReport.findings[0].message = 'A fabricated replacement message.' + Assert-CompositionReport $alteredFindingReport '*SUPER_LEAF_CONTENT_MISMATCH*' + $removedFindingReport = $capturedFindingReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $removedFindingReport.'sub-results'[0].findings = @() + $removedFindingReport.'sub-results'[0].summary.counts.minor = 0 + $removedFindingReport.findings = @() + $removedFindingReport.summary.counts.minor = 0 + Assert-CompositionReport $removedFindingReport '*SUPER_LEAF_CONTENT_MISMATCH*' + Set-Content -LiteralPath $compositionPath -Value ($expectedComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + foreach ($case in @( @{ Pattern = '*SUPER_IDENTITY_MISMATCH*'; Change = { param($candidate) $candidate.skill.id = 'al-other-review' } } @{ Pattern = '*SUPER_IDENTITY_MISMATCH*'; Change = { param($candidate) $candidate.skill.version = 2 } } @@ -359,6 +472,9 @@ try { ) Assert-CompositionReport $fabricatedSkip '*SUPER_UNEXPECTED_SKIP*' + $emptyAcceptedComposition = $expectedComposition | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $emptyAcceptedComposition.acceptedResults = @() + Set-Content -LiteralPath $compositionPath -Value ($emptyAcceptedComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM $noResults = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json $noResults.'sub-results' = @() $noResults.summary.coverage.'worklist-size' = 0 @@ -366,9 +482,37 @@ try { $noResults.outcome = 'not-applicable' Assert-CompositionReport $noResults '*SUPER_OUTCOME_MISMATCH*' $noResults.outcome = 'failed' - $noResults | Add-Member -NotePropertyName 'outcome-reason' -NotePropertyValue 'No selected leaf could be evaluated.' + $noResults | Add-Member -NotePropertyName 'outcome-reason' -NotePropertyValue 'al-style-review and al-security-review could not be evaluated.' Assert-CompositionReport $noResults + $oneMissingId = $noResults | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $oneMissingId.'outcome-reason' = 'al-security-review could not be evaluated.' + Assert-CompositionReport $oneMissingId '*SUPER_MISSING_LEAF_REASON*' + foreach ($baseReport in @($missingLeafReport, $noResults, $validSuperReport)) { + $capturedComposition = if ($baseReport.outcome -ceq 'completed') { $expectedComposition } else { $incompleteComposition } + Set-Content -LiteralPath $compositionPath -Value ($capturedComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + $selfReviewReport = $baseReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $selfReviewReport.findings = @(@{ + id = 'agent:cross-domain-gap' + domain = 'Agent' + severity = 'minor' + confidence = 'medium' + message = 'A cross-domain issue needs attention.' + references = @() + 'from-sub-skill' = 'agent' + }) + $selfReviewReport.summary.counts.minor = 1 + if ($baseReport.outcome -ceq 'completed') { + Assert-CompositionReport $selfReviewReport + } + elseif ($baseReport.outcome -ceq 'failed') { + Assert-CompositionReport $selfReviewReport '*Invalid findings-report JSON or schema*' + } + else { + Assert-CompositionReport $selfReviewReport '*SUPER_AGENT_REVIEW_INCOMPLETE*' + } + } + $allFailed = $noResults | ConvertTo-Json -Depth 20 | ConvertFrom-Json $allFailed.'sub-results' = @($completedLeaf, $completedSecurityLeaf) | ConvertTo-Json -Depth 20 | ConvertFrom-Json foreach ($leaf in $allFailed.'sub-results') { @@ -376,11 +520,15 @@ try { $leaf.summary.coverage.'items-evaluated' = 0 $leaf | Add-Member -NotePropertyName 'outcome-reason' -NotePropertyValue 'Invocation failed.' } + $failedComposition = $expectedComposition | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $failedComposition.acceptedResults = @(Save-AcceptedLeafReports $allFailed.'sub-results') + Set-Content -LiteralPath $compositionPath -Value ($failedComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM Assert-CompositionReport $allFailed $skipComposition = $expectedComposition | ConvertTo-Json -Depth 20 | ConvertFrom-Json $skipComposition.subSkills = @($skipComposition.subSkills[0]) $skipComposition.skipped = @(@{ id = 'al-security-review'; version = 1; reason = 'not-applicable' }) + $skipComposition.acceptedResults = @($skipComposition.acceptedResults[0]) Set-Content -LiteralPath $compositionPath -Value ($skipComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM $validSkippedReport = $fabricatedSkip | ConvertTo-Json -Depth 20 | ConvertFrom-Json $validSkippedReport.outcome = 'completed' @@ -406,6 +554,7 @@ try { @{ id = $_.id; version = $_.version; reason = 'configuration' } }) $allSkippedComposition.subSkills = @() + $allSkippedComposition.acceptedResults = @() Set-Content -LiteralPath $compositionPath -Value ($allSkippedComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM $allSkippedReport = $noResults | ConvertTo-Json -Depth 20 | ConvertFrom-Json $allSkippedReport.outcome = 'not-applicable' @@ -464,7 +613,6 @@ try { $declaredSkill = @($fixtureIndex.skills | Where-Object path -CEQ $declaredPath)[0] @{ id = $_.id; version = $declaredSkill.version; reason = $_.reason; declaredPath = $declaredPath } }) - Set-Content -LiteralPath $compositionPath -Value ($resolved | ConvertTo-Json -Depth 30) -Encoding utf8NoBOM $resolvedReport = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json $resolvedReport.'sub-results' = @($resolved.subSkills | ForEach-Object { $leafReport = $completedLeaf | ConvertTo-Json -Depth 20 | ConvertFrom-Json @@ -477,6 +625,8 @@ try { $resolvedReport | Add-Member -NotePropertyName 'skipped-sub-skills' -NotePropertyValue @( $resolved.skipped | ForEach-Object { @{ skill = @{ id = $_.id; version = $_.version }; reason = $_.reason } } ) + $resolved | Add-Member -NotePropertyName 'acceptedResults' -NotePropertyValue @(Save-AcceptedLeafReports $resolvedReport.'sub-results') + Set-Content -LiteralPath $compositionPath -Value ($resolved | ConvertTo-Json -Depth 30) -Encoding utf8NoBOM Assert-CompositionReport $resolvedReport } Set-Content -LiteralPath $compositionPath -Value ($expectedComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM @@ -643,6 +793,9 @@ try { Set-Content -LiteralPath $reportPath -Value ($partialSuperReport | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM $acceptedPartialSuper = & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super Assert-True (-not $acceptedPartialSuper.normalized) 'partial super-skill excludes failed coverage from its rollup' + $partialComposition = $expectedComposition | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $partialComposition.acceptedResults = @(Save-AcceptedLeafReports @($completedLeaf, $failedLeaf)) + Set-Content -LiteralPath $compositionPath -Value ($partialComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM Assert-CompositionReport $partialSuperReport $failedLeafLeakage = $partialSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json diff --git a/tools/Validate-FindingsReport.ps1 b/tools/Validate-FindingsReport.ps1 index cf42c5b7..d5e5dcea 100644 --- a/tools/Validate-FindingsReport.ps1 +++ b/tools/Validate-FindingsReport.ps1 @@ -38,6 +38,7 @@ catch { $expectedComposition = $null $expectedLeaves = [Collections.Generic.Dictionary[string, object]]::new([StringComparer]::Ordinal) $expectedSkips = [Collections.Generic.Dictionary[string, object]]::new([StringComparer]::Ordinal) +$acceptedLeaves = [Collections.Generic.Dictionary[string, object]]::new([StringComparer]::Ordinal) if ($ExpectedCompositionPath) { if ($SkillKind -cne 'super') { throw 'Expected composition is supported only for super-skill reports.' @@ -55,10 +56,22 @@ if ($ExpectedCompositionPath) { } $compositionSchema = @{ type = 'object' - required = @('superSkill', 'subSkills', 'skipped') + required = @('superSkill', 'subSkills', 'skipped', 'acceptedResults') properties = @{ superSkill = $identitySchema subSkills = @{ type = 'array'; items = $identitySchema } + acceptedResults = @{ + type = 'array' + items = @{ + type = 'object' + required = @('id', 'version', 'reportPath') + properties = @{ + id = $identitySchema.properties.id + version = $identitySchema.properties.version + reportPath = @{ type = 'string'; minLength = 1 } + } + } + } skipped = @{ type = 'array' items = @{ type = 'object'; required = @('id', 'version', 'reason'); properties = $skipProperties } @@ -84,6 +97,31 @@ if ($ExpectedCompositionPath) { } $expectedSkips.Add([string]$skip.id, $skip) } + $compositionDirectory = Split-Path -Parent (Resolve-Path -LiteralPath $ExpectedCompositionPath).Path + foreach ($accepted in @($expectedComposition.acceptedResults)) { + if (-not $expectedLeaves.ContainsKey([string]$accepted.id) -or + $accepted.version -ne $expectedLeaves[$accepted.id].version -or + $acceptedLeaves.ContainsKey([string]$accepted.id)) { + throw "Accepted result '$($accepted.id)' must uniquely match a selected leaf and version." + } + $acceptedPath = if ([IO.Path]::IsPathRooted($accepted.reportPath)) { + $accepted.reportPath + } + else { + Join-Path $compositionDirectory $accepted.reportPath + } + $acceptedRaw = Get-Content -LiteralPath $acceptedPath -Raw + if (-not ($acceptedRaw | Test-Json -SchemaFile $schemaPath -ErrorAction Stop)) { + throw "Accepted result '$($accepted.id)' does not satisfy the report schema." + } + $acceptedReport = $acceptedRaw | ConvertFrom-Json -Depth 100 + if ($acceptedReport.skill.id -cne $accepted.id -or $acceptedReport.skill.version -ne $accepted.version -or + $acceptedReport.PSObject.Properties.Name -ccontains 'sub-results' -or + $acceptedReport.PSObject.Properties.Name -ccontains 'skipped-sub-skills') { + throw "Accepted result '$($accepted.id)' must be a leaf report with the captured identity." + } + $acceptedLeaves.Add([string]$accepted.id, $acceptedReport) + } } catch { throw "Invalid expected composition: $($_.Exception.Message)" @@ -105,6 +143,39 @@ function Test-HasProperty { return $null -ne $Object -and $Object.PSObject.Properties.Name -ccontains $Name } +function Test-JsonContentEqual { + param([object] $First, [object] $Second) + + if ($null -eq $First -or $null -eq $Second) { + return $null -eq $First -and $null -eq $Second + } + if ($First -is [pscustomobject] -or $Second -is [pscustomobject]) { + if ($First -isnot [pscustomobject] -or $Second -isnot [pscustomobject] -or + @($First.PSObject.Properties).Count -ne @($Second.PSObject.Properties).Count) { + return $false + } + foreach ($property in $First.PSObject.Properties) { + if (-not (Test-HasProperty $Second $property.Name) -or + -not (Test-JsonContentEqual $property.Value $Second.PSObject.Properties[$property.Name].Value)) { + return $false + } + } + return $true + } + if ($First -is [array] -or $Second -is [array]) { + if ($First -isnot [array] -or $Second -isnot [array] -or $First.Count -ne $Second.Count) { + return $false + } + for ($index = 0; $index -lt $First.Count; $index++) { + if (-not (Test-JsonContentEqual $First[$index] $Second[$index])) { + return $false + } + } + return $true + } + return $First.GetType() -eq $Second.GetType() -and $First -ceq $Second +} + function Get-SourceLineCount { param([string] $Path) @@ -441,6 +512,14 @@ function Get-SemanticErrors { Add-Error 'SUPER_LEAF_VERSION_MISMATCH' "$ReportPathPrefix.sub-results[$index].skill.version" ` "Unexpected version for '$($identity.id)'." } + if (-not $acceptedLeaves.ContainsKey([string]$identity.id)) { + Add-Error 'SUPER_LEAF_NOT_ACCEPTED' "$ReportPathPrefix.sub-results[$index]" ` + "Leaf '$($identity.id)' has no host-captured accepted result." + } + elseif (-not (Test-JsonContentEqual $subResults[$index] $acceptedLeaves[$identity.id])) { + Add-Error 'SUPER_LEAF_CONTENT_MISMATCH' "$ReportPathPrefix.sub-results[$index]" ` + "Leaf '$($identity.id)' differs from its host-captured accepted result." + } $slot = [Array]::IndexOf($orderedIds, $identity.id) if ($slot -le $previousSlot) { Add-Error 'SUPER_SUB_RESULT_ORDER' "$ReportPathPrefix.sub-results[$index].skill" ` @@ -451,6 +530,16 @@ function Get-SemanticErrors { foreach ($leaf in @($expectedComposition.subSkills)) { if (-not $producerIds.Contains([string]$leaf.id)) { $missingResults++ + if ($acceptedLeaves.ContainsKey([string]$leaf.id)) { + Add-Error 'SUPER_ACCEPTED_LEAF_MISSING' "$ReportPathPrefix.sub-results" ` + "Host-captured accepted leaf '$($leaf.id)' must be included." + } + $reason = if (Test-HasProperty $Current 'outcome-reason') { $Current.'outcome-reason' } else { '' } + $idPattern = '(? Date: Fri, 2 Oct 2026 10:21:05 +0200 Subject: [PATCH 3/3] Preserve literal strings in composed report acceptance --- docs/contributing.md | 3 +- docs/standalone-runner.md | 3 ++ tools/Test-ReviewContract.ps1 | 53 +++++++++++++++++++++++++++++++ tools/Validate-FindingsReport.ps1 | 24 ++++++++------ 4 files changed, 73 insertions(+), 10 deletions(-) diff --git a/docs/contributing.md b/docs/contributing.md index 51f6d0fc..e0a51c34 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -154,7 +154,8 @@ not a deployable or compiled application. ## Before opening a PR From your BCQuality checkout, use the existing validators. The Python -validator needs Python and PyYAML; the fixture harness needs PowerShell 7. +validator needs Python and PyYAML; the fixture harness needs PowerShell 7.5 or later +so findings-report parsing preserves timestamp-shaped JSON strings verbatim. If PyYAML is not installed in your development environment, install it with `python -m pip install pyyaml`. diff --git a/docs/standalone-runner.md b/docs/standalone-runner.md index 74b2f8ed..8980b034 100644 --- a/docs/standalone-runner.md +++ b/docs/standalone-runner.md @@ -78,6 +78,9 @@ its own task-level applicability and reports `not-applicable` or ## Composition acceptance +The findings-report validator requires PowerShell 7.5 or later to preserve +literal JSON strings with `ConvertFrom-Json -DateKind String`. + A report can be internally consistent while omitting a selected review. Bind the final acceptance gate to the host's selection, not just the returned reports. The private JSON input to `-ExpectedCompositionPath` follows the diff --git a/tools/Test-ReviewContract.ps1 b/tools/Test-ReviewContract.ps1 index a567e4ce..db1c1b19 100644 --- a/tools/Test-ReviewContract.ps1 +++ b/tools/Test-ReviewContract.ps1 @@ -451,6 +451,59 @@ try { $removedFindingReport.findings = @() $removedFindingReport.summary.counts.minor = 0 Assert-CompositionReport $removedFindingReport '*SUPER_LEAF_CONTENT_MISMATCH*' + + $capturedCorrectionLeaf = $capturedFindingLeaf | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $capturedCorrectionLeaf.findings[0] | Add-Member -NotePropertyName 'suggested-code' -NotePropertyValue 'exit(1);' + $correctionComposition = $expectedComposition | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $correctionComposition.acceptedResults = @(Save-AcceptedLeafReports @($capturedCorrectionLeaf, $completedSecurityLeaf)) + Set-Content -LiteralPath $compositionPath -Value ($correctionComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + $capturedCorrectionReport = $capturedFindingReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $capturedCorrectionReport.'sub-results'[0] = $capturedCorrectionLeaf + $capturedCorrectionReport.findings[0] | Add-Member -NotePropertyName 'suggested-code' -NotePropertyValue 'exit(1);' + Assert-CompositionReport $capturedCorrectionReport + $correctionCapturePath = $correctionComposition.acceptedResults[0].reportPath + $immutableCorrectionCapture = [IO.File]::ReadAllText($correctionCapturePath) + foreach ($codePoint in @(0x0000, 0x00AD, 0x200B, 0xFEFF)) { + $alteredCorrectionReport = $capturedCorrectionReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $alteredCorrection = 'ex' + [char]$codePoint + 'it(1);' + $alteredCorrectionReport.'sub-results'[0].findings[0].'suggested-code' = $alteredCorrection + $alteredCorrectionReport.findings[0].'suggested-code' = $alteredCorrection + Assert-CompositionReport $alteredCorrectionReport '*SUPER_LEAF_CONTENT_MISMATCH*' + $rolledOnlyCorrectionReport = $capturedCorrectionReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $rolledOnlyCorrectionReport.findings[0].'suggested-code' = $alteredCorrection + Assert-CompositionReport $rolledOnlyCorrectionReport '*SUPER_FINDING_MISMATCH*' + Assert-True ([string]::Equals([IO.File]::ReadAllText($correctionCapturePath), $immutableCorrectionCapture, [StringComparison]::Ordinal)) ` + 'rejecting altered corrections leaves the immutable host capture unchanged' + } + $timestampReason = '2026-10-02T09:00:00Z' + $timestampLeaf = $completedLeaf | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $timestampLeaf | Add-Member -NotePropertyName 'outcome-reason' -NotePropertyValue $timestampReason + $timestampComposition = $expectedComposition | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $timestampComposition.acceptedResults = @(Save-AcceptedLeafReports @($timestampLeaf, $completedSecurityLeaf)) + Set-Content -LiteralPath $compositionPath -Value ($timestampComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + $timestampReport = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $timestampReport.'sub-results'[0] = $timestampLeaf + foreach ($alteredTimestamp in @('2026-10-02T09:00:00.000Z', '2026-10-02T09:00:00+00:00')) { + $timestampLeaf.'outcome-reason' = $alteredTimestamp + Assert-CompositionReport $timestampReport '*SUPER_LEAF_CONTENT_MISMATCH*' + } + $timestampLeaf.'outcome-reason' = $timestampReason + Set-Content -LiteralPath $reportPath -Value ($timestampReport | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + $acceptedTimestampReport = & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super ` + -ExpectedCompositionPath $compositionPath + $acceptedReason = $acceptedTimestampReport.report.'sub-results'[0].'outcome-reason' + Assert-True ($acceptedReason -is [string] -and [string]::Equals($acceptedReason, $timestampReason, [StringComparison]::Ordinal)) ` + 'accepted timestamp-shaped JSON text remains the original literal string' + $normalizedTimestampReport = $validReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $normalizedTimestampReport | Add-Member -NotePropertyName 'outcome-reason' -NotePropertyValue '2026-10-02T09:00:00.000Z' + $normalizedTimestampReport.findings[0].location.range.'start-line' = 1 + Set-Content -LiteralPath $reportPath -Value ($normalizedTimestampReport | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + $acceptedNormalizedTimestamp = & $validator -ReportPath $reportPath -BCQualityRoot $Root -SourceRoot $tmp ` + -SourcePaths $sourcePath -RetrievedArticlePaths $articlePath -AllowBoundedNormalization + Assert-True $acceptedNormalizedTimestamp.normalized 'bounded normalization still applies to an eligible range' + Assert-True ($acceptedNormalizedTimestamp.report.'outcome-reason' -is [string] -and + [string]::Equals($acceptedNormalizedTimestamp.report.'outcome-reason', $normalizedTimestampReport.'outcome-reason', [StringComparison]::Ordinal)) ` + 'bounded normalization preserves unrelated timestamp-shaped text exactly' Set-Content -LiteralPath $compositionPath -Value ($expectedComposition | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM foreach ($case in @( diff --git a/tools/Validate-FindingsReport.ps1 b/tools/Validate-FindingsReport.ps1 index d5e5dcea..b3c41b39 100644 --- a/tools/Validate-FindingsReport.ps1 +++ b/tools/Validate-FindingsReport.ps1 @@ -1,3 +1,4 @@ +#Requires -Version 7.5 <# .SYNOPSIS Validates a BCQuality findings-report against its structural and semantic contract. @@ -29,7 +30,7 @@ try { if (-not ($raw | Test-Json -SchemaFile $schemaPath -ErrorAction Stop)) { throw 'Report does not satisfy schemas/findings-report.schema.json.' } - $report = $raw | ConvertFrom-Json -Depth 100 + $report = $raw | ConvertFrom-Json -Depth 100 -DateKind String } catch { throw "Invalid findings-report JSON or schema: $($_.Exception.Message)" @@ -83,7 +84,7 @@ if ($ExpectedCompositionPath) { if (-not ($compositionRaw | Test-Json -Schema $compositionSchema -ErrorAction Stop)) { throw 'Expected composition does not satisfy its input contract.' } - $expectedComposition = $compositionRaw | ConvertFrom-Json -Depth 100 + $expectedComposition = $compositionRaw | ConvertFrom-Json -Depth 100 -DateKind String $expectedIds = [Collections.Generic.HashSet[string]]::new([StringComparer]::Ordinal) foreach ($leaf in @($expectedComposition.subSkills)) { if (-not $expectedIds.Add([string]$leaf.id)) { @@ -114,7 +115,7 @@ if ($ExpectedCompositionPath) { if (-not ($acceptedRaw | Test-Json -SchemaFile $schemaPath -ErrorAction Stop)) { throw "Accepted result '$($accepted.id)' does not satisfy the report schema." } - $acceptedReport = $acceptedRaw | ConvertFrom-Json -Depth 100 + $acceptedReport = $acceptedRaw | ConvertFrom-Json -Depth 100 -DateKind String if ($acceptedReport.skill.id -cne $accepted.id -or $acceptedReport.skill.version -ne $accepted.version -or $acceptedReport.PSObject.Properties.Name -ccontains 'sub-results' -or $acceptedReport.PSObject.Properties.Name -ccontains 'skipped-sub-skills') { @@ -173,6 +174,10 @@ function Test-JsonContentEqual { } return $true } + if ($First -is [string] -or $Second -is [string]) { + return $First -is [string] -and $Second -is [string] -and + [string]::Equals($First, $Second, [StringComparison]::Ordinal) + } return $First.GetType() -eq $Second.GetType() -and $First -ceq $Second } @@ -271,9 +276,10 @@ function Get-SemanticErrors { $firstHasCode = Test-HasProperty $First 'suggested-code' $secondHasCode = Test-HasProperty $Second 'suggested-code' if ($firstHasCode -or $secondHasCode) { - return $firstHasCode -and $secondHasCode -and $First.'suggested-code' -ceq $Second.'suggested-code' + return $firstHasCode -and $secondHasCode -and + [string]::Equals($First.'suggested-code', $Second.'suggested-code', [StringComparison]::Ordinal) } - return $First.message -ceq $Second.message + return [string]::Equals($First.message, $Second.message, [StringComparison]::Ordinal) } function Test-ReferencesInclude { @@ -314,7 +320,7 @@ function Get-SemanticErrors { $RolledFinding.id -cne $expectedId -or $RolledFinding.severity -cne $LeafFinding.severity -or $RolledFinding.confidence -cne $LeafFinding.confidence -or - $RolledFinding.message -cne $LeafFinding.message -or + -not [string]::Equals($RolledFinding.message, $LeafFinding.message, [StringComparison]::Ordinal) -or $rolledReferences.Count -ne $leafReferences.Count -or -not (Test-ReferencesInclude $rolledReferences $leafReferences)) { return $false @@ -323,7 +329,7 @@ function Get-SemanticErrors { $rolledHasProperty = Test-HasProperty $RolledFinding $name $leafHasProperty = Test-HasProperty $LeafFinding $name if ($rolledHasProperty -ne $leafHasProperty -or - ($rolledHasProperty -and $RolledFinding.$name -cne $LeafFinding.$name)) { + ($rolledHasProperty -and -not [string]::Equals($RolledFinding.$name, $LeafFinding.$name, [StringComparison]::Ordinal))) { return $false } } @@ -339,7 +345,7 @@ function Get-SemanticErrors { $sameCorrection = Test-SameCorrection $RolledFinding $LeafFinding $correctionsConflict = (Test-HasProperty $RolledFinding 'suggested-code') -and (Test-HasProperty $LeafFinding 'suggested-code') -and - $RolledFinding.'suggested-code' -cne $LeafFinding.'suggested-code' + -not [string]::Equals($RolledFinding.'suggested-code', $LeafFinding.'suggested-code', [StringComparison]::Ordinal) $explicitCrossRuleMerge = $leafReferences.Count -and $rolledReferences.Count -gt $leafReferences.Count -and -not $correctionsConflict -and @@ -718,7 +724,7 @@ if ($errors.Count -and $AllowBoundedNormalization) { $otherErrors = @($errors | Where-Object Code -CNE 'RANGE_START_MISMATCH') $rangeErrors = @($errors | Where-Object Code -CEQ 'RANGE_START_MISMATCH') if (-not $otherErrors.Count -and $rangeErrors.Count) { - $candidate = $report | ConvertTo-Json -Depth 100 | ConvertFrom-Json -Depth 100 + $candidate = $report | ConvertTo-Json -Depth 100 | ConvertFrom-Json -Depth 100 -DateKind String $eligible = $true foreach ($finding in @($candidate.findings)) { if (-not (Test-HasProperty $finding 'location') -or