Conversation
WalkthroughAdds filesystem-based log rotation and pruning to emain/log.ts. New helpers: findHighestLogNumber(logsDir) and pruneOldLogs(logsDir) (keeps five most recent rotated logs), and rotateLogIfNeeded() which rotates waveapp.log when it exceeds 10 MB, creating the logs directory if missing. Rotation/pruning are invoked at module load inside a try/catch and any non-fatal outcomes/errors are recorded to a module-level variable and emitted via the logger after it’s configured. Keeps waveapp.log as the active file, adds fs imports, and preserves a console transport in development mode. Exported log(...msg: any[]) remains unchanged. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
emain/log.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
emain/log.ts (1)
emain/platform.ts (1)
getWaveDataDir(263-263)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Build for TestDriver.ai
🔇 Additional comments (1)
emain/log.ts (1)
78-80: LGTM! Correct execution order.The invocation sequence is correct: rotation precedes pruning, and both execute before logger configuration. This ensures the log file is rotated if needed before the logger starts writing.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
emain/log.ts (1)
52-54: Continue pruning even if one unlink fails.Right now a single
fs.unlinkSyncerror (e.g., EACCES on one stale log) will throw and abort the whole prune, so the remaining old files are never deleted and the same failure repeats every startup. Wrap each unlink in its own try/catch so we log the failure but keep pruning the rest.Apply this diff:
- for (const logFile of toDelete) { - fs.unlinkSync(path.join(logsDir, logFile.name)); - } + for (const logFile of toDelete) { + const filePath = path.join(logsDir, logFile.name); + try { + fs.unlinkSync(filePath); + } catch (err) { + console.error(`Failed to delete old log ${filePath}:`, err); + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
emain/log.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Build for TestDriver.ai
emain/log.ts
Outdated
| logger.error("error rotating/pruning logs (non-fatal):", logRotateError); | ||
| } | ||
| if (rotatedPath != null) { | ||
| logger.info("rotated old log file to:", rotatedPath); | ||
| } | ||
| if (prunedFiles.length > 0) { | ||
| logger.info("pruned old log files:", prunedFiles.join(", ")); |
There was a problem hiding this comment.
Fix dropped metadata in winston logs.
Because the logger uses a printf formatter that only renders info.message, the second argument you pass to logger.error/info never shows up—no error stack, no rotated file path, no pruned filenames. Inline the data into the message (or extend the formatter) so we actually see these details.
Apply this diff:
-if (logRotateError != null) {
- logger.error("error rotating/pruning logs (non-fatal):", logRotateError);
-}
-if (rotatedPath != null) {
- logger.info("rotated old log file to:", rotatedPath);
-}
-if (prunedFiles.length > 0) {
- logger.info("pruned old log files:", prunedFiles.join(", "));
-}
+if (logRotateError != null) {
+ const errStr = logRotateError instanceof Error ? logRotateError.stack ?? logRotateError.message : `${logRotateError}`;
+ logger.error(`error rotating/pruning logs (non-fatal): ${errStr}`);
+}
+if (rotatedPath != null) {
+ logger.info(`rotated old log file to: ${rotatedPath}`);
+}
+if (prunedFiles.length > 0) {
+ logger.info(`pruned old log files: ${prunedFiles.join(", ")}`);
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In emain/log.ts around lines 109 to 115, the winston logger uses a printf
formatter that only renders message text so the second argument is dropped;
change the three calls to inline the metadata into the message string: include
logRotateError (use logRotateError.stack || String(logRotateError)) in the error
message, include rotatedPath directly in the info message, and include
prunedFiles.join(", ") inside the info message so the details are part of
info.message and will render with the current formatter.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
emain/log.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
emain/log.ts (1)
emain/platform.ts (1)
getWaveDataDir(263-263)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (go)
- GitHub Check: Build for TestDriver.ai
| if (logRotateError != null) { | ||
| log("error rotating/pruning logs (non-fatal):", logRotateError); | ||
| } | ||
| if (rotatedPath != null) { | ||
| log("rotated old log file to:", rotatedPath); | ||
| } | ||
| if (prunedFiles.length > 0) { | ||
| log("pruned old log files:", prunedFiles.join(", ")); | ||
| } | ||
| if (pruneError != null) { | ||
| log("error pruning some log files (non-fatal):", pruneError); | ||
| } |
There was a problem hiding this comment.
Restore error-level logging for rotation failures.
logRotateError and pruneError used to be emitted through logger.error, but now they go through log(...), which always logs at INFO. That masks actual failures from error-level monitors/alerts. Please keep these cases at ERROR while still formatting the message inline.
-if (logRotateError != null) {
- log("error rotating/pruning logs (non-fatal):", logRotateError);
-}
+if (logRotateError != null) {
+ logger.error(format("error rotating/pruning logs (non-fatal): %o", logRotateError));
+}
...
-if (pruneError != null) {
- log("error pruning some log files (non-fatal):", pruneError);
-}
+if (pruneError != null) {
+ logger.error(format("error pruning some log files (non-fatal): %o", pruneError));
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (logRotateError != null) { | |
| log("error rotating/pruning logs (non-fatal):", logRotateError); | |
| } | |
| if (rotatedPath != null) { | |
| log("rotated old log file to:", rotatedPath); | |
| } | |
| if (prunedFiles.length > 0) { | |
| log("pruned old log files:", prunedFiles.join(", ")); | |
| } | |
| if (pruneError != null) { | |
| log("error pruning some log files (non-fatal):", pruneError); | |
| } | |
| if (logRotateError != null) { | |
| logger.error(format("error rotating/pruning logs (non-fatal): %o", logRotateError)); | |
| } | |
| if (rotatedPath != null) { | |
| log("rotated old log file to:", rotatedPath); | |
| } | |
| if (prunedFiles.length > 0) { | |
| log("pruned old log files:", prunedFiles.join(", ")); | |
| } | |
| if (pruneError != null) { | |
| logger.error(format("error pruning some log files (non-fatal): %o", pruneError)); | |
| } |
🤖 Prompt for AI Agents
In emain/log.ts around lines 129 to 140, error cases for logRotateError and
pruneError are being emitted via log(...) (INFO) instead of logger.error; change
those two branches to call logger.error and include the formatted inline message
plus the error object (e.g. logger.error("error rotating/pruning logs
(non-fatal): %s", String(logRotateError), logRotateError)) so the error-level
monitor receives the event and the error details/stack are preserved; keep the
rotatedPath and prunedFiles branches using log(...) as-is.
Merges 59 commits from wavetermdev/waveterm v0.12.0 release into a5af fork. Resolves 49 merge conflicts across frontend, backend, and configuration files. ## Major Upstream Features Integrated ### AI Enhancements (v0.12.0) - AI Response Feedback + Copy Buttons (wavetermdev#2457) - Reasoning Deltas Display (wavetermdev#2443) - Google AI File Summarization (wavetermdev#2455) - `wsh ai` Command Reimplementation (wavetermdev#2435) - Terminal Context Improvements (wavetermdev#2444) - Batch Tool Approval System - Enhanced AI Panel with welcome message - Context menu support for AI messages ### Infrastructure Updates - Mobile User Agent Emulation for web widgets (wavetermdev#2454) - OSC 7 Support for Fish & PowerShell shells (wavetermdev#2456) - Log Rotation System (wavetermdev#2432) - Onboarding improvements - React 19 compatibility updates - Tailwind v4 migration progress - Dependency updates (50+ commits) ## Fork Features Preserved ✅ **Horizontal Widget Bar** (tabbar.tsx) - Widgets remain in horizontal tab bar (not reverted to sidebar) - Fork-specific layout maintained ✅ **Optional Pane Title Labels** (blockframe.tsx) - Auto-generated pane titles preserved - Custom block rendering logic intact ✅ **Layout Model Modifications** (layoutModel.ts) - Fork's widget positioning logic maintained - Horizontal layout integration preserved ## Conflict Resolution Summary **Configuration (8 files):** - Accepted upstream: .golangci.yml, Taskfile.yml, package.json, go.mod, etc. - All dependencies updated to v0.12.0 levels **Backend AI (13 files):** - Accepted upstream: All pkg/aiusechat/ files - New AI tools: read_dir, screenshot, terminal context - Enhanced OpenAI backend with reasoning support **Frontend AI Panel (12 files):** - Accepted upstream: All frontend/app/aipanel/ files - New features: reasoning display, feedback buttons, welcome message - Enhanced message handling and UX **Backend Infrastructure (7 files):** - Accepted upstream: emain, pkg/telemetry, pkg/wcore, pkg/wshrpc - Updated RPC types and telemetry data structures **Frontend Fork Features (8 files):** - Preserved fork: blockframe.tsx, tabbar.tsx, layoutModel.ts - Accepted upstream: keymodel.ts, wshclientapi.ts, termwrap.ts, etc. **Deleted Files (1 file):** - Removed: frontend/app/modals/tos.tsx (deleted upstream) ## Files Changed - Configuration: 8 files - Backend: 20+ files - Frontend: 25+ files - Total staged: 135 files ## Testing Required 1. Verify AI panel functionality (reasoning, feedback, tools) 2. Test horizontal widget bar (fork feature) 3. Test pane title labels (fork feature) 4. Build verification: `npm install && npm run build:dev` 5. Backend build: `go build ./...` 6. Full test suite: `npm test` ## Known Issues⚠️ Widget bar integration may need review - upstream removed widget sidebar⚠️ Layout changes may conflict with horizontal widget positioning⚠️ React 19 compatibility should be tested thoroughly ## Rollback If issues arise, rollback available at: - Tag: fork-v0.11.6-pre-v0.12-merge - Branch: backup-pre-v0.12-merge ## Next Steps 1. Test all functionality thoroughly 2. Fix any runtime errors from merge 3. Review fork feature integration points 4. Update documentation if needed 5. Create PR to main after validation --- 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 participant
No description provided.