[Ehaan-Dadarkar]

What

• New warnMain() helper (electron/src/main/session-log/main-warn.ts) writes to process.stderr and sessionLog.append('main', …) in one call, reusing the supervisor's note() shape.
• All five stderr-only sites now use it: locale/theme/window-state preference save failures, onSidecarCrash, and the startup failure in index.ts.
• The hand-written [main] prefixes are gone from the call sites; the log's source argument carries it instead.
• docs/architecture/issue-reports.md lists main among the log's sources; the Known gaps line is deleted.
• No new dependency.

Why

Fixes #1960. The session log is meant to be "what this run of the app has logged", but main's own warnings went to the terminal only — and a packaged app has no terminal, so they went nowhere. These are the one-line warnings that explain puzzling reports (window in the wrong place, theme reverting, language ignored).

Design note

The startup failure also goes through the helper rather than staying on stderr alone: one way to log (two ways is how the next one gets missed), the stderr output is unchanged, and the log line is harmless — no window exists to read it when startup fails.

How to test

cd surfsense_local/electron
pnpm test
pnpm typecheck

New test in session-log.test.ts: calls warnMain and asserts the line lands in lines() tagged [main] and on stderr with the [main] prefix. 167/167 tests pass, typecheck clean.

High-level PR Summary

This PR routes main process warnings into the session log by introducing a warnMain() helper that writes to both stderr and the session log simultaneously. Previously, these warnings only went to the terminal (which doesn't exist in packaged apps), causing them to be lost. The changes update five call sites (locale/theme/window-state preference save failures, sidecar crashes, and startup failures) to use this new helper, removing hand-written [main] prefixes in favor of a structured source argument.

⏱️ Estimated Review Time: 5-15 minutes

💡 Review Order Suggestion

Order	File Path
1	surfsense_local/electron/src/main/session-log/main-warn.ts
2	surfsense_local/electron/src/main/session-log/session-log.test.ts
3	docs/architecture/issue-reports.md
4	surfsense_local/electron/src/main/i18n/locale-prefs.ts
5	surfsense_local/electron/src/main/theme-prefs.ts
6	surfsense_local/electron/src/main/window-state.ts
7	surfsense_local/electron/src/main/index.ts

Summary by CodeRabbit

• Bug Fixes
  • Session logs now include application warnings alongside sidecar, renderer, and updater output. This includes errors when saving locale, theme, or window preferences, as well as sidecar startup failures and crashes.
  • Main-process warnings continue to appear in the terminal and are now also available in the session log.

[vercel]

@Ehaan-Dadarkar is attempting to deploy a commit to the Rohan Verma's projects Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: MODSetter/SurfSense/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 6ddb967f-e8fc-48fa-88f2-fb0975fce050

📥 Commits

Reviewing files that changed from the base of the PR and between 91b0e09 and f5addaf.

📒 Files selected for processing (7)

• docs/architecture/issue-reports.md
• surfsense_local/electron/src/main/i18n/locale-prefs.ts
• surfsense_local/electron/src/main/index.ts
• surfsense_local/electron/src/main/session-log/main-warn.ts
• surfsense_local/electron/src/main/session-log/session-log.test.ts
• surfsense_local/electron/src/main/theme-prefs.ts
• surfsense_local/electron/src/main/window-state.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

Main-process warnings now use a shared helper that writes to stderr and the session log. Locale, theme, and window-state save failures, sidecar crashes, and startup failures use this helper. The architecture documentation now lists main-process warnings as a session-log source.

Changes

Main-process warning logging

Layer / File(s)	Summary
Shared warning helper and test surfsense_local/electron/src/main/session-log/main-warn.ts, surfsense_local/electron/src/main/session-log/session-log.test.ts	warnMain writes a [main]-prefixed warning to stderr and appends the text to the session log with source main. The test checks both outputs.
Warning call sites and documentation surfsense_local/electron/src/main/i18n/locale-prefs.ts, surfsense_local/electron/src/main/theme-prefs.ts, surfsense_local/electron/src/main/window-state.ts, surfsense_local/electron/src/main/index.ts, docs/architecture/issue-reports.md	Preference-save failures, sidecar crashes, and startup failures now use warnMain. The documentation lists main-process warnings as a session-log source and removes the prior known-gap note.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to f5add

The warning-routing change is ready to merge with no identified actionable risk.

Security Architecture Review

Security architecture risk: 🔵 Low · up to f5add

Warnings can now appear in issue reports. Logs are not uploaded automatically, and users can exclude or edit them before sharing. Home-directory redaction remains in place, but it does not guarantee removal of every sensitive detail from error messages.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The incremental exposure is warning text becoming available to the current application renderer and its existing clipboard/report workflow, rather than a new network service or privileged operation. Some startup failures can occur after a window exists, so lack of a reader is not a universal safeguard.

Trust Boundaries and Controls

• observed — The unchanged IPC handler returns logs only when the sender is the current main window's webContents; other senders receive an empty array. New warning producers do not change that reader authority.
• observed — The unchanged report form requires user submission and offers a log-inclusion toggle, enabled by default. Logs are copied to the clipboard, not placed in the GitHub URL; users are told to review them before manually pasting into a public issue. Clipboard failure stops the external-open action.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 6 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: routing main-process warnings into the session log.
Linked Issues check	✅ Passed	Issue #1960 is directly linked and has coding requirements. The PR adds one warnMain() helper that writes [main] to process.stderr and appends source main to the session log. The reported call…
Out of Scope Changes check	✅ Passed	The reviewed changes stay within issue #1960. They add the shared warning helper, route the five specified main-process warning sites through it, add the required focused test, and update the related …

Full details: Docstring Coverage

Explanation

Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 6 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[MODSetter]

Merged into dev, thanks, and #1960 is closed. All five sites go through the helper, the terminal output is unchanged, and the test fails on dev. Two things for a follow-up PR, neither blocking:

1. issue-reports.md:9 says more than the code does. It says the log carries "a preference file it could not read or write". But the three load functions swallow every read error silently (theme-prefs.ts:32-34, locale-prefs.ts:28-30, window-state.ts:78-80), so only failed writes reach it. Either warn on a read failure other than "file not found", which is what would explain The main process's own warnings never reach the session log #1960's examples, or say "write".
2. Node's and Electron's own process warnings still reach only the terminal. Main registers no process.on("warning"), so a deprecation or MaxListenersExceeded never reaches the log. Either add process.on("warning", (w) => sessionLog.append("main", String(w))), or keep a narrower Known gaps line. Also keep the ## Known gaps heading itself: the other architecture docs keep it, writing "None." when empty.