Repository navigation
fix(dsh): read the current session format for recall sources and tool results - #176
Merged
Merged
Conversation
… results
Session format v4 admits only producer-owned message source kinds and refuses
the released `{ kind: 'plugin', plugin }` wrapper. The injected recall message
therefore failed admission, so every turn that reached recall ended before its
first step with:
format v4 message requires a producer-owned source kind
The same format carries a tool result's call identity on the message itself and
no longer wraps it in a `tool-result` content block. Tool memory read only the
wrapper, so it collected no result ids at all — which also dropped
tool-call-only assistant messages, because a call is emitted only once its
result has been correlated. Measured against a real v4 session of ten completed
turns with `includeToolMemory` enabled: zero tool messages, each turn reduced to
one user and one assistant message.
The current agent preset also moved out of the session header into the
`agentPreset` session projection, leaving the header with only its initial
value. A session that switched preset mid-flight tagged every later memory with
the stale preset.
Read whichever tool-result shape the runtime carries, name the plugin's own
source kind through the platform's merge-extensible source map, and take the
preset from the projection when the runtime exposes one, falling back to the
header otherwise.
Refs MemTensor#174
3 tasks done
Contributor
Author
|
Since workflows from a fork need approval before they run, here is the current ( legacy-0.1.0-rc.6 (same suite, Also clean locally: The legacy leg is the reason both readers keep the released shapes: |
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.
What this fixes
Three ways the plugin predates the current DSH session format (v4). The first is a hard failure; the other two are silent.
1. Recall injection is refused at admission — every turn fails
source: { kind: 'plugin', plugin: 'memos-cloud', form: 'recall' }is the retired wrapper. v4 refuses it (format v4 message requires a producer-owned source kind), and because the message is injected at step 1 of every turn, no turn completes. This is #174.The plugin now names its own producer kind,
plugin:memos-cloud, registered through the platform's merge-extensibleMessageSourceMap. That value is what the platform's own v3→v4 migration already produces for this plugin id (producerKind('memos-cloud')→plugin:memos-cloudfor any plugin outside its first-party allowlists), so newly written recall rows and rows migrated from older logs agree.plugin:memos-cloudis accepted by both runtimes:dsh-llm@0.1.2-rc.1'screateMessagedoes not validatesourceat all, anddsh-session@0.1.2-rc.1only requires a non-emptykindstring.2. Tool memory silently collected nothing
captureTurnread the call identity from the releasedtool-resultcontent wrapper. v4 retired that wrapper: the id now rides the message (message.toolCallId, mirrored onmessage.source.callId) andcontentholds plain blocks. SoresultCallIdsstayed empty, and sincetoolCalls()emits a call only once its result has been correlated, tool-call-only assistant messages were dropped as well.Measured against a real
0.2.0-rc.2session (ten completed turns,includeToolMemory: true):The reader now accepts either shape, so the released wrapper keeps working.
3. Agent preset came from the header
lifecycle.tstagged writes and recall filters withsession.header.agentPreset, which holds only the initial value; current DSH keeps the live preset in theagentPresetsession projection (Sessionexposes no projection accessor). A session that switched preset mid-flight tagged every later memory with the stale preset.The preset is now read from the projection when the runtime exposes one, falling back to the header for runtimes that do not — and a projection that reports no preset leaves it absent rather than reviving the header.
Tests
pnpm testinpackages/dsh: 168 passed, 4 skipped. Two existing assertions encoded the retired shapes and were updated; everything else passes unchanged, including the released-wrapper tool-result coverage.tsc --noEmitclean;pnpm buildclean.Notes for reviewers
memos-cloud;plugin:memos-cloudalso passes admission. I picked the prefixed form because it is exactly what the platform's own migration assigns this plugin, so migrated history and new writes stay identical. Happy to switch to a bare producer kind if you prefer.@deepseek-ai/dsh-*is pinned to0.1.2-rc.1, the last line where both retired shapes were valid, so the suite cannot exercise the current format. This PR does not bump them (that would likely require rewriting much of the suite); the new tests spell out the current shapes directly instead. Bumping them — and widening thepeerDependenciesrange past^0.1.2-rc.1— would surface the skew at install time. Happy to take that on in a follow-up if you want it in scope.Refs #174