Skip to content

gui: Refresh empty wallets automatically - #66

Open
BenWestgate wants to merge 3 commits into
gui-reviewability-v1from
codex/gui-wallet-refresh-v1
Open

BenWestgate wants to merge 3 commits into
gui-reviewability-v1from
codex/gui-wallet-refresh-v1

Conversation

@BenWestgate

@BenWestgate BenWestgate commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

Follow-up to #65.

Refresh the eligible Bitcoin Core wallet list once per second while the wallet chooser is visible, matching the CLI's waiting behavior. The selected wallet is preserved by exact name, and polling shares the existing serialized worker boundary so it cannot overlap an import or correction job.

The manual “Check again” action is removed; a wallet created in Bitcoin Core appears automatically. Polling pauses while the chooser is hidden and stops after it leaves the navigation stack. Transient refresh failures are retried without abandoning recovery. If a selected wallet disappears, no destination remains selected and Continue is disabled until the operator explicitly chooses again.

Read-only wallet polls use daemon workers so a slow/unresponsive bitcoin-cli poll cannot keep the process alive after the last GUI window closes. Mutating jobs still use non-daemon workers so their cleanup/relocking finally blocks remain guaranteed. The shared serialization gate is unchanged.

Validation on current head dfffb00:

  • prior branch validation: 974 tests normally and under python -O, Ruff, strict mypy, GTK/Xvfb walkthrough;
  • review follow-up c26de06: walkthrough covers transient poll failure, disappeared selection, explicit reselection and polling shutdown;
  • shutdown-policy follow-up ac56aab + dfffb00: tests/test_gui_boundaries.py passes 41/41 and statically pins run() as non-daemon versus poll() as daemon;
  • fresh GitHub Python-package run 443 completed successfully.

Review-size policy: this PR raises the separately enforced GUI cap from <2000 to <2050 logical lines. The maintainer explicitly approved the GUI limit for v1 in the Codex task on 2026-09-30; the library <5200 limit is a separate authorization.

All inline review threads are resolved. AI assistance on the final shutdown-policy follow-up is disclosed by commit history.

@BenWestgate BenWestgate added the gate: adversarial review Resolve, merge, or explicitly defer before the next full adversarial review. label Sep 29, 2026 — with ChatGPT Codex Connector

@BenWestgate BenWestgate left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI-generated review, posted at the maintainer's request.

Not ACKing 9b4066e: if the selected wallet disappears on refresh, target falls through to len(current) and silently activates Create a new wallet. Continue can then change the destination without an explicit user choice.

Copy link
Copy Markdown
Owner Author

Review note: the polling change does remove the standalone #65 Check again path that dropped restoring, so the stacked #65→#66 candidate fixes that reviewed bug. One separate item still needs maintainer disposition: this PR raises the GUI review budget from 2,000 to 2,050 logical lines. The explicit budget authorization on record is for the installed library (<5200), not for changing the separately enforced GUI cap. Prefer keeping the GUI under 2,000 if that can be done without a risky pre-release refactor; otherwise the 2,050 cap should be an explicit human review/approval rather than an incidental test change.

@BenWestgate BenWestgate added area: gui Graphical user interface behavior. area: wallet/core Wallet integration and Bitcoin Core boundaries. enhancement New feature or request labels Sep 29, 2026 — with ChatGPT Codex Connector
@BenWestgate
BenWestgate force-pushed the codex/gui-wallet-refresh-v1 branch from 9b4066e to e552e60 Compare September 29, 2026 23:38

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e552e60f31

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/codex32_gui/pages.py Outdated
Poll Bitcoin Core once per second while the wallet chooser is visible, matching the CLI's waiting behavior. Preserve the selected wallet by exact name and serialize polling with other background work so refreshes cannot overlap imports or correction.

The GTK walkthrough covers automatic appearance, selection preservation, removal of the manual refresh button, and stopping after navigation. Full normal and optimized suites, Ruff, strict mypy, and the walkthrough pass.
@BenWestgate
BenWestgate force-pushed the codex/gui-wallet-refresh-v1 branch from e552e60 to c26de06 Compare September 30, 2026 00:10
@BenWestgate

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c26de065d1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/codex32_gui/work.py Outdated
@BenWestgate BenWestgate added the area: security Security invariants, hardening, and security-sensitive boundaries. label Sep 30, 2026 — with ChatGPT Codex Connector

@BenWestgate BenWestgate left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI-generated release-gate review, posted at the maintainer's request.

ACK dfffb00.

  • The disappearing-selection regression is fixed: refresh preserves a wallet by exact name when present; if it disappears, no replacement/Create row is selected and Continue stays disabled until explicit reselection.
  • Transient read-only poll failures leave the chooser intact and retry.
  • work.poll() now uses daemon workers so a stuck read-only bitcoin-cli poll cannot hold process shutdown; work.run() remains non-daemon for mutation/relocking cleanup.
  • All six Python-package matrix jobs on the exact head completed successfully, including optimized tests/constants on Ubuntu 3.13.

No remaining code blocker from this review. #71–#74 are separately tracked Tails presentation/copy follow-ups and do not invalidate this #66 behavior ACK.

Copy link
Copy Markdown
Owner Author

Integration follow-up: #77 now carries the four Tails field-test items #71–#74 as one focused one-file PR on top of this head. Review order for the clean GUI stack is #65 → #66 → #77. #77 has a code ACK; its final outstanding qualification is the manual Tails guest-resolution fit check for #74. The reviewed restore-authentication GUI delta from #28 is still replayed only after #57 is integrated into the settled library/CLI base.

Copy link
Copy Markdown
Owner Author

Agent release-gate re-review at exact head dfffb00: the wallet-refresh behavior reviewed earlier is unchanged; the later delta only makes read-only poll workers daemon threads while explicitly keeping mutating run() workers non-daemon so relocking/cleanup remains process-blocking. The AST regression pins that distinction, git diff --check is clean, and exact-head run 36655064471 succeeded. No remaining code-review blocker found. This PR is required with #65; the unresolved #65 refresh-mode thread is eliminated by this stacked replacement rather than fixed in the parent.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: gui Graphical user interface behavior. area: security Security invariants, hardening, and security-sensitive boundaries. area: wallet/core Wallet integration and Bitcoin Core boundaries. enhancement New feature or request gate: adversarial review Resolve, merge, or explicitly defer before the next full adversarial review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant