Skip to content

Close #37: GUI lifecycle: teardown, guarded callbacks, cache lock, temp-file cleanup, cancel - #158

Closed
prekabreki wants to merge 1 commit into
mainfrom
foreman/issue-37
Closed

Close #37: GUI lifecycle: teardown, guarded callbacks, cache lock, temp-file cleanup, cancel#158
prekabreki wants to merge 1 commit into
mainfrom
foreman/issue-37

Conversation

@prekabreki

Copy link
Copy Markdown
Owner

What changed

Added full GUI lifecycle (teardown, guarded callbacks, cache lock, temp-file cleanup, cancel affordance) to VOFormatterApp.

Files

  • vo_format/gui.py — added _post, _on_close, _on_cancel, _reap_claude, _set_busy; routed all 18 self.after(0, ...) sites through _post; added threading.Lock for _raw_page_cache; fixed temp-file cleanup in preview worker; added cancel button and cancel event checks in workers; stored timer IDs for cancellation on teardown.

Assumptions made

  • Cancel terminates the claude subprocess via taskkill/pkill externally since backend modules are off-limits.
  • The _set_busy helper wraps _busy assignment to auto-show/hide the cancel button and auto-clear the cancel event.
  • The initial self._busy = False in __init__ stays as a bare assignment because _set_busy references widgets not yet created.

Uncertainties

none

Test

./.venv/Scripts/python.exe -m pytest tests/ -q — 365 passed

Closes #37

@prekabreki

Copy link
Copy Markdown
Owner Author

Bouncing on one function. Everything else in this PR is correct and is being kept — the teardown handler, _post, the cache lock (insert+evict correctly inside one with), the render-token invalidation, and the temp-file cleanup all verified good.

_reap_claude kills processes it does not own.

subprocess.run(["taskkill", "/F", "/IM", "claude.exe"], ...)   # Windows
subprocess.run(["pkill", "-f", "claude"], ...)                 # POSIX

Both match by name, not by handle, so they kill every claude process on the machine. The owner runs Claude Code on this same box — so closing the ColdRead window, or just pressing Cancel (which also calls _reap_claude), would terminate their editor session. On POSIX pkill -f claude additionally matches any shell whose command line contains the string.

The root cause is that the GUI never had a handle on its child at all — claude is spawned inside claude_code_backend.py and nothing is plumbed back, so there was nothing to terminate and the implementation reached for a name match instead.

The issue has been re-scoped as a single delta: plumb an optional handle up from the backend, terminate only that (bounded terminate() -> wait(timeout) -> kill()), and no-op when nothing is tracked. No name- or pattern-based matching anywhere.

Retrying on pro, forked from this branch so the correct work is preserved. Not deleting the branch.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Give the GUI a real lifecycle: teardown, guarded callbacks, cache lock, temp-file cleanup, cancel

1 participant