From 45689cb30d8aa0275957ca8fe89f617bf871107d Mon Sep 17 00:00:00 2001 From: Ed Zynda Date: Thu, 7 May 2026 12:23:15 +0300 Subject: [PATCH] extract duplicated subagent + event conversion to internal/extbridge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same ~40-line block — building a kit.SubagentConfig, wrapping OnEvent through sdkEventToSubagentEvent, calling kitInstance.Subagent, and translating the SDK result into extensions.SubagentResult — was copy-pasted three times: * cmd/root.go (interactive TUI Context, line 1148) * cmd/root.go (post-SessionStart runtime Context, line 1446) * internal/acpserver/session.go (ACP server Context, line 154) A separate sdkEventToSubagentEvent function was duplicated byte-for-byte between cmd/root.go and internal/acpserver/session.go. Both are now consolidated in a new internal/extbridge package which is the only module-internal home that can legitimately import both pkg/kit/ (the public SDK) and internal/extensions/. cmd/ and internal/acpserver/ both import it, so SDK-event-to-extension-event schema changes only have one site to update. Also fixes pkg/kit/events.go godoc comment that named the underlying LLM library, per AGENTS.md 'No Dependency Name Leakage' rule for exported SDK symbols. go test -race ./... passes. --- .kit/prompts/code-audit.md | 146 ++++++++++++++++++++++++++++++++ cmd/root.go | 118 +------------------------- internal/acpserver/session.go | 71 +--------------- internal/extbridge/extbridge.go | 97 +++++++++++++++++++++ pkg/kit/events.go | 6 +- 5 files changed, 251 insertions(+), 187 deletions(-) create mode 100644 .kit/prompts/code-audit.md create mode 100644 internal/extbridge/extbridge.go diff --git a/.kit/prompts/code-audit.md b/.kit/prompts/code-audit.md new file mode 100644 index 00000000..c228e57d --- /dev/null +++ b/.kit/prompts/code-audit.md @@ -0,0 +1,146 @@ +--- +description: Read-only audit for dead code, duplication, boundary violations, and refactor opportunities +--- + +Perform a comprehensive **read-only** audit of this repository and report +findings. **Do not edit, rename, or delete any files.** Optional focus / scope +hints from the user: $@ + +## Scope + +If the user supplied focus hints above (a package path, a subsystem name, a +concern like "TUI" or "extensions"), scope the audit accordingly. Otherwise +audit the whole repo, prioritising the highest-traffic packages first +(`cmd/`, `internal/`, `pkg/kit/` for this repo). + +## Steps + +1. **Map the repo first**: + - `ls` / `find` the top-level layout and list every Go package + - Read `AGENTS.md`, `README.md`, and any `pkg/*/doc.go` to understand the + intended architectural boundaries (SDK vs internal vs TUI vs cmd vs + extension surface) + - Note the public SDK surface (`pkg/kit/`) and any documented invariants + (e.g. "no dependency name leakage", "UI never imports extensions + directly") — these define what counts as a violation + +2. **Hunt for dead code**: + - Run `go vet ./...` and capture warnings + - Use `grep` to find exported symbols (`^func [A-Z]`, `^type [A-Z]`, + `^var [A-Z]`, `^const [A-Z]`) and cross-reference call sites. Symbols + with zero non-test references inside the module are suspects + - Check for unreferenced files, `// TODO: remove` markers, commented-out + blocks, and `_ = x` discard patterns + - If `staticcheck`, `deadcode`, or `unused` are available on PATH, run + them and include their output verbatim + - **Do not delete anything** — list candidates with file:line and a + confidence level (high / medium / low) + +3. **Find unnecessary duplication**: + - Look for near-identical function bodies, struct shapes, or switch + statements across packages — `grep` for repeated function signatures + and copy-pasted string literals / error messages is a fast first pass + - Distinguish *coincidental* duplication (two things that happen to look + alike but evolve independently) from *unnecessary* duplication (same + intent, drifting in lockstep) — only flag the latter + - For each cluster, propose where the extracted helper should live + (which package, which file) and whether it crosses a boundary + +4. **Check concerns / boundary violations**: + - **SDK leakage**: grep `pkg/kit/` for imports of `internal/...` types + in exported signatures, and for dependency-name leakage in exported + names / godoc (e.g. library jargon appearing in `LLM*` types) + - **UI ↔ extensions**: grep `internal/ui/` for any import of + `internal/extensions/` — per AGENTS.md the UI must not import + extensions directly; converters in `cmd/root.go` should bridge them + - **cmd vs internal**: business logic living in `cmd/` that should be + in `internal/` (and vice versa) + - **Cyclic risk**: packages that import each other transitively or that + reach across sibling boundaries unexpectedly + - For each violation, cite the offending import / signature with + file:line + +5. **Spot refactor opportunities**: + - Long functions (>80 lines) doing multiple unrelated things + - Deeply nested conditionals that flatten well with early returns + - Repeated `if err != nil { return fmt.Errorf("...: %w", err) }` chains + that could become helpers — but only where the wrapping context is + genuinely uniform + - Structs with too many fields that hint at split responsibilities + - Exported APIs that would be cleaner with options structs / functional + options + - Tests that share setup boilerplate ripe for a helper + - Flag each with: location, current shape (1-2 lines), proposed shape + (1-2 lines), and estimated risk (low / medium / high) + +6. **Cross-check against project rules**: + - Re-read `AGENTS.md` "Key Patterns" section and verify nothing in your + findings contradicts the documented gotchas (Yaegi interface ban, + `prog.Send()` from `Update()`, function-field bug, etc.) — if a + "refactor" would reintroduce a known pitfall, drop it from the report + and note why + +7. **Write the report** as your final message (do not write it to disk) + structured as: + + ``` + # Code Audit Report + + ## Summary + - N dead-code candidates + - N duplication clusters + - N boundary violations + - N refactor opportunities + + ## Dead Code + ### High confidence + - path/to/file.go:LINE — symbol — reason + + ### Medium confidence + ... + + ## Duplication + ### Cluster: + - Sites: file:line, file:line, … + - Suggested home: package/path + - Notes: … + + ## Boundary Violations + - Rule: + - Offender: file:line + - Fix sketch: … + + ## Refactor Opportunities + - Location: file:line + - Current: … + - Proposed: … + - Risk: low/medium/high + - Why it's worth it: … + + ## Suggested Next Steps + 1. … + 2. … + ``` + +8. **End the report with an explicit reminder** that no files were modified, + and recommend the user pick the highest-leverage items to act on + manually (or via a follow-up `/fix-issue` style prompt) rather than + running a sweeping refactor. + +## Guidelines + +- **Read-only, always**: no `edit`, no `write`, no `git commit`, no `go mod + tidy`. Use only `read`, `grep`, `find`, `ls`, and read-only `bash` + commands (`go vet`, `go build -o /tmp/...`, `staticcheck`, etc.) +- **Cite every finding** with `path/to/file.go:LINE` so the user can jump + straight to it +- **Be honest about confidence**: false positives in a code audit are + expensive — prefer "medium confidence, worth a look" over confidently + wrong claims +- **Quantity isn't quality**: 10 sharp findings beat 100 nitpicks. Cut + anything that's purely stylistic unless it directly causes one of the + four issue categories above +- **Skip generated code** (`*.pb.go`, `*_gen.go`, anything under + `vendor/`) and obvious third-party copies +- **Don't propose architectural rewrites** — stay within the existing + shape of the repo and recommend incremental, reviewable changes diff --git a/cmd/root.go b/cmd/root.go index 91ee6b1e..93155dbe 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -14,6 +14,7 @@ import ( "github.com/mark3labs/kit/internal/app" "github.com/mark3labs/kit/internal/auth" "github.com/mark3labs/kit/internal/config" + "github.com/mark3labs/kit/internal/extbridge" "github.com/mark3labs/kit/internal/extensions" "github.com/mark3labs/kit/internal/models" "github.com/mark3labs/kit/internal/prompts" @@ -1140,45 +1141,8 @@ func runNormalMode(ctx context.Context) error { } }, SpawnSubagent: func(config extensions.SubagentConfig) (*extensions.SubagentHandle, *extensions.SubagentResult, error) { - // In-process subagent via SDK. - sdkCfg := kit.SubagentConfig{ - Prompt: config.Prompt, - Model: config.Model, - SystemPrompt: config.SystemPrompt, - Timeout: config.Timeout, - NoSession: config.NoSession, - } - // Bridge SDK events to extension SubagentEvents. - if config.OnEvent != nil { - sdkCfg.OnEvent = func(e kit.Event) { - se := sdkEventToSubagentEvent(e) - if se.Type != "" { - config.OnEvent(se) - } - } - } - result, err := kitInstance.Subagent(ctx, sdkCfg) - if result == nil { - return nil, &extensions.SubagentResult{Error: err}, err - } - extResult := &extensions.SubagentResult{ - Response: result.Response, - Error: err, - SessionID: result.SessionID, - Elapsed: result.Elapsed, - } - if result.Usage != nil { - extResult.Usage = &extensions.SubagentUsage{ - InputTokens: result.Usage.InputTokens, - OutputTokens: result.Usage.OutputTokens, - } - } - return nil, extResult, err + return extbridge.SpawnSubagent(ctx, kitInstance, config) }, - - // ------------------------------------------------------------------------- - // Tree Navigation API (Phase 1 Bridge) - // ------------------------------------------------------------------------- GetTreeNode: func(entryID string) *extensions.TreeNode { node := kitInstance.GetTreeNode(entryID) if node == nil { @@ -1438,45 +1402,8 @@ func runNormalMode(ctx context.Context) error { } }, SpawnSubagent: func(config extensions.SubagentConfig) (*extensions.SubagentHandle, *extensions.SubagentResult, error) { - // In-process subagent via SDK. - sdkCfg := kit.SubagentConfig{ - Prompt: config.Prompt, - Model: config.Model, - SystemPrompt: config.SystemPrompt, - Timeout: config.Timeout, - NoSession: config.NoSession, - } - // Bridge SDK events to extension SubagentEvents. - if config.OnEvent != nil { - sdkCfg.OnEvent = func(e kit.Event) { - se := sdkEventToSubagentEvent(e) - if se.Type != "" { - config.OnEvent(se) - } - } - } - result, err := kitInstance.Subagent(ctx, sdkCfg) - if result == nil { - return nil, &extensions.SubagentResult{Error: err}, err - } - extResult := &extensions.SubagentResult{ - Response: result.Response, - Error: err, - SessionID: result.SessionID, - Elapsed: result.Elapsed, - } - if result.Usage != nil { - extResult.Usage = &extensions.SubagentUsage{ - InputTokens: result.Usage.InputTokens, - OutputTokens: result.Usage.OutputTokens, - } - } - return nil, extResult, err + return extbridge.SpawnSubagent(ctx, kitInstance, config) }, - - // ------------------------------------------------------------------------- - // Tree Navigation API (Phase 1 Bridge) - Second Context - // ------------------------------------------------------------------------- GetTreeNode: func(entryID string) *extensions.TreeNode { node := kitInstance.GetTreeNode(entryID) if node == nil { @@ -2178,42 +2105,3 @@ func runInteractiveModeBubbleTea(_ context.Context, appInstance *app.App, modelN _, runErr := program.Run() return runErr } - -// sdkEventToSubagentEvent converts an SDK event to an extension-facing -// SubagentEvent. Returns a zero-value event (Type=="") for events that -// don't map to anything useful. -func sdkEventToSubagentEvent(e kit.Event) extensions.SubagentEvent { - switch ev := e.(type) { - case kit.MessageUpdateEvent: - return extensions.SubagentEvent{Type: "text", Content: ev.Chunk} - case kit.ReasoningDeltaEvent: - return extensions.SubagentEvent{Type: "reasoning", Content: ev.Delta} - case kit.ToolCallEvent: - return extensions.SubagentEvent{ - Type: "tool_call", ToolCallID: ev.ToolCallID, - ToolName: ev.ToolName, ToolKind: ev.ToolKind, ToolArgs: ev.ToolArgs, - } - case kit.ToolExecutionStartEvent: - return extensions.SubagentEvent{ - Type: "tool_execution_start", ToolCallID: ev.ToolCallID, - ToolName: ev.ToolName, ToolKind: ev.ToolKind, - } - case kit.ToolExecutionEndEvent: - return extensions.SubagentEvent{ - Type: "tool_execution_end", ToolCallID: ev.ToolCallID, - ToolName: ev.ToolName, ToolKind: ev.ToolKind, - } - case kit.ToolResultEvent: - return extensions.SubagentEvent{ - Type: "tool_result", ToolCallID: ev.ToolCallID, - ToolName: ev.ToolName, ToolKind: ev.ToolKind, - ToolResult: ev.Result, IsError: ev.IsError, - } - case kit.TurnStartEvent: - return extensions.SubagentEvent{Type: "turn_start"} - case kit.TurnEndEvent: - return extensions.SubagentEvent{Type: "turn_end"} - default: - return extensions.SubagentEvent{} - } -} diff --git a/internal/acpserver/session.go b/internal/acpserver/session.go index a98b71ee..af18a862 100644 --- a/internal/acpserver/session.go +++ b/internal/acpserver/session.go @@ -8,6 +8,7 @@ import ( "github.com/charmbracelet/log" + "github.com/mark3labs/kit/internal/extbridge" "github.com/mark3labs/kit/internal/extensions" kit "github.com/mark3labs/kit/pkg/kit" ) @@ -152,38 +153,7 @@ func (r *sessionRegistry) create(ctx context.Context, cwd string) (*acpSession, return kitInstance.ExecuteCompletion(context.Background(), req) }, SpawnSubagent: func(config extensions.SubagentConfig) (*extensions.SubagentHandle, *extensions.SubagentResult, error) { - sdkCfg := kit.SubagentConfig{ - Prompt: config.Prompt, - Model: config.Model, - SystemPrompt: config.SystemPrompt, - Timeout: config.Timeout, - NoSession: config.NoSession, - } - if config.OnEvent != nil { - sdkCfg.OnEvent = func(e kit.Event) { - se := sdkEventToSubagentEvent(e) - if se.Type != "" { - config.OnEvent(se) - } - } - } - result, err := kitInstance.Subagent(context.Background(), sdkCfg) - if result == nil { - return nil, &extensions.SubagentResult{Error: err}, err - } - extResult := &extensions.SubagentResult{ - Response: result.Response, - Error: err, - SessionID: result.SessionID, - Elapsed: result.Elapsed, - } - if result.Usage != nil { - extResult.Usage = &extensions.SubagentUsage{ - InputTokens: result.Usage.InputTokens, - OutputTokens: result.Usage.OutputTokens, - } - } - return nil, extResult, err + return extbridge.SpawnSubagent(context.Background(), kitInstance, config) }, // Render — fall back to logging. @@ -269,40 +239,3 @@ func (s *acpSession) clearCancel() { defer s.cancelMu.Unlock() s.cancelFn = nil } - -// sdkEventToSubagentEvent converts an SDK event to an extension SubagentEvent. -func sdkEventToSubagentEvent(e kit.Event) extensions.SubagentEvent { - switch ev := e.(type) { - case kit.MessageUpdateEvent: - return extensions.SubagentEvent{Type: "text", Content: ev.Chunk} - case kit.ReasoningDeltaEvent: - return extensions.SubagentEvent{Type: "reasoning", Content: ev.Delta} - case kit.ToolCallEvent: - return extensions.SubagentEvent{ - Type: "tool_call", ToolCallID: ev.ToolCallID, - ToolName: ev.ToolName, ToolKind: ev.ToolKind, ToolArgs: ev.ToolArgs, - } - case kit.ToolExecutionStartEvent: - return extensions.SubagentEvent{ - Type: "tool_execution_start", ToolCallID: ev.ToolCallID, - ToolName: ev.ToolName, ToolKind: ev.ToolKind, - } - case kit.ToolExecutionEndEvent: - return extensions.SubagentEvent{ - Type: "tool_execution_end", ToolCallID: ev.ToolCallID, - ToolName: ev.ToolName, ToolKind: ev.ToolKind, - } - case kit.ToolResultEvent: - return extensions.SubagentEvent{ - Type: "tool_result", ToolCallID: ev.ToolCallID, - ToolName: ev.ToolName, ToolKind: ev.ToolKind, - ToolResult: ev.Result, IsError: ev.IsError, - } - case kit.TurnStartEvent: - return extensions.SubagentEvent{Type: "turn_start"} - case kit.TurnEndEvent: - return extensions.SubagentEvent{Type: "turn_end"} - default: - return extensions.SubagentEvent{} - } -} diff --git a/internal/extbridge/extbridge.go b/internal/extbridge/extbridge.go new file mode 100644 index 00000000..4d8d7aa5 --- /dev/null +++ b/internal/extbridge/extbridge.go @@ -0,0 +1,97 @@ +// Package extbridge wires the public Kit SDK to the internal extensions +// package. It exists so that cmd/ and internal/acpserver/ don't both +// reimplement the same SDK→extension event/subagent conversions. +package extbridge + +import ( + "context" + + "github.com/mark3labs/kit/internal/extensions" + kit "github.com/mark3labs/kit/pkg/kit" +) + +// SDKEventToSubagentEvent converts an SDK [kit.Event] into the +// extension-facing [extensions.SubagentEvent]. Returns a zero-value event +// (Type=="") for events that don't map to anything useful — callers should +// drop those. +func SDKEventToSubagentEvent(e kit.Event) extensions.SubagentEvent { + switch ev := e.(type) { + case kit.MessageUpdateEvent: + return extensions.SubagentEvent{Type: "text", Content: ev.Chunk} + case kit.ReasoningDeltaEvent: + return extensions.SubagentEvent{Type: "reasoning", Content: ev.Delta} + case kit.ToolCallEvent: + return extensions.SubagentEvent{ + Type: "tool_call", ToolCallID: ev.ToolCallID, + ToolName: ev.ToolName, ToolKind: ev.ToolKind, ToolArgs: ev.ToolArgs, + } + case kit.ToolExecutionStartEvent: + return extensions.SubagentEvent{ + Type: "tool_execution_start", ToolCallID: ev.ToolCallID, + ToolName: ev.ToolName, ToolKind: ev.ToolKind, + } + case kit.ToolExecutionEndEvent: + return extensions.SubagentEvent{ + Type: "tool_execution_end", ToolCallID: ev.ToolCallID, + ToolName: ev.ToolName, ToolKind: ev.ToolKind, + } + case kit.ToolResultEvent: + return extensions.SubagentEvent{ + Type: "tool_result", ToolCallID: ev.ToolCallID, + ToolName: ev.ToolName, ToolKind: ev.ToolKind, + ToolResult: ev.Result, IsError: ev.IsError, + } + case kit.TurnStartEvent: + return extensions.SubagentEvent{Type: "turn_start"} + case kit.TurnEndEvent: + return extensions.SubagentEvent{Type: "turn_end"} + default: + return extensions.SubagentEvent{} + } +} + +// SpawnSubagent runs a subagent in-process via the Kit SDK and translates +// the result/events back into the extension-facing types. The returned +// handle is always nil — the SDK path runs synchronously and does not +// expose a separate process handle. Callers that need non-blocking +// behaviour should run this in their own goroutine. +// +// This function consolidates the previously-duplicated wiring in +// cmd/root.go (interactive + runtime contexts) and +// internal/acpserver/session.go. +func SpawnSubagent(ctx context.Context, k *kit.Kit, cfg extensions.SubagentConfig) (*extensions.SubagentHandle, *extensions.SubagentResult, error) { + sdkCfg := kit.SubagentConfig{ + Prompt: cfg.Prompt, + Model: cfg.Model, + SystemPrompt: cfg.SystemPrompt, + Timeout: cfg.Timeout, + NoSession: cfg.NoSession, + } + if cfg.OnEvent != nil { + sdkCfg.OnEvent = func(e kit.Event) { + se := SDKEventToSubagentEvent(e) + if se.Type != "" { + cfg.OnEvent(se) + } + } + } + + result, err := k.Subagent(ctx, sdkCfg) + if result == nil { + return nil, &extensions.SubagentResult{Error: err}, err + } + + extResult := &extensions.SubagentResult{ + Response: result.Response, + Error: err, + SessionID: result.SessionID, + Elapsed: result.Elapsed, + } + if result.Usage != nil { + extResult.Usage = &extensions.SubagentUsage{ + InputTokens: result.Usage.InputTokens, + OutputTokens: result.Usage.OutputTokens, + } + } + return nil, extResult, err +} diff --git a/pkg/kit/events.go b/pkg/kit/events.go index 2129f322..7f82cebf 100644 --- a/pkg/kit/events.go +++ b/pkg/kit/events.go @@ -148,9 +148,9 @@ func parseToolArgs(toolArgs string) map[string]any { // --------------------------------------------------------------------------- // Finish reasons reported by the LLM provider on a completed turn. These -// mirror fantasy.FinishReason string values so comparisons against -// TurnEndEvent.StopReason / TurnResult.StopReason are stable across -// providers. +// mirror the underlying provider's finish reason string values so +// comparisons against TurnEndEvent.StopReason / TurnResult.StopReason are +// stable across providers. const ( // FinishReasonStop: the model produced a natural stop (e.g. stop sequence // or end-of-turn signal).