fix(pairing): clarify reconnect and startup state#616
Conversation
|
Codex review: found issues before merge. Reviewed June 3, 2026, 1:37 AM ET / 05:37 UTC. Summary Reproducibility: yes. for the review finding: source inspection shows the PR deletes the HKCU Run value after scheduled-task registration while current docs still tell users to verify or remove only that Run entry. I did not run a Windows logon smoke in this read-only review. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Land this after maintainers accept the scheduled-task startup direction, docs/diagnostics cover both scheduled-task and Run-key paths, and Windows validation proves task-backed startup, fallback, cleanup, and the connection-label behavior. Do we have a high-confidence way to reproduce the issue? Yes for the review finding: source inspection shows the PR deletes the HKCU Run value after scheduled-task registration while current docs still tell users to verify or remove only that Run entry. I did not run a Windows logon smoke in this read-only review. Is this the best way to solve the issue? Not yet. Scheduled-task-first startup may be the right direction, but the maintainable merge path is to align docs/diagnostics and validate the Windows startup behavior before removing the old fallback from successful opt-ins. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 53ff55aff974. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
6d83c03 to
e365ba3
Compare
|
Thanks for the cleanup work here. I don't think this is safe to merge yet because the startup/scheduled-task behavior still needs author clarification and hardening before we can be confident it won't misreport or change autostart state incorrectly. Could you please address the scheduled-task scope/enabled-state concerns directly — especially whether the task is per-user vs all-user, how the UI determines the true enabled state, and how reconnect/startup status behaves after upgrades or missing tasks? Once that's explicit in code/tests/proof, this can get another focused review. |
Summary
1.0.0app-version display for offline orphan paired-node rows, while preserving matched/online versionsValidation
git diff --check.NETvalidation blocked on this mac shell:dotnetandpwshare not installed/resolving, and./build.ps1is not directly executable hereNotes