diff --git a/mateclaw-server/src/main/java/vip/mate/channel/feishu/FeishuChannelAdapter.java b/mateclaw-server/src/main/java/vip/mate/channel/feishu/FeishuChannelAdapter.java index 735f9311..d0f3ab87 100644 --- a/mateclaw-server/src/main/java/vip/mate/channel/feishu/FeishuChannelAdapter.java +++ b/mateclaw-server/src/main/java/vip/mate/channel/feishu/FeishuChannelAdapter.java @@ -1857,9 +1857,17 @@ public class FeishuChannelAdapter extends AbstractChannelAdapter implements Stre if (finalContent.isBlank()) { finalContent = "(无回复内容)"; } - streamingCardManager.finishCard(sessionKey, finalContent); + // Strip any /api/v1/files/generated/{id} URLs out of the card + // text (replacing each with a "📎 filename" marker) AND send + // each cache-resolved file as a native Feishu attachment. The + // streaming card can only render markdown — without this hop + // the user sees a broken-looking download link instead of the + // actual file. Cache-miss URLs fall back to the user-facing + // retry hint that GeneratedFileScrubber emits. + String renderedContent = scrubAndSendAttachments(receiveId, finalContent); + streamingCardManager.finishCard(sessionKey, renderedContent); log.info("[feishu-stream] Card streaming completed: sessionKey={}, contentLen={}", - sessionKey, finalContent.length()); + sessionKey, renderedContent.length()); return finalContent; } catch (Exception e) { @@ -1902,12 +1910,54 @@ public class FeishuChannelAdapter extends AbstractChannelAdapter implements Stre ? message.getReplyToken() : (message.getChatId() != null ? message.getChatId() : message.getSenderId()); if (replyTarget != null) { - sendMessage(replyTarget, finalContent); + // Same scrub-and-upload hop as the streaming card finish path — + // a generated-file URL in plain text would otherwise reach the + // user as a markdown link that opens to nothing useful in IM. + String renderedContent = scrubAndSendAttachments(replyTarget, finalContent); + sendMessage(replyTarget, renderedContent); } } return finalContent; } + /** + * Run {@link GeneratedFileScrubber} over outbound text. For every + * {@code /api/v1/files/generated/{id}} URL the cache resolves, send + * the bytes as a native Feishu attachment to {@code targetId} via + * the existing upload path. Returns the rewritten text (URLs replaced + * with {@code "📎 filename"} markers for hits, retry hints for + * cache-misses) so the caller can use it for the text bubble / card. + * + *

No-op when scrubber / uploader / channel entity is missing — + * returns input unchanged so legacy callers (3-arg ctor, tests) + * continue to behave like the pre-scrubber adapter. The cost of the + * regex scan on a normal reply with no generated URLs is one + * Matcher.find() that returns false, negligible. + */ + // Package-private so tests can call directly without driving the full + // streaming pipeline. Production callers are processStream and + // processStreamAsText, both internal. + String scrubAndSendAttachments(String targetId, String content) { + if (content == null || content.isEmpty() + || generatedFileScrubber == null + || mediaUploader == null + || channelEntity == null + || channelEntity.getId() == null) { + return content; + } + GeneratedFileScrubber.ScrubResult scrubbed = generatedFileScrubber.scrub(content); + Long channelId = channelEntity.getId(); + for (GeneratedFileScrubber.AttachmentHit hit : scrubbed.attachments()) { + uploadAndSendAttachment(targetId, channelId, + new MediaSource.Bytes(hit.bytes()), + hit.fileName(), + hit.mediaType(), + hit.mimeType(), + null); + } + return scrubbed.rewrittenText(); + } + /** Resolve the best id to receive a streaming card — prefer reply token, then chat, then sender. */ private static String pickReceiveId(ChannelMessage message) { if (message == null) return null; @@ -2255,10 +2305,12 @@ public class FeishuChannelAdapter extends AbstractChannelAdapter implements Stre * downgrade note as a follow-up text bubble so users understand * why a bubble isn't native. */ - private void uploadAndSendAttachment(String targetId, Long channelId, - MediaSource source, String fileName, - String mediaType, String contentType, - Integer durationMillis) { + // Package-private + non-final so tests can override and capture upload + // attempts without standing up a real Feishu HTTP client. + void uploadAndSendAttachment(String targetId, Long channelId, + MediaSource source, String fileName, + String mediaType, String contentType, + Integer durationMillis) { try { MediaUploadResult result = mediaUploader.upload(new MediaUploadRequest( channelId, source, fileName, mediaType, contentType, durationMillis)); diff --git a/mateclaw-server/src/test/java/vip/mate/channel/feishu/FeishuScrubOnFinishTest.java b/mateclaw-server/src/test/java/vip/mate/channel/feishu/FeishuScrubOnFinishTest.java new file mode 100644 index 00000000..6359ed39 --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/channel/feishu/FeishuScrubOnFinishTest.java @@ -0,0 +1,207 @@ +package vip.mate.channel.feishu; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import vip.mate.channel.ChannelMessageRouter; +import vip.mate.channel.media.GeneratedFileScrubber; +import vip.mate.channel.media.MediaSource; +import vip.mate.channel.model.ChannelEntity; +import vip.mate.tool.document.GeneratedFileCache; + +import java.util.ArrayList; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +/** + * Pin the scrub-on-finish contract for outbound Feishu replies. + * + *

