fix: release lease on unused timeout when hooks are configured

[raballew]

When a lease times out without any client connection and hooks are configured, _cleanup_after_lease skipped the afterLease hook (correct) but also skipped calling _request_lease_release (bug). This left the exporter permanently stuck in LeaseReady status because the controller was never notified that the lease should be freed.

Add _request_lease_release() call in the else branch of _cleanup_after_lease so the controller always frees the lease, regardless of whether the afterLease hook ran.

Fixes: #237

[coderabbitai]

📝 Walkthrough

Walkthrough

Await self._request_lease_release() was added to Exporter._cleanup_after_lease() on the shutdown branch when an unused lease ends; tests were added to assert the lease-release request is awaited once in several unused-lease scenarios.

Changes

Cohort / File(s)	Summary
Exporter shutdown cleanup python/packages/jumpstarter/jumpstarter/exporter/exporter.py	Added awaiting call to self._request_lease_release() in _cleanup_after_lease() when _stop_requested is true and the AVAILABLE status report path is skipped.
Tests for lease-release behavior python/packages/jumpstarter/jumpstarter/exporter/exporter_test.py	Added three async tests in TestUnusedLeaseTimeout asserting _request_lease_release is awaited exactly once for: with hooks, without hooks, and during shutdown (shutdown test also asserts AVAILABLE is not reported).

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• build(deps-dev): update sphinx-inline-tabs requirement from >=2023.4.21 to >=2025.12.21.14 in /python #589: touches exporter shutdown/lease-release flow and the same _request_lease_release() call site.
• Fix exporter state machine: stuck exporters on lease-end during hooks #349: modifies _cleanup_after_lease and lease teardown logic.
• build(deps): bump actions/upload-artifact from 4 to 7 #592: updates Exporter teardown/async context management and overlaps on shutdown/teardown paths.

Suggested reviewers

• evakhoni
• kirkbrauer
• mangelajo

Poem

🐰 A silent lease beneath the moonlit sky,
I tapped the shutdown path and watched it sigh,
One gentle request — a soft, releasing breeze,
Hops of closure through the exporter trees 🌿

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: fixing lease release when hooks are configured during unused timeout.
Description check	✅ Passed	The description is directly related to the changeset, explaining the bug (skipped _request_lease_release call) and the fix (adding it to else branch).
Linked Issues check	✅ Passed	The PR successfully addresses issue #237 by ensuring _request_lease_release is called when a lease times out without client connection, even with hooks configured.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the lease release issue: one-line fix in exporter.py and three new test cases verifying the fix.
Docstring Coverage	✅ Passed	Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@python/packages/jumpstarter/jumpstarter/exporter/exporter.py`:
- Line 625: The guard that skips sending the release RPC when
lease_ended.is_set() causes controller-initiated lease ends to never send the
RPC; update _request_lease_release to accept an optional force: bool parameter
(default False) and only skip when lease_ended.is_set() && !force, then call it
with force=True from the controller-managed path where cleanup runs (e.g., from
_cleanup_after_lease and from run_after_lease_hook / request_lease_release in
hooks.py) so the RPC is sent even if lease_ended was already set; alternatively,
if you prefer ordering, move the call to _request_lease_release() to occur
before lease_ended.set() in serve()’s controller branch, but do one of these
changes so the release RPC is guaranteed to be sent.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 976d3d4e-82a4-4fb4-93df-6c3cd6553189

📥 Commits

Reviewing files that changed from the base of the PR and between f92ac89 and 1fcdcef.

📒 Files selected for processing (2)

• python/packages/jumpstarter/jumpstarter/exporter/exporter.py
• python/packages/jumpstarter/jumpstarter/exporter/exporter_test.py

[coderabbitai]

♻️ Duplicate comments (2)

python/packages/jumpstarter/jumpstarter/exporter/exporter.py (1)

646-646: ⚠️ Potential issue | 🟠 Major

This still won’t send the release RPC in the natural timeout path.

_request_lease_release() bails out once lease_ended is set, and the controller-driven timeout flow sets that flag before _cleanup_after_lease() runs. So this await can still be a no-op in the exact scenario this PR is meant to fix, leaving the exporter stuck in LeaseReady.

Move the release request before lease_ended.set(), or add a force path that bypasses the guard.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/packages/jumpstarter/jumpstarter/exporter/exporter.py` at line 646,
The current await self._request_lease_release() can be skipped because
lease_ended is set before _cleanup_after_lease() runs; update the cleanup so the
release RPC is always attempted: either call self._request_lease_release()
before setting lease_ended (move the call above lease_ended.set()), or modify
_request_lease_release() to accept a force/retry flag that bypasses the
lease_ended guard so the exporter leaves LeaseReady reliably; adjust calls in
_cleanup_after_lease() and any controller-driven timeout path to use the new
ordering or force path and ensure errors are logged/handled.

