Skip to content

fix: wrap yield in try-finally in UsethisConfig.set() and change_cwd()#1834

Merged
nathanjmcdougall merged 2 commits intomainfrom
copilot/fix-contextmanager-state-restoration
Apr 7, 2026
Merged

fix: wrap yield in try-finally in UsethisConfig.set() and change_cwd()#1834
nathanjmcdougall merged 2 commits intomainfrom
copilot/fix-contextmanager-state-restoration

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 7, 2026

@contextmanager cleanup code placed after yield doesn't execute on exception — the exception re-raises at the yield point, skipping everything below it. This left usethis_config in a corrupted state (e.g. quiet=True permanently) when exceptions propagated through with usethis_config.set(...) blocks.

Changes

  • src/usethis/_config.py — Wrap yield in try-finally in UsethisConfig.set() so _restore() always runs
  • src/usethis/_test.py — Same fix for change_cwd() so os.chdir(old_dir) always runs
  • tests/usethis/test_config.py — Add tests verifying state restoration after exception
  • .agents/skills/usethis-python-code/SKILL.md — Add this as a documented common mistake in the context manager guidance (v1.9)
# Before: cleanup skipped on exception
self.project_dir = project_dir
yield
self._restore(old)  # never reached if body raises

# After: cleanup guaranteed
self.project_dir = project_dir
try:
    yield
finally:
    self._restore(old)

…), update skill, add tests

Agent-Logs-Url: https://github.com/usethis-python/usethis-python/sessions/429f9231-8554-40bd-b168-73c3f152f417

Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix contextlib.contextmanager state restoration on exception fix: wrap yield in try-finally in UsethisConfig.set() and change_cwd() Apr 7, 2026
Copilot AI requested a review from nathanjmcdougall April 7, 2026 02:54
@nathanjmcdougall nathanjmcdougall marked this pull request as ready for review April 7, 2026 03:44
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 7, 2026

Merging this PR will not alter performance

✅ 2 untouched benchmarks


Comparing copilot/fix-contextmanager-state-restoration (ed0639a) with main (61f88eb)

Open in CodSpeed

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@nathanjmcdougall nathanjmcdougall merged commit e6862cd into main Apr 7, 2026
20 checks passed
@nathanjmcdougall nathanjmcdougall deleted the copilot/fix-contextmanager-state-restoration branch April 7, 2026 04:14
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.

agent: contextlib.contextmanager does NOT restore state on exception unless try-finally is used

2 participants