Skip to content

Commit a322632

Browse files
kubafloCopilot
andcommitted
Publish the try-fix panel and the independent review in the PR body
The fix phase runs five cross-pollinated try-fix candidates and publishes one, but only the winner ever reached the pull request. A reader could not tell a fix selected from five competing approaches from the single candidate that happened to run, and the body read identically either way. That is not hypothetical: before the tidiness fix, four of five candidates were routinely blocked for changing no file at all. The body now carries a Try-fix panel table - every candidate, its model, its result, and its approach or the reason it was rejected, with the published one marked - alongside the independent review of the winning diff. Both report and never refuse: an absent record renders "Not measured" rather than nothing, because silence is indistinguishable from a feature nobody wired up, which is how the regression cross-reference stayed dead for its entire life. Three defects found and fixed while building it: - Reading candidate properties by dot threw under StrictMode when a record omitted one, letting a display detail abort a fix phase that had already produced a winning diff. Now read through PSObject.Properties. The existing suite caught this, which is what it is for. - A pipe in model-written prose ended its table cell and silently shifted every later column, so a row could misattribute a result to the wrong candidate. Pipes are escaped, not stripped: the character is load-bearing in the C# candidates describe. - Nothing tested that the candidate gate allowlists the manifest fields the orchestrator writes. This pipeline has already lost finished runs to exactly that. The two are now pinned together, so the next new field fails in milliseconds on a laptop instead of after the reproduction, the fix, and every arm have been paid for. Every panel string is bounded with -Prose: a presentation bound must never be able to discard the work it describes, which has destroyed four completed runs here already. 17 new tests, each mutation-tested (8 mutations, all killed, including a guard-the-guard that proves the allowlist cross-check is not vacuous). 60 suites: 4159 passed, 35 failed - the same pre-existing review-half failures, unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 735ac9a2-7bec-4baa-ad19-c298e5bc795a
1 parent eaed006 commit a322632

5 files changed

Lines changed: 761 additions & 2 deletions

File tree

