Storing process logs

[Kitenite]

Auto-committed for PR creation

Description

Related Issues

Type of Change

• Bug fix
• New feature
• Documentation
• Refactor
• Other (please describe):

Testing

Screenshots (if applicable)

Additional Notes

Summary by CodeRabbit

• New Features

  • Terminal output history is now saved to log files for later review.

• Bug Fixes

  • Terminal processes are now properly terminated when tabs are deleted, preventing orphaned processes.

[coderabbitai]

Walkthrough

The changes enhance terminal lifecycle management by introducing synchronous directory operations and adding persistent logging of terminal output history. When terminals are killed, their output is saved to log files. Additionally, when tabs are deleted, all associated terminal processes—including those in nested groups—are recursively terminated before tab removal.

Changes

Cohort / File(s)	Summary
Terminal Output Logging apps/desktop/src/main/lib/tmux-manager.ts	Switches from async to synchronous directory creation using mkdirSync. Adds new private method saveTerminalLog() to persist terminal output history under ~/.superset/logs/processes. Integrates logging call in kill() method to capture output before process termination.
Tab Deletion with Cleanup apps/desktop/src/main/lib/workspace/tab-operations.ts	Introduces internal helper function for recursive terminal process termination across tab hierarchies. Integrates pre-cleanup step into tab deletion flow to kill all associated terminals (including nested) before removing tab and updating mosaic tree.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant TabOps as Tab Operations
    participant TmuxMgr as Tmux Manager
    participant FS as File System
    
    User->>TabOps: Delete Tab
    TabOps->>TabOps: Traverse tab tree recursively
    TabOps->>TmuxMgr: Kill terminal in tab
    TmuxMgr->>FS: Save output to log file
    TmuxMgr->>FS: Kill tmux session
    TabOps->>TabOps: Remove tab from tree
    TabOps->>TabOps: Update mosaic layout
    TabOps-->>User: Tab deleted

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

• Areas requiring attention:
  • Recursive terminal cleanup logic in tab deletion to ensure no orphaned processes remain
  • Directory creation path and file permissions for ~/.superset/logs/processes
  • Session data availability check and handling in saveTerminalLog() to prevent log write failures
  • Integration point between kill() method and new logging to verify output capture timing

Poem

🐰 When tabs go away, the terminals go too,
Their whispers are logged in a cozy home-brew,
No orphans left wandering, no messy affairs,
Just tidy transitions and history to spare! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description is entirely empty except for the template scaffold; no actual content, rationale, or implementation details are provided.	Fill in all required template sections including a clear description of changes, related issues, type of change, testing steps, and any additional context about the logging implementation.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Storing process logs' directly aligns with the main change: adding terminal output log persistence via the new saveTerminalLog method.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch when-closing-terminal-kill-the-thread-in-pcbr

---

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

apps/desktop/src/main/lib/tmux-manager.ts (1)

663-697: Consider adding safeguards for log file size and retention.

The implementation correctly saves terminal output to log files with proper error handling. However, consider the following enhancements:

1. File size limits: If a terminal runs for extended periods or produces verbose output, outputHistory could consume significant disk space. Consider truncating extremely large logs or implementing a maximum size threshold.

2. Log rotation: The logs accumulate indefinitely in ~/.superset/logs/processes without cleanup. Consider implementing a retention policy (e.g., delete logs older than 30 days) or limiting the total number of log files.

apps/desktop/src/main/lib/workspace/tab-operations.ts (1)

175-191: Consider adding error handling for terminal process termination.

The recursive cleanup logic is well-structured, but consider the following improvements:

1. Error handling: If tmuxManager.kill() fails for any terminal, the function continues silently. Consider logging failures or collecting them to report back to the caller.

2. Consistency with other imports: The function uses dynamic import for tmuxManager, but createTab (line 105) also imports it dynamically in the same pattern. This is acceptable for lazy loading, but ensure it's intentional.

Example with error handling:

 async function killTerminalProcesses(tab: Tab): Promise<void> {
 	const tmuxManager = await import("../tmux-manager").then((m) => m.default);
 
 	if (tab.type === "terminal") {
-		// Kill the terminal process for this tab
-		// This will trigger log saving in tmuxManager.kill()
-		tmuxManager.kill(tab.id);
+		// Kill the terminal process for this tab
+		// This will trigger log saving in tmuxManager.kill()
+		const success = tmuxManager.kill(tab.id);
+		if (!success) {
+			console.warn(`[TabOperations] Failed to kill terminal process for tab ${tab.id}`);
+		}
 	} else if (tab.type === "group" && tab.tabs) {
 		// Recursively kill terminals in child tabs
 		for (const childTab of tab.tabs) {
 			await killTerminalProcesses(childTab);
 		}
 	}
 }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e18716b and a390968.

📒 Files selected for processing (2)

• apps/desktop/src/main/lib/tmux-manager.ts (4 hunks)
• apps/desktop/src/main/lib/workspace/tab-operations.ts (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

apps/desktop/src/main/lib/workspace/tab-operations.ts (2)

apps/desktop/src/shared/types.ts (1)

• Tab (44-56)

apps/desktop/src/main/lib/workspace/tab-helpers.ts (1)

• findTab (11-22)

🔇 Additional comments (3)

apps/desktop/src/main/lib/tmux-manager.ts (2)

3-3: LGTM: Synchronous directory operations are appropriate here.

The switch from async mkdir to mkdirSync is consistent with the non-async nature of saveSessionsToDisk and saveTerminalLog. Since directory creation is infrequent and fast, blocking operations are acceptable.

Also applies to: 650-650, 678-678

---

517-523: LGTM: Log saving is properly placed before session termination.

The pre-kill logic correctly retrieves the session and saves the output history before destroying the PTY client and tmux session. This ensures terminal logs are captured even when terminals are forcefully killed.

apps/desktop/src/main/lib/workspace/tab-operations.ts (1)

212-219: LGTM: Terminal cleanup is properly integrated into tab deletion.

The changes correctly ensure that all terminal processes (including those in nested groups) are terminated before the tab is removed. The pre-cleanup step works well with the log-saving mechanism in TmuxManager.kill().