open-webui/open-terminal

Fork 0

mirror of https://github.com/open-webui/open-terminal.git synced 2026-11-05 01:27:28 +00:00

[PR #149] fix: process log retention never reached files with no in-memory record #138

New issue

Open

opened 2026-10-18 21:11:08 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-18 21:11:08 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/open-webui/open-terminal/pull/149
Author: @dvystrcil
Created: 8/1/2026
Status: 🔄 Open

Base: main ← Head: fix/log-retention-sweep

📝 Commits (1)

601c58f fix: process log retention never reached files with no in-memory record

📊 Changes

3 files changed (+72 additions, -1 deletions)

View changed files

📝 CHANGELOG.md (+6 -0)
📝 open_terminal/main.py (+12 -1)
📝 open_terminal/utils/log.py (+54 -0)

📄 Description

Problem

_cleanup_expired()'s log-file deletion only runs when a finished log file also has a matching in-memory BackgroundProcess record:

for process_id in expired:
    bp = _processes.pop(process_id)
    if bp.log_path and bp.finished_at and now - bp.finished_at > PROCESS_LOG_RETENTION:
        os.remove(bp.log_path)

_processes is an in-memory dict that resets to empty on every process restart. The JSONL log files under LOG_DIR/processes/ live on disk (frequently a persistent volume in containerized deployments) and survive restarts. So any log file older than the last restart is permanently unreachable by this cleanup path, regardless of age -- the in-memory record it would need to be found via is simply gone.

I ran into this concretely: a long-forgotten process log (months old) was still sitting on disk, and it contained the plaintext output of a command that had echoed a secret to stdout. PROCESS_LOG_RETENTION defaults to 7 days, but nothing was actually enforcing that once the server had restarted even once since that log was written.

Fix

_cleanup_expired() now also sweeps the log directory by file mtime directly (open_terminal/utils/log.py's new sweep_expired_log_files), independent of any in-memory record. Since _cleanup_expired() runs on every process-status-check request (/execute list, individual status polls), the actual directory scan is rate-limited to once per 5 minutes (sweep_expired_log_files_rate_limited) to avoid adding a full os.listdir() on every request under active polling.

Kept the fix minimal and reactive (piggybacking on the existing _cleanup_expired() call sites) rather than introducing new background-task/scheduling infrastructure, since none currently exists in this codebase.

Testing

No test harness exists in this repo currently, so verified manually and end-to-end:

Unit-level: confirmed sweep_expired_log_files deletes only .jsonl files older than the retention window, leaves recent and non-.jsonl files alone, and the rate-limited wrapper correctly no-ops on a second immediate call.
End-to-end: started the real server, manually placed an orphaned .jsonl file (old mtime, no in-memory record -- simulating exactly the reported scenario), hit GET /execute, confirmed the file was deleted.

Happy to add a tests/ directory with these as real pytest cases if you'd like that as part of this PR rather than a separate contribution.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/open-webui/open-terminal/pull/149 **Author:** [@dvystrcil](https://github.com/dvystrcil) **Created:** 8/1/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/log-retention-sweep` --- ### 📝 Commits (1) - [`601c58f`](https://github.com/open-webui/open-terminal/commit/601c58f323afe40a14455cea556a03d93b988ad5) fix: process log retention never reached files with no in-memory record ### 📊 Changes **3 files changed** (+72 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `CHANGELOG.md` (+6 -0) 📝 `open_terminal/main.py` (+12 -1) 📝 `open_terminal/utils/log.py` (+54 -0) </details> ### 📄 Description ## Problem `_cleanup_expired()`'s log-file deletion only runs when a finished log file *also* has a matching in-memory `BackgroundProcess` record: ```python for process_id in expired: bp = _processes.pop(process_id) if bp.log_path and bp.finished_at and now - bp.finished_at > PROCESS_LOG_RETENTION: os.remove(bp.log_path) ``` `_processes` is an in-memory dict that resets to empty on every process restart. The JSONL log files under `LOG_DIR/processes/` live on disk (frequently a persistent volume in containerized deployments) and survive restarts. So any log file older than the last restart is permanently unreachable by this cleanup path, regardless of age -- the in-memory record it would need to be found via is simply gone. I ran into this concretely: a long-forgotten process log (months old) was still sitting on disk, and it contained the plaintext output of a command that had echoed a secret to stdout. `PROCESS_LOG_RETENTION` defaults to 7 days, but nothing was actually enforcing that once the server had restarted even once since that log was written. ## Fix `_cleanup_expired()` now also sweeps the log directory by file mtime directly (`open_terminal/utils/log.py`'s new `sweep_expired_log_files`), independent of any in-memory record. Since `_cleanup_expired()` runs on every process-status-check request (`/execute` list, individual status polls), the actual directory scan is rate-limited to once per 5 minutes (`sweep_expired_log_files_rate_limited`) to avoid adding a full `os.listdir()` on every request under active polling. Kept the fix minimal and reactive (piggybacking on the existing `_cleanup_expired()` call sites) rather than introducing new background-task/scheduling infrastructure, since none currently exists in this codebase. ## Testing No test harness exists in this repo currently, so verified manually and end-to-end: - Unit-level: confirmed `sweep_expired_log_files` deletes only `.jsonl` files older than the retention window, leaves recent and non-`.jsonl` files alone, and the rate-limited wrapper correctly no-ops on a second immediate call. - End-to-end: started the real server, manually placed an orphaned `.jsonl` file (old mtime, no in-memory record -- simulating exactly the reported scenario), hit `GET /execute`, confirmed the file was deleted. Happy to add a `tests/` directory with these as real pytest cases if you'd like that as part of this PR rather than a separate contribution. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-18 21:11:08 +00:00

No labels

bug

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

open-webui/open-terminal#138

No description provided.

Rows
Columns