chore: stage generic ACP base for Muse integration - #3
Draft
naytewilson wants to merge 15 commits into
Draft
Conversation
# Conflicts: # apps/server/scripts/acp-mock-agent.ts # apps/server/src/provider/acp/AcpSessionRuntime.ts
…etain PR's optional auth and config-option callbacks
- Consume session/update available_commands_update and publish it as the provider instance's slashCommands (menu populates after first session). - When no mode alias matches, keep the agent's current mode instead of picking the first non-plan mode.
… id/name
- The availability probe reads _meta.availableCommands, so an agent's slash
commands are known before any session and persist in the provider cache.
- Mode aliases no longer match on descriptions: forge's planning mode
('without code changes') and research mode ('codebase exploration') both
matched the implement alias 'code', silently putting the agent in a
read-only mode. Match ids and names, and never pick a plan mode as the
working mode.
T3 re-applies its requested mode on every turn. Forcing a mode when none of T3's aliases match undid a mode the user chose through the agent itself, so forge's /muse reverted to forge on the next prompt.
Session config options still win once a session reports them; without this an ACP provider's model picker was empty until a session existed.
…ions Config options observed from an earlier session can predate the agent build the probe just ran; the initialize list is current.
… models It only stood in while no models were known; with the agent's current model marked as default it was a second way to say the same thing.
github-actions
Bot
added
vouch:trusted
Confirmed against the source and fixed (verdicts from a per-finding review): - Plan-mode filter dropped valid working modes: isPlanMode matched on the mode description, so a working mode (e.g. `agent`) whose prose mentions planning was excluded from workingModes and never selected, leaving the agent in its previous mode. Match by id/name only. - Invalid standard config values failed the whole turn: a stale/invalid select value was forwarded to setConfigOption and rejected with -32602. Validate string select values against the option's allowed values first, matching the model path; booleans are left to the runtime. - Model selection silently dropped on whitespace: displayed slugs are trimmed but validation compared untrimmed values. Match on trimmed values and send the matching raw value. - Images dropped for capable agents: embedding was gated on promptCapabilities.image === true, silently dropping images (and failing image-only turns) for agents that accept images without advertising the flag -- a regression from the shared-core refactor. Embed unless the agent explicitly declines (image === false). - Failed initialize cached forever: Effect.cached memoized the full Exit, so a failed handshake was replayed to every later start()/initialize() call, contradicting start()'s retry contract. Cache only success and invalidate on failure (cachedInvalidateWithTTL). - MCP toolkit dropped silently: HTTP/SSE MCP servers (incl. T3's own t3-code) are filtered out when the agent omits the capability flag. Keep the gate but log a warning so the loss isn't silent. - Withdrawn slash commands persisted: parseAdvertisedCommands collapsed "absent" and "explicitly empty" both to [], and the caller only wrote on length > 0, so an explicit empty availableCommands could not withdraw stale commands. Return undefined for absent vs [] for empty, and write whenever a list is advertised. - Web: text-gen write handlers used the unfiltered provider list, so the resolver's fallback could persist an ACP instance as the text-generation model (no server-side text gen). Use the same ACP-filtered list as the read path. Server + web typecheck clean; all ACP test suites pass (51 tests). Deferred (need a design decision, noted on the PR): generic SessionModelState models are inert in the standard path (needs the session model state threaded into the apply path, mirroring GrokAcpSupport's setSessionModel usage); the ACP provider icon is a wide wordmark in square slots (needs a square glyph). The acceptForSession -> allow_always finding was reviewed and is NOT a bug: "Always allow this session" is correctly encoded as allow_always. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012kJsM2KuUdGPPYukuHHLuy
Models advertised through SessionModelState (e.g. initialize `_meta.modelState`) were shown in the picker but never applied by the generic "standard" path: it only handled `category:'model'` config options and never called setSessionModel. The selection was silently dropped, session.model stayed unset, and the reactor treated every model-bearing turn as a change -- tearing the session down and dropping agent context. Thread the session model state into the apply path: - AcpSessionRuntime exposes getModelState (SessionModelState), populated from the session setup response and updated after a successful session/set_model, mirroring the existing getModeState/modeStateRef. - The standard apply path, when a selection matches no config-option model, looks it up in the model state and applies it via session/set_model (only when it actually changes), recording appliedModel so session.model reflects it and the reactor no longer restarts the session each turn. Regression test: selecting a SessionModelState-only model (grok-mock-alt) now sends session/set_model with that id and never misroutes it as a config-option write. Server typecheck clean; ACP suites pass (incl. the new test). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012kJsM2KuUdGPPYukuHHLuy
The ACP provider icon was a ~2.6:1 wordmark rendered in square icon slots (ModelListRow, ProviderInstanceIcon, and the provider settings dialogs), where it letterboxed to a thin horizontal sliver instead of reading as a peer icon. Replace it with a square plug glyph, matching the mobile app which already uses a plug for the same driver. The former ACPRegistryIcon wordmark had no other use and is removed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012kJsM2KuUdGPPYukuHHLuy
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
3 participants
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.
Internal staging PR for Muse Code integration. Base is the isolated current-upstream branch forge/muse-code-msp-provider. Head is the upstream generic ACP work from JDLondon7/t3code#9242. Do not merge to fork main. This exists only to let GitHub adjudicate/reconcile the ACP stack against current T3 before Muse-specific work.