Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion src/note_mcp/api/embeds.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,14 @@
# Example: https://speakerdeck.com/tomohisa/introducing-decider-pattern-with-event-sourcing (Issue #223)
SPEAKERDECK_PATTERN = re.compile(r"^https?://speakerdeck\.com/[\w-]+/[\w-]+$")

# Generic external URL fallback: any other http(s) URL that appears alone on a
# line is embedded as an 'external-article' card via the same
# /v2/embed_by_external_api endpoint already used for Zenn.dev / Qiita /
# connpass. This matches note.com's own editor behavior, where pasting any URL
# on its own line creates a link card. Specific patterns below take precedence
# (checked first in get_embed_service).
GENERIC_EXTERNAL_PATTERN = re.compile(r"^https?://\S+$")

# Data-driven pattern to service mapping (Issue #235: DRY principle)
# Note: GIST_PATTERN and GITHUB_REPO_PATTERN are mutually exclusive by design
# (GIST_PATTERN matches gist.github.com, GITHUB_REPO_PATTERN matches github.com only).
Expand Down Expand Up @@ -123,10 +131,16 @@ def get_embed_service(url: str) -> str | None:
Returns:
Service type ('youtube', 'twitter', 'note', 'gist', 'githubRepository',
'googlepresentation', 'speakerdeck', 'oembed', 'external-article') or None if unsupported.
Any http(s) URL that matches no specific pattern falls back to
'external-article' (generic link card). Non-http(s) strings return None.
"""
for pattern, service in EMBED_PATTERNS:
if pattern.match(url):
return service
# Fallback: treat any other standalone http(s) URL as an external-article
# card, matching note.com's editor behavior on paste.
if GENERIC_EXTERNAL_PATTERN.match(url):
return "external-article"
return None


Expand Down Expand Up @@ -470,6 +484,15 @@ async def resolve_embed_keys(
except NoteAPIError as e:
# Log warning and continue processing other embeds
logger.warning("Embed key fetch failed for %s: %s", url, e.message)
# Original placeholder key is preserved
# Degrade the figure to a plain link paragraph. A figure left with
# an unregistered placeholder key is not rendered by note.com's
# frontend at all (Issue #116), which would silently drop the URL
# from the article. A visible link is the safer fallback.
element_id = str(uuid.uuid4())
link_html = (
f'<p name="{element_id}" id="{element_id}">'
f'<a href="{data_src}">{data_src}</a></p>'
)
result = result.replace(match.group(0), link_html)

return result
6 changes: 5 additions & 1 deletion src/note_mcp/utils/markdown_to_html.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import uuid
from collections.abc import Callable, Iterator
from contextlib import contextmanager
from html import unescape as html_unescape

from markdown_it import MarkdownIt

Expand Down Expand Up @@ -268,7 +269,10 @@ def _convert_standalone_embed_urls(html: str) -> str:
"""

def replace_embed_url(match: re.Match[str]) -> str:
url = match.group(2).strip()
# Unescape HTML entities first: markdown-it escapes '&' in query
# strings to '&amp;' (e.g. Amazon affiliate URLs), which would break
# pattern matching and double-escape in generate_embed_html.
url = html_unescape(match.group(2).strip())

# Check if this URL is a supported embed URL
service = get_embed_service(url)
Expand Down
59 changes: 43 additions & 16 deletions tests/unit/test_embeds.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,26 @@ def test_gist_url(self) -> None:
assert get_embed_service("https://gist.github.com/user-name/abc123def") == "gist"
assert get_embed_service("http://gist.github.com/user/gist123") == "gist"

def test_unsupported_url_returns_none(self) -> None:
"""Test that unsupported URLs return None."""
def test_generic_http_url_falls_back_to_external_article(self) -> None:
"""Generic http(s) URLs fall back to 'external-article' (link card)."""
from note_mcp.api.embeds import get_embed_service