The agent often replies with a markdown link like + * [report.pdf](/api/v1/files/generated/abc-123) after + * generating a file. The IM client can't open the + * tenant-token-protected backend endpoint that link points at — so + * without intervention the user gets a dead link instead of the file. + * + *

The fix is the {@code scrubAndSendAttachments} hop, called by + * both {@code processStream} (CardKit streaming path) and + * {@code processStreamAsText} (fallback). This test pins that hop: + * + *

    + *
  1. The URL is replaced with a {@code "📎 filename"} marker so + * the card text reads cleanly.
  2. + *
  3. An upload is enqueued for the cache-resolved bytes, with the + * right mediaType / fileName / mimeType from the cache entry.
  4. + *
  5. Cache misses degrade to a user-facing retry hint instead of + * leaving the dead link in the bubble.
  6. + *
  7. Replies with no generated-file URL pass through unchanged and + * trigger zero uploads.
  8. + *
+ */ +class FeishuScrubOnFinishTest { + + private GeneratedFileCache cache; + private GeneratedFileScrubber scrubber; + private RecordingAdapter adapter; + + @BeforeEach + void setUp() { + cache = new GeneratedFileCache(); + scrubber = new GeneratedFileScrubber(cache); + adapter = new RecordingAdapter(scrubber); + } + + @Test + @DisplayName("agent reply with cached PDF URL → text rewritten + upload enqueued") + void cacheHitTriggersUploadAndRewrite() { + byte[] bytes = "%PDF-1.4 fake pdf body".getBytes(); + String id = cache.put(bytes, "report.pdf", "application/pdf"); + String agentReply = "✅ PDF 已生成\n[report.pdf](/api/v1/files/generated/" + id + ")\n" + + "点击上方链接即可下载,链接 10 分钟内有效"; + + String rendered = adapter.scrubAndSendAttachments("ou_user_123", agentReply); + + // Text bubble shows the marker, not the dead link. + assertTrue(rendered.contains("📎 report.pdf"), + "rewrittenText should embed the filename marker: " + rendered); + assertFalse(rendered.contains("/api/v1/files/generated/" + id), + "rewrittenText should NOT still carry the generated URL: " + rendered); + // The "(/api/...)" part of the markdown link disappears with the + // URL, leaving the bracketed display name + the marker. Both are + // user-friendly text — no live link rot. + + // Exactly one upload enqueued, with the right metadata. + assertEquals(1, adapter.uploads.size(), "expected exactly one attachment send"); + RecordedUpload upload = adapter.uploads.get(0); + assertEquals("ou_user_123", upload.targetId); + assertEquals("report.pdf", upload.fileName); + assertEquals("file", upload.mediaType, "PDF should classify as 'file', not 'image'"); + assertEquals("application/pdf", upload.contentType); + assertTrue(upload.source instanceof MediaSource.Bytes, + "upload should be a Bytes source so the uploader skips a remote fetch"); + assertEquals(bytes.length, + ((MediaSource.Bytes) upload.source).data().length); + } + + @Test + @DisplayName("PNG cache hit classifies as 'image' so vision-aware bubble renders inline") + void imageMimeTriggersImageUpload() { + byte[] bytes = new byte[]{(byte) 0x89, 'P', 'N', 'G'}; + String id = cache.put(bytes, "chart.png", "image/png"); + String agentReply = "这是图表: /api/v1/files/generated/" + id; + + String rendered = adapter.scrubAndSendAttachments("oc_chat_x", agentReply); + + assertTrue(rendered.contains("📎 chart.png")); + assertEquals(1, adapter.uploads.size()); + assertEquals("image", adapter.uploads.get(0).mediaType, + "image/png MIME must route to the image endpoint, not file"); + assertEquals("image/png", adapter.uploads.get(0).contentType); + } + + @Test + @DisplayName("cache miss → retry hint in bubble, no upload attempted") + void cacheMissDoesNotUpload() { + String agentReply = "你的文件: [doc.pdf](/api/v1/files/generated/never-existed-uuid)"; + + String rendered = adapter.scrubAndSendAttachments("ou_user_x", agentReply); + + assertFalse(rendered.contains("/api/v1/files/generated/never-existed-uuid"), + "dead URL should be replaced with the missing-reference notice"); + assertTrue(rendered.contains(GeneratedFileCache.MISSING_REFERENCE_NOTICE), + "user-visible retry hint expected: " + rendered); + assertTrue(adapter.uploads.isEmpty(), + "cache miss must NOT enqueue an upload — bytes don't exist"); + } + + @Test + @DisplayName("plain reply with no generated URL is forwarded unchanged, zero uploads") + void plainTextIsForwarded() { + String agentReply = "Sure, here's a quick summary: ..."; + + String rendered = adapter.scrubAndSendAttachments("ou_user_x", agentReply); + + assertEquals(agentReply, rendered, "no scrubbable URL → exact pass-through"); + assertTrue(adapter.uploads.isEmpty(), + "non-generated reply must not trigger any upload work"); + } + + @Test + @DisplayName("multiple generated URLs in one reply enqueue one upload per cache hit") + void multipleUrlsEnqueueMultipleUploads() { + String aId = cache.put("AAAA".getBytes(), "a.pdf", "application/pdf"); + String bId = cache.put("BBBB".getBytes(), "b.png", "image/png"); + String agentReply = "两个产物:\n- /api/v1/files/generated/" + aId + + "\n- /api/v1/files/generated/" + bId; + + adapter.scrubAndSendAttachments("ou_user_x", agentReply); + + assertEquals(2, adapter.uploads.size(), + "one upload per generated URL — got: " + + adapter.uploads.stream().map(u -> u.fileName).toList()); + assertEquals("a.pdf", adapter.uploads.get(0).fileName); + assertEquals("file", adapter.uploads.get(0).mediaType); + assertEquals("b.png", adapter.uploads.get(1).fileName); + assertEquals("image", adapter.uploads.get(1).mediaType); + } + + @Test + @DisplayName("scrubber missing → no-op pass-through, no NPE (legacy 3-arg ctor case)") + void scrubberAbsentIsNoop() { + // Build adapter without the scrubber — simulates the 3-arg ctor + // path / unit tests that don't wire the media beans. + RecordingAdapter legacyAdapter = new RecordingAdapter(null); + String text = "agent reply with /api/v1/files/generated/abc"; + String rendered = legacyAdapter.scrubAndSendAttachments("ou_x", text); + assertEquals(text, rendered); + assertTrue(legacyAdapter.uploads.isEmpty()); + } + + // ------------------------------------------------------------------ + // Test fixture + // ------------------------------------------------------------------ + + /** Captured upload arguments — what the test asserts on. */ + private record RecordedUpload(String targetId, MediaSource source, String fileName, + String mediaType, String contentType) {} + + /** + * Subclass that records every {@link FeishuChannelAdapter#uploadAndSendAttachment} + * call instead of actually doing the HTTP send. Constructor wires a + * synthetic {@link ChannelEntity} with id=1 so the channelId check + * inside {@code scrubAndSendAttachments} passes; mediaUploader is + * mocked because the recording adapter overrides the call path that + * would use it anyway. + */ + private static class RecordingAdapter extends FeishuChannelAdapter { + final List uploads = new ArrayList<>(); + + RecordingAdapter(GeneratedFileScrubber scrubber) { + super(testEntity(), + mock(ChannelMessageRouter.class), + new ObjectMapper(), + scrubber == null ? null : mock(FeishuMediaUploader.class), + scrubber); + } + + @Override + void uploadAndSendAttachment(String targetId, Long channelId, + MediaSource source, String fileName, + String mediaType, String contentType, + Integer durationMillis) { + uploads.add(new RecordedUpload(targetId, source, fileName, mediaType, contentType)); + } + + private static ChannelEntity testEntity() { + ChannelEntity e = new ChannelEntity(); + e.setId(1L); + e.setChannelType("feishu"); + e.setConfigJson("{\"app_id\":\"x\",\"app_secret\":\"y\"}"); + return e; + } + } +}