.github/scripts/Publish-Replication.Tests.ps1

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ BeforeAll {
4545
'Get-ReplicationPullRequestMarker',
4646
'New-ReplicationBranchName',
4747
'Get-ReplicationCandidateText',
48+
'Get-ReplicationIndependentReviewBlock',
49+
'Get-ReplicationFixPanelBlock',
4850
'Get-ValidatedFixFiles',
4951
'Assert-ReplicationStagedFix',
5052
'Remove-ReplicationPlatformTitlePrefix',
@@ -2169,3 +2171,188 @@ Describe 'The pre-flight upstream duplicate gate refuses only what it measured'
21692171
} finally { Remove-Item -Recurse -Force $root -ErrorAction SilentlyContinue }
21702172
}
21712173
}
2174+
2175+
Describe 'The pull request body reports the independent review of the winning fix' {
2176+
BeforeAll {
2177+
$script:ReviewCandidate = [pscustomobject]@{
2178+
fixIndependentReview = [pscustomobject]@{
2179+
model = 'gpt-5.6-sol'
2180+
summary = 'The diff restores the null guard and the test covers it.'
2181+
findings = @(
2182+
[pscustomobject]@{ severity = 'blocking'; detail = 'The guard is not applied on the Android path.' }
2183+
[pscustomobject]@{ severity = 'minor'; detail = 'The new field could be readonly.' }
2184+
)
2185+
}
2186+
}
2187+
}
2188+
2189+
It 'attributes the review to a model that did not write the diff' {
2190+
$block = Get-ReplicationIndependentReviewBlock -Candidate $script:ReviewCandidate
2191+
2192+
$block | Should -Match 'Independent review'
2193+
$block | Should -Match 'gpt-5\.6-sol'
2194+
$block | Should -Match 'without having written it'
2195+
$block | Should -Match 'restores the null guard'
2196+
}
2197+
2198+
It 'surfaces each finding with its severity' {
2199+
$block = Get-ReplicationIndependentReviewBlock -Candidate $script:ReviewCandidate
2200+
2201+
$block | Should -Match '\*\*blocking\.\*\* The guard is not applied on the Android path\.'
2202+
$block | Should -Match '\*\*minor\.\*\* The new field could be readonly\.'
2203+
}
2204+
2205+
It 'says plainly that findings did not block publication' {
2206+
# The arm reports, never refuses: its false-positive rate is unmeasured
2207+
# because every reviewed pull request in the validating corpus carried a
2208+
# blocking finding, so there is no negative control. A wrong paragraph
2209+
# costs a reader a minute; a wrong refusal costs a certified fix.
2210+
Get-ReplicationIndependentReviewBlock -Candidate $script:ReviewCandidate |
2211+
Should -Match 'did not block publication'
2212+
}
2213+
2214+
It 'caps the findings it renders so one verbose review cannot flood the body' {
2215+
$many = 1..12 | ForEach-Object { [pscustomobject]@{ severity = 'minor'; detail = "finding number $_" } }
2216+
$candidate = [pscustomobject]@{
2217+
fixIndependentReview = [pscustomobject]@{ model = 'gpt-5.6-sol'; summary = 'ok'; findings = $many }
2218+
}
2219+
2220+
$block = Get-ReplicationIndependentReviewBlock -Candidate $candidate
2221+
2222+
@([regex]::Matches($block, '(?m)^- \*\*minor\.\*\*')).Count | Should -Be 6
2223+
}
2224+
2225+
It 'reports "Not measured" when the review is absent or unusable' {
2226+
# Silence is indistinguishable from a feature nobody wired up, which is
2227+
# exactly how the regression cross-reference stayed dead for its whole
2228+
# life while passing every behavioural test it had.
2229+
Get-ReplicationIndependentReviewBlock -Candidate ([pscustomobject]@{ fixFiles = @('a.cs') }) |
2230+
Should -Match 'Not measured'
2231+
Get-ReplicationIndependentReviewBlock -Candidate ([pscustomobject]@{ fixIndependentReview = $null }) |
2232+
Should -Match 'Not measured'
2233+
Get-ReplicationIndependentReviewBlock -Candidate ([pscustomobject]@{
2234+
fixIndependentReview = [pscustomobject]@{ model = 'gpt-5.6-sol'; summary = ' '; findings = @() } }) |
2235+
Should -Match 'Not measured'
2236+
}
2237+
}
2238+
2239+
Describe 'The pull request body records the try-fix panel, not only its winner' {
2240+
BeforeAll {
2241+
$script:PanelCandidate = [pscustomobject]@{
2242+
fixPanel = @(
2243+
[pscustomobject]@{ attempt = 1; model = 'claude-opus-5'; result = 'Blocked'; detail = 'reported a pass without changing any file'; won = $false }
2244+
[pscustomobject]@{ attempt = 2; model = 'gpt-5.6-sol'; result = 'Fail'; detail = 'oracle failed 1 of 3 runs'; won = $false }
2245+
[pscustomobject]@{ attempt = 3; model = 'claude-opus-5'; result = 'Pass'; detail = 'Guard the native flow-direction mapper'; won = $true }
2246+
)
2247+
}
2248+
}
2249+
2250+
It 'names every candidate that competed, with its model and result' {
2251+
$block = Get-ReplicationFixPanelBlock -Candidate $script:PanelCandidate
2252+
2253+
# The whole point of the disclosure: a reader can tell a fix selected
2254+
# from competing approaches from the one candidate that happened to run.
2255+
$block | Should -Match 'gpt-5\.6-sol'
2256+
$block | Should -Match 'claude-opus-5'
2257+
$block | Should -Match 'Blocked'
2258+
$block | Should -Match 'oracle failed 1 of 3 runs'
2259+
}
2260+
2261+
It 'marks exactly the candidate whose diff was published' {
2262+
$block = Get-ReplicationFixPanelBlock -Candidate $script:PanelCandidate
2263+
2264+
@([regex]::Matches($block, '\(selected\)')).Count | Should -Be 1
2265+
$block | Should -Match 'Pass \*\*\(selected\)\*\*'
2266+
}
2267+
2268+
It 'escapes a pipe in candidate prose so the table cannot silently shift its columns' {
2269+
$candidate = [pscustomobject]@{
2270+
fixPanel = @(
2271+
[pscustomobject]@{ attempt = 1; model = 'claude-opus-5'; result = 'Pass'; detail = 'restore the a|b fallback'; won = $true }
2272+
)
2273+
}
2274+
2275+
$block = Get-ReplicationFixPanelBlock -Candidate $candidate
2276+
2277+
# An unescaped pipe ends the cell, so every later column reports the
2278+
# wrong candidate's value. A row that misattributes a result is worse
2279+
# than no row at all.
2280+
$block | Should -Match 'a\\\|b'
2281+
$row = @($block -split "`n" | Where-Object { $_ -match 'claude-opus-5' })[0]
2282+
@($row -split '(?<!\\)\|').Count | Should -Be 6
2283+
}
2284+
2285+
It 'says the panel was not measured rather than rendering nothing' {
2286+
Get-ReplicationFixPanelBlock -Candidate ([pscustomobject]@{ fixFiles = @('a.cs') }) |
2287+
Should -Match 'Not measured'
2288+
Get-ReplicationFixPanelBlock -Candidate ([pscustomobject]@{ fixPanel = @() }) |
2289+
Should -Match 'Not measured'
2290+
}
2291+
2292+
It 'is actually invoked by the body builder, not merely defined' {
2293+
# A test that exercises a new function in isolation says nothing about
2294+
# the call site, and the call site is where this class of defect lives:
2295+
# the regression cross-reference passed every behavioural test it had
2296+
# while returning nothing in production for its entire life.
2297+
$errors = $null
2298+
$ast = [System.Management.Automation.Language.Parser]::ParseFile(
2299+
(Join-Path $PSScriptRoot 'shared/Publish-ReplicationPR.ps1'), [ref]$null, [ref]$errors)
2300+
$errors | Should -BeNullOrEmpty
2301+
2302+
$body = $ast.Find({
2303+
$args[0] -is [System.Management.Automation.Language.FunctionDefinitionAst] -and
2304+
$args[0].Name -eq 'New-ReplicationPullRequestBody'
2305+
}, $true)
2306+
$body | Should -Not -BeNullOrEmpty
2307+
2308+
foreach ($name in @('Get-ReplicationFixPanelBlock', 'Get-ReplicationIndependentReviewBlock')) {
2309+
$calls = @($body.FindAll({
2310+
$args[0] -is [System.Management.Automation.Language.CommandAst] -and
2311+
$args[0].GetCommandName() -eq $name
2312+
}, $true))
2313+
$calls.Count | Should -BeGreaterThan 0 -Because "$name must be called by the body builder"
2314+
}
2315+
}
2316+
}
2317+
2318+
Describe 'The candidate gate allowlists every manifest field the orchestrator writes' {
2319+
It 'accepts each fix* key the orchestrator emits, so a new field cannot destroy a completed run' {
2320+
# This pipeline has already lost finished runs to exactly this: the
2321+
# orchestrator gains a manifest field, the gate does not know the name,
2322+
# and the run is refused after the reproduction, the fix, and every arm
2323+
# have already been paid for. Pinning the two together makes the next
2324+
# field fail here - in milliseconds, on a laptop - instead of there.
2325+
$validator = Join-Path $PSScriptRoot 'shared/Validate-ReplicationCandidate.ps1'
2326+
$errors = $null
2327+
$ast = [System.Management.Automation.Language.Parser]::ParseFile($validator, [ref]$null, [ref]$errors)
2328+
$errors | Should -BeNullOrEmpty
2329+
2330+
$function = $ast.Find({
2331+
$args[0] -is [System.Management.Automation.Language.FunctionDefinitionAst] -and
2332+
$args[0].Name -eq 'Read-ReplicationManifest'
2333+
}, $true)
2334+
$function | Should -Not -BeNullOrEmpty
2335+
2336+
$assignment = $function.Find({
2337+
$args[0] -is [System.Management.Automation.Language.AssignmentStatementAst] -and
2338+
$args[0].Left.Extent.Text -eq '$allowedProperties'
2339+
}, $true)
2340+
$assignment | Should -Not -BeNullOrEmpty
2341+
2342+
$allowed = & ([scriptblock]::Create($assignment.Right.Extent.Text))
2343+
$allowed.Count | Should -BeGreaterThan 20
2344+
2345+
$orchestrator = Join-Path $PSScriptRoot 'Replicate-Issue.ps1'
2346+
$written = @([regex]::Matches(
2347+
(Get-Content -LiteralPath $orchestrator -Raw), '(?m)^\s{12}(fix[A-Za-z]+)\s*=') |
2348+
ForEach-Object { $_.Groups[1].Value } |
2349+
Sort-Object -Unique)
2350+
2351+
# Guards the guard: if the extraction stops matching, the comparison
2352+
# below passes vacuously and this test protects nothing.
2353+
$written.Count | Should -BeGreaterThan 4
2354+
$written | Should -Contain 'fixPanel'
2355+
2356+
@($written | Where-Object { $allowed -cnotcontains $_ }) | Should -BeNullOrEmpty
2357+
}
2358+
}

