Repository navigation
fix(installer): bound retained display inventory queries - #350
Conversation
Keep retained-device and post-repair CIM/DISM queries behind the existing bounded process edge. Fail closed on timeout or invalid results, retain version/trust and device-policy safeguards, and preserve runtime restoration. Validate 45 production guard fixtures with real inventory child processes and mocked Windows edges; confirm the original hang with a negative control. No live Windows driver qualification. Related report LizardByte#660 remains unconfirmed. Prepared with OpenAI Codex on behalf of the repository owner.
Nonary
left a comment
There was a problem hiding this comment.
Independent round 1 review — verdict: clear (no actionable introduced findings).
Session: 01a166b4-aa57-7216-af4f-10842b0360fb.
Reviewed head: 4d8ff65649c50ad10c1b5c2e32c33f0b8c3cb705.
Target: canary at 8f94e2932a22afa2411324a85c644f37a46e1cf0.
Fresh refs still match immediately before posting. GitHub reports an open, mergeable draft. Current integration commit c9abdcd49bb377aaba72e2372844dc3ca010c54a has those base/head parents and the same tree as the reviewed head.
Used the actual installed UltraCode skill with native gpt-6-luna / high controls. Completed 22 independent helpers within the requested 72 Luna High ceiling, maximum six concurrent: six scoped discovery reviewers, then four fresh confirmer/denier pairs for each selected material hypothesis:
- C1 rejected: PS5.1 JSON projection changing health/version/repair decisions. Independent zero/one/many probes preserved arrays and consumed Boolean/numeric/string fields; the existing ambiguity checks, maximum observed version floor, Driver Store/OEM identity and catalog/trust checks remain intact.
- C2 rejected: retained-query failure bypassing restoration, continuing installation, or claiming unchanged driver state after repair. Errors and invalid results throw; post-repair health stays inside the restoration
finally; initialization requires valid healthy results; retained failure wording defers further changes. The outer retained branch exits before bundled installation.
Windows verification:
- PASS: all 45
test_retained_display_device.ps1fixtures and MSI opt-out wiring on Windows PowerShell 5.1.26100.9444 (75.8 seconds). Production generated queries, child launch, timeout/termination and JSON decoding ran; OS inventory/signature/mutation boundaries were mocked. Timed-out child PID exit, post-repair restoration, fail-closed markers/results and warning assertions passed. - PASS: Sunshine/SudoVDA
test_display_inventory_query.ps1;git diff --check; clean source checkout. - Negative control reproduced: the exact canary installer's mocked health-timeout case remained blocked until an external 10-second watchdog terminated it.
Validation limits: the raw Windows aggregate passed all three safety matrices, initial-query fixtures and 564 rollback checks, then stopped in the unchanged provenance test because Get-FileHash was unavailable. A run with the standard Utility module globally preloaded progressed to an OS Access denied at scripts/download_libvirtualdisplay_release.ps1:159, moving mocked staging files (including consumer-release-lock.json) into the provenance fixture's temporary cache. That denied move was stopped without retry; the full aggregate is unpassed. CI run 39892629364 is still in progress; its installer safety step was skipped, so no CI safety-test clearance is inferred. Required checks remain for the final review/merge gates.
Live CIM/DISM timing, descendant-process cleanup, package builds and real upgrades remain unverified. No source edits, driver installation, service/trust changes, pushes, merges or draft promotion occurred. This review does not establish or resolve Vibepollo LizardByte#660's reported symptoms. PR remains draft for the parent's final review.
AI-generated review by OpenAI Codex on behalf of the repository owner.
|
Scope clarification for round 1 review: the referenced report is Nonary/Vibepollo issue 660. GitHub rendered the review’s unqualified issue number as an upstream link. This review makes no diagnosis or resolution claim for the Vibepollo report. |
Nonary
left a comment
There was a problem hiding this comment.
Independent final review (round 2) — verdict: clear. No unresolved actionable introduced findings.
Session: 01a166cc-1f3d-7511-9115-4fd2a95d10ed.
Reviewed head: 4d8ff65649c50ad10c1b5c2e32c33f0b8c3cb705.
Target canary: 8f94e2932a22afa2411324a85c644f37a46e1cf0.
Fresh refs still match. Complete two-file diff and surrounding consumers reviewed afresh. Integration commit c9abdcd49bb377aaba72e2372844dc3ca010c54a has those exact base/head parents and tree 3354de0ee1a9c6b5d56c833564774456f6c7f8cf, identical to the head.
Used the actual installed UltraCode skill and native gpt-6-luna / high controls: 22 distinct completed helpers within the 72 Luna High ceiling, at most six concurrent. Six scoped discovery reviewers, followed by four fresh independent confirmer/denier pairs per selected material hypothesis:
- C1 rejected: JSON/PowerShell 5.1 projection or non-inbox filtering changing health/version/trust decisions. The projections retain all consumed fields and explicit arrays; missing/ambiguous/malformed envelopes throw. Existing unknown-health/ambiguity guards, maximum observed version floor, Driver Store/OEM INF identity and version parity, catalog membership and Windows trust checks remain.
- C2 rejected: retained inventory hanging/failing open, bypassing restoration, continuing into bundled installation, or claiming unchanged driver state after repair. The new callsites use the existing asynchronous pipe capture and finite per-query deadline. Query/decoder failures throw; post-repair health remains inside the broker/service restoration
finally; the retained branch exits before bundled installation; query errors replace the stale unchanged-driver claim. Slow-query failure and leaving earlier repair operations in place are deliberate fail-closed behavior with existing partial-repair semantics, not a new rollback defect.
Round 1 is from distinct session 01a166b4-aa57-7216-af4f-10842b0360fb. I checked its actual session record, all 22 helper starts/completions, allocation and logs. There were no actionable first-pass findings to dispose of. Its recorded Windows PowerShell 5.1.26100.9444 retained fixture command exited 0 after 75.806 seconds, with all 45 fixtures and MSI opt-out wiring passing. The Sunshine/SudoVDA initial-query tests exited 0; the unchanged-canary negative control required the external 10-second watchdog. These are verified prior results at this same head, not new tests from this final session.
Validation limits:
- This session's attempted fixture launch was denied by PowerShell AuthorizationManager before execution. I stopped without retry or bypass. Local Git access to the prior task's checkout also returned permission denied; that action stopped. Fresh source/diff/integration were read through the authorized GitHub connector; local review text was reconciled against returned source.
- The full aggregate remains unpassed: round 1 first encountered unavailable
Get-FileHash, then a preloaded-module run reached Access denied atscripts/download_libvirtualdisplay_release.ps1:159moving mocked staging files, includingconsumer-release-lock.json, into the fixture cache. That denied move was not retried or bypassed. - Termination targets the direct PowerShell child and is best effort; failed-kill/native-descendant behavior and real CIM/DISM timing remain unverified. The 15-second budget is per inventory query, not the whole installer. Real upgrades and actual driver/trust/service restoration were not exercised.
- Current-head CI run 39892629364 completed success. Windows build, MSI packaging and packaged-MSI validation succeeded; installer driver safety/rollback and Windows test execution were skipped, so CI is not evidence that the full safety suite passed.
Readiness policy was checked using allowed metadata: canary branch metadata reports classic required-check enforcement off with empty check/context lists; its active ruleset 25668682 contains deletion/non-fast-forward rules only. The other branch ruleset applies to master. No required status checks are configured for canary in that evidence. The previously denied branch-protection endpoint was not retried. I will perform the final fresh-state preflight and verify the ready transition separately.
No product-source edits, pushes, merges, releases, driver installation, or service/trust changes. This review does not diagnose or resolve Nonary/Vibepollo issue 660.
AI-generated review by OpenAI Codex on behalf of the repository owner.
|
Related issue: #351 — Driver installation is skipped when safety inventory exceeds the 15-second timeout. This PR addresses the installer inventory timeout described there. |
Summary
Harden a concrete remaining installer hang surface identified while triaging Nonary/Vibepollo#660. This does not establish the cause of that report or claim to resolve its installer, log-export, or game-focus symptoms.
The initial safety inventory already has a timeout, but equal/newer retained-driver health checks and retained-package lookup still queried CIM/DISM synchronously in the installer. The post-repair health check used the same unbounded path.
Only the installer script and its retained-device regression fixture change. Based on canary 8f94e29; the two source files are unchanged from the tested initial base 6b4e1a3.
Validation
Command: pwsh -NoProfile -File packaging/windows/test_retained_display_device.ps1
Limits
Native Windows PowerShell 5.1, real CIM/DISM timing, full Windows package builds and live upgrades are unverified. No driver installation, service changes, trust changes or release actions were performed. This bounds retained inventory waits, not every possible installer stall. Keep draft pending independent review and Windows validation.
Prepared with OpenAI Codex on behalf of the repository owner.