fix: reject oversized qq inbound images - #2
Open
clown145 wants to merge 1 commit into
Open
Conversation
Reviewer's Guide为 QQ 公众号图片实现了按大小感知的入站媒体处理，并将媒体拒绝信息向上传播到 agent runner，使得超大图片可以在早期就以结构化错误被拒绝（包括后端特定的限制），同时更新所有调用点和测试以适配新的 Agent runner 中 QQ 入站媒体拒绝的时序图sequenceDiagram
participant AgentRunner as runAgentForMessageInternal
participant persistInboundMedia
participant persistQqOfficialInboundMedia
participant downloadQqOfficialImage
participant appendRunStep
participant sendFinalMessage
participant completeRun
AgentRunner->>persistInboundMedia: persistInboundMedia(env, inboundMessage)
persistInboundMedia->>persistQqOfficialInboundMedia: persistQqOfficialInboundMedia(env, message)
persistQqOfficialInboundMedia->>downloadQqOfficialImage: downloadQqOfficialImage(sourceUrl, limitBytes)
downloadQqOfficialImage-->>persistQqOfficialInboundMedia: { ok: false, reason: too_large }
persistQqOfficialInboundMedia-->>persistInboundMedia: InboundMediaResult(rejection)
persistInboundMedia-->>AgentRunner: InboundMediaResult(media)
alt [media.rejection]
AgentRunner->>appendRunStep: appendRunStep(env.AGENT_DB, ...)
AgentRunner->>sendFinalMessage: sendFinalMessage(env, runId, message, responseText)
AgentRunner->>completeRun: completeRun(env.AGENT_DB, runId, completed)
else [no rejection]
AgentRunner->>AgentRunner: resolveActiveModelCapabilities(...)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 以：
Getting HelpOriginal review guide in EnglishReviewer's GuideImplements size-aware inbound media handling for QQ official images and propagates media rejection up to the agent runner so oversized images are rejected early with a structured error, including backend-specific limits, while updating call sites and tests to handle the new InboundMediaResult shape. Sequence diagram for QQ inbound media rejection in agent runnersequenceDiagram
participant AgentRunner as runAgentForMessageInternal
participant persistInboundMedia
participant persistQqOfficialInboundMedia
participant downloadQqOfficialImage
participant appendRunStep
participant sendFinalMessage
participant completeRun
AgentRunner->>persistInboundMedia: persistInboundMedia(env, inboundMessage)
persistInboundMedia->>persistQqOfficialInboundMedia: persistQqOfficialInboundMedia(env, message)
persistQqOfficialInboundMedia->>downloadQqOfficialImage: downloadQqOfficialImage(sourceUrl, limitBytes)
downloadQqOfficialImage-->>persistQqOfficialInboundMedia: { ok: false, reason: too_large }
persistQqOfficialInboundMedia-->>persistInboundMedia: InboundMediaResult(rejection)
persistInboundMedia-->>AgentRunner: InboundMediaResult(media)
alt [media.rejection]
AgentRunner->>appendRunStep: appendRunStep(env.AGENT_DB, ...)
AgentRunner->>sendFinalMessage: sendFinalMessage(env, runId, message, responseText)
AgentRunner->>completeRun: completeRun(env.AGENT_DB, runId, completed)
else [no rejection]
AgentRunner->>AgentRunner: resolveActiveModelCapabilities(...)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces size limit validation for inbound media attachments, specifically for the QQ official adapter. It adds logic to determine effective size limits based on the configured storage backend and updates the agent runner to handle media rejections by sending a failure message to the user and terminating the run. The review feedback suggests refactoring the storage backend detection logic to avoid duplication and improve maintainability. Additionally, a security concern was raised regarding potential memory exhaustion when downloading images; it is recommended to use streaming to enforce size limits before fully loading the response into memory.
Comment on lines
+147
to
+155
| function effectiveBlobStorageBackend(env: Env): BlobStorageBackend { | ||
| if ( | ||
| env.OBJECT_STORAGE_BACKEND === "d1_lite" || | ||
| (env.OBJECT_STORAGE_BACKEND !== "s3" && !env.AGENT_BUCKET) | ||
| ) { | ||
| return "d1_lite"; | ||
| } | ||
| return env.OBJECT_STORAGE_BACKEND === "s3" ? "s3" : "r2"; | ||
| } |
There was a problem hiding this comment.
The logic for determining the effective storage backend is duplicated here and likely mirrors the internal logic of createBlobStorage. This creates a maintenance risk if the storage configuration logic changes in the future.
Instead of re-implementing this check, you should use the backend property already exposed by the BlobStorage instance. Consider instantiating the storage once in persistQqOfficialInboundMedia and passing it (or the derived limit) down to the helper functions to ensure consistency and reduce redundant environment checks.
Comment on lines
99
to
102
| const bytes = new Uint8Array(await response.arrayBuffer()); | ||
| if (bytes.byteLength > MAX_QQ_OFFICIAL_IMAGE_BYTES) { | ||
| return undefined; | ||
| if (bytes.byteLength > limitBytes) { | ||
| return { ok: false, reason: "too_large" }; | ||
| } |
There was a problem hiding this comment.
Downloading the entire image into memory using response.arrayBuffer() before checking its size can lead to memory exhaustion (OOM) if the server omits the Content-Length header or sends a body significantly larger than the limit.
While the subsequent check on bytes.byteLength prevents storing oversized images, the memory has already been allocated, which can be exploited for Denial of Service (DoS) attacks. For better resilience, consider reading the response body as a stream and aborting the download as soon as the limitBytes threshold is exceeded.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 4 个问题
供 AI 代理使用的提示
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="tests/unit/qq-official/inbound-media.test.ts" line_range="64-85" />
<code_context>
expect(stored.get("attachments/agent-1/msg_qq/qq_attachment_0")?.contentType).toBe("image/png");
});
+
+ it("returns an oversized rejection before downloading when content length exceeds the limit", async () => {
+ const stored = new Map<string, { bytes: Uint8Array; contentType?: string }>();
+ const fetchMock = vi.fn(
+ async () =>
+ new Response("too-large", {
+ headers: {
+ "content-length": String(8 * 1024 * 1024 + 1)
+ }
+ })
+ );
+ globalThis.fetch = fetchMock as unknown as typeof fetch;
+
+ const result = await persistQqOfficialInboundMedia(envWithBucket(stored), message());
+
+ expect(result.rejection).toMatchObject({
+ code: "attachment_too_large",
+ attachmentIds: ["qq_attachment_0"],
+ responseText: "The image exceeds the 8 MiB size limit."
+ });
+ expect(result.message.attachments[0]).not.toHaveProperty("r2Key");
+ expect(stored.size).toBe(0);
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** 添加一个测试用例：即使 `content-length` 在限制内，实际 body 超过限制。
目前我们只覆盖了 `content-length` 超过限制并在读取前拒绝的情况。请再添加一个测试：在该测试中，要么缺失 `content-length`，要么其值低于限制，但实际下载的 body 超过 `limitBytes`，以验证我们仍然会拒绝该图片，而且不会持久化或写入任何存储。
```suggestion
it("returns an oversized rejection before downloading when content length exceeds the limit", async () => {
const stored = new Map<string, { bytes: Uint8Array; contentType?: string }>();
const fetchMock = vi.fn(
async () =>
new Response("too-large", {
headers: {
"content-length": String(8 * 1024 * 1024 + 1)
}
})
);
globalThis.fetch = fetchMock as unknown as typeof fetch;
const result = await persistQqOfficialInboundMedia(envWithBucket(stored), message());
expect(result.rejection).toMatchObject({
code: "attachment_too_large",
attachmentIds: ["qq_attachment_0"],
responseText: "The image exceeds the 8 MiB size limit."
});
expect(result.message.attachments[0]).not.toHaveProperty("r2Key");
expect(stored.size).toBe(0);
});
it("returns an oversized rejection after downloading when body exceeds the limit", async () => {
const stored = new Map<string, { bytes: Uint8Array; contentType?: string }>();
const limitPlusOne = 8 * 1024 * 1024 + 1;
const imageBytes = new Uint8Array(limitPlusOne);
imageBytes.fill(1);
const fetchMock = vi.fn(
async () =>
new Response(imageBytes, {
headers: {
"content-type": "image/png"
// intentionally omit content-length so we exercise the streaming limit
}
})
);
globalThis.fetch = fetchMock as unknown as typeof fetch;
const result = await persistQqOfficialInboundMedia(envWithBucket(stored), message());
expect(result.rejection).toMatchObject({
code: "attachment_too_large",
attachmentIds: ["qq_attachment_0"],
responseText: "The image exceeds the 8 MiB size limit."
});
expect(result.message.attachments[0]).not.toHaveProperty("r2Key");
expect(stored.size).toBe(0);
});
```
</issue_to_address>
### Comment 2
<location path="tests/unit/qq-official/inbound-media.test.ts" line_range="83-84" />
<code_context>
+ attachmentIds: ["qq_attachment_0"],
+ responseText: "The image exceeds the 8 MiB size limit."
+ });
+ expect(result.message.attachments[0]).not.toHaveProperty("r2Key");
+ expect(stored.size).toBe(0);
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** 考虑测试混合/多个附件，以验证部分拒绝行为以及 `attachmentIds` 的聚合。
目前我们只覆盖了单个超大图片及其拒绝/持久化行为。由于 `persistQqOfficialInboundMedia` 会聚合 `attachmentResults` 并扁平化 `rejection.attachmentIds`，请添加一个包含混合附件的测试（例如，一个超大图片加一个合法图片，或一个图片加一个非图片）。该测试应验证：合法附件被持久化并带有 `r2Key`，`rejection.attachmentIds` 仅包含被拒绝的附件 ID，且 `message.attachments` 保持预期的顺序和内容。
建议实现如下：
```typescript
const result = await persistQqOfficialInboundMedia(envWithBucket(stored), message());
expect(result.rejection).toMatchObject({
code: "attachment_too_large",
attachmentIds: ["qq_attachment_0"],
responseText: "The image exceeds the 8 MiB size limit."
});
expect(result.message.attachments[0]).not.toHaveProperty("r2Key");
expect(stored.size).toBe(0);
});
it("persists valid attachments and rejects oversized ones while aggregating attachmentIds", async () => {
const stored = new Map<string, ArrayBuffer>();
const smallImageBytes = new Uint8Array(1024); // 1 KiB
const largeImageBytes = new Uint8Array(9 * 1024 * 1024); // > 8 MiB
const fetchMock = vi
.fn()
// First attachment: valid image
.mockResolvedValueOnce(
new Response(smallImageBytes, {
status: 200,
headers: { "content-type": "image/png" }
})
)
// Second attachment: oversized image
.mockResolvedValueOnce(
new Response(largeImageBytes, {
status: 200,
headers: { "content-type": "image/png" }
})
);
globalThis.fetch = fetchMock as unknown as typeof fetch;
const mixedMessage = message();
mixedMessage.attachments = [
{
...mixedMessage.attachments[0],
id: "qq_attachment_0",
url: "https://cdn.qq.com/image-small.png"
},
{
...mixedMessage.attachments[0],
id: "qq_attachment_1",
url: "https://cdn.qq.com/image-large.png"
}
];
const result = await persistQqOfficialInboundMedia(envWithBucket(stored), mixedMessage);
expect(fetchMock).toHaveBeenNthCalledWith(1, "https://cdn.qq.com/image-small.png");
expect(fetchMock).toHaveBeenNthCalledWith(2, "https://cdn.qq.com/image-large.png");
// Valid attachment gets persisted and has an r2Key
expect(result.message.attachments[0]).toMatchObject({
id: "qq_attachment_0",
r2Key: "attachments/agent-1/msg_qq/qq_attachment_0",
mimeType: "image/png",
size: smallImageBytes.byteLength
});
// Oversized attachment is present but has no r2Key
expect(result.message.attachments[1]).toMatchObject({
id: "qq_attachment_1"
});
expect(result.message.attachments[1]).not.toHaveProperty("r2Key");
// Only the oversized attachment id is aggregated in the rejection
expect(result.rejection).toMatchObject({
code: "attachment_too_large",
attachmentIds: ["qq_attachment_1"]
});
// Only one object stored in the bucket for the valid attachment
expect(stored.size).toBe(1);
expect(stored.has("attachments/agent-1/msg_qq/qq_attachment_0")).toBe(true);
});
```
该补丁假设：
1. `message()` 返回的对象包含一个可变的 `attachments` 数组，并且 `attachments[0]` 的结构与 QQ 附件 schema 兼容（即展开并重写 `id`/`url` 是合法的）。
2. 桶的键模式 `attachments/agent-1/msg_qq/<attachmentId>` 与 `persistQqOfficialInboundMedia` 实际使用的模式一致。
3. 测试环境中可以使用 `Response`（如果不能，请从你的 fetch 实现中导入，例如 `undici` 或 `node-fetch`）。如果你现有的 helper 不同（例如 `message` 支持通过参数配置 attachments），请相应调整 `mixedMessage` 的构造方式，但要保持对 `r2Key`、`rejection.attachmentIds`、`message.attachments` 顺序以及 `stored` 大小/键的期望不变。
</issue_to_address>
### Comment 3
<location path="tests/unit/agent-runner-inbound-media.test.ts" line_range="68-77" />
<code_context>
+ });
+ });
+
+ it("sends the media rejection directly and skips agent execution", async () => {
+ await expect(runAgentForMessage({} as Env, message())).resolves.toEqual(expect.any(String));
+
+ expect(mocks.sendFinalMessage).toHaveBeenCalledWith(
+ {},
+ expect.any(String),
+ expect.objectContaining({ id: "msg_qq" }),
+ "The image exceeds the 8 MiB size limit."
+ );
+ expect(mocks.completeRun).toHaveBeenCalledWith(undefined, expect.any(String), "completed");
+ expect(mocks.executeAgentToolLoop).not.toHaveBeenCalled();
+ });
+});
</code_context>
<issue_to_address>
**suggestion (testing):** 为无拒绝路径添加一个互补测试，以确保正常的 agent 执行流程仍然会运行。
为了充分覆盖 `runAgentForMessage` 的媒体处理逻辑，请添加一个测试，使 `persistInboundMedia` 解析为 `{ message, rejection: undefined }`。在这种情况下，断言 `executeAgentToolLoop` 被调用，`sendFinalMessage` 不会在该路径中被调用，并且运行通过正常流程完成。这有助于捕获在无拒绝路径上被意外短路的回归问题。
建议实现如下：
```typescript
mocks.persistInboundMedia.mockResolvedValue({
message: message(),
rejection: {
code: "attachment_too_large",
attachmentIds: ["qq_attachment_0"],
responseText: "The image exceeds the 8 MiB size limit.",
summary: "Inbound image exceeds the 8 MiB size limit"
}
});
});
it("continues normal agent execution when media is accepted", async () => {
mocks.persistInboundMedia.mockResolvedValue({
message: message(),
rejection: undefined
});
await expect(runAgentForMessage({} as Env, message())).resolves.toEqual(
expect.any(String)
);
expect(mocks.executeAgentToolLoop).toHaveBeenCalled();
expect(mocks.sendFinalMessage).not.toHaveBeenCalled();
expect(mocks.completeRun).toHaveBeenCalledWith(
undefined,
expect.any(String),
"completed"
);
});
```
- 请确保这个新测试放在与现有媒体拒绝测试相同的 `describe` 块中，以便复用相同的 `mocks` 和 `message()` 设置。
- 如果该文件中的其他测试也依赖 `mocks.sendFinalMessage` 或 `mocks.executeAgentToolLoop`，请确认任何共享的 `beforeEach` 会正确重置这些 mock（例如使用 `jest.clearAllMocks()`），从而保证这个新测试在一个干净的状态下开始运行。
</issue_to_address>
### Comment 4
<location path="tests/unit/agent-runner-inbound-media.test.ts" line_range="69-78" />
<code_context>
+ });
+
+ it("sends the media rejection directly and skips agent execution", async () => {
+ await expect(runAgentForMessage({} as Env, message())).resolves.toEqual(expect.any(String));
+
+ expect(mocks.sendFinalMessage).toHaveBeenCalledWith(
+ {},
+ expect.any(String),
+ expect.objectContaining({ id: "msg_qq" }),
+ "The image exceeds the 8 MiB size limit."
+ );
+ expect(mocks.completeRun).toHaveBeenCalledWith(undefined, expect.any(String), "completed");
+ expect(mocks.executeAgentToolLoop).not.toHaveBeenCalled();
+ });
+});
</code_context>
<issue_to_address>
**suggestion (testing):** 断言拒绝摘要通过 `appendRunStep` 进行了传递。
由于 `runAgentForMessage` 现在会使用 `media.rejection.summary` 追加一个已完成的步骤到运行中，本测试也应该验证这一点。在 Promise 解析之后，添加如下断言：
```ts
expect(mocks.appendRunStep).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({ summary: "Inbound image exceeds the 8 MiB size limit" })
);
```
这可以确认拒绝信息不仅通过最终消息发送出去，也已记录在运行历史中。
</issue_to_address>帮助我变得更有用！请在每条评论上点击 👍 或 👎，我会根据你的反馈改进后续的审查。
Original comment in English
Hey - I've found 4 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="tests/unit/qq-official/inbound-media.test.ts" line_range="64-85" />
<code_context>
expect(stored.get("attachments/agent-1/msg_qq/qq_attachment_0")?.contentType).toBe("image/png");
});
+
+ it("returns an oversized rejection before downloading when content length exceeds the limit", async () => {
+ const stored = new Map<string, { bytes: Uint8Array; contentType?: string }>();
+ const fetchMock = vi.fn(
+ async () =>
+ new Response("too-large", {
+ headers: {
+ "content-length": String(8 * 1024 * 1024 + 1)
+ }
+ })
+ );
+ globalThis.fetch = fetchMock as unknown as typeof fetch;
+
+ const result = await persistQqOfficialInboundMedia(envWithBucket(stored), message());
+
+ expect(result.rejection).toMatchObject({
+ code: "attachment_too_large",
+ attachmentIds: ["qq_attachment_0"],
+ responseText: "The image exceeds the 8 MiB size limit."
+ });
+ expect(result.message.attachments[0]).not.toHaveProperty("r2Key");
+ expect(stored.size).toBe(0);
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test case where the body exceeds the limit even though `content-length` is within the limit.
Currently we only cover the case where `content-length` exceeds the limit and we reject before reading. Please also add a test where `content-length` is missing or below the limit but the downloaded body exceeds `limitBytes`, to verify we still reject the image and never persist it or write it to storage.
```suggestion
it("returns an oversized rejection before downloading when content length exceeds the limit", async () => {
const stored = new Map<string, { bytes: Uint8Array; contentType?: string }>();
const fetchMock = vi.fn(
async () =>
new Response("too-large", {
headers: {
"content-length": String(8 * 1024 * 1024 + 1)
}
})
);
globalThis.fetch = fetchMock as unknown as typeof fetch;
const result = await persistQqOfficialInboundMedia(envWithBucket(stored), message());
expect(result.rejection).toMatchObject({
code: "attachment_too_large",
attachmentIds: ["qq_attachment_0"],
responseText: "The image exceeds the 8 MiB size limit."
});
expect(result.message.attachments[0]).not.toHaveProperty("r2Key");
expect(stored.size).toBe(0);
});
it("returns an oversized rejection after downloading when body exceeds the limit", async () => {
const stored = new Map<string, { bytes: Uint8Array; contentType?: string }>();
const limitPlusOne = 8 * 1024 * 1024 + 1;
const imageBytes = new Uint8Array(limitPlusOne);
imageBytes.fill(1);
const fetchMock = vi.fn(
async () =>
new Response(imageBytes, {
headers: {
"content-type": "image/png"
// intentionally omit content-length so we exercise the streaming limit
}
})
);
globalThis.fetch = fetchMock as unknown as typeof fetch;
const result = await persistQqOfficialInboundMedia(envWithBucket(stored), message());
expect(result.rejection).toMatchObject({
code: "attachment_too_large",
attachmentIds: ["qq_attachment_0"],
responseText: "The image exceeds the 8 MiB size limit."
});
expect(result.message.attachments[0]).not.toHaveProperty("r2Key");
expect(stored.size).toBe(0);
});
```
</issue_to_address>
### Comment 2
<location path="tests/unit/qq-official/inbound-media.test.ts" line_range="83-84" />
<code_context>
+ attachmentIds: ["qq_attachment_0"],
+ responseText: "The image exceeds the 8 MiB size limit."
+ });
+ expect(result.message.attachments[0]).not.toHaveProperty("r2Key");
+ expect(stored.size).toBe(0);
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider testing mixed/multiple attachments to validate partial rejection behavior and attachmentIds aggregation.
Right now we only cover a single oversized image and the rejection/persistence behavior. Since `persistQqOfficialInboundMedia` aggregates `attachmentResults` and flattens `rejection.attachmentIds`, please add a test with mixed attachments (e.g., one oversized and one valid image, or an image plus a non-image). That test should verify that valid attachments are persisted with an `r2Key`, `rejection.attachmentIds` only contains the rejected attachment IDs, and `message.attachments` preserves the expected order and contents.
Suggested implementation:
```typescript
const result = await persistQqOfficialInboundMedia(envWithBucket(stored), message());
expect(result.rejection).toMatchObject({
code: "attachment_too_large",
attachmentIds: ["qq_attachment_0"],
responseText: "The image exceeds the 8 MiB size limit."
});
expect(result.message.attachments[0]).not.toHaveProperty("r2Key");
expect(stored.size).toBe(0);
});
it("persists valid attachments and rejects oversized ones while aggregating attachmentIds", async () => {
const stored = new Map<string, ArrayBuffer>();
const smallImageBytes = new Uint8Array(1024); // 1 KiB
const largeImageBytes = new Uint8Array(9 * 1024 * 1024); // > 8 MiB
const fetchMock = vi
.fn()
// First attachment: valid image
.mockResolvedValueOnce(
new Response(smallImageBytes, {
status: 200,
headers: { "content-type": "image/png" }
})
)
// Second attachment: oversized image
.mockResolvedValueOnce(
new Response(largeImageBytes, {
status: 200,
headers: { "content-type": "image/png" }
})
);
globalThis.fetch = fetchMock as unknown as typeof fetch;
const mixedMessage = message();
mixedMessage.attachments = [
{
...mixedMessage.attachments[0],
id: "qq_attachment_0",
url: "https://cdn.qq.com/image-small.png"
},
{
...mixedMessage.attachments[0],
id: "qq_attachment_1",
url: "https://cdn.qq.com/image-large.png"
}
];
const result = await persistQqOfficialInboundMedia(envWithBucket(stored), mixedMessage);
expect(fetchMock).toHaveBeenNthCalledWith(1, "https://cdn.qq.com/image-small.png");
expect(fetchMock).toHaveBeenNthCalledWith(2, "https://cdn.qq.com/image-large.png");
// Valid attachment gets persisted and has an r2Key
expect(result.message.attachments[0]).toMatchObject({
id: "qq_attachment_0",
r2Key: "attachments/agent-1/msg_qq/qq_attachment_0",
mimeType: "image/png",
size: smallImageBytes.byteLength
});
// Oversized attachment is present but has no r2Key
expect(result.message.attachments[1]).toMatchObject({
id: "qq_attachment_1"
});
expect(result.message.attachments[1]).not.toHaveProperty("r2Key");
// Only the oversized attachment id is aggregated in the rejection
expect(result.rejection).toMatchObject({
code: "attachment_too_large",
attachmentIds: ["qq_attachment_1"]
});
// Only one object stored in the bucket for the valid attachment
expect(stored.size).toBe(1);
expect(stored.has("attachments/agent-1/msg_qq/qq_attachment_0")).toBe(true);
});
```
This patch assumes:
1. `message()` returns an object with a mutable `attachments` array and an `attachments[0]` shape compatible with the QQ attachment schema (i.e., spreading it and overriding `id`/`url` is valid).
2. The bucket key pattern `attachments/agent-1/msg_qq/<attachmentId>` matches what `persistQqOfficialInboundMedia` actually uses.
3. `Response` is available in the test environment (if not, import it from your fetch implementation, e.g., `undici` or `node-fetch`).
If your existing helpers differ (e.g., `message` supports a configuration argument for attachments), adapt the construction of `mixedMessage` accordingly, but keep the expectations around `r2Key`, `rejection.attachmentIds`, order of `message.attachments`, and `stored` size/keys the same.
</issue_to_address>
### Comment 3
<location path="tests/unit/agent-runner-inbound-media.test.ts" line_range="68-77" />
<code_context>
+ });
+ });
+
+ it("sends the media rejection directly and skips agent execution", async () => {
+ await expect(runAgentForMessage({} as Env, message())).resolves.toEqual(expect.any(String));
+
+ expect(mocks.sendFinalMessage).toHaveBeenCalledWith(
+ {},
+ expect.any(String),
+ expect.objectContaining({ id: "msg_qq" }),
+ "The image exceeds the 8 MiB size limit."
+ );
+ expect(mocks.completeRun).toHaveBeenCalledWith(undefined, expect.any(String), "completed");
+ expect(mocks.executeAgentToolLoop).not.toHaveBeenCalled();
+ });
+});
</code_context>
<issue_to_address>
**suggestion (testing):** Add a complementary test for the no-rejection path to ensure the normal agent execution still runs.
To fully exercise `runAgentForMessage`’s media handling, please add a test where `persistInboundMedia` resolves to `{ message, rejection: undefined }`. In that case, assert that `executeAgentToolLoop` is called, `sendFinalMessage` is not invoked from this path, and the run completes via the normal flow. This will help catch regressions where the non-rejection path is accidentally short-circuited.
Suggested implementation:
```typescript
mocks.persistInboundMedia.mockResolvedValue({
message: message(),
rejection: {
code: "attachment_too_large",
attachmentIds: ["qq_attachment_0"],
responseText: "The image exceeds the 8 MiB size limit.",
summary: "Inbound image exceeds the 8 MiB size limit"
}
});
});
it("continues normal agent execution when media is accepted", async () => {
mocks.persistInboundMedia.mockResolvedValue({
message: message(),
rejection: undefined
});
await expect(runAgentForMessage({} as Env, message())).resolves.toEqual(
expect.any(String)
);
expect(mocks.executeAgentToolLoop).toHaveBeenCalled();
expect(mocks.sendFinalMessage).not.toHaveBeenCalled();
expect(mocks.completeRun).toHaveBeenCalledWith(
undefined,
expect.any(String),
"completed"
);
});
```
- Ensure this new test is placed within the same `describe` block as the existing media rejection test so it shares the same `mocks` and `message()` setup.
- If other tests in this file rely on `mocks.sendFinalMessage` or `mocks.executeAgentToolLoop`, confirm that any shared `beforeEach` correctly resets mocks (e.g. `jest.clearAllMocks()`), so this new test starts from a clean state.
</issue_to_address>
### Comment 4
<location path="tests/unit/agent-runner-inbound-media.test.ts" line_range="69-78" />
<code_context>
+ });
+
+ it("sends the media rejection directly and skips agent execution", async () => {
+ await expect(runAgentForMessage({} as Env, message())).resolves.toEqual(expect.any(String));
+
+ expect(mocks.sendFinalMessage).toHaveBeenCalledWith(
+ {},
+ expect.any(String),
+ expect.objectContaining({ id: "msg_qq" }),
+ "The image exceeds the 8 MiB size limit."
+ );
+ expect(mocks.completeRun).toHaveBeenCalledWith(undefined, expect.any(String), "completed");
+ expect(mocks.executeAgentToolLoop).not.toHaveBeenCalled();
+ });
+});
</code_context>
<issue_to_address>
**suggestion (testing):** Assert that the rejection summary is propagated via `appendRunStep`.
Since `runAgentForMessage` now appends a completed step using `media.rejection.summary`, this test should also verify that. After the promise resolves, add an assertion such as:
```ts
expect(mocks.appendRunStep).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({ summary: "Inbound image exceeds the 8 MiB size limit" })
);
```
This confirms the rejection is recorded in the run history, not only sent in the final message.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+64
to
+85
| it("returns an oversized rejection before downloading when content length exceeds the limit", async () => { | ||
| const stored = new Map<string, { bytes: Uint8Array; contentType?: string }>(); | ||
| const fetchMock = vi.fn( | ||
| async () => | ||
| new Response("too-large", { | ||
| headers: { | ||
| "content-length": String(8 * 1024 * 1024 + 1) | ||
| } | ||
| }) | ||
| ); | ||
| globalThis.fetch = fetchMock as unknown as typeof fetch; | ||
|
|
||
| const result = await persistQqOfficialInboundMedia(envWithBucket(stored), message()); | ||
|
|
||
| expect(result.rejection).toMatchObject({ | ||
| code: "attachment_too_large", | ||
| attachmentIds: ["qq_attachment_0"], | ||
| responseText: "The image exceeds the 8 MiB size limit." | ||
| }); | ||
| expect(result.message.attachments[0]).not.toHaveProperty("r2Key"); | ||
| expect(stored.size).toBe(0); | ||
| }); |
There was a problem hiding this comment.
suggestion (testing): 添加一个测试用例：即使 content-length 在限制内，实际 body 超过限制。
目前我们只覆盖了 content-length 超过限制并在读取前拒绝的情况。请再添加一个测试：在该测试中，要么缺失 content-length，要么其值低于限制，但实际下载的 body 超过 limitBytes，以验证我们仍然会拒绝该图片，而且不会持久化或写入任何存储。
Suggested change
| it("returns an oversized rejection before downloading when content length exceeds the limit", async () => { | |
| const stored = new Map<string, { bytes: Uint8Array; contentType?: string }>(); | |
| const fetchMock = vi.fn( | |
| async () => | |
| new Response("too-large", { | |
| headers: { | |
| "content-length": String(8 * 1024 * 1024 + 1) | |
| } | |
| }) | |
| ); | |
| globalThis.fetch = fetchMock as unknown as typeof fetch; | |
| const result = await persistQqOfficialInboundMedia(envWithBucket(stored), message()); | |
| expect(result.rejection).toMatchObject({ | |
| code: "attachment_too_large", | |
| attachmentIds: ["qq_attachment_0"], | |
| responseText: "The image exceeds the 8 MiB size limit." | |
| }); | |
| expect(result.message.attachments[0]).not.toHaveProperty("r2Key"); | |
| expect(stored.size).toBe(0); | |
| }); | |
| it("returns an oversized rejection before downloading when content length exceeds the limit", async () => { | |
| const stored = new Map<string, { bytes: Uint8Array; contentType?: string }>(); | |
| const fetchMock = vi.fn( | |
| async () => | |
| new Response("too-large", { | |
| headers: { | |
| "content-length": String(8 * 1024 * 1024 + 1) | |
| } | |
| }) | |
| ); | |
| globalThis.fetch = fetchMock as unknown as typeof fetch; | |
| const result = await persistQqOfficialInboundMedia(envWithBucket(stored), message()); | |
| expect(result.rejection).toMatchObject({ | |
| code: "attachment_too_large", | |
| attachmentIds: ["qq_attachment_0"], | |
| responseText: "The image exceeds the 8 MiB size limit." | |
| }); | |
| expect(result.message.attachments[0]).not.toHaveProperty("r2Key"); | |
| expect(stored.size).toBe(0); | |
| }); | |
| it("returns an oversized rejection after downloading when body exceeds the limit", async () => { | |
| const stored = new Map<string, { bytes: Uint8Array; contentType?: string }>(); | |
| const limitPlusOne = 8 * 1024 * 1024 + 1; | |
| const imageBytes = new Uint8Array(limitPlusOne); | |
| imageBytes.fill(1); | |
| const fetchMock = vi.fn( | |
| async () => | |
| new Response(imageBytes, { | |
| headers: { | |
| "content-type": "image/png" | |
| // intentionally omit content-length so we exercise the streaming limit | |
| } | |
| }) | |
| ); | |
| globalThis.fetch = fetchMock as unknown as typeof fetch; | |
| const result = await persistQqOfficialInboundMedia(envWithBucket(stored), message()); | |
| expect(result.rejection).toMatchObject({ | |
| code: "attachment_too_large", | |
| attachmentIds: ["qq_attachment_0"], | |
| responseText: "The image exceeds the 8 MiB size limit." | |
| }); | |
| expect(result.message.attachments[0]).not.toHaveProperty("r2Key"); | |
| expect(stored.size).toBe(0); | |
| }); |
Original comment in English
suggestion (testing): Add a test case where the body exceeds the limit even though content-length is within the limit.
Currently we only cover the case where content-length exceeds the limit and we reject before reading. Please also add a test where content-length is missing or below the limit but the downloaded body exceeds limitBytes, to verify we still reject the image and never persist it or write it to storage.
Suggested change
| it("returns an oversized rejection before downloading when content length exceeds the limit", async () => { | |
| const stored = new Map<string, { bytes: Uint8Array; contentType?: string }>(); | |
| const fetchMock = vi.fn( | |
| async () => | |
| new Response("too-large", { | |
| headers: { | |
| "content-length": String(8 * 1024 * 1024 + 1) | |
| } | |
| }) | |
| ); | |
| globalThis.fetch = fetchMock as unknown as typeof fetch; | |
| const result = await persistQqOfficialInboundMedia(envWithBucket(stored), message()); | |
| expect(result.rejection).toMatchObject({ | |
| code: "attachment_too_large", | |
| attachmentIds: ["qq_attachment_0"], | |
| responseText: "The image exceeds the 8 MiB size limit." | |
| }); | |
| expect(result.message.attachments[0]).not.toHaveProperty("r2Key"); | |
| expect(stored.size).toBe(0); | |
| }); | |
| it("returns an oversized rejection before downloading when content length exceeds the limit", async () => { | |
| const stored = new Map<string, { bytes: Uint8Array; contentType?: string }>(); | |
| const fetchMock = vi.fn( | |
| async () => | |
| new Response("too-large", { | |
| headers: { | |
| "content-length": String(8 * 1024 * 1024 + 1) | |
| } | |
| }) | |
| ); | |
| globalThis.fetch = fetchMock as unknown as typeof fetch; | |
| const result = await persistQqOfficialInboundMedia(envWithBucket(stored), message()); | |
| expect(result.rejection).toMatchObject({ | |
| code: "attachment_too_large", | |
| attachmentIds: ["qq_attachment_0"], | |
| responseText: "The image exceeds the 8 MiB size limit." | |
| }); | |
| expect(result.message.attachments[0]).not.toHaveProperty("r2Key"); | |
| expect(stored.size).toBe(0); | |
| }); | |
| it("returns an oversized rejection after downloading when body exceeds the limit", async () => { | |
| const stored = new Map<string, { bytes: Uint8Array; contentType?: string }>(); | |
| const limitPlusOne = 8 * 1024 * 1024 + 1; | |
| const imageBytes = new Uint8Array(limitPlusOne); | |
| imageBytes.fill(1); | |
| const fetchMock = vi.fn( | |
| async () => | |
| new Response(imageBytes, { | |
| headers: { | |
| "content-type": "image/png" | |
| // intentionally omit content-length so we exercise the streaming limit | |
| } | |
| }) | |
| ); | |
| globalThis.fetch = fetchMock as unknown as typeof fetch; | |
| const result = await persistQqOfficialInboundMedia(envWithBucket(stored), message()); | |
| expect(result.rejection).toMatchObject({ | |
| code: "attachment_too_large", | |
| attachmentIds: ["qq_attachment_0"], | |
| responseText: "The image exceeds the 8 MiB size limit." | |
| }); | |
| expect(result.message.attachments[0]).not.toHaveProperty("r2Key"); | |
| expect(stored.size).toBe(0); | |
| }); |
Comment on lines
+83
to
+84
| expect(result.message.attachments[0]).not.toHaveProperty("r2Key"); | ||
| expect(stored.size).toBe(0); |
There was a problem hiding this comment.
suggestion (testing): 考虑测试混合/多个附件，以验证部分拒绝行为以及 attachmentIds 的聚合。
目前我们只覆盖了单个超大图片及其拒绝/持久化行为。由于 persistQqOfficialInboundMedia 会聚合 attachmentResults 并扁平化 rejection.attachmentIds，请添加一个包含混合附件的测试（例如，一个超大图片加一个合法图片，或一个图片加一个非图片）。该测试应验证：合法附件被持久化并带有 r2Key，rejection.attachmentIds 仅包含被拒绝的附件 ID，且 message.attachments 保持预期的顺序和内容。
建议实现如下：
const result = await persistQqOfficialInboundMedia(envWithBucket(stored), message());
expect(result.rejection).toMatchObject({
code: "attachment_too_large",
attachmentIds: ["qq_attachment_0"],
responseText: "The image exceeds the 8 MiB size limit."
});
expect(result.message.attachments[0]).not.toHaveProperty("r2Key");
expect(stored.size).toBe(0);
});
it("persists valid attachments and rejects oversized ones while aggregating attachmentIds", async () => {
const stored = new Map<string, ArrayBuffer>();
const smallImageBytes = new Uint8Array(1024); // 1 KiB
const largeImageBytes = new Uint8Array(9 * 1024 * 1024); // > 8 MiB
const fetchMock = vi
.fn()
// First attachment: valid image
.mockResolvedValueOnce(
new Response(smallImageBytes, {
status: 200,
headers: { "content-type": "image/png" }
})
)
// Second attachment: oversized image
.mockResolvedValueOnce(
new Response(largeImageBytes, {
status: 200,
headers: { "content-type": "image/png" }
})
);
globalThis.fetch = fetchMock as unknown as typeof fetch;
const mixedMessage = message();
mixedMessage.attachments = [
{
...mixedMessage.attachments[0],
id: "qq_attachment_0",
url: "https://cdn.qq.com/image-small.png"
},
{
...mixedMessage.attachments[0],
id: "qq_attachment_1",
url: "https://cdn.qq.com/image-large.png"
}
];
const result = await persistQqOfficialInboundMedia(envWithBucket(stored), mixedMessage);
expect(fetchMock).toHaveBeenNthCalledWith(1, "https://cdn.qq.com/image-small.png");
expect(fetchMock).toHaveBeenNthCalledWith(2, "https://cdn.qq.com/image-large.png");
// Valid attachment gets persisted and has an r2Key
expect(result.message.attachments[0]).toMatchObject({
id: "qq_attachment_0",
r2Key: "attachments/agent-1/msg_qq/qq_attachment_0",
mimeType: "image/png",
size: smallImageBytes.byteLength
});
// Oversized attachment is present but has no r2Key
expect(result.message.attachments[1]).toMatchObject({
id: "qq_attachment_1"
});
expect(result.message.attachments[1]).not.toHaveProperty("r2Key");
// Only the oversized attachment id is aggregated in the rejection
expect(result.rejection).toMatchObject({
code: "attachment_too_large",
attachmentIds: ["qq_attachment_1"]
});
// Only one object stored in the bucket for the valid attachment
expect(stored.size).toBe(1);
expect(stored.has("attachments/agent-1/msg_qq/qq_attachment_0")).toBe(true);
});该补丁假设：
message()返回的对象包含一个可变的attachments数组，并且attachments[0]的结构与 QQ 附件 schema 兼容（即展开并重写id/url是合法的）。- 桶的键模式
attachments/agent-1/msg_qq/<attachmentId>与persistQqOfficialInboundMedia实际使用的模式一致。 - 测试环境中可以使用
Response（如果不能，请从你的 fetch 实现中导入，例如undici或node-fetch）。如果你现有的 helper 不同（例如message支持通过参数配置 attachments），请相应调整mixedMessage的构造方式，但要保持对r2Key、rejection.attachmentIds、message.attachments顺序以及stored大小/键的期望不变。
Original comment in English
suggestion (testing): Consider testing mixed/multiple attachments to validate partial rejection behavior and attachmentIds aggregation.
Right now we only cover a single oversized image and the rejection/persistence behavior. Since persistQqOfficialInboundMedia aggregates attachmentResults and flattens rejection.attachmentIds, please add a test with mixed attachments (e.g., one oversized and one valid image, or an image plus a non-image). That test should verify that valid attachments are persisted with an r2Key, rejection.attachmentIds only contains the rejected attachment IDs, and message.attachments preserves the expected order and contents.
Suggested implementation:
const result = await persistQqOfficialInboundMedia(envWithBucket(stored), message());
expect(result.rejection).toMatchObject({
code: "attachment_too_large",
attachmentIds: ["qq_attachment_0"],
responseText: "The image exceeds the 8 MiB size limit."
});
expect(result.message.attachments[0]).not.toHaveProperty("r2Key");
expect(stored.size).toBe(0);
});
it("persists valid attachments and rejects oversized ones while aggregating attachmentIds", async () => {
const stored = new Map<string, ArrayBuffer>();
const smallImageBytes = new Uint8Array(1024); // 1 KiB
const largeImageBytes = new Uint8Array(9 * 1024 * 1024); // > 8 MiB
const fetchMock = vi
.fn()
// First attachment: valid image
.mockResolvedValueOnce(
new Response(smallImageBytes, {
status: 200,
headers: { "content-type": "image/png" }
})
)
// Second attachment: oversized image
.mockResolvedValueOnce(
new Response(largeImageBytes, {
status: 200,
headers: { "content-type": "image/png" }
})
);
globalThis.fetch = fetchMock as unknown as typeof fetch;
const mixedMessage = message();
mixedMessage.attachments = [
{
...mixedMessage.attachments[0],
id: "qq_attachment_0",
url: "https://cdn.qq.com/image-small.png"
},
{
...mixedMessage.attachments[0],
id: "qq_attachment_1",
url: "https://cdn.qq.com/image-large.png"
}
];
const result = await persistQqOfficialInboundMedia(envWithBucket(stored), mixedMessage);
expect(fetchMock).toHaveBeenNthCalledWith(1, "https://cdn.qq.com/image-small.png");
expect(fetchMock).toHaveBeenNthCalledWith(2, "https://cdn.qq.com/image-large.png");
// Valid attachment gets persisted and has an r2Key
expect(result.message.attachments[0]).toMatchObject({
id: "qq_attachment_0",
r2Key: "attachments/agent-1/msg_qq/qq_attachment_0",
mimeType: "image/png",
size: smallImageBytes.byteLength
});
// Oversized attachment is present but has no r2Key
expect(result.message.attachments[1]).toMatchObject({
id: "qq_attachment_1"
});
expect(result.message.attachments[1]).not.toHaveProperty("r2Key");
// Only the oversized attachment id is aggregated in the rejection
expect(result.rejection).toMatchObject({
code: "attachment_too_large",
attachmentIds: ["qq_attachment_1"]
});
// Only one object stored in the bucket for the valid attachment
expect(stored.size).toBe(1);
expect(stored.has("attachments/agent-1/msg_qq/qq_attachment_0")).toBe(true);
});This patch assumes:
message()returns an object with a mutableattachmentsarray and anattachments[0]shape compatible with the QQ attachment schema (i.e., spreading it and overridingid/urlis valid).- The bucket key pattern
attachments/agent-1/msg_qq/<attachmentId>matches whatpersistQqOfficialInboundMediaactually uses. Responseis available in the test environment (if not, import it from your fetch implementation, e.g.,undiciornode-fetch).
If your existing helpers differ (e.g.,messagesupports a configuration argument for attachments), adapt the construction ofmixedMessageaccordingly, but keep the expectations aroundr2Key,rejection.attachmentIds, order ofmessage.attachments, andstoredsize/keys the same.
Comment on lines
+68
to
+77
| it("sends the media rejection directly and skips agent execution", async () => { | ||
| await expect(runAgentForMessage({} as Env, message())).resolves.toEqual(expect.any(String)); | ||
|
|
||
| expect(mocks.sendFinalMessage).toHaveBeenCalledWith( | ||
| {}, | ||
| expect.any(String), | ||
| expect.objectContaining({ id: "msg_qq" }), | ||
| "The image exceeds the 8 MiB size limit." | ||
| ); | ||
| expect(mocks.completeRun).toHaveBeenCalledWith(undefined, expect.any(String), "completed"); |
There was a problem hiding this comment.
suggestion (testing): 为无拒绝路径添加一个互补测试，以确保正常的 agent 执行流程仍然会运行。
为了充分覆盖 runAgentForMessage 的媒体处理逻辑，请添加一个测试，使 persistInboundMedia 解析为 { message, rejection: undefined }。在这种情况下，断言 executeAgentToolLoop 被调用，sendFinalMessage 不会在该路径中被调用，并且运行通过正常流程完成。这有助于捕获在无拒绝路径上被意外短路的回归问题。
建议实现如下：
mocks.persistInboundMedia.mockResolvedValue({
message: message(),
rejection: {
code: "attachment_too_large",
attachmentIds: ["qq_attachment_0"],
responseText: "The image exceeds the 8 MiB size limit.",
summary: "Inbound image exceeds the 8 MiB size limit"
}
});
});
it("continues normal agent execution when media is accepted", async () => {
mocks.persistInboundMedia.mockResolvedValue({
message: message(),
rejection: undefined
});
await expect(runAgentForMessage({} as Env, message())).resolves.toEqual(
expect.any(String)
);
expect(mocks.executeAgentToolLoop).toHaveBeenCalled();
expect(mocks.sendFinalMessage).not.toHaveBeenCalled();
expect(mocks.completeRun).toHaveBeenCalledWith(
undefined,
expect.any(String),
"completed"
);
});- 请确保这个新测试放在与现有媒体拒绝测试相同的
describe块中，以便复用相同的mocks和message()设置。 - 如果该文件中的其他测试也依赖
mocks.sendFinalMessage或mocks.executeAgentToolLoop，请确认任何共享的beforeEach会正确重置这些 mock（例如使用jest.clearAllMocks()），从而保证这个新测试在一个干净的状态下开始运行。
Original comment in English
suggestion (testing): Add a complementary test for the no-rejection path to ensure the normal agent execution still runs.
To fully exercise runAgentForMessage’s media handling, please add a test where persistInboundMedia resolves to { message, rejection: undefined }. In that case, assert that executeAgentToolLoop is called, sendFinalMessage is not invoked from this path, and the run completes via the normal flow. This will help catch regressions where the non-rejection path is accidentally short-circuited.
Suggested implementation:
mocks.persistInboundMedia.mockResolvedValue({
message: message(),
rejection: {
code: "attachment_too_large",
attachmentIds: ["qq_attachment_0"],
responseText: "The image exceeds the 8 MiB size limit.",
summary: "Inbound image exceeds the 8 MiB size limit"
}
});
});
it("continues normal agent execution when media is accepted", async () => {
mocks.persistInboundMedia.mockResolvedValue({
message: message(),
rejection: undefined
});
await expect(runAgentForMessage({} as Env, message())).resolves.toEqual(
expect.any(String)
);
expect(mocks.executeAgentToolLoop).toHaveBeenCalled();
expect(mocks.sendFinalMessage).not.toHaveBeenCalled();
expect(mocks.completeRun).toHaveBeenCalledWith(
undefined,
expect.any(String),
"completed"
);
});- Ensure this new test is placed within the same
describeblock as the existing media rejection test so it shares the samemocksandmessage()setup. - If other tests in this file rely on
mocks.sendFinalMessageormocks.executeAgentToolLoop, confirm that any sharedbeforeEachcorrectly resets mocks (e.g.jest.clearAllMocks()), so this new test starts from a clean state.
Comment on lines
+69
to
+78
| await expect(runAgentForMessage({} as Env, message())).resolves.toEqual(expect.any(String)); | ||
|
|
||
| expect(mocks.sendFinalMessage).toHaveBeenCalledWith( | ||
| {}, | ||
| expect.any(String), | ||
| expect.objectContaining({ id: "msg_qq" }), | ||
| "The image exceeds the 8 MiB size limit." | ||
| ); | ||
| expect(mocks.completeRun).toHaveBeenCalledWith(undefined, expect.any(String), "completed"); | ||
| expect(mocks.executeAgentToolLoop).not.toHaveBeenCalled(); |
There was a problem hiding this comment.
suggestion (testing): 断言拒绝摘要通过 appendRunStep 进行了传递。
由于 runAgentForMessage 现在会使用 media.rejection.summary 追加一个已完成的步骤到运行中，本测试也应该验证这一点。在 Promise 解析之后，添加如下断言：
expect(mocks.appendRunStep).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({ summary: "Inbound image exceeds the 8 MiB size limit" })
);这可以确认拒绝信息不仅通过最终消息发送出去，也已记录在运行历史中。
Original comment in English
suggestion (testing): Assert that the rejection summary is propagated via appendRunStep.
Since runAgentForMessage now appends a completed step using media.rejection.summary, this test should also verify that. After the promise resolves, add an assertion such as:
expect(mocks.appendRunStep).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({ summary: "Inbound image exceeds the 8 MiB size limit" })
);This confirms the rejection is recorded in the run history, not only sent in the final message.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
处理超大 QQ 图片的入站媒体拒绝，并将其在媒体管道和 agent runner 中向下传播。
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Handle inbound media rejections for oversized QQ images and propagate them through the media pipeline and agent runner.
Bug Fixes:
Enhancements:
Tests: