feat(desktop): refine session status reminder summaries#446
Conversation
github-actions
Bot
added
the
area:agent-service
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent 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)
🧰 Additional context used🪛 ast-grep (0.43.0)apps/desktop/src/services/AgentService/task/center.ts[warning] 50-52: 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. (regexp-from-variable) [warning] 53-55: 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. (regexp-from-variable) [warning] 56-58: 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. (regexp-from-variable) [warning] 59-61: 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. (regexp-from-variable) [warning] 62-64: 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. (regexp-from-variable) 🔇 Additional comments (13)
📝 WalkthroughSummary by CodeRabbit
WalkthroughReplaces whitespace-based reminder summarization with a locale-aware, markdown/HTML-to-plain-text pipeline (summarizeNotificationText) and applies it to completed, waiting-approval, waiting-user-question, and failed session reminders; tests validate sanitization, command previews, and edge cases. ChangesNotification text sanitization pipeline
Sequence DiagramsequenceDiagram
participant Caller
participant summarizeNotificationText
participant reminderMarkdownParser
Caller->>summarizeNotificationText: request(text, mode, locale)
summarizeNotificationText->>reminderMarkdownParser: parse markdown/HTML tokens
summarizeNotificationText->>summarizeNotificationText: extract clauses, dedupe, join, truncate
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)
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!
| const REMINDER_MARKDOWN_ESCAPE_PATTERN = /\\([\\`*_{}[\]()#+.!>-])/g; | ||
| const REMINDER_PATH_LIKE_TOKEN_PATTERN = /\S*[\\/]\S*/g; | ||
| const REMINDER_PATH_WRAPPER_TRIM_PATTERN = /^[("'[{]+|[)"'\],.;:!?}]+$/g; | ||
| const REMINDER_PATH_SEGMENT_PATTERN = '(?:[A-Za-z0-9._-]+|\\*{1,2})'; |
There was a problem hiding this comment.
REMINDER_PATH_SEGMENT_PATTERN currently does not allow @, so scoped package paths like packages/@touchai/__tests__/center.test.ts are not treated as path-like. That means the markdown pass can still parse __tests__ as emphasis and corrupt the notification path. Please add a regression test for scoped package paths and loosen the segment matcher.
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.