refactor(agent): Agent Looper refactor phase2, restructure pipeline and rename loop files to agent by sky5454 · Pull Request #2585 · sipeed/picoclaw

sky5454 · 2026-04-19T01:46:49Z

📝 Description

Refactor pkg/agent package by renaming loop_* files to agent_* and splitting the monolithic loop.go (~1500 lines) into a structured Pipeline architecture.

Key changes:

loop.go → agent.go + pipeline_*.go (SetupTurn, CallLLM, ExecuteTools, Finalize)
loop_turn.go → turn_coord.go (runTurn becomes thin coordinator)
All loop_*.go → agent_*.go
New turn_coord_test.go tests Pipeline methods and coordinator

🗣️ Type of Change

🐞 Bug fix
✨ New feature
📖 Documentation update
⚡ Code refactoring (no functional changes, no api changes)

🤖 AI Code Generation

[] 🤖 Fully AI-generated (100% AI, 0% Human)
🛠️ Mostly AI-generated (AI draft, Human verified/modified)
[] 👨‍💻 Mostly Human-written (Human lead, AI assisted or none)

🔗 Related Issue

📚 Technical Context

Reference URL: N/A
Reasoning:
- Original loop.go was ~1500 lines with 40+ local variables and goto statements, hard to test and maintain
- After splitting into Pipeline methods, each method has single responsibility and can be tested independently
- runTurn becomes thin coordinator, passing turnExecution state through Pipeline methods

🧪 Test Environment

Hardware: PC
OS: Windows 11
Model/Provider: N/A (test framework only)
Channels: N/A

📸 Evidence

Build: ✓
Tests: ✓ (go test ./pkg/agent/... -skip "TestGlobal|TestSeahorse")
Lint: ✓ (golangci-lint run 0 issues)
Merged main without conflicts

☑️ Checklist

Phase 2 of loop.go refactor — dependency inversion using adapter pattern. - Add interfaces.MessageBus and interfaces.ChannelManager interfaces - Create adapters/messagebus.go wrapping *bus.MessageBus - Create adapters/channelmanager.go wrapping *channels.Manager - Update AgentLoop to use interfaces instead of concrete types - Update registerSharedTools to accept interfaces.MessageBus Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Pipeline refactoring: - Split pipeline.go (1400 lines) into focused files: - pipeline_setup.go (~115 lines): SetupTurn method - pipeline_llm.go (~519 lines): CallLLM method - pipeline_execute.go (~693 lines): ExecuteTools method - pipeline_finalize.go (~78 lines): Finalize method - Pipeline struct and NewPipeline remain in pipeline.go (~39 lines) Agent file renaming: - Rename loop_*.go to agent_*.go for consistent naming: - loop.go -> agent.go, loop_message.go -> agent_message.go, etc. - Merge turn.go + turn_exec.go into turn_state.go - Rename loop_turn.go -> turn_coord.go Documentation: - Update docs/pipeline-restructuring-plan.md - Add docs/agent-rename-plan.md Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Copilot

Pull request overview

Refactors pkg/agent turn execution by replacing the monolithic runTurn implementation with a coordinator (turn_coord.go) plus a structured Pipeline (pipeline_*.go), alongside broad loop_* → agent_*/turn_* renames and updated architecture docs/tests.

Changes:

Introduces Pipeline with SetupTurn, CallLLM, ExecuteTools, and Finalize to modularize the turn lifecycle.
Adds turn_coord.go as the new runTurn coordinator and centralizes turn-related state/types in turn_state.go.
Updates tests and documentation to match the new file layout and turn execution structure.

Reviewed changes

