feat: consolidate aps chat lanes#35
Merged
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
feat: wire ACP MCP bridge tools
feat: implement ACP skill invocation
feat: support adapter create device types
feat: execute CLI skill scripts
|
|
||
| func execSkillScript(ctx context.Context, skill *skills.Skill, scriptName, scriptPath string, args []string, profileID string, stdin io.Reader, stdout, stderr io.Writer) error { | ||
| command, commandArgs := skillScriptCommand(scriptName, scriptPath, args) | ||
| cmd := exec.CommandContext(ctx, command, commandArgs...) |
There was a problem hiding this comment.
Pull request overview
This PR consolidates the “aps-chat” lane groundwork by introducing a native core chat subsystem, a new aps chat CLI/TUI surface (including multi-profile participant parsing), and wiring chat-mode execution into the messenger runtime. It also extends ACP to support WebSocket transport and advances skill execution paths in both CLI and ACP.
Changes:
- Added native chat primitives in
internal/core/chat(turn-taking, transcripts, persistence, LLM config resolution, provider preflight/client wiring) plusSessionTypeChat. - Added
aps chatCLI command group + TUI and session persistence, plus e2e coverage for one-shot and REPL/attach flows. - Enabled chat-mode execution for messenger services (
options.execution: chat) and added ACP WebSocket transport + richer MCP tool bridging; expanded adapter type/strategy validation and implementedaps skill run.
Reviewed changes
Copilot reviewed 51 out of 52 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/chat/main_test.go | New e2e harness compiling aps and validating chat once/REPL/attach behaviors via registry + PTY. |
| internal/core/session/types.go | Adds SessionTypeChat to the unified session registry type system. |
| internal/core/profile.go | Extends profiles with llm: config for model/provider/router resolution. |
| internal/core/chat/types.go | Defines chat roles, turns, transcripts, and completer interface. |
| internal/core/chat/turntaking.go | Adds round-robin policy + auto-turn session state machine. |
| internal/core/chat/turntaking_test.go | Unit tests for turn policy and auto-turn caps/commands. |
| internal/core/chat/store.go | Implements transcript JSON persistence under APS data dir. |
| internal/core/chat/service.go | Adds chat service to open/register sessions and send turns via LLM client. |
| internal/core/chat/service_test.go | Verifies session registration and transcript persistence across turns. |
| internal/core/chat/routellm.go | Resolves LLM config precedence and routellm router/provider URIs. |
| internal/core/chat/routellm_test.go | Tests config precedence and provider inference behavior. |
| internal/core/chat/preflight.go | Resolves provider API keys from profile secrets for candidate URIs. |
| internal/core/chat/preflight_test.go | Tests secret lookup and unauthorized error messaging. |
| internal/core/chat/persona.go | Renders system prompt from profile identity/scope/limits. |
| internal/core/chat/persona_test.go | Tests system prompt rendering and field elision. |
| internal/core/chat/participants.go | Builds participant prompts and composes multi-speaker system prompt. |
| internal/core/chat/participants_test.go | Tests active speaker marking and unknown speaker errors. |
| internal/core/chat/client.go | Builds LLM client/providers with fallback support. |
| internal/core/assets/docs/CLI.md | Documents the new aps chat command surface and messenger chat-mode config. |
| internal/core/adapter/types.go | Marks more adapter types implemented; adds strategy validation and defaults. |
| internal/core/adapter/manager.go | Validates loading strategy during adapter creation. |
| internal/core/adapter/errors.go | Adds helper to detect invalid strategy errors. |
| internal/core/adapter/create_types_test.go | Tests default strategies and create coverage for implemented types. |
| internal/cli/skill/skill.go | Implements aps skill run script execution with validation + telemetry/error shaping. |
| internal/cli/skill/skill_test.go | Adds tests for skill script execution success/failure cases. |
| internal/cli/groups.go | Places chat under the interact help group. |
| internal/cli/groups_test.go | Updates group membership expectations to include chat. |
| internal/cli/chat/types.go | Defines CLI/core engine boundary and chat CLI options/types. |
| internal/cli/chat/tui.go | Adds Bubble Tea TUI for chat, including streaming display path. |
| internal/cli/chat/session.go | Persists chat transcript metadata into the session registry. |
| internal/cli/chat/multi.go | Parses participants/invites and meta-commands for REPL control. |
| internal/cli/chat/multi_test.go | Tests participant parsing and meta-command recognition. |
| internal/cli/chat/engine.go | Adds core-backed engine + fake engine for test/e2e. |
| internal/cli/chat/cmd.go | Introduces aps chat command + once mode + multi-profile system prompt wiring. |
| internal/cli/chat.go | Registers the chat command on the root CLI. |
| internal/cli/adapter/create.go | Updates adapter create help and validates loading strategies early. |
| internal/cli/adapter/create_test.go | Tests create behavior for advertised types and invalid strategy/type cases. |
| internal/cli/acp/toggle.go | Expands ACP toggle to support ws/websocket transport selection. |
| internal/cli/acp/server.go | Starts ACP server using profile-configured stdio vs WebSocket transport. |
| internal/adapters/messenger/telegram_provider_test.go | Adds coverage ensuring chat-mode routes through native chat runner + provider reply delivery. |
| internal/adapters/messenger/handler.go | Wires chat execution mode into service runtime executor and handler options. |
| internal/adapters/messenger/chat_executor.go | Adds chat message executor and native runner bridge into core chat service. |
| internal/acp/transport_ws.go | Improves MCP bridge tool listing/calling with typed interfaces and deterministic ordering. |
| internal/acp/transport_ws_test.go | Adds tests for MCP tool listing, calling, and error conditions. |
| internal/acp/skills.go | Uses the default skills config/registry and implements skill invoke via terminal manager. |
| internal/acp/skills_test.go | Updates skill list/get tests and adds invoke execution + validation coverage. |
| internal/acp/server.go | Adds WebSocket transport support with HTTP server, address reporting, and timeouts. |
| internal/acp/acp_test.go | Adds test asserting WS HTTP server read header timeout is configured. |
| go.mod | Adds indirect deps for anthropic/openai + tidwall libs used by updated kits. |
| go.sum | Adds corresponding module sums for new indirect deps. |
| docs/stories/055-aps-chat-llm-repl.md | Updates story status to implementation and notes lane dependencies. |
| CHANGELOG.md | Documents messenger chat-mode bridge and notes pending companion integrations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+55
to
+56
| func (s *Store) path(sessionID string) string { | ||
| return filepath.Join(s.root, sessionID+".json") |
Comment on lines
+45
to
+49
| engine, err := newEngine(cmd.Context(), activeProfile, opts, systemPrompt) | ||
| if err != nil { | ||
| fmt.Fprintln(os.Stderr, err) | ||
| return err | ||
| } |
Comment on lines
+33
to
+37
| binName := "aps-chat-test" | ||
| if runtime.GOOS == "windows" { | ||
| binName += ".exe" | ||
| } | ||
| apsBinary = filepath.Join(os.TempDir(), binName) |
Comment on lines
+48
to
+52
| var params []string | ||
| if apiKey != "" { | ||
| params = append(params, "api_key="+apiKey) | ||
| } | ||
| if resolved.Config.BaseURL != "" { |
Comment on lines
+99
to
+103
| info := &session.SessionInfo{ | ||
| ID: id, | ||
| ProfileID: s.profile.ID, | ||
| Command: "aps chat", | ||
| Status: session.SessionActive, |
Comment on lines
+73
to
+77
| s.messages = append(s.messages, Message{Role: roleAssistant, Content: content}) | ||
| } else { | ||
| s.messages[len(s.messages)-1].Content = content | ||
| } | ||
| return s.persist() |
Comment on lines
+219
to
+223
| term, err := s.terminalManager.CreateTerminal( | ||
| skill.GetScriptPath(params.Script), | ||
| nil, | ||
| skill.BasePath, | ||
| map[string]string{ |
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.
Summary
Validation