[Bug] TUI session rollback (from #704) uses the global DB for namespaced sessions, leaks a DBWorker, and its "Force" option is misleading

Closed
  • Is something wrong?
#742 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

A pull request for this has already been merged.

  • #756 by @fderuiter — merged

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
45/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
python
Domain
database, desktop, testing

Research direction

Start with run_rollback_worker in app/ui/tui.py:5842-5876, then read the CLI undo path and the related issue #709 to understand how session database paths are resolved. Review tests/test_tui_history_rollback.py:98-197 for existing coverage and worker waiting behavior. Done means rollback uses the session's database set, releases its worker, handles the Force option and modal state correctly, and has deterministic tests.

Written by the indexing model from the issue text.

Description

bug

Summary

The History modal rollback added in #704 (merged as a7ae7a8) has several problems. It was merged while a review of it was in progress, so they are filed here instead.

Location

  • app/ui/tui.py:5842-5876: run_rollback_worker
  • tests/test_tui_history_rollback.py:98-197

Problems

  1. Wrong database for namespaced sessions. The worker always opens get_app_dir()/autosorter.db and get_app_dir()/cache.db (tui.py:5856-5857), even when history_db_path points to a per-session history.db. The CLI undo path uses the database that sits next to the session's history DB, so a TUI rollback of a namespaced session updates the wrong document index and cache.
  2. Thread leak. A new DBWorker() is created for every rollback (tui.py:5855) and is never stopped, so each rollback leaves a worker thread running.
  3. "Force rollback" option is misleading. ignore_missing has no effect for sessions with step-ledger rows, which is every session the TUI produces. That path already skips missing files. When a file already exists at the original location, it deletes the moved file instead of restoring it (see #715). The confirm dialog mentions neither behaviour.
  4. Escape closes the modal mid-rollback. While the worker is running, Escape still dismisses the History modal, so the success or failure result is never shown.
  5. Flaky tests. tests/test_tui_history_rollback.py waits for the thread worker with a fixed number of pilot.pause() calls instead of waiting for the worker to finish. This caused a macOS CI failure on #704 and also fails locally under load.

Expected vs actual

  • Expected: rollback writes to the same DB set the CLI undo uses for that session, releases its worker, and accurately describes what "Force" does. The modal stays open until the rollback finishes, and the tests are deterministic.
  • Actual: see above.

Suggested fix

  • Resolve the autosorter.db and cache.db paths the same way the CLI undo path does, from the session's history DB location (ideally through one shared helper; see #709).
  • Stop the DBWorker in a finally block.
  • Either make ignore_missing meaningful for ledger-backed sessions or remove the checkbox, and spell out the collision behaviour in the confirm dialog.
  • Block Escape and Cancel while a rollback is in flight.
  • In the tests, await the worker with app.workers.wait_for_complete() or poll for the status label.

Severity

High. Items 1 and 3 can leave the index wrong or lose data on a user-triggered undo.

Dominant language
Python
Stars
1
Forks
0
Avg merge
10h 6m
Merged PRs (30d)
367

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from fderuiter/sortify

All issues in fderuiter/sortify

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.