fix(app): don't crash when a save's filesystem prep fails

[wkentaro]

save_labels wraps Path.mkdir (creating the output directory) and os.path.relpath (computing the image's stored relative path) in the same try as write_label_file, but caught only LabelFileError. write_label_file re-wraps its own I/O errors into LabelFileError, so writes were covered, but mkdir and relpath are not: a read-only or full --output directory raises PermissionError/OSError, and on Windows an image and output on different drives raises ValueError. Both escaped uncaught and took the whole app down through the global exception hook, including during auto-save (which runs on every edit).

They are now caught alongside the existing write failures, matching the raw-OSError/ValueError handling already used elsewhere at the UI boundary (_open_image_into_state, _on_setting_changed): the save-error dialog is shown and save_labels returns False instead of crashing.

Test plan

• tests/e2e/save_error_handling_test.py: parametrized over both documented failure modes (Path.mkdir -> PermissionError, os.path.relpath -> ValueError); each asserts save_labels returns False and shows the error dialog. Both cases fail on main and pass with the fix.
• Drove the real MainWindow (app's real excepthook installed) into a genuinely read-only output dir and triggered auto-save: with the fix the app survives with the dialog and exits cleanly; on main the same input hits the excepthook with a PermissionError traceback and crashes.
• uv run ruff check, ruff format --check, ty check, full unit/e2e suite (883 passed) green

[wkentaro]

This was generated by AI during PR processing.

Verdict: recommend-merge

Reviewed the diff (medium-effort /code-review, no findings needing a fix). The change broadens the save_labels exception handler from LabelFileError to (LabelFileError, OSError, ValueError), so a failing Path.mkdir (read-only/full --output dir → PermissionError, an OSError) or a cross-drive os.path.relpath on Windows (ValueError) now surfaces the save-error dialog and returns False instead of crashing the app through the global exception hook (including during auto-save).

Checks:

• Branch is up to date with main (0 commits behind the merge-base, no file overlap) — no rebase needed; the green CI reflects the merged result.
• All 11 CI jobs green (lint + test on ubuntu/macos/windows × 3.12/3.13/3.14).
• The raise RuntimeError(...) duplicate-file guard and the assert preconditions inside the try are not subclasses of OSError/ValueError, so they still propagate — no guard is masked by the broadening.
• New tests/e2e/save_error_handling_test.py drives the real save flow with monkeypatched Path.mkdir/os.path.relpath failures and asserts no crash + error dialog shown.
• Single clean conventional commit; nothing to reshape.

[wkentaro]

This was generated by AI during PR processing.

Withdrew recommend-merge. main has advanced far enough that this branch now conflicts with it (mergeStateStatus: DIRTY), so the verdict was no longer true: the PR cannot be merged as it stands, whatever its contents.

Nothing is wrong with the change itself, and no re-review is implied. This needs a rebase onto current main and a fresh CI run, after which it will be re-verdicted. It is back in the processing queue; ticks rebase one branch at a time to avoid overlapping rebase chains, so this will be picked up in an upcoming tick.

Merging the approved backlog sooner would stop these from going stale: several PRs are queued behind the same moving main.

[wkentaro]

This was generated by AI during PR processing.

Rebased onto current main (bd3ca1d) to clear the conflict that withdrew the previous verdict. The conflict was purely additive in CHANGELOG.md (this entry vs. #2391's, both new lines in the same Fixed list); both entries are kept. No code hunks conflicted, and the branch is still one logical commit (d300ac9f).

Ran a report-only two-axis review over the rebased diff. No meaningful findings, so I skipped the full review-fix loop:

• Standards: clean. The changelog entry is filed correctly, the one-line except widening is surgical, and the new e2e test follows house style (Final constant, @pytest.mark.gui, the MainWinFactory / show_window_and_wait_for_imagedata / close_or_pause fixtures, string-target monkeypatch.setattr, full annotations). Only a judgement call: _record_critical is near-verbatim from tests/e2e/label_dialog_validation_test.py, but e2e files here are deliberately self-contained with no shared dialog-stub module, so extracting it would introduce a convention rather than follow one. Left as-is.
• Spec: clean. The review specifically checked the main risk of widening a catch over a whole try body, namely whether a successful write could now be mis-reported as a failure. It cannot: write_label_file already catches (OSError, TypeError, ValueError) internally and re-raises as LabelFileWriteError, Annotation is a plain dataclass with no validation, and the post-write findItems/setCheckState bookkeeping raises neither OSError nor ValueError. The asserts raise AssertionError and are untouched.

Local verification on the rebased tree: ruff check, ruff format --check, ty check, and mdformat --check all pass over the tracked tree, and the full suite is green (893 passed). The two new parametrized cases pass here and fail on main.

One cosmetic note on this PR's own description, not the code: it cites _open_image_into_state and _on_setting_changed as precedent for raw OSError/ValueError handling, but _open_image_into_state (labelme/_app.py:2089) catches only OSError. _on_setting_changed (:2488) does catch both, so the precedent holds via that one. Not worth a force-push to reword.

Awaiting CI on the rebased head; I will verdict once the checks report.

[wkentaro]

This was generated by AI during PR processing.

Verdict: recommend-merge

CI on the rebased head (d300ac9f) has reported: all 11 jobs green (lint + test on ubuntu/macos/windows × 3.12/3.13/3.14, plus cla). Merge state is CLEAN, and the branch sits directly on top of current main (bd3ca1d, 0 commits behind the merge-base), so this green reflects exactly the merged result.

Restoring the verdict withdrawn on 2026-07-25: the withdrawal was purely about the CHANGELOG.md conflict with main, which the rebase resolved additively (both this entry and #2391's are kept). The change itself was unchanged by the rebase and re-reviewed clean on both the standards and spec axes, with the full local suite green (893 passed).

Nothing further from me. History is a single logical conventional commit; no reshaping needed.