feat(desktop): refine session status reminder summaries#446
Conversation
github-actions
Bot
added
the
area:agent-service
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces whitespace-based reminder summarization with a locale-aware, markdown/HTML-to-plain-text pipeline ( ChangesNotification text sanitization pipeline
Sequence DiagramsequenceDiagram
participant Caller
participant summarizeNotificationText
participant reminderMarkdownParser
Caller->>summarizeNotificationText: request(text, mode, locale)
summarizeNotificationText->>summarizeNotificationText: normalize text & protect paths
summarizeNotificationText->>reminderMarkdownParser: parse markdown/HTML tokens
summarizeNotificationText->>summarizeNotificationText: extract clauses, dedupe
summarizeNotificationText->>summarizeNotificationText: join with mode-aware separators
summarizeNotificationText->>summarizeNotificationText: truncate to max chars
summarizeNotificationText->>Caller: return sanitized plain-text summary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
|
Is this solution secure and stable enough? I might lean more towards using an existing Markdown library for parsing |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/services/AgentService/task/center.ts`:
- Around line 51-66: extractReminderInlineText currently assumes a non-standard
token.type === 'link' and also has fallback fields (text/raw) while the parser
and markstream-vue use markdown-it tokens; update extractReminderInlineText to
prefer token.content but also recognize and handle markdown-it link_open and
link_close tokens by recursing into token.children (or the tokens between
link_open/link_close) to extract inner text, rather than only checking for type
'link' or text/raw fields; reference ReminderMarkdownToken,
extractReminderInlineText, and token types 'link', 'link_open', 'link_close'
when locating and updating the logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2d74536f-d945-4769-a9ac-15a47578260d
📒 Files selected for processing (1)
apps/desktop/src/services/AgentService/task/center.ts
📜 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). (6)
- GitHub Check: Frontend Quality
- GitHub Check: Rust Checks
- GitHub Check: Frontend Tests
- GitHub Check: CodeQL (rust)
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: Desktop E2E Smoke (Windows)
🔇 Additional comments (7)
apps/desktop/src/services/AgentService/task/center.ts (7)
3-4: LGTM!
95-126: LGTM!
127-172: LGTM!
174-228: LGTM!
230-323: LGTM!
325-426: LGTM!
428-481: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/services/AgentService/task/center.ts`:
- Around line 135-156: The regexes inside isReminderPathLikeToken are rebuilt on
every call; hoist pathSegment and the five RegExp instances
(posixRelativePattern, windowsRelativePattern, posixAbsolutePattern,
windowsAbsolutePattern, windowsUncPattern) to module scope as constants
(alongside REMINDER_PATH_WRAPPER_TRIM_PATTERN) and have isReminderPathLikeToken
reuse those precompiled patterns, preserving the same test logic and return
conditions; ensure the module-scope patterns use the same string templates so
behavior remains identical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4a6bacf5-9969-4fac-a3dd-232ac1ac2507
📒 Files selected for processing (2)
apps/desktop/src/services/AgentService/task/center.tsapps/desktop/tests/services/AgentService/task/center-reminder.test.ts
📜 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). (6)
- GitHub Check: Rust Checks
- GitHub Check: Frontend Quality
- GitHub Check: Frontend Tests
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (rust)
- GitHub Check: Desktop E2E Smoke (Windows)
🧰 Additional context used
🪛 ast-grep (0.43.0)
apps/desktop/src/services/AgentService/task/center.ts
[warning] 138-138: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^(?:${pathSegment}/)+${pathSegment}$)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 139-139: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^(?:${pathSegment}\\\\)+${pathSegment}$)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 140-142: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(
^(?:/|\\.{1,2}/|~/)(?:${pathSegment}/)*${pathSegment}$
)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 143-145: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(
^(?:[A-Za-z]:\\\\|\\.{1,2}\\\\|~\\\\)(?:${pathSegment}\\\\)*${pathSegment}$
)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 146-146: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^\\\\\\\\${pathSegment}(?:\\\\${pathSegment})+$)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
🔇 Additional comments (7)
apps/desktop/tests/services/AgentService/task/center-reminder.test.ts (1)
218-250: LGTM!apps/desktop/src/services/AgentService/task/center.ts (6)
1-69: LGTM!
158-202: LGTM!
204-224: LGTM!
249-257: LGTM!
276-482: LGTM!
484-600: LGTM!
3 participants
Summary
Refine desktop session status reminder notifications so they read like plain-text summaries instead of raw markdown fragments.
User-facing impact:
--joined fragments.Related issue or RFC
Link the issue or RFC:
For code changes, link the tracking issue. Only documentation wording, link fixes, or comment-only cleanups may skip the issue-first flow.
AI assistance disclosure
If AI tools materially assisted this contribution, disclose that here or point to the relevant commit trailer.
Testing evidence
List the commands you ran and the results you observed.
pnpm --filter desktop exec tsc --noEmit --pretty false— passedpnpm --filter desktop test -- --run tests/services/AgentService/task/center-reminder.test.ts— passedpnpm --filter desktop test -- --run tests/composables/SearchView/useSearchPage.test.ts— passedpnpm --filter desktop test -- --run tests/services/native-service.test.ts— passedpnpm --filter desktop test -- --run tests/services/NotificationService/i18n.test.ts— passedpnpm tauri build— passed1.1.1; verified reminder notifications against manual desktop testing flowNot run locally:
pnpm test:prpnpm test:coverage:rustpnpm test:e2eDid you follow TDD (test-first) for feature and fix work? Strongly recommended. See docs/testing/testing.md.
Risk notes
Screenshots or recordings
Checklist
[WIP]or similar title prefixes.AgentService, runtime, MCP, or schema boundaries, there is an accepted RFC.pnpm test:prfor this code PR, or this is a docs-only change.pnpm test:coverage:rustor relied on CI coverage evidence.pnpm test:e2elocally or documented why CI is the first valid proof.