Skip to content

Route long tool resources to file attachments - #79

Merged
formulahendry merged 3 commits into
formulahendry:mainfrom
Eskibear:eskibear-runtime-resource-setting
Aug 15, 2026
Merged

formulahendry merged 3 commits into
formulahendry:mainfrom
Eskibear:eskibear-runtime-resource-setting

Conversation

@Eskibear

Copy link
Copy Markdown
Contributor

Summary

  • add a configurable 1000 character inline limit for tool text resources, with longer content sent as file attachments
  • scope showResources, --hide-resources, and bridge.resources to intermediate tool output
  • keep explicit agent resources and attach_file results visible when tool resources are hidden
  • keep the artifact MCP server available regardless of the tool resource visibility setting
  • identify bridge-owned attachment links by their wechat-acp://artifact/ URI

Testing

  • npm run build
  • node --import tsx/esm --test tests/client.test.ts tests/config.test.ts tests/session.test.ts tests/bridge-new-session.test.ts
  • node --import tsx/esm --test --test-name-pattern "artifact MCP requires auth" tests/artifact.test.ts

Closes #78

Copilot AI balanced review requested due to automatic review settings August 14, 2026 16:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Routes oversized tool text resources to file attachments while preserving explicit resource delivery.

Changes:

  • Adds configurable inline limits and runtime resource visibility.
  • Distinguishes tool resources from explicit artifacts.
  • Updates tests and documentation.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/acp/client.ts Routes and filters resources.
src/acp/session.ts Applies per-session visibility.
src/artifacts/store.ts Reuses artifact URI helpers.
src/artifacts/types.ts Defines artifact URI utilities.
src/bridge.ts Always starts artifact support.
src/config.ts Adds inline-limit configuration.
bin/wechat-acp.ts Adds the CLI option.
tests/client.test.ts Tests resource routing.
tests/session.test.ts Tests runtime settings.
tests/config.test.ts Tests limit validation.
tests/bridge-new-session.test.ts Tests resource configuration.
README.md Documents behavior.
CHANGELOG.md Records the change.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/acp/client.ts Outdated
Comment thread src/acp/client.ts Outdated
Copilot AI review requested due to automatic review settings August 14, 2026 16:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/acp/client.ts:1159

  • A hidden blob resource with no optional MIME type has routesToImagePipeline === false, so this return allows a mirrored rawOutput.binaryResultsForLlm image fallback to be sent. That leaks an incidental tool resource while showResources=false; fallback deduplication should not depend only on resource MIME metadata.
      opts.log(`[resource] skipped tool image (showImages=false, ${name})`);
      return true;
    }

src/acp/client.ts:896

  • When an ordinary tool resource_link omits its optional mimeType, routesToImagePipeline is false, so this hidden link does not count as handled. The subsequent rawOutput.binaryResultsForLlm image fallback can then deliver a mirrored image despite showResources=false. Suppression/deduplication needs to correlate the fallback without relying solely on optional link MIME metadata.

This issue also appears on line 1157 of the same file.

    if (source === "tool" && !explicitArtifact && opts.showResources === false) {
      opts.log(`[resource-link] skipped (showResources=false, ${name})`);
      return routesToImagePipeline;

src/acp/client.ts:1177

  • This rewrites every declared image MIME type to text/plain, including textual resources such as image/svg+xml. That conflicts with the requirement to preserve the original resource MIME type when creating the attachment. The attachment path should bypass native-image routing without changing the declared MIME.
      }
      const inlineLimit = opts.resourceInlineLimit ?? DEFAULT_RESOURCE_INLINE_LIMIT;
      if (source === "tool" && resource.text.length > inlineLimit) {
        const attachmentName = resourceAttachmentName(resource.uri);

Copilot AI review requested due to automatic review settings August 14, 2026 16:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.

Suppressed comments (1)

README.md:343

  • This now describes every listed setting as controlling only intermediate output, but bridge.audio is applied to all audio blocks, including explicit agent_message_chunk audio (src/acp/client.ts:486,1080). Keep the general wording as “output forwarding” and scope the intermediate-only statement specifically to bridge.resources, otherwise users may expect explicit audio to bypass bridge.audio off.
- The built-in `bridge.thoughts`, `bridge.diffs`, `bridge.audio`, and `bridge.resources` options control intermediate output forwarding for the current WeChat user's session. `bridge.resources off` hides tool resources, including entries from `tool_call_update.rawOutput.contents[]`. When tool resources are on, `agent.resourceInlineLimit` controls whether tool text resources are rendered inline or sent as attachments. `bridge.images` controls only inline tool images. Explicit agent resources, explicit response images, and files sent with `attach_file` are always delivered. These options use the startup config as defaults, accept `on` or `off`, and are not persisted across session resets or bridge restarts.

@formulahendry formulahendry left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at 6aef462. The filename and MIME follow-ups look correct, and the resource visibility / attachment routing matches #78. LGTM.

@formulahendry
formulahendry merged commit 4b787a5 into formulahendry:main Aug 15, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Separate tool resource visibility from explicit file attachments

3 participants