fix(chat): preserve multi-mode arrays through ChatOrchestrator - #2237
Merged
Conversation
Multi-mode chat requests (e.g. agent_modes=['cluckin-chuck','chat']) were silently collapsed to a single junk mode like 'cluckin-chuckchat' because ChatOrchestrator joined the array to a comma-string at the boundary and downstream sanitize_key() calls stripped the comma. Net effect: mode-restricted tool allowlist filters (host plugins using 'datamachine_resolved_tools' to lock down a public chat surface to a specific tool subset) silently no-op'd. Tools that should be gated by a custom mode would leak through whenever 'chat' was also in the active modes — defeating the whole point of the multi-mode resolution pattern. Symptom in a real downstream (Cluckin' Chuck): - frontend chat injects client_context.agent_modes = ['cluckin-chuck','chat'] - the 'chat' mode is required as the execution surface; 'cluckin-chuck' carries the custom directive + tool allowlist filter - Without this fix the joined string becomes 'cluckin-chuck,chat' → sanitize_key strips the comma → 'cluckin-chuckchat' (junk) - The allowlist filter's in_array(SLUG, modes) check fails → filter exits early → every chat-mode tool (read_instagram, publish_*, etc.) is reachable from the public surface Fix: - processChat (line 212): pass 'modes' => $modes array alongside the joined 'mode' string. executeConversationTurn already prefers the array when present (see line 715), so the multi-mode resolution survives. - processContinue (line 388): recover the modes array from session.mode by splitting on comma instead of trusting sanitize_key. Existing sessions that were stored before this fix as junk single-mode strings will resume with their broken mode, which is acceptable — the DB row is the source of truth and can be patched out-of-band. - createSession (line 641): preserve comma-joined multi-mode strings when writing to session.mode (sanitize each part individually) so subsequent processContinue calls can recover the array. Verified end-to-end with a downstream that depends on this for security- adjacent surface restriction: stored='chat' → modes=[chat] (admin surface — correct) stored='cluckin-chuck' → modes=[cluckin-chuck] (custom — correct) stored='cluckin-chuck,chat' → modes=[cluckin-chuck,chat] (multi — FIXED) stored='cluckin-chuckchat' → modes=[cluckin-chuckchat] (legacy junk, unchanged) Reported by Extra-Chill / Cluckin' Chuck integration. Closes the upstream half of chubes4/cluckin-chuck#12.
Contributor
Homeboy Results —
|
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.
Closes the upstream half of chubes4/cluckin-chuck#12.
Problem
Multi-mode chat requests get silently collapsed to a single junk mode whenever they flow through
ChatOrchestrator. For a request likeagent_modes = ['cluckin-chuck', 'chat']:processChatline 71 normalizes the array correctly:$modes = ['cluckin-chuck', 'chat']$mode = 'cluckin-chuck,chat''mode' => $mode) toexecuteConversationTurnexecuteConversationTurnline 715:normalizeModes($options['mode'])runssanitize_key('cluckin-chuck,chat')sanitize_keystrips the comma →'cluckin-chuckchat'(single junk mode)cluckin-chuckmode no longer exists in the resolved listprocessContinueline 388 has the same shape, andcreateSessionline 641sanitize_keys the multi-mode string when writing tosession.mode— so the broken state persists across session resumption.Why this matters
Mode-restricted tool allowlist filters (host plugins that hook
datamachine_resolved_toolsto lock down a public chat surface to a specific tool subset) silently no-op when their mode slug doesn't appear in the active mode list. That defeats the entire point of the multi-mode resolution pattern: a downstream that intentionally composes a custom mode +chatas the execution surface ends up with the unrestrictedchatsurface.Concrete downstream example
Cluckin' Chuck uses this exact pattern for a public chat agent. The
cluckin-chuckmode carries the wing-business directive + a tool allowlist filter that strips everything except 13 wing tools.chatis required as the execution surface or DM treats the call as pipeline mode and the filter never runs.With this bug present:
client_context.agent_modes = ['cluckin-chuck', 'chat']['cluckin-chuckchat']in_array('cluckin-chuck', $modes, true)is falseread_instagramfrom a public chat sessionFix
Three small changes inside
ChatOrchestrator:processChat(line 212): also pass'modes' => $modes(array) alongside the joined'mode'string.executeConversationTurnline 715 already prefers the array form when present (!empty($options['modes'])), so the multi-mode resolution survives without further refactor.processContinue(line 388): recover the modes array fromsession.modeby splitting on comma instead of trustingsanitize_key. Same dual-key pattern ('modes' + 'mode').createSession(line 641): preserve comma-joined multi-mode strings when writing tosession.mode. Sanitize each comma-separated part individually so the multi-mode shape survives subsequentprocessContinuecalls.Verification
End-to-end simulation against the live filter chain in a downstream that depends on this for surface restriction:
The legacy-junk row is unchanged on purpose — sessions created before this fix carry the broken
session.modevalue, and the DB is the source of truth. Operators can patch those out-of-band if needed; downstream Cluckin' Chuck just ranUPDATE wp_datamachine_chat_sessions SET mode='cluckin-chuck,chat' WHERE session_id=...for the one affected session.Out of scope
A proper fix would persist
agent_modesas a JSON array column in the session table, so the modes are first-class state instead of being recovered from a comma-string heuristic. That's a schema migration and a bigger PR. This change is the minimum required to close the surface-escape security issue without touching the schema.Versioning
Patch bump (no breaking changes to public APIs —
'mode'is still passed for backward compat,'modes'is additive). Handled by your release tooling.