fix(window): reject transient restore thumbnail bounds - #345
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
P2
|
Codex review: needs maintainer review before merge. Reviewed August 9, 2026, 8:54 PM ET / August 10, 2026, 00:54 UTC. ClawSweeper reviewWhat this changesThe PR makes background window restore retain its verified bounds unless refreshed public inventory confirms the same window, owner process generation, restored state, and near-identical frame. Regression provenancePossible regression — probable (reproduction; reviewed change). No predecessor PR is attributed. Merge readinessCurrent main still accepts any same-ID post-restore inventory record, while this branch checks the verified receipt before allowing refreshed metadata. The focused collaborator-authored patch has no actionable correctness or security finding and needs ordinary maintainer approval. Priority: P2 Review scores
Verification
How this fits togetherThe CLI restores a minimized macOS window through the automation service, then refreshes public window inventory for output. The refresh must preserve the verified restore receipt when inventory is delayed or temporarily reports an animated Dock thumbnail. flowchart LR
A[CLI restore request] --> B[Verified window receipt]
B --> C[Native window restore]
C --> D[Public inventory refresh]
D --> E{Identity and bounds match?}
E -->|yes| F[Use refreshed window metadata]
E -->|no or missing| G[Keep verified restore bounds]
F --> H[CLI result]
G --> H
Decision needed
Why: The change is a focused native-automation reliability repair with strong branch evidence, but collaborator-authored PRs require explicit maintainer judgment before landing. Before merge
Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Land the receipt-pinning check after maintainer approval and the remaining required CI result, preserving verified restore bounds until public inventory reports a matching restored window. Do we have a high-confidence way to reproduce the issue? Yes—current main directly replaces the verified result with the first post-restore inventory record, and the branch tests reproduce transient thumbnail, identity-drift, and inventory-miss cases. Is this the best way to solve the issue? Yes—the guard preserves the established verified receipt and permits refreshed metadata only when it independently matches the expected window and frame, avoiding a new public API or retry policy. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 6e70b21f5bdb. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
082cff7 to
e1fab20
Compare
clawsweeper
Bot
added
proof: sufficient
1 participant
Summary
40x81Dock-thumbnail frame, identity drift, bounded rounding tolerance, inventory misses, and idempotent visible restoreRoot cause
After a verified background restore, public window inventory can briefly expose the exact CG window ID at its animated Dock-thumbnail frame. The CLI's best-effort display refresh accepted that same-ID descriptor without checking its owner-generation and bounds receipt, so a successful restore could report a tiny Dock-positioned
new_boundsvalue.Proof
swift test --package-path Apps/CLI --filter WindowRestoreOutputTests --no-parallel— 6/6pnpm run test:safe— 718 unit tests and 72 runtime tests passed1200x852frame while 4 immediate public listings were transientNo AppleScript path is involved.