fix: the 12 confirmed findings of the 2026-09-28 code review - #275
Merged
Merged
Conversation
Thread safety (Magnification API): - MagThreadInvoke: a call abandoned at the 250 ms timeout never runs later (pending/running/done/abandoned state); the caller waits it out if the servicer already started. The three callers that captured stack refs now capture by value / return through a shared block. - MagHost::setSamplingMode is marshalled; the mode is recorded as applied only on success, with a bounded retry (3 tries, 1 s apart), because mode 0 reports FALSE on every call on this rig. - RenderEngine hide/show system cursor and the crash filter are marshalled, with a direct call as fallback. - EnsureCompositePulse closes its thread handle. Transform / hybrid: - transform->render reveal overlap uses TicksAtHz(3, hz), like the other path. - MpoGhost::create destroys the previous window before re-creating on retarget. - edgeClip switched off mid-zoom releases the clip at once. - CursorSprite::setScale (dead code) removed. Settings / config / installer: - WindConfig checks the setConfig ini write and shows "Couldn't save the setting" on failure (configWriteFailed). - The uninstaller removes the signed-in user's %LOCALAPPDATA%\Wind (owner of this session's explorer.exe via installer/user-localappdata.ps1), not the elevating account's. - LoadConfig returns ParseConfig(DefaultIniText()); a generated test pins the template to the struct defaults (52 keys). Plan: docs/superpowers/plans/2026-09-28-review-fixes.md. Version 0.10.2. Closes #274 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPUNAWcwghXdHCApcKKjSG
The caller that loses the abandon race to a servicer already running the call waited INFINITE, which could freeze the tick thread or the crash filter if that call stalled. It now waits one more 250 ms and returns regardless; safe because every MagThreadInvoke caller captures by value. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPUNAWcwghXdHCApcKKjSG
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #274. Plan:
docs/superpowers/plans/2026-09-28-review-fixes.md. Version 0.10.2.Fixes
Thread safety (Magnification API)
MagThreadInvoke: a call abandoned at the 250 ms timeout never runs later (pending/running/done/abandoned CAS); a caller that loses the race to a servicer already running the call waits one more bounded 250 ms and then returns. The three callers that captured stack references now capture by value (getInputTransform returns through a shared block).MagHost::setSamplingModeis marshalled; recorded as applied only on success, with a bounded retry (3 tries, 1 s apart). Mode 0 reports FALSE on every call on this rig (245/245 log lines) while working, so an unbounded retry would have re-issued it every tick.RenderEnginehide/show system cursor, shutdown and the crash filter are marshalled, with a direct call as fallback.EnsureCompositePulsecloses its thread handle.Transform / hybrid
TicksAtHz(3, hz)at both sites.MpoGhost::createdestroys the previous window before re-creating on retarget.edgeClipswitched off mid-zoom releases the clip at once (gated so an unused edgeClip costs no per-tick syscall).CursorSprite::setScale(dead code) removed.Settings / config / installer
setConfigini write; on failure logs it and the page shows "Couldn't save the setting".%LOCALAPPDATA%\Wind(owner of this session's explorer.exe,installer/user-localappdata.ps1, PSModulePath pinned), not the elevating account's; falls back to$LOCALAPPDATA.LoadConfigreturnsParseConfig(DefaultIniText()); a generated test pins all 52 template keys to the struct defaults.Verification
build.bat test: 304 passed (52 new template/default checks).build.bat checkclean. Playwright: 57 passed (new save-failure dialog test).tools/release.ps1elevated through pwsh 7 (CI path): all installer checks passed, including local signing and uninstall with the new data-folder helper.txHookWrite=1in render and transform (the marshalling paths) clean, no warnings, no crash dumps; sampling apply now 3 tries then accepted.🤖 Generated with Claude Code
https://claude.ai/code/session_01KPUNAWcwghXdHCApcKKjSG