Repository navigation
Conversation
Fixes manaflow-ai#2044 Native macOS notifications display raw markdown syntax (e.g. **bold**, ## headings, `code`) when triggered by AI agent output. This adds a stripMarkdown helper that removes common markdown formatting before the notification body and subtitle are sent to UNUserNotificationCenter and the custom notification command, so users see clean plain text. Patterns stripped: headings, bold, italic, inline code, links, blockquotes, list markers (unordered and ordered), and horizontal rules.
|
@anthhub is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded a private Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Sources/TerminalNotificationStore.swift (1)
1098-1109: Redundant heading regex - first pattern is superseded by the multiline version.The first regex (lines 1099-1103) with
.anchoredonly matches headings at string start. The second regex (lines 1105-1109) with(?m)^matches headings at the start of any line, including the start of the string. The first call is therefore redundant.♻️ Proposed simplification
- // Headings: ## Heading → Heading - result = result.replacingOccurrences( - of: #"^#{1,6}\s+"#, - with: "", - options: [.regularExpression, .anchored] - ) - // Also handle headings mid-line (after a newline) + // Headings: ## Heading → Heading result = result.replacingOccurrences( of: #"(?m)^#{1,6}\s+"#, with: "", options: .regularExpression )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/TerminalNotificationStore.swift` around lines 1098 - 1109, The first replacingOccurrences call that uses of: #"^#{1,6}\s+"# with options [.regularExpression, .anchored] is redundant because the subsequent call using of: #"(?m)^#{1,6}\s+"# already matches headings at the start of the string and at the start of any line; remove the initial call (the block that targets ^ with .anchored) so only the multiline regex replacement on variable `result` remains, keeping the behavior for both string-start and mid-line headings intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/TerminalNotificationStore.swift`:
- Around line 1098-1109: The first replacingOccurrences call that uses of:
#"^#{1,6}\s+"# with options [.regularExpression, .anchored] is redundant because
the subsequent call using of: #"(?m)^#{1,6}\s+"# already matches headings at the
start of the string and at the start of any line; remove the initial call (the
block that targets ^ with .anchored) so only the multiline regex replacement on
variable `result` remains, keeping the behavior for both string-start and
mid-line headings intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b35efd8c-e2a9-4957-8f0f-7c3e14b2c7c9
📒 Files selected for processing (1)
Sources/TerminalNotificationStore.swift
Greptile SummaryThis PR adds a Key changes:
Issues found:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[TerminalNotification arrives] --> B{Suppressed?}
B -- No --> C[scheduleUserNotification]
B -- Yes --> D[playSuppressedNotificationFeedback]
C --> E[resolvedNotificationTitle]
C --> F["stripMarkdown(subtitle)"]
C --> G["stripMarkdown(body)"]
F & G --> H[UNMutableNotificationContent]
H --> I[UNUserNotificationCenter.add]
I --> J["runCustomCommand(title, stripped subtitle, stripped body)"]
D --> K["stripMarkdown(subtitle)"]
D --> L["stripMarkdown(body)"]
K & L --> M["runCustomCommand(title, stripped subtitle, stripped body)"]
D --> N[playSelectedSound]
subgraph stripMarkdown
direction TB
S1[Strip headings] --> S2[Strip bold+italic ***]
S2 --> S3[Strip bold **]
S3 --> S4["Strip italic * / _ ⚠️ no word-boundary guard"]
S4 --> S5[Strip inline code]
S5 --> S6[Strip links]
S6 --> S7[Strip blockquotes]
S7 --> S8[Strip list markers]
S8 --> S9[Strip horizontal rules]
S9 --> S10[Collapse blank lines]
S10 --> S11[Trim whitespace]
end
Reviews (1): Last reviewed commit: "fix: strip markdown syntax from desktop ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| result = result.replacingOccurrences( | ||
| of: #"_(.+?)_"#, | ||
| with: "$1", | ||
| options: .regularExpression | ||
| ) |
There was a problem hiding this comment.
_italic_ regex mangles snake_case identifiers
The pattern _(.+?)_ does not check word boundaries, so it matches the first pair of underscores in any technical identifier and concatenates the surrounding fragments. For example:
brew_upgrade_formula→_upgrade_is matched →brewupgrade_formula(the leading word loses its separator)CMUX_NOTIFICATION_BODY→_NOTIFICATION_is matched →CMUXBODYCompleted task_1 and task_2→_1 and task_is matched →Completed task1 and task2(data loss between the two items)
This would mangle any notification text that contains technical content with underscores — shell commands, file names, env var names, etc.
Standard Markdown parsers (including CommonMark) do not treat _ as emphasis when it appears inside a word (i.e. flanked by alphanumeric characters on both sides). A word-boundary guard on the italic patterns avoids the false positive:
| result = result.replacingOccurrences( | |
| of: #"_(.+?)_"#, | |
| with: "$1", | |
| options: .regularExpression | |
| ) | |
| result = result.replacingOccurrences( | |
| of: #"(?<!\w)_(.+?)_(?!\w)"#, | |
| with: "$1", | |
| options: .regularExpression | |
| ) |
The same word-boundary issue applies to the bold _{2} pattern at lines 1129-1133, though in practice double underscores in non-markdown contexts are far less common.
| // Headings: ## Heading → Heading | ||
| result = result.replacingOccurrences( | ||
| of: #"^#{1,6}\s+"#, | ||
| with: "", | ||
| options: [.regularExpression, .anchored] | ||
| ) | ||
| // Also handle headings mid-line (after a newline) | ||
| result = result.replacingOccurrences( | ||
| of: #"(?m)^#{1,6}\s+"#, | ||
| with: "", | ||
| options: .regularExpression | ||
| ) |
There was a problem hiding this comment.
The first replacingOccurrences call (lines 1099-1103) uses [.regularExpression, .anchored], which anchors the match to the very start of the string. However, the immediately following call (lines 1105-1109) uses (?m)^#{1,6}\s+ with the multiline flag, which makes ^ match at the start of every line — including the very first character of the string. The first call is therefore entirely redundant: it can never match anything that the second call won't already remove.
| // Headings: ## Heading → Heading | |
| result = result.replacingOccurrences( | |
| of: #"^#{1,6}\s+"#, | |
| with: "", | |
| options: [.regularExpression, .anchored] | |
| ) | |
| // Also handle headings mid-line (after a newline) | |
| result = result.replacingOccurrences( | |
| of: #"(?m)^#{1,6}\s+"#, | |
| with: "", | |
| options: .regularExpression | |
| ) | |
| // Headings: ## Heading → Heading (all lines) | |
| result = result.replacingOccurrences( | |
| of: #"(?m)^#{1,6}\s+"#, | |
| with: "", | |
| options: .regularExpression | |
| ) |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/TerminalNotificationStore.swift`:
- Around line 1129-1139: The italic-stripping regexes are too broad and mangle
plain text; tighten the two replacingOccurrences calls that operate on result by
adding word-boundary lookarounds so they only match standalone markdown italics
(e.g., use patterns with (?<!\w) and (?!\w) around the delimiters for both the
asterisk and underscore variants), leaving the rest of the replacement logic
intact so result still strips legitimate markdown without corrupting arithmetic
or underscored identifiers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 312b8c01-17dd-4af2-9baf-6590f87727d2
📒 Files selected for processing (1)
Sources/TerminalNotificationStore.swift
Bare `_(.+?)_` and `\*(.+?)\*` patterns corrupt snake_case identifiers like CMUX_NOTIFICATION_BODY by treating embedded underscores/asterisks as italic delimiters. Wrap both patterns with `(?<!\w)..(?!\w)` so they only match when the delimiter is not surrounded by word characters. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nderscore regex
Apply the same (?<!\w)...(?!\w) guards to the _{2} and _{3} variants that
were already added to the single-underscore italic regex, preventing false
matches inside snake_case identifiers like my_variable_name or ENV_VAR_NAME.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Hi @austinywang @lawrencecchen 👋 Quick ping — this strips markdown syntax from desktop notification text so users see clean readable text instead of raw |
|
Quick triage request on this PR. This still looks like a useful quality-of-life bugfix to me, but I know there was some review feedback around the markdown-stripping behavior. If maintainers still want this landed, I can do a final pass on the remaining comments and refresh the branch. If not, I am happy to close it and avoid leaving stale review surface around. |
teamleaderleo
added
area: notifications
This branch has not been deployed
2 participants
Summary
**bold**,## headings,`code`,- list items— instead of clean plain text.stripMarkdown(_ text: String) -> Stringprivate helper onTerminalNotificationStorethat removes common markdown formatting before delivering text toUNUserNotificationCenterand the custom notification shell command.What's stripped
## Task completeTask complete**important**important*note*note***critical***critical`git status`git status[see docs](https://example.com)see docs> some quotesome quote- itemitem1. itemitem---Design decisions
TerminalNotification.body/.subtitleso the in-app notifications panel (which may render markdown in the future) keeps the original text.UNUserNotificationCenter(content.subtitle,content.body) and to the custom shell command (CMUX_NOTIFICATION_SUBTITLE,CMUX_NOTIFICATION_BODY) is stripped.Test plan
**bold**or## Heading) and confirm the desktop banner shows plain textCMUX_NOTIFICATION_BODYas plain textTerminalNotificationStoreTestspass without regressions🤖 Generated with Claude Code
Summary by cubic
Fixes #2044. Desktop notifications now strip Markdown from the subtitle and body so macOS shows clean plain text instead of raw formatting.
Bug Fixes
stripMarkdown(_:)inTerminalNotificationStoreto remove headings, emphasis, inline code, links, blockquotes, list markers, and horizontal rules; collapses extra blank lines and adds word-boundary guards for italics and underscore-based__bold__/___bold+italic___to avoid manglingsnake_caseidentifiers.UNUserNotificationCenterand the custom notification command; stored notification text stays unchanged. Titles are not modified.Refactors
stripMarkdown(_:)to simplify the implementation without changing behavior.Written for commit f40ff9a. Summary will update on new commits.
Summary by CodeRabbit