python/packages/jumpstarter/jumpstarter/exporter/exporter_test.py (1)

190-244: ⚠️ Potential issue | 🟠 Major

These tests don’t exercise the real regression path.

They only verify the mocked _request_lease_release() call. None of them reproduces the controller-initiated timeout sequence where lease_ended is already set, so they can still pass even if the production code skips the release RPC.

Please add coverage that follows the actual cleanup order, or assert the guard is bypassed in the same path the exporter uses at runtime.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/packages/jumpstarter/jumpstarter/exporter/exporter_test.py` around
lines 190 - 244, Add a test that reproduces the controller-initiated timeout
path by setting lease_ctx.lease_ended.set() before calling
exporter._cleanup_after_lease so the guard path taken at runtime is exercised;
specifically, create a lease_ctx with client_name="" and
lease_ctx.before_lease_hook.set(), then set lease_ctx.lease_ended.set() before
invoking exporter._cleanup_after_lease, and assert
exporter._request_lease_release.assert_awaited_once(); do the same variant for
the shutdown case by also setting exporter._stop_requested = True and verifying
the release is still requested and any status reporting behavior (e.g.,
ExporterStatus.AVAILABLE not emitted) remains correct.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@python/packages/jumpstarter/jumpstarter/exporter/exporter_test.py`:
- Around line 190-244: Add a test that reproduces the controller-initiated
timeout path by setting lease_ctx.lease_ended.set() before calling
exporter._cleanup_after_lease so the guard path taken at runtime is exercised;
specifically, create a lease_ctx with client_name="" and
lease_ctx.before_lease_hook.set(), then set lease_ctx.lease_ended.set() before
invoking exporter._cleanup_after_lease, and assert
exporter._request_lease_release.assert_awaited_once(); do the same variant for
the shutdown case by also setting exporter._stop_requested = True and verifying
the release is still requested and any status reporting behavior (e.g.,
ExporterStatus.AVAILABLE not emitted) remains correct.

In `@python/packages/jumpstarter/jumpstarter/exporter/exporter.py`:
- Line 646: The current await self._request_lease_release() can be skipped
because lease_ended is set before _cleanup_after_lease() runs; update the
cleanup so the release RPC is always attempted: either call
self._request_lease_release() before setting lease_ended (move the call above
lease_ended.set()), or modify _request_lease_release() to accept a force/retry
flag that bypasses the lease_ended guard so the exporter leaves LeaseReady
reliably; adjust calls in _cleanup_after_lease() and any controller-driven
timeout path to use the new ordering or force path and ensure errors are
logged/handled.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6b6d9c2e-261f-474d-8ac2-3a5cd51afb22

📥 Commits

Reviewing files that changed from the base of the PR and between d3815de and 403fa8b.

📒 Files selected for processing (2)

• python/packages/jumpstarter/jumpstarter/exporter/exporter.py
• python/packages/jumpstarter/jumpstarter/exporter/exporter_test.py