.github/scripts/Replicate-Issue.Tests.ps1

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,9 @@ BeforeAll {
167167
'Test-ReplicationScopeMatchesHead',
168168
'Read-ReplicationFixScope',
169169
'Read-ReplicationFixWinner',
170+
'Read-ReplicationFixReview',
171+
'Invoke-ReplicationFixReview',
172+
'Get-ReplicationFixPanelRecord',
170173
'Get-ReplicationFixArmEvidence',
171174
'Invoke-ReplicationFixArms',
172175
'Write-ReplicationFixArmResults',
@@ -8810,6 +8813,7 @@ Describe 'A fix phase is told a different truth than a reproduction phase' {
88108813
$script:agentDir = '/tmp/artifacts/agent'
88118814
$script:fixScopePath = '/tmp/artifacts/agent/fix-scope.json'
88128815
$script:fixWinnerPath = '/tmp/artifacts/agent/fix-winner.json'
8816+
$script:fixReviewPath = '/tmp/artifacts/agent/fix-review.json'
88138817
$script:fixOracleRunnerPath = '/tmp/artifacts/fix/run-oracle.ps1'
88148818
$script:appiumPlanPath = '/tmp/repo/appium-plan.json'
88158819
$script:sandboxProposalPath = '/tmp/artifacts/agent/sandbox-proposal.json'
@@ -10908,6 +10912,7 @@ Describe 'Every way the fix phase can fail still ships the reproduction' {
1090810912

1090910913
$script:fixScopePath = Join-Path $script:agentDir 'fix-scope.json'
1091010914
$script:fixWinnerPath = Join-Path $script:agentDir 'winner.json'
10915+
$script:fixReviewPath = Join-Path $script:agentDir 'review.json'
1091110916
$script:fixPatchPath = Join-Path $script:repoRoot 'fix.patch'
1091210917
$script:fixOracleRunnerPath = Join-Path $script:repoRoot 'run-oracle.ps1'
1091310918

@@ -11293,6 +11298,7 @@ Describe 'A fix phase may only ask to write files that can be granted' {
1129311298
$script:IssueNumber = 12345
1129411299
$script:fixScopePath = Join-Path $script:agentDir 'fix-scope.json'
1129511300
$script:fixWinnerPath = Join-Path $script:agentDir 'fix-winner.json'
11301+
$script:fixReviewPath = Join-Path $script:agentDir 'fix-review.json'
1129611302
$script:fixPatchPath = Join-Path $script:repoRoot 'fix.patch'
1129711303
$script:fixOracleRunnerPath = Join-Path $script:repoRoot 'run-oracle.ps1'
1129811304
$script:granted = [Collections.Generic.List[object]]::new()
@@ -14350,3 +14356,146 @@ Describe 'A name that exists is a missing using, not a wrong name' {
1435014356
}
1435114357
}
1435214358
}
14359+
14360+
Describe 'The independent review cannot cost the fix it describes' {
14361+
BeforeAll {
14362+
$script:ReviewSource = Get-Content -LiteralPath (Join-Path $PSScriptRoot 'Replicate-Issue.ps1') -Raw
14363+
$script:ReviewAst = [System.Management.Automation.Language.Parser]::ParseInput(
14364+
$script:ReviewSource, [ref]$null, [ref]$null)
14365+
$script:FixPhaseFn = $script:ReviewAst.Find({
14366+
param($n)
14367+
$n -is [System.Management.Automation.Language.FunctionDefinitionAst] -and
14368+
$n.Name -eq 'Invoke-ReplicationFixPhase'
14369+
}, $true)
14370+
}
14371+
14372+
It 'writes the four-arm results before it asks for a review' {
14373+
# A step timeout KILLS the process, so no try/catch inside the review
14374+
# can contain it - and 23% of runs that reach the fix panel time out
14375+
# inside it. Were the review to run first, a timeout during the model
14376+
# call would destroy the fix-control and restoration evidence of a fix
14377+
# that had already passed every arm.
14378+
#
14379+
# This arm publishes a paragraph that nothing acts on. It must never be
14380+
# able to discard the work it describes.
14381+
$script:FixPhaseFn | Should -Not -BeNullOrEmpty
14382+
14383+
$calls = $script:FixPhaseFn.FindAll({
14384+
param($n) $n -is [System.Management.Automation.Language.CommandAst]
14385+
}, $true) | Where-Object {
14386+
$_.GetCommandName() -in @('Write-ReplicationFixArmResults', 'Invoke-ReplicationFixReview')
14387+
}
14388+
14389+
$writeAt = @($calls | Where-Object { $_.GetCommandName() -eq 'Write-ReplicationFixArmResults' } |
14390+
ForEach-Object { $_.Extent.StartOffset })
14391+
$reviewAt = @($calls | Where-Object { $_.GetCommandName() -eq 'Invoke-ReplicationFixReview' } |
14392+
ForEach-Object { $_.Extent.StartOffset })
14393+
14394+
$writeAt | Should -Not -BeNullOrEmpty
14395+
$reviewAt | Should -Not -BeNullOrEmpty
14396+
($writeAt | Measure-Object -Maximum).Maximum |
14397+
Should -BeLessThan ($reviewAt | Measure-Object -Minimum).Minimum
14398+
}
14399+
14400+
It 'contains every failure inside the review rather than letting it reach the fix phase' {
14401+
$reviewFn = $script:ReviewAst.Find({
14402+
param($n)
14403+
$n -is [System.Management.Automation.Language.FunctionDefinitionAst] -and
14404+
$n.Name -eq 'Invoke-ReplicationFixReview'
14405+
}, $true)
14406+
$reviewFn | Should -Not -BeNullOrEmpty
14407+
14408+
# Every model call and every property read in this arm must sit inside a
14409+
# try, because StrictMode turns an absent property into a throw and that
14410+
# throw would land in the fix phase rather than here.
14411+
$copilotCalls = $reviewFn.FindAll({
14412+
param($n)
14413+
$n -is [System.Management.Automation.Language.CommandAst] -and
14414+
$n.GetCommandName() -eq 'Invoke-ReplicationCopilot'
14415+
}, $true)
14416+
@($copilotCalls) | Should -Not -BeNullOrEmpty
14417+
14418+
foreach ($call in $copilotCalls) {
14419+
$guarded = $false
14420+
$node = $call
14421+
while ($node) {
14422+
if ($node -is [System.Management.Automation.Language.TryStatementAst]) { $guarded = $true; break }
14423+
$node = $node.Parent
14424+
}
14425+
$guarded | Should -BeTrue -Because 'a throw here would cost a fix that has already passed all four arms'
14426+
}
14427+
}
14428+
}
14429+
14430+
Describe 'The fix panel record reaches the published manifest' {
14431+
It 'records every candidate that competed, not only the one that won' {
14432+
$results = @(
14433+
[pscustomobject]@{ Attempt = 1; Model = 'claude-opus-5'; Result = 'Blocked'; Rejection = 'changed no file'; Approach = '' }
14434+
[pscustomobject]@{ Attempt = 2; Model = 'gpt-5.6-sol'; Result = 'Fail'; Rejection = ''; Approach = 'widen the guard' }
14435+
[pscustomobject]@{ Attempt = 3; Model = 'claude-opus-5'; Result = 'Pass'; Rejection = ''; Approach = 'restore the null check' }
14436+
)
14437+
14438+
$record = @(Get-ReplicationFixPanelRecord -Results $results -WinnerAttempt $results[2])
14439+
14440+
$record.Count | Should -Be 3
14441+
@($record | Where-Object { $_.won }).Count | Should -Be 1
14442+
($record | Where-Object { $_.won }).attempt | Should -Be 3
14443+
($record | Where-Object { $_.attempt -eq 2 }).model | Should -Be 'gpt-5.6-sol'
14444+
}
14445+
14446+
It 'prefers the rejection for a blocked candidate, whose approach is empty' {
14447+
# A blocked candidate is the most informative row in the table - it is
14448+
# the one a reader cannot reconstruct from the published diff - and it
14449+
# is exactly the row whose approach field is empty.
14450+
$results = @(
14451+
[pscustomobject]@{ Attempt = 1; Model = 'claude-opus-5'; Result = 'Blocked'; Rejection = 'changed protected files'; Approach = '' }
14452+
[pscustomobject]@{ Attempt = 2; Model = 'gpt-5.6-sol'; Result = 'Pass'; Rejection = ''; Approach = 'restore the null check' }
14453+
)
14454+
14455+
$record = @(Get-ReplicationFixPanelRecord -Results $results -WinnerAttempt $results[1])
14456+
14457+
($record | Where-Object { $_.attempt -eq 1 }).detail | Should -Be 'changed protected files'
14458+
($record | Where-Object { $_.attempt -eq 2 }).detail | Should -Be 'restore the null check'
14459+
}
14460+
14461+
It 'bounds panel prose without discarding it, because nothing downstream parses it' {
14462+
# Four completed runs in this pipeline have been destroyed by a bound
14463+
# that refused instead of trimming. A presentation field must never be
14464+
# able to do that, so every panel string is converted with -Prose.
14465+
$results = @(
14466+
[pscustomobject]@{ Attempt = 1; Model = 'claude-opus-5'; Result = 'Pass'; Rejection = ''; Approach = ('x' * 5000) }
14467+
)
14468+
14469+
{ Get-ReplicationFixPanelRecord -Results $results -WinnerAttempt $results[0] } | Should -Not -Throw
14470+
14471+
$record = @(Get-ReplicationFixPanelRecord -Results $results -WinnerAttempt $results[0])
14472+
$record[0].detail.Length | Should -BeLessOrEqual 300
14473+
$record[0].detail | Should -Not -BeNullOrEmpty
14474+
}
14475+
14476+
It 'survives a candidate record that carries none of the optional properties' {
14477+
# StrictMode turns a missing property into a thrown error, so reading
14478+
# by dot would let a display detail abort a fix phase that had already
14479+
# produced a winning diff. The panel is the last thing that should be
14480+
# able to destroy a fix, and the existing suite caught exactly this.
14481+
Set-StrictMode -Version Latest
14482+
14483+
$bare = @([pscustomobject]@{ Attempt = 1 })
14484+
14485+
{ Get-ReplicationFixPanelRecord -Results $bare -WinnerAttempt $bare[0] } | Should -Not -Throw
14486+
14487+
$record = @(Get-ReplicationFixPanelRecord -Results $bare -WinnerAttempt $bare[0])
14488+
$record.Count | Should -Be 1
14489+
$record[0].attempt | Should -Be 1
14490+
$record[0].won | Should -BeTrue
14491+
}
14492+
14493+
It 'is written into the candidate manifest under fixPanel' {
14494+
# The renderer is worthless if the field never reaches the manifest the
14495+
# publisher reads. This is the call-site half of the panel disclosure.
14496+
$source = Get-Content -LiteralPath (Join-Path $PSScriptRoot 'Replicate-Issue.ps1') -Raw
14497+
14498+
$source | Should -Match '(?m)^\s{12}fixPanel\s*='
14499+
$source | Should -Match 'Panel = @\(Get-ReplicationFixPanelRecord'
14500+
}
14501+
}

0 commit comments

Comments
 (0)