Reset contextvar tokens in resolved_dependencies

[chrisguidry]

The resolved_dependencies function was setting contextvars without capturing reset tokens, which left stale references after the context manager exited. This could cause issues in reentrant or sequential calls, like "stack is closed" errors when the AsyncExitStack was already closed but still referenced.

Now we capture tokens from all contextvar.set() calls and reset them in finally blocks to restore prior state. This follows the same pattern suggested in the fastmcp PR review: jlowin/fastmcp#2318

Changes

• Modified resolved_dependencies to capture reset tokens from all 5 contextvar sets
• Added nested try/finally blocks to ensure proper cleanup even on exceptions
• Added 5 new tests that verify contextvar isolation and cleanup

Testing

Added comprehensive tests including:

• test_contextvar_reset_on_reentrant_call - Demonstrates the stale contextvar issue (fails without fix, passes with fix)
• test_contextvar_not_leaked_to_caller - Verifies no leakage outside context
• test_contextvar_isolation_between_tasks - Confirms isolation between sequential tasks
• test_contextvar_cleanup_after_task - Ensures cleanup after task completion
• test_async_exit_stack_cleanup - Verifies AsyncExitStack cleanup

All 272 tests pass with 100% coverage maintained in src/docket/dependencies.py.

🤖 Generated with Claude Code

[claude]

Claude finished @chrisguidry's task —— View job

---

Code Review Complete ✅

Summary

This PR correctly addresses a contextvar cleanup issue in the resolved_dependencies function. The implementation follows best practices for contextvar management and includes comprehensive test coverage.

✅ Strengths

1. Correct Pattern Implementation: The fix properly captures reset tokens from all 5 contextvar set() calls and resets them in nested try/finally blocks, following Python's documented best practices for contextvar cleanup.

2. Proper Exception Handling: The nested try/finally structure ensures cleanup happens even if exceptions occur:

   • Inner finally resets stack token
   • Outer finally resets all other tokens in reverse order

3. Comprehensive Test Coverage: Five well-designed tests verify:

   • Reentrant call behavior
   • No leakage to caller context
   • Isolation between sequential tasks
   • Proper cleanup after task completion
   • AsyncExitStack resource cleanup

4. 100% Coverage Maintained: All changes maintain the project's 100% test coverage requirement.

🔍 Code Quality

The implementation is clean and follows the existing codebase patterns:

• Proper token capture at the beginning (docket_token, worker_token, etc.)
• Correct reset order in finally blocks (reverse of set order)
• No performance impact - just adds token tracking

✅ Security Considerations

No security concerns identified. The change actually improves security by preventing potential context leakage between tasks.

✅ Performance

Minimal performance impact - only adds token capture/reset operations which are negligible compared to task execution overhead.

🎯 Bug Fix Verification

The fix correctly addresses the "stack is closed" errors that could occur in reentrant scenarios. The test test_contextvar_reset_on_reentrant_call specifically verifies this case works properly now.

Recommendation

APPROVED ✅ - This is a solid fix that correctly implements contextvar cleanup. The pattern matches Python's recommended approach and the test coverage is thorough. Ready to merge.

[github-actions]

📚 Documentation has been built for this PR!

You can download the documentation directly here:
https://github.com/chrisguidry/docket/actions/runs/18979058840/artifacts/4431861829

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (2822eaa) to head (37d43c9).

Additional details and impacted files

@@            Coverage Diff             @@
##              main      #178    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           34        34            
  Lines         5107      5216   +109     
  Branches       264       265     +1     
==========================================
+ Hits          5107      5216   +109     

Flag	Coverage Δ
python-3.10	100.00% <100.00%> (ø)
python-3.11	98.79% <100.00%> (+0.02%)	⬆️
python-3.12	100.00% <100.00%> (ø)
python-3.13	100.00% <100.00%> (ø)
python-3.14	100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/docket/dependencies.py	100.00% <100.00%> (ø)
tests/test_dependencies.py	100.00% <100.00%> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jakekaplan]

This makes sense to me as described! Although it does seem like there is a test failing still and I can't understand why 🤔

[jakekaplan]

Oh actually that's unrelated and just a task ordering expectation... carry on!