fix: address CodeRabbit review findings from release PR

[jeanfbrito]

Summary

Addresses the legitimate findings from CodeRabbit's review on the release PR #3289.

• Log dedup state reset — error messages now clear the dedup key so subsequent non-error messages aren't wrongly suppressed (info A → error → info A no longer drops the second info A)
• SSL cert regex — now matches "self signed certificate" (spaces/hyphens) in addition to "self.signed" (dots)
• Cookie header redaction — cookie and set-cookie added to sensitive keys in Outlook error context sanitizer
• PdfContent duplicate navigations — useEffect depends only on url now, with proper timeout cleanup
• PDF link guard — uses closest('a') for nested elements, case-insensitive .pdf check, handles query strings and fragments

Test plan

• Verify log dedup: consecutive identical info messages are still suppressed, but info → error → same info logs all three
• Verify SSL cert errors with spaces (self signed certificate) are classified correctly
• Verify context sanitizer redacts cookie headers
• Open a PDF in document viewer, verify no duplicate loads on URL change
• Click a .PDF (uppercase) link inside PDF viewer, verify it's blocked

Summary by CodeRabbit

• Bug Fixes

  • Improved error logging behavior to prevent suppression of subsequent non-error messages after error events
  • Enhanced PDF viewer link handling with improved URL resolution and proper cleanup procedures
  • Expanded error classification to recognize additional SSL/TLS certificate error variations

• Security

  • Strengthened sensitive data protection by detecting cookies in error logs

[coderabbitai]

Walkthrough

Three targeted improvements to logging deduplication, error classification patterns, and PDF content handling. Updates include clearing deduplication keys on errors, expanding TLS/SSL error pattern matching with separator variants, adding cookie-based sensitive key detection, and improving document URL synchronization with proper cleanup and anchor resolution logic.

Changes

Cohort / File(s)	Summary
Logging & Deduplication src/logging/dedup.ts	Modified error-handling behavior in shouldLog and createFileHook to reset deduplication keys (lastIpcKey and lastFileKey) when processing error-level messages, preventing non-error messages from being suppressed by pre-error deduplication state.
Error Classification src/outlookCalendar/errorClassification.ts	Expanded TLS/SSL error regex patterns to match "self-signed" and "certificate expired" variants with flexible separators (., whitespace, -); added cookie and set-cookie to sensitive context key detection.
PDF Content Rendering src/ui/components/ServersView/PdfContent.tsx	Refactored useEffect dependency list and added proper setTimeout cleanup; updated click handler to resolve anchor elements via closest(), parse URLs safely with try/catch, and detect PDF files by path extension rather than tag name.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

type: bug

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'fix: address CodeRabbit review findings from release PR' refers to addressing review findings but does not clearly summarize the actual technical changes made. It's generic and vague, lacking specific context about what was actually fixed.	Consider a more specific title that highlights a primary change, such as 'fix: reset dedup state on error logs and improve PDF link handling' or similar.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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 and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/outlookCalendar/errorClassification.ts (1)

69-70: Minor inconsistency in character class separators.

The self[.\s-]signed pattern includes hyphen - as a separator, but certificate[.\s]has[.\s]expired does not. For consistency, consider adding hyphen to both patterns if "certificate-has-expired" variants may occur in error messages.

Otherwise, the regex updates look correct for matching the common "self signed certificate" and "self-signed certificate" variants.

Optional: Add hyphen separator for consistency

     pattern:
-      /SSL_ERROR|UNABLE_TO_VERIFY|CERT_|ERR_TLS|self[.\s-]signed|certificate[.\s]has[.\s]expired/i,
+      /SSL_ERROR|UNABLE_TO_VERIFY|CERT_|ERR_TLS|self[.\s-]signed|certificate[.\s-]has[.\s-]expired/i,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/outlookCalendar/errorClassification.ts` around lines 69 - 70, Update the
regex pattern used in error classification so both variants accept a hyphen as a
separator: modify the existing pattern entry (the `pattern:` value in
errorClassification.ts) to include `-` in the `certificate[.\s]has[.\s]expired`
portion (make it `certificate[.\s-]has[.\s-]expired`) to match
"certificate-has-expired" and keep consistency with `self[.\s-]signed`.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/outlookCalendar/errorClassification.ts`:
- Around line 69-70: Update the regex pattern used in error classification so
both variants accept a hyphen as a separator: modify the existing pattern entry
(the `pattern:` value in errorClassification.ts) to include `-` in the
`certificate[.\s]has[.\s]expired` portion (make it
`certificate[.\s-]has[.\s-]expired`) to match "certificate-has-expired" and keep
consistency with `self[.\s-]signed`.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6f943b01-a99d-4ae8-a355-9a14cc3fa987

