fix: backward-compatible renderMessage option for tool messages - #37
Open
ranxianglei wants to merge 2 commits into
Open
fix: backward-compatible renderMessage option for tool messages#37ranxianglei wants to merge 2 commits into
ranxianglei wants to merge 2 commits into
Conversation
renderMessage() prepended ACP tags (<acp tokens="XX" type="YY">REF</acp>) to ALL messages including tool-call arguments stored in CoreMessage.text. In proxy adapters that store wire-format JSON in .text (e.g. OpenAI function.arguments), this corrupted the JSON and caused downstream SchemaError (Missing key at ["command"] etc). Tool messages don't need ACP tags: they are structured metadata, not displayable text. assignRefs still allocates refs for them (compression ranges can still target them), and adjustBoundariesForToolPairs auto- expands ranges to include tool_call+tool_result pairs. The model never needs to see a tool message's ref to compress it. Plugin (opencode-acp) is unaffected: tool args live in Part.input (a separate JSON field), not CoreMessage.text, so tags on .text were always cosmetic metadata, never load-bearing.
Redesign based on review feedback: instead of unconditionally skipping
tool messages (breaking change for existing callers), extract rendering
into composable building blocks with opt-in behavior.
Changes:
- renderMessage() and renderVisibleRefs() accept optional RenderOptions
parameter ({ skipToolMessages?: boolean }). Default behavior unchanged.
- Config gains optional render?: RenderConfig ({ skipToolMessageTags?: boolean }).
- renderRefsNode reads ctx.config.render?.skipToolMessageTags via
optionsFromConfig() helper. Pipeline callers with no config change get
the original behavior.
- renderMessage() is now exported (was private) so adapters can reuse it.
Plugin (opencode-acp): no config change needed, identical behavior.
Proxy (acp-proxy): sets config.render.skipToolMessageTags = true.
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
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.
Problem
renderMessage()tags ALL messages including tool-call arguments inCoreMessage.text. In proxy adapters (e.g.acp-proxy) that store wire-format JSON in.text(OpenAIfunction.arguments), this corrupted the JSON →SchemaError(Missing key at ["command"]).Approach (redesigned per review)
Unconditionally skip tool messages.← rejected: breaking change for existing callers.New approach: opt-in via
RenderOptions+RenderConfig, zero behavior change for existing callers.API changes
Who opts in?
config.render = { skipToolMessageTags: true }Tests (211/211 pass)
renderVisibleRefs tags tool messages by default (backward compat)— existing behavior preservedrenderVisibleRefs skips tool messages when options.skipToolMessages is set— opt-in worksrenderRefsNode respects config.render.skipToolMessageTags— pipeline integrationWhy this is safe
renderMessage()default path: identical to beforerenderVisibleRefs()default path: identical to beforerenderRefsNodewith noconfig.render: identical to beforerenderMessage()now exported: adapters can reuse without reimplementingassignRefsruns beforerender-refs)adjustBoundariesForToolPairsauto-expands ranges)