Copilot reviewed 23 out of 35 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
pkg/agent/turn_state.go	Adds coordinator/pipeline control enums and introduces `turnExecution` state container.
pkg/agent/turn_coord.go	New `runTurn` coordinator extracted from legacy turn loop; retains side-question/model routing helpers.
pkg/agent/pipeline.go	Introduces Pipeline dependency container and constructor.
pkg/agent/pipeline_setup.go	Extracts one-time turn initialization (context assembly, message building, candidate selection).
pkg/agent/pipeline_llm.go	Extracts LLM call path (hooks, retries, fallback, tool-call normalization).
pkg/agent/pipeline_execute.go	Extracts tool execution loop (hooks, media delivery, steering injection).
pkg/agent/pipeline_finalize.go	Extracts finalization logic (persist assistant msg, optional compact, return result).
pkg/agent/subturn.go	Updates SubTurn execution to call `runTurn` with a Pipeline instance.
pkg/agent/turn_coord_test.go	Adds tests for pipeline methods and coordinator integration paths.
pkg/agent/interfaces/interfaces.go	Introduces MessageBus/ChannelManager interfaces for dependency inversion.
pkg/agent/adapters/messagebus.go	Adds adapter wrapper for `*bus.MessageBus` → `interfaces.MessageBus`.
pkg/agent/adapters/channelmanager.go	Adds adapter wrapper for `*channels.Manager` → `interfaces.ChannelManager`.
pkg/agent/agent.go	Updates AgentLoop to store bus/channelManager as interfaces and to run turns via Pipeline.
pkg/agent/agent_init.go	Updates tool registration to accept the MessageBus interface.
pkg/agent/agent_inject.go	Keeps injection helpers; channel manager type aligns with new interface field.
pkg/agent/agent_message.go	Renamed/moved message routing + dispatch logic (now in agent_* naming).
pkg/agent/agent_outbound.go	Renamed/moved outbound publishing helpers.
pkg/agent/agent_steering.go	Renamed/moved steering continuation logic.
pkg/agent/agent_transcribe.go	Renamed/moved transcription pipeline for audio media.
pkg/agent/agent_media.go	Renamed/moved media ref resolution helpers.
pkg/agent/agent_event.go	Renamed/moved event emission/logging helpers.
pkg/agent/agent_command.go	Renamed/moved command handling and explicit skill selection.
pkg/agent/agent_mcp.go	Renamed/moved MCP initialization/runtime handling.
pkg/agent/agent_mcp_test.go	Adds/updates MCP runtime reset tests.
pkg/agent/agent_utils.go	Renamed/moved general helpers used across agent runtime.
pkg/agent/hooks_test.go	Updates test wiring to account for interface-typed bus field.
pkg/agent/loop_turn.go	Removes the legacy monolithic `runTurn` implementation (replaced by coordinator + pipeline).
docs/architecture/session-system.md	Updates file references to new agent/turn file names.
docs/architecture/routing-system.md	Updates runtime integration references to new agent/turn file names.
docs/architecture/agent-refactor/pipeline-restructuring-plan.md	Documents Pipeline file split plan and current structure.
docs/architecture/agent-refactor/pipeline-restructuring-plan.zh.md	Chinese version of Pipeline restructuring plan.
docs/architecture/agent-refactor/loop-split.md	Updates historical split doc to point to new naming and pipeline architecture.
docs/architecture/agent-refactor/agent-rename-plan.md	Documents the `loop_` → `agent_`/`turn_*` rename plan and resulting structure.
docs/architecture/agent-refactor/agent-rename-plan.zh.md	Chinese version of the rename plan.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-19T01:52:06Z

+	messages := exec.messages
+	pendingMessages := exec.pendingMessages
+	maxMediaSize := pipeline.Cfg.Agents.Defaults.GetMaxMediaSize()


pendingMessages is initialized from exec.pendingMessages, but the coordinator later appends to the local pendingMessages slice (and injects it) without clearing or updating exec.pendingMessages. Because the loop condition checks len(exec.pendingMessages) > 0, initial steering messages can keep the turn loop alive indefinitely and can be re-injected on subsequent iterations. Consider using exec.pendingMessages as the single source of truth (append into it), and after injection explicitly clear/drain it (and/or reset the local slice) so messages are injected exactly once.

Copilot · 2026-04-19T01:52:07Z

+		case ControlBreak:
+			// Ensure empty response falls back to DefaultResponse
+			if finalContent == "" {
+				finalContent = ts.opts.DefaultResponse
+			}
+			return pipeline.Finalize(ctx, turnCtx, ts, exec, turnStatus, finalContent)
+		case ControlToolLoop:


