fix: support embedded context in ACP mode for Zed @ file references#1264
fix: support embedded context in ACP mode for Zed @ file references#1264yetone wants to merge 2 commits intoMoonshotAI:mainfrom
Conversation
- Set embedded_context=True in ACP PromptCapabilities so clients (e.g. Zed) are allowed to send embedded resource blocks - Handle EmbeddedResourceContentBlock in acp_blocks_to_content_parts by extracting TextResourceContents into tagged text blocks - Handle ResourceContentBlock as a resource link fallback - Add tests for embedded resource and resource link conversion Closes MoonshotAI#1054
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR fixes Zed editor's @ file reference feature in ACP mode by enabling embedded context support and implementing handlers for resource content blocks.
Changes:
- Enabled
embedded_context=Truein ACP server capabilities to allow clients to send embedded resource blocks - Added conversion logic for
EmbeddedResourceContentBlock(embedded file content) andResourceContentBlock(file references) to XML-tagged text format - Added comprehensive unit tests for the new conversion paths
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/kimi_cli/acp/server.py | Changed embedded_context from False to True in ACP PromptCapabilities to enable resource block support |
| src/kimi_cli/acp/convert.py | Added handlers for EmbeddedResourceContentBlock and ResourceContentBlock to convert them to XML-tagged text format for LLM consumption |
| tests/ui_and_conv/test_acp_convert.py | Added three new test functions covering embedded text resources, resource links, and mixed block scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| content.append( | ||
| TextPart(text=f"<resource_link uri={block.uri!r} name={block.name!r} />") |
There was a problem hiding this comment.
Consider handling the case where block.name might be None or empty. If name is None, the current code will format it as name='None' in the XML tag, which may not be the intended behavior. Consider using block.name or block.uri.split('/')[-1] to extract a filename from the URI as a fallback, or omitting the name attribute if it's None.
| content.append( | |
| TextPart(text=f"<resource_link uri={block.uri!r} name={block.name!r} />") | |
| name = block.name or (block.uri.split("/")[-1] if getattr(block, "uri", None) else None) | |
| name_attr = f" name={name!r}" if name else "" | |
| content.append( | |
| TextPart(text=f"<resource_link uri={block.uri!r}{name_attr} />") |
| def test_acp_blocks_to_content_parts_handles_embedded_text_resource(): | ||
| block = acp.schema.EmbeddedResourceContentBlock( | ||
| type="resource", | ||
| resource=acp.schema.TextResourceContents( | ||
| uri="file:///path/to/foo.py", | ||
| text="print('hello')", | ||
| ), | ||
| ) | ||
| parts = acp_blocks_to_content_parts([block]) | ||
| assert len(parts) == 1 | ||
| assert isinstance(parts[0], TextPart) | ||
| assert "file:///path/to/foo.py" in parts[0].text | ||
| assert "print('hello')" in parts[0].text |
There was a problem hiding this comment.
The test coverage should include edge cases with special characters in URIs and text content, such as URIs containing quotes, angle brackets, or ampersands, and text containing XML special characters. This would help verify whether the XML formatting handles these cases correctly.
| else: | ||
| logger.warning( | ||
| "Unsupported embedded resource type: {type}", | ||
| type=type(resource).__name__, | ||
| ) |
There was a problem hiding this comment.
Consider adding a test for non-text embedded resource types (e.g., BlobResourceContents) to verify that the warning is properly logged and the block is gracefully skipped. This would ensure the unsupported resource type handling path is tested.
Problem
When using Zed editor's
@syntax to reference files, kimi ACP mode doesn't recognize the file content. This is because:embedded_contextwas set toFalsein ACPPromptCapabilities, so Zed never sends the embedded resource blocksacp_blocks_to_content_partshad no handler forEmbeddedResourceContentBlockorResourceContentBlockFix
embedded_context=Truein the ACP server'sPromptCapabilitiesto allow clients to includeContentBlock::Resourcein prompt requestsEmbeddedResourceContentBlockby extractingTextResourceContentsinto XML-tagged text blocks with the resource URIResourceContentBlockas a resource link fallback with URI and nameTesting
pytest tests/ui_and_conv/test_acp_convert.py)Closes #1054