fix(comfyui): stop -WhatIf from reporting a converged host as empty #885

Merged
coilysiren merged 1 commit from fix/comfyui-whatif-verification into main 2026-08-20 05:20:41 +00:00
Owner

What

just windows-comfyui-converge -WhatIf reported a fully converged kai-tower-3026 as completely empty, planning 56.42 GiB of downloads that were already on disk and byte-for-byte correct.

Get-FileHash returns nothing when -WhatIf binds at the top level of a SupportsShouldProcess script, so Test-Sha256 compared $null against the pin and every model failed verification. Beyond the misleading preview, the preflight gate sizes $requiredFreeBytes from that phantom bundle, so a converged host with under about 56 GiB free could not preview its own state at all.

How

The fix. Test-Sha256 now hashes through System.Security.Cryptography.SHA256 over a FileStream instead of Get-FileHash. A pure read inside a converger should not be suppressible by a preview flag, and going through .NET removes the cmdlet-preference coupling rather than papering over it at the call site with -WhatIf:$false.

Why the suite missed it. This is the interesting half. write_wrapper built a plain wrapper that invoked the converger as & script -WhatIf, and in that shape Get-FileHash works fine. Only a top-level -WhatIf, the shape just actually produces via powershell.exe -File, triggers it. Minimal repro:

shape A: wrapper calls '& inner -WhatIf'   -> hash=[A70580F0...]
shape B: -File inner.ps1 -WhatIf           -> hash=[]
shape C: top-level -WhatIf on the wrapper  -> hash=[]

So the harness was structurally incapable of reproducing production. The wrapper is now an advanced script that takes -WhatIf itself (shape C), and the existing preview test drives it that way.

New case. test_what_if_verifies_an_already_converged_model seeds a verified model and asserts the preview reads it as verified, never planned, with no model download required. The previous preview test only ever ran against an empty tree, where planned is the correct answer, which is the gap that let this through.

Verification

The new test discriminates. Reverting only the .ps1 hunk and rerunning:

FAIL: test_what_if_verifies_an_already_converged_model
AssertionError: 'model: verified test_models/fixture.safetensors' not found in
  'What if: Performing the operation "Retrieve the value for property 'ProviderPath'" ...
   model: planned test_models/fixture.safetensors (21 bytes)'

That is the production symptom exactly, ProviderPath line and all. With the fix restored, 6 of 6 pass.

Live on kai-tower-3026, the preview now tells the truth:

preflight: 685.99 GiB free, no model download required
model: verified diffusion_models/qwen_image_fp8_e4m3fn.safetensors (20430635136 bytes)
model: verified diffusion_models/qwen_image_edit_2511_fp8mixed.safetensors (20533762817 bytes)
model: verified text_encoders/qwen_2.5_vl_7b_fp8_scaled.safetensors (9384670680 bytes)
model: verified vae/qwen_image_vae.safetensors (253806246 bytes)
model: verified loras/qwen_image_union_diffsynth_lora.safetensors (943906720 bytes)
model: verified background_removal/birefnet.safetensors (444473596 bytes)

pre-commit run --all-files passes clean.

Note on the residual RESULT=CHANGED

The preview still ends RESULT=CHANGED on the tower, which is correct rather than a leftover. The launcher write and the scheduled-task steps are ShouldProcess gated, so under preview they report as pending work. Only the model and checkpoint verification was lying, and that is what this changes.

closes #882

🤖 Generated with Claude Code

