Milestone 5 Bugfixes #52

New Issue

2026-09-09T11:09:24+01:00

gronod commented

2026-09-09 11:09:24 +01:00

Install “Overwrite” from the collision alert is a no-op.
beginInstallProfile sets forceOverwrite: !askBeforeOverwrite and collisionPolicy: .overwrite. The alert then calls resolveInstallCollision(policy: .overwrite) without setting forceOverwrite = true. ProfileInstaller.install: existing dest + forceOverwrite == false + .overwrite falls through to copyFailed("destination already exists"). Rename/Cancel work; Overwrite does not. ProfileInstallerTests never call install() against a real dest — they reimplement copy/move inline — so this is untested. (#27)
VerificationHistoryStore.append without load() wipes the file.
records starts []. append writes that plus the new row. Stage5View.onAppear → loadHistory() is a detached Task. Verify can run first and replace verification_history.json with one row. load() documents “never overwrite unparseable file”; append after a failed load still overwrites. append must load() (or refuse) first. (#26)
createdProfileURL is session-only.
After relaunch, wizard gating can restore Stage 5 from disk .icc, but canVerify / Install require createdProfileURL != nil, which is only set inside createProfile(). Resolve from ArtefactProbe.resolveProfile on Stage 4/5 appear. (#23 / #25 / #27)
Drift alert is not consecutive and hides the profcheck warning.
Spec: ≥ 2 consecutive Warning runs, distinct UTC days or ≥ 1 hour. DriftAlert.compute pairs any two .poor records, ignoring intervening Good/Excellent. Header shows profcheckWarning only when driftAlert == nil, so a drift badge swallows an unparseable-run warning. (#25 / #26)
printerName persisted as "", displayed as Unknown only in the picker.
Spec: wizardState.printerName or "Unknown". makeVerificationRecord uses wizard.printerName ?? "". Empty-string printers, empty CSV field, filter tag "" colliding with “All”. (#26)
ApplycalArgsTests.unapplyNotEmitted asserts the opposite of its name.
Builder does emit -u when unapply: true. UI never sets it (good). Rename the test; keep a guard that ProfileWorkflowViewModel never passes unapply: true. (#24)
Installer dest is always {stem}.icc.
Source .icm is copied to stem.icc. #69 prefers existing .icm. Preserve extension. Stem check contains("..") rejects foo..bar which is not traversal. (#27)
#24 AC “.gam produced next to profile” is best-effort in the VM.
runIccgamut failure is an info banner; Create still go(to: .verifyInstall). M5 UI test happens to see the mock .gam. Product vs ticket: either weaken #24 AC or fail Create when iccgamut fails.

Suggested extra ACs on #50 (process only)

collect() returns after process exit even if a pipe EOF never arrives (timeout).
Killing captured applycal never replaceItemAts the input profile; tmp is deleted; original .icc byte-identical.
killAll on quit covers colprof_*, iccgamut_*, profcheck_*, applycal_*, and chartread_* (XY still q\n + 500 ms first).
Handheld fixture unskip (#50 original) still required; M5 UI tests do not count.

Install overwrite, history wipe, and createdProfileURL restore stay off this ticket unless we edit #50 Scope In.

1. **Install “Overwrite” from the collision alert is a no-op.** `beginInstallProfile` sets `forceOverwrite: !askBeforeOverwrite` and `collisionPolicy: .overwrite`. The alert then calls `resolveInstallCollision(policy: .overwrite)` **without** setting `forceOverwrite = true`. `ProfileInstaller.install`: existing dest + `forceOverwrite == false` + `.overwrite` falls through to `copyFailed("destination already exists")`. Rename/Cancel work; Overwrite does not. `ProfileInstallerTests` never call `install()` against a real dest — they reimplement copy/move inline — so this is untested. (#27) 2. **`VerificationHistoryStore.append` without `load()` wipes the file.** `records` starts `[]`. `append` writes that plus the new row. `Stage5View.onAppear` → `loadHistory()` is a detached `Task`. Verify can run first and replace `verification_history.json` with one row. `load()` documents “never overwrite unparseable file”; `append` after a failed load still overwrites. `append` must `load()` (or refuse) first. (#26) 3. **`createdProfileURL` is session-only.** After relaunch, wizard gating can restore Stage 5 from disk `.icc`, but `canVerify` / Install require `createdProfileURL != nil`, which is only set inside `createProfile()`. Resolve from `ArtefactProbe.resolveProfile` on Stage 4/5 appear. (#23 / #25 / #27) 4. **Drift alert is not consecutive and hides the profcheck warning.** Spec: ≥ 2 **consecutive** Warning runs, distinct UTC days **or** ≥ 1 hour. `DriftAlert.compute` pairs any two `.poor` records, ignoring intervening Good/Excellent. Header shows `profcheckWarning` only when `driftAlert == nil`, so a drift badge swallows an unparseable-run warning. (#25 / #26) 5. **`printerName` persisted as `""`, displayed as Unknown only in the picker.** Spec: `wizardState.printerName or "Unknown"`. `makeVerificationRecord` uses `wizard.printerName ?? ""`. Empty-string printers, empty CSV field, filter tag `""` colliding with “All”. (#26) 6. **`ApplycalArgsTests.unapplyNotEmitted` asserts the opposite of its name.** Builder **does** emit `-u` when `unapply: true`. UI never sets it (good). Rename the test; keep a guard that `ProfileWorkflowViewModel` never passes `unapply: true`. (#24) 7. **Installer dest is always `{stem}.icc`.** Source `.icm` is copied to `stem.icc`. `#69` prefers existing `.icm`. Preserve extension. Stem check `contains("..")` rejects `foo..bar` which is not traversal. (#27) 8. **`#24` AC “`.gam` produced next to profile” is best-effort in the VM.** `runIccgamut` failure is an info banner; Create still `go(to: .verifyInstall)`. M5 UI test happens to see the mock `.gam`. Product vs ticket: either weaken #24 AC or fail Create when iccgamut fails. --- ### Suggested extra ACs on #50 (process only) - [ ] `collect()` returns after process exit even if a pipe EOF never arrives (timeout). - [ ] Killing captured `applycal` never `replaceItemAt`s the input profile; tmp is deleted; original `.icc` byte-identical. - [ ] `killAll` on quit covers `colprof_*`, `iccgamut_*`, `profcheck_*`, `applycal_*`, and `chartread_*` (XY still `q\n` + 500 ms first). - [ ] Handheld fixture unskip (#50 original) still required; M5 UI tests do not count. Install overwrite, history wipe, and `createdProfileURL` restore stay off this ticket unless we edit #50 Scope In.

gronod added this to the M5 — Profile, verification & install milestone 2026-09-09 11:09:24 +01:00

gronod added the Kind/Bug

Bug/Backend Bug/UI labels 2026-09-09 11:09:24 +01:00

gronod added a new dependency 2026-09-09 11:10:34 +01:00

#23 Stage 4: colprof UI & argv builder

gronod added a new dependency 2026-09-09 11:11:15 +01:00