Skip to content

Fix tmux session cleanup for OMX job lifecycle#7

Merged
vkehfdl1 merged 1 commit intodevfrom
Feature/#4
Mar 22, 2026
Merged

Fix tmux session cleanup for OMX job lifecycle#7
vkehfdl1 merged 1 commit intodevfrom
Feature/#4

Conversation

@vkehfdl1
Copy link
Copy Markdown
Contributor

Summary

  • add explicit session finalization in DaniService so completed and failed jobs actively close their tmux session
  • record session end metadata (ended_at, termination_reason) for lifecycle visibility
  • cover service-level finalization and low-level tmux close behavior with regression tests

Why

Issue #4 is satisfied under the omx resume interpretation of lifecycle: the durable context lives in omx_session_id, while tmux is only the temporary execution shell. That means we should close tmux after each run and recreate a fresh tmux shell on resume, rather than leave stray tmux sessions behind.

Validation

  • python -m pytest -q
  • python -m ruff check .

Closes #4

Session cleanup was implicit and depended on tmux exiting on its own, which left lifecycle completion ambiguous and could strand tmux sessions after successful or failed runs.

This change adds an explicit session finalization step in DaniService, records when and why a session ended, and teaches OmxRunner how to kill an existing tmux session safely. Tests now cover both successful and failed finalization paths as well as the low-level tmux close behavior.

Constraint: Must preserve the existing omx resume flow that uses omx_session_id across fresh tmux shells
Rejected: Full stage lifecycle state machine | too broad for the issue and unnecessary to stop stray tmux sessions
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep tmux cleanup centralized in service finalization so future lifecycle changes do not duplicate termination logic
Tested: python -m pytest -q; python -m ruff check .
Not-tested: Real tmux + omx integration against a live GitHub event stream
@vkehfdl1 vkehfdl1 merged commit 578ec2f into dev Mar 22, 2026
6 checks passed
@vkehfdl1 vkehfdl1 mentioned this pull request Mar 22, 2026
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.

1 participant