📥 Commits

Reviewing files that changed from the base of the PR and between 41e65be and a949bda.

📒 Files selected for processing (3)

• src/logging/dedup.ts
• src/outlookCalendar/errorClassification.ts
• src/ui/components/ServersView/PdfContent.tsx

📜 Review details

⏰ 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: check (windows-latest)
• GitHub Check: check (macos-latest)
• GitHub Check: check (ubuntu-latest)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use TypeScript strict mode enabled in TypeScript configuration
Use React functional components with hooks instead of class components
Follow FSA (Flux Standard Action) pattern for Redux actions
Use camelCase for file names and PascalCase for component file names
All code must pass ESLint and TypeScript checks
Write self-documenting code with clear naming; avoid unnecessary comments except for complex business logic or non-obvious decisions
Use Fuselage components from @rocket.chat/fuselage for all UI work and only create custom components when Fuselage doesn't provide what's needed
Check Theme.d.ts for valid color tokens when using Fuselage components
Use defensive coding with optional chaining and fallbacks for Linux-only APIs (process.getuid(), process.getgid(), process.geteuid(), process.getegid()) to ensure cross-platform compatibility across Windows, macOS, and Linux

Files:

• src/logging/dedup.ts
• src/outlookCalendar/errorClassification.ts
• src/ui/components/ServersView/PdfContent.tsx

src/outlookCalendar/**/*.{ts,tsx}

📄 CodeRabbit inference engine (src/outlookCalendar/AGENTS.md)

src/outlookCalendar/**/*.{ts,tsx}: Use createClassifiedError() from errorClassification.ts for user-facing errors to provide error categorization, user-friendly messages, and structured error context
Always use outlookError() for errors as it logs regardless of verbose mode settings, ensuring errors are always visible to users

Files:

• src/outlookCalendar/errorClassification.ts

🧠 Learnings (3)

📓 Common learnings

Learnt from: Ram-sah19
Repo: RocketChat/Rocket.Chat.Electron PR: 3254
File: .github/workflows/build-release.yml:80-94
Timestamp: 2026-03-11T06:38:40.426Z
Learning: In the RocketChat/Rocket.Chat.Electron repository, the issues flagged in `.github/workflows/build-release.yml` (e.g., `node12` runtime in the release action and missing `snapcraft_token` input), i18n files, and `electron-builder.json` are pre-existing in the `develop` branch and are pulled in during merge conflict resolution. Do not flag these as new issues introduced by PRs that only modify `src/injected.ts` and `src/ui/main/rootWindow.ts`.

📚 Learning: 2026-03-06T19:31:11.433Z

Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: src/outlookCalendar/AGENTS.md:0-0
Timestamp: 2026-03-06T19:31:11.433Z
Learning: Applies to src/outlookCalendar/**/*.{ts,tsx} : Use `createClassifiedError()` from `errorClassification.ts` for user-facing errors to provide error categorization, user-friendly messages, and structured error context

Applied to files:

• src/outlookCalendar/errorClassification.ts

📚 Learning: 2026-03-06T19:31:11.433Z

Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: src/outlookCalendar/AGENTS.md:0-0
Timestamp: 2026-03-06T19:31:11.433Z
Learning: Applies to src/outlookCalendar/**/*.{ts,tsx} : Always use outlookError() for errors as it logs regardless of verbose mode settings, ensuring errors are always visible to users

Applied to files:

• src/outlookCalendar/errorClassification.ts

🔇 Additional comments (6)

src/logging/dedup.ts (2)

46-49: Good fix: reset IPC dedup state on errors.

Resetting lastIpcKey on error prevents stale dedup state from suppressing the next non-error IPC message.

---

70-74: Good fix: file-hook error path now safely resets dedup state.

Passing through missing messages and clearing lastFileKey on error keeps file transport dedup behavior consistent after failures.

src/ui/components/ServersView/PdfContent.tsx (2)

20-33: Good fix for URL sync and duplicate navigation prevention.

The narrowed dependency ([url]) plus timeout cleanup makes this effect deterministic and prevents stale delayed navigations.

---

49-56: PDF link interception logic is now robust.

closest('a') + case-insensitive pathname matching is a solid improvement for nested targets and .PDF links with query/fragment suffixes.

src/outlookCalendar/errorClassification.ts (2)

232-239: Good addition of cookie headers to sensitive keys.

Adding cookie and set-cookie to the redaction list is a proper security enhancement. These headers can contain session tokens and authentication data that should not appear in logs.

---

241-251: LGTM - defensive sensitive key detection.

The includes('cookie') and includes('authorization') checks provide defense-in-depth by catching variations like proxy-authorization, x-custom-cookie, or any header names containing these sensitive terms. This is appropriate for security-sensitive redaction logic.