The ControlBreak path finalizes immediately without checking ts.hardAbortRequested(). Pipeline.CallLLM can set a hard-abort (e.g., HookActionHardAbort) and return ControlBreak, which will currently mark the turn completed and persist a final message instead of restoring session state via abortTurn. Add a hard-abort check here (set turnStatus = TurnEndStatusAborted and call al.abortTurn(ts)) before finalizing.

Copilot · 2026-04-19T01:52:07Z

+		logger.InfoCF("agent", "SWITCH ctrl", map[string]any{"ctrl": int(ctrl), "iter": iteration})
+		switch ctrl {
+		case ControlContinue:
+			logger.InfoCF("agent", "CASE ControlContinue", map[string]any{"iter": iteration})
+			continue


logger.InfoCF calls like "SWITCH ctrl" / "CASE ControlContinue" look like temporary debugging and will add high-volume INFO logs on every turn iteration. These should be removed or downgraded to Debug (and ideally use a consistent message format with other agent logs) to avoid noisy production logs.

Copilot · 2026-04-19T01:52:07Z

+				logger.WarnCF("agent", "Failed to save session after tool delivery",
+					map[string]any{
+						"agent_id": ts.agent.ID,
+						"error":    err.Error(),
+					})


When exec.allResponsesHandled is true, a session save failure is only logged and the turn still completes successfully. In the pre-refactor implementation this path returned an error and emitted an EventKindError, so this is a behavioral change that can silently drop conversation history. Consider emitting the same error event and propagating the error up to runTurn (e.g., by returning an error from ExecuteTools or storing it on exec for the coordinator to return).

Suggested change

logger.WarnCF("agent", "Failed to save session after tool delivery",

map[string]any{

"agent_id": ts.agent.ID,

"error": err.Error(),

})

saveErr := fmt.Errorf("failed to save session after tool delivery: %w", err)

logger.WarnCF("agent", "Failed to save session after tool delivery",

map[string]any{

"agent_id": ts.agent.ID,

"error": saveErr.Error(),

})

exec.err = saveErr

al.events.Publish(bus.Event{

Kind: constants.EventKindError,

Data: map[string]any{

"agent_id": ts.agent.ID,

"error": saveErr.Error(),

},

})

return ToolControlBreak

Copilot · 2026-04-19T01:52:07Z

+			case HookActionAbortTurn:
+				_ = ts.requestHardAbort()
+				return ToolControlBreak
+			case HookActionHardAbort:
+				_ = ts.requestHardAbort()
+				return ToolControlBreak
+			}


HookActionAbortTurn is currently treated the same as HookActionHardAbort (it requests a hard abort and returns), but elsewhere in the hook system AbortTurn is handled as an error via hookAbortError(...) (with an EventKindError). This changes semantics and makes aborts harder to diagnose. Consider distinguishing the two actions here (e.g., record a hookAbortError for AbortTurn and propagate an error back to the coordinator).

sky5454 and others added 6 commits April 17, 2026 13:38

fix(agent): code format fixed

11f9d96

refactor(agent): code test file added/renamed

4efa51b

Merge branch 'main' into looper_refactor_phase2

dc5ccaf

docs(agent): update agent refactor docs

23d9b65

Copilot AI review requested due to automatic review settings April 19, 2026 01:46

Copilot started reviewing on behalf of sky5454 April 19, 2026 01:47 View session

Copilot AI reviewed Apr 19, 2026

View reviewed changes

sipeed-bot bot added domain: agent go Pull requests that update go code labels Apr 19, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(agent): Agent Looper refactor phase2, restructure pipeline and rename loop files to agent#2585

refactor(agent): Agent Looper refactor phase2, restructure pipeline and rename loop files to agent#2585
sky5454 wants to merge 6 commits intosipeed:mainfrom
sky5454:looper_refactor_phase2

sky5454 commented Apr 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 19, 2026

Uh oh!

Copilot AI Apr 19, 2026

Uh oh!

Copilot AI Apr 19, 2026

Uh oh!

Copilot AI Apr 19, 2026

Uh oh!

Copilot AI Apr 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

sky5454 commented Apr 19, 2026

📝 Description

🗣️ Type of Change

🤖 AI Code Generation

🔗 Related Issue

📚 Technical Context

🧪 Test Environment

📸 Evidence

☑️ Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants