feat(desktop-main): rotate Winston logs in userData with daily rotation#226
Merged
Merged
Conversation
…ency Refs #149. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…th DailyRotateFile Refs #149. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…fore bootstrap Refs #149. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…h resolution Refs #149. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…path endpoints Refs #149. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n AppModule Refs #149. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…vice Refs #149. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…toscroll Refs #149. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…icsPanel Refs #149. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds daily-rotating file logging to the Electron main process (writing under the OS-specific userData/logs/ directory) and surfaces basic log diagnostics through new /api/diagnostics/* endpoints plus a Settings → Diagnostics UI panel in the web frontend.
Changes:
- Add
winston-daily-rotate-fileand refactor the desktop-main logger into acreateLogger(logDir)factory that installs a daily-rotating file transport (14-day retention). - Initialize the desktop-main logger using Electron’s
app.getPath('userData')and wire a new diagnostics controller/service to tail today’s log + expose its path. - Add a web
DiagnosticsPanel(polling tail every 5s) and mount it in Settings, with associated tests and API client methods.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| package-lock.json | Locks winston-daily-rotate-file and its transitive deps. |
| app/packages/desktop-main/package.json | Adds winston-daily-rotate-file dependency to desktop-main. |
| app/packages/desktop-main/src/logger.ts | Refactors logger into console fallback + createLogger(logDir) with DailyRotateFile transport. |
| app/packages/desktop-main/src/logger.test.ts | Tests createLogger transport setup and live-binding singleton reassignment. |
| app/packages/desktop-main/src/main.ts | Initializes logger under userData/logs before bootstrapping Nest microservice. |
| app/packages/desktop-main/src/main.test.ts | Verifies createLogger is called with derived userData/logs path during startup. |
| app/packages/desktop-main/src/app.module.ts | Wires diagnostics controller/service and provides DIAGNOSTICS_LOG_DIR token. |
| app/packages/desktop-main/src/services/DiagnosticsService.ts | Implements today’s log path resolution and tail reading (ENOENT-safe). |
| app/packages/desktop-main/src/services/DiagnosticsService.test.ts | Unit tests for log path and tail behavior. |
| app/packages/desktop-main/src/controllers/diagnostics.controller.ts | Adds /diagnostics/tail and /diagnostics/path endpoints. |
| app/packages/desktop-main/src/controllers/diagnostics.controller.test.ts | Unit tests for diagnostics controller behavior. |
| app/packages/web/src/api.service.ts | Adds api.diagnosticsTail() and api.diagnosticsLogPath() client helpers. |
| app/packages/web/src/components/DiagnosticsPanel.tsx | Adds polling diagnostics UI panel with loading/error states + autoscroll. |
| app/packages/web/src/components/DiagnosticsPanel.test.tsx | Tests DiagnosticsPanel loading, success, error, and polling behavior. |
| app/packages/web/src/pages/settings.page.tsx | Adds a new “Diagnostics” section and mounts DiagnosticsPanel. |
| app/packages/web/src/pages/settings.page.test.tsx | Updates SettingsPage tests to include Diagnostics section/panel. |
Comments suppressed due to low confidence (1)
app/packages/desktop-main/src/main.ts:21
- The Electron runtime guard runs after
createLogger(path.join(app.getPath('userData'), 'logs')). In a non-Electron execution (e.g.node dist/main.js),appmay be undefined/incorrect and this will throw before the intended readable error is raised. Move theprocess.versions['electron']check above anyelectronAPI usage (and only callcreateLoggeronce the guard passes).
applyFixPath();
createLogger(path.join(app.getPath('userData'), 'logs'));
// ElectronIPCTransport requires ipcMain, which is only available inside an
// Electron main process. Fail fast with a readable message rather than a
// cryptic module-resolution error when someone runs `node dist/main.js`.
if (!process.versions['electron']) {
throw new Error(
'desktop-main must run inside an Electron main process. ' +
'Launch via Electron — running with plain Node is not supported.',
);
…ach poll readTail() previously read the entire log file with readFile() on every 5-second poll. Replaced with fs.open/FileHandle.read to seek directly to max(0, size - 200 KB) and read only the tail window, capping memory and I/O regardless of how large the daily log grows. Refs #149. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CoderCoco
changed the title
…tron import
- main.ts: move process.versions['electron'] guard above app.getPath() so a
friendly error is raised before any Electron API is touched in non-Electron envs
- app.module.ts: remove static `import { app } from 'electron'`; DIAGNOSTICS_LOG_DIR
factory now uses createRequire lazily, guarded by process.versions['electron'],
with DIAGNOSTICS_LOG_DIR env-var fallback for non-Electron contexts (test-main)
- DiagnosticsService: use bytesRead from FileHandle.read() to slice the buffer
before toString(), preventing null-byte corruption on short reads
Refs #149.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Split logger formats so Console gets colorized output in dev while DailyRotateFile uses a plain format with no ANSI escape codes, keeping log files safe for parsers and the diagnostics tail endpoint. Guard DiagnosticsPanel fetchData state updates behind the cancellation flag so in-flight requests resolved after unmount do not attempt to set state on an unmounted component. Refs #149. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the static `import { app } from 'electron'` with a top-level
`await import('electron')` placed after the `process.versions['electron']`
guard. The npm electron package does not export `app` in plain Node
environments, so the static import would silently give `undefined`. The
dynamic import runs only when the guard has confirmed we are inside an
Electron main process, preventing any evaluation failure before the
friendly error message can be thrown.
Refs #149.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Panel vi.clearAllTimers() must only be called while fake timers are active. The polling test already calls vi.useRealTimers() at the end of the test body, and RTL cleanup handles component unmount for all other tests, so the global afterEach call is unnecessary. Refs #149. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Electron Falling back to '' caused getTodayLogPath() to return a relative path, which could accidentally open a same-named file from the process CWD instead of returning ENOENT. os.tmpdir() is an absolute, writable directory that is unlikely to contain a matching log file, so readTail() safely returns [] in non-Electron / test environments. Refs #149. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…lete first line When offset > 0, readTail() now reads from offset-1 rather than offset. The extra byte ensures the first split element is always either '' (offset landed on a newline) or a partial fragment (offset landed mid-line) — safe to drop in both cases. Previously, if the offset coincided exactly with the start of a complete line, that line was incorrectly discarded. Adds a test asserting the first line at an exact boundary survives. Refs #149. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace hard-coded POSIX separators with node:path so the assertions are platform-independent on Windows as well as POSIX systems. Refs #149. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The block had a stale comment claiming it reset module state but its body was empty. `initialLogger` is captured at static import time (before any test runs) so isolation is already guaranteed; no reset hook is needed. Refs #149. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment on lines
+76
to
+78
| it('should poll for new lines every 5 seconds', async () => { | ||
| vi.useFakeTimers({ shouldAdvanceTime: true }); | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #149
Summary
winston-daily-rotate-file ^5.0.0in@hyveon/desktop-mainand refactorslogger.tsto acreateLogger(logDir)factory that adds aDailyRotateFiletransport writingmain-YYYY-MM-DD.logunder a caller-supplied directory (14-day retention, daily rotation at midnight)main.tsresolvesapp.getPath('userData')from Electron and callscreateLogger(path.join(userData, 'logs'))beforebootstrap(), ensuring log files land in the correct OS-specific user data directory on every platformDiagnosticsService(reads last N lines from today's log file, ENOENT-safe) andDiagnosticsController(GET /api/diagnostics/tail,GET /api/diagnostics/path) wired intoAppModulewith aDIAGNOSTICS_LOG_DIRinjection tokenDiagnosticsPanelReact component that fetchesapi.diagnosticsTail()on mount, polls every 5 seconds, autoscrolls to the bottom, and surfaces loading/error states; mounted in a new "Diagnostics" section onSettingsPageImplementation notes
logger.tssingleton is exported asexport let logger(ESM live binding) so existing import sites (LogsService, etc.) receive the updated instance aftermain.tscallscreateLogger— no refactor of call sites neededwinston-daily-rotate-filev5.0.0 is CJS but importable from ESM hosts in Node.js; v5 also ships bundled TypeScript declarations, so no@types/package is requiredDIAGNOSTICS_LOG_DIRAppModule provider usesuseFactory: () => path.join(electronApp.getPath('userData'), 'logs')— this call is safe because the factory only runs during Nest bootstrap, whichmain.tsdefers until aftercreateLoggerhas already resolved the pathwindow.gsd.diagnostics) was intentionally omitted — the web layer uses the same HTTPGET /api/*pattern as every other feature; no IPC bridge is neededdiagnosticsLogPathis fetched once on mount only (stable across the component lifetime);diagnosticsTailis re-fetched every 5 secondsTest plan
npm run app:test— 525 tests pass (50 test files, 0 failures)npm run app:lint— 0 errors~/Library/Application Support/<app>/logs/main-YYYY-MM-DD.log(macOS) or~/.config/<app>/logs/...(Linux)