assert get_embed_service("https://example.com") == "external-article"
assert get_embed_service("https://google.com") == "external-article"
assert get_embed_service("https://vimeo.com/123456") == "external-article"
# Query strings (e.g. Amazon affiliate URLs) are accepted
assert (
get_embed_service("https://www.amazon.co.jp/dp/4086315408?tag=abc-22&th=1")
== "external-article"
)

def test_non_http_url_returns_none(self) -> None:
"""Non-http(s) strings still return None."""
from note_mcp.api.embeds import get_embed_service

assert get_embed_service("https://example.com") is None
assert get_embed_service("https://google.com") is None
assert get_embed_service("https://vimeo.com/123456") is None
assert get_embed_service("not a url") is None
assert get_embed_service("ftp://example.com/file") is None
assert get_embed_service("mailto:user@example.com") is None


class TestIsEmbedUrl:
Expand Down Expand Up @@ -100,11 +112,15 @@ def test_gist_urls_are_embed_urls(self) -> None:
assert is_embed_url("https://gist.github.com/user-name/abc123") is True

def test_unsupported_urls_are_not_embed_urls(self) -> None:
"""Test that unsupported URLs are not recognized as embed URLs."""
"""Non-http(s) strings are not recognized as embed URLs.

Generic http(s) URLs ARE embed URLs since the external-article
fallback (they become link cards, like in note.com's editor).
"""
from note_mcp.api.embeds import is_embed_url

assert is_embed_url("https://example.com") is False
assert is_embed_url("https://google.com") is False
assert is_embed_url("https://example.com") is True
assert is_embed_url("https://google.com") is True
assert is_embed_url("not a url") is False


Expand Down Expand Up @@ -223,12 +239,20 @@ def test_url_escaping(self) -> None:
assert "&amp;" in html or "feature=share" in html
assert '"<script>' not in html # Should be escaped

def test_generic_url_uses_external_article(self) -> None:
"""Generic http(s) URLs generate external-article embeds (fallback)."""
from note_mcp.api.embeds import generate_embed_html

html = generate_embed_html("https://example.com")
assert 'embedded-service="external-article"' in html
assert 'data-src="https://example.com"' in html

def test_unsupported_url_raises_error(self) -> None:
"""Test that unsupported URL raises ValueError."""
"""Test that non-http(s) URL raises ValueError."""
from note_mcp.api.embeds import generate_embed_html

with pytest.raises(ValueError, match="Unsupported embed URL"):
generate_embed_html("https://example.com")
generate_embed_html("ftp://example.com/file")

def test_embed_key_parameter(self) -> None:
"""Test that embed_key parameter is used when provided."""
Expand Down Expand Up @@ -561,7 +585,7 @@ async def test_fetch_embed_key_unsupported_url(self) -> None:
)

with pytest.raises(ValueError, match="Unsupported embed URL"):
await fetch_embed_key(session, "https://example.com", "n1234567890ab")
await fetch_embed_key(session, "ftp://example.com/file", "n1234567890ab")

@pytest.mark.asyncio
async def test_fetch_embed_key_api_error(self) -> None:
Expand Down Expand Up @@ -867,9 +891,12 @@ async def test_api_error_logs_warning_and_continues(self) -> None:
# Should NOT raise - error is logged and processing continues
result = await resolve_embed_keys(session, html_body, "n1234567890ab")

# First embed keeps original key (failed), second is replaced (succeeded)
assert 'embedded-content-key="embrandom1"' in result # unchanged
assert 'embedded-content-key="embserverkey2"' in result # replaced
# First embed (failed) degrades to a visible link paragraph;
# an unregistered placeholder key would render as nothing.
assert 'embedded-content-key="embrandom1"' not in result
assert '<a href="https://note.com/user/n/nfailarticle">' in result
# Second embed (succeeded) gets the server-registered key
assert 'embedded-content-key="embserverkey2"' in result
assert mock_fetch.call_count == 2

@pytest.mark.asyncio
Expand Down Expand Up @@ -1001,11 +1028,11 @@ def test_note_embed_with_server_key(self) -> None:
assert f'embedded-content-key="{embed_key}"' in html