## What `just windows-comfyui-converge -WhatIf` reported a fully converged `kai-tower-3026` as completely empty, planning 56.42 GiB of downloads that were already on disk and byte-for-byte correct. `Get-FileHash` returns nothing when `-WhatIf` binds at the top level of a `SupportsShouldProcess` script, so `Test-Sha256` compared `$null` against the pin and every model failed verification. Beyond the misleading preview, the preflight gate sizes `$requiredFreeBytes` from that phantom bundle, so a converged host with under about 56 GiB free could not preview its own state at all. ## How **The fix.** `Test-Sha256` now hashes through `System.Security.Cryptography.SHA256` over a `FileStream` instead of `Get-FileHash`. A pure read inside a converger should not be suppressible by a preview flag, and going through .NET removes the cmdlet-preference coupling rather than papering over it at the call site with `-WhatIf:$false`. **Why the suite missed it.** This is the interesting half. `write_wrapper` built a plain wrapper that invoked the converger as `& script -WhatIf`, and in that shape `Get-FileHash` works fine. Only a top-level `-WhatIf`, the shape `just` actually produces via `powershell.exe -File`, triggers it. Minimal repro: ``` shape A: wrapper calls '& inner -WhatIf' -> hash=[A70580F0...] shape B: -File inner.ps1 -WhatIf -> hash=[] shape C: top-level -WhatIf on the wrapper -> hash=[] ``` So the harness was structurally incapable of reproducing production. The wrapper is now an advanced script that takes `-WhatIf` itself (shape C), and the existing preview test drives it that way. **New case.** `test_what_if_verifies_an_already_converged_model` seeds a verified model and asserts the preview reads it as `verified`, never `planned`, with `no model download required`. The previous preview test only ever ran against an empty tree, where `planned` is the correct answer, which is the gap that let this through. ## Verification The new test discriminates. Reverting only the `.ps1` hunk and rerunning: ``` FAIL: test_what_if_verifies_an_already_converged_model AssertionError: 'model: verified test_models/fixture.safetensors' not found in 'What if: Performing the operation "Retrieve the value for property 'ProviderPath'" ... model: planned test_models/fixture.safetensors (21 bytes)' ``` That is the production symptom exactly, `ProviderPath` line and all. With the fix restored, 6 of 6 pass. Live on `kai-tower-3026`, the preview now tells the truth: ``` preflight: 685.99 GiB free, no model download required model: verified diffusion_models/qwen_image_fp8_e4m3fn.safetensors (20430635136 bytes) model: verified diffusion_models/qwen_image_edit_2511_fp8mixed.safetensors (20533762817 bytes) model: verified text_encoders/qwen_2.5_vl_7b_fp8_scaled.safetensors (9384670680 bytes) model: verified vae/qwen_image_vae.safetensors (253806246 bytes) model: verified loras/qwen_image_union_diffsynth_lora.safetensors (943906720 bytes) model: verified background_removal/birefnet.safetensors (444473596 bytes) ``` `pre-commit run --all-files` passes clean. ## Note on the residual RESULT=CHANGED The preview still ends `RESULT=CHANGED` on the tower, which is correct rather than a leftover. The launcher write and the scheduled-task steps are `ShouldProcess` gated, so under preview they report as pending work. Only the model and checkpoint verification was lying, and that is what this changes. closes #882 🤖 Generated with [Claude Code](https://claude.com/claude-code)
fix(comfyui): stop -WhatIf from reporting a converged host as empty
All checks were successful
TruffleHog / Scan for secrets (pull_request) Successful in 5s
CI / lint (pull_request) Successful in 41s
9408ce6ce6
Get-FileHash returns nothing when -WhatIf binds at the top level of a
SupportsShouldProcess script, so Test-Sha256 compared $null against the
pin and every model reported unverified. The preview then demanded
56.42 GiB of downloads already on disk, and the preflight gate would
throw Insufficient free disk on a host needing zero bytes.

Hash through System.Security.Cryptography instead. A pure read inside a
converger must not be suppressible by a preview flag.

The existing preview test could not catch this: it invoked the converger
as `& script -WhatIf` from a plain wrapper, and that shape leaves
Get-FileHash working. Only a top-level -WhatIf, the shape the justfile
uses, triggers it. The wrapper is now an advanced script taking -WhatIf
itself, and a new case asserts a seeded, already-verified model still
reads as verified under preview.

Verified on kai-tower-3026. The preview now reports no model download
required and verifies all six models. The new test fails against the old
hashing path and passes against the new one.

closes #882

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
coilyco-flight-deck/infrastructure!885
No description provided.