def test_unsupported_url_raises_error(self) -> None:
"""Test that unsupported URL raises ValueError."""
"""Test that non-http(s) URL raises ValueError."""
from note_mcp.api.embeds import generate_embed_html_with_key

with pytest.raises(ValueError, match="Unsupported embed URL"):
generate_embed_html_with_key("https://example.com", "emb123")
generate_embed_html_with_key("ftp://example.com/file", "emb123")

def test_url_escaping(self) -> None:
"""Test that special characters in URL are properly escaped."""
Expand Down
43 changes: 27 additions & 16 deletions tests/unit/test_markdown_to_html.py
Original file line number Diff line number Diff line change
Expand Up @@ -632,16 +632,14 @@ class TestStandaloneUrl:
対応しているのはYouTube、Twitter、note.com記事のみです。
"""

def test_standalone_url_becomes_link(self) -> None:
"""単独行のURLはリンクテキストになる(埋め込みにはならない)"""
def test_standalone_url_becomes_external_article_embed(self) -> None:
"""単独行のURLは external-article 埋め込み(リンクカード)になる"""
markdown = "https://example.com/article"
result = markdown_to_html(markdown)

# 埋め込み属性がないことを確認
assert "data-embed-service" not in result
assert "embedded-service" not in result
# URLはテキストとして含まれる
assert "https://example.com/article" in result
# external-article の figure に変換される(noteエディタの貼り付けと同じ挙動)
assert 'embedded-service="external-article"' in result
assert 'data-src="https://example.com/article"' in result

def test_url_in_text_preserved(self) -> None:
"""文中のURLは保持される"""
Expand Down Expand Up @@ -767,14 +765,23 @@ def test_embed_url_as_link_not_converted(self) -> None:
assert 'href="https://www.youtube.com/watch?v=dQw4w9WgXcQ"' in result
assert 'embedded-service="youtube"' not in result

def test_unsupported_url_not_converted(self) -> None:
"""サポートされていないURLは埋め込みに変換されない"""
def test_generic_url_converted_to_external_article(self) -> None:
"""専用パターン外のURLも external-article 埋め込みに変換される"""
markdown = "https://vimeo.com/123456"
result = markdown_to_html(markdown)

assert "<figure" not in result
assert "embedded-service" not in result
assert "https://vimeo.com/123456" in result
assert "<figure" in result
assert 'embedded-service="external-article"' in result

def test_amp_in_query_string_not_double_escaped(self) -> None:
"""クエリ文字列の & が二重エスケープされない(Amazonアフィリエイト等)"""
markdown = "https://www.amazon.co.jp/dp/4086315408?tag=abc-22&th=1"
result = markdown_to_html(markdown)

assert 'embedded-service="external-article"' in result
# data-src は1段エスケープ(&amp;)のみ。&amp;amp; は二重エスケープの兆候
assert "tag=abc-22&amp;th=1" in result
assert "&amp;amp;" not in result


class TestHasEmbedUrl:
Expand All @@ -799,10 +806,14 @@ def test_gist_url_detected(self) -> None:
assert has_embed_url("https://gist.github.com/defunkt/2059") is True
assert has_embed_url("https://gist.github.com/user-name/abc123") is True

def test_unsupported_url_not_detected(self) -> None:
"""サポートされていないURLは検出されない"""
assert has_embed_url("https://vimeo.com/123456") is False
assert has_embed_url("https://example.com") is False
def test_generic_url_detected_as_embed(self) -> None:
"""専用パターン外の http(s) URL も埋め込み対象として検出される"""
assert has_embed_url("https://vimeo.com/123456") is True
assert has_embed_url("https://example.com") is True

def test_non_http_url_not_detected(self) -> None:
"""http(s) 以外は検出されない"""
assert has_embed_url("ftp://example.com/file") is False

def test_url_in_text_detected(self) -> None:
"""テキスト内の埋め込みURLも検出される"""
Expand Down