chore: round 4 dead-code cleanup (transport helpers, detect, orphan methods) (#1156)

* chore(agentctl): delete duplicate procattr files in adapter package

setProcGroup and killProcessGroup are defined in both
agentctl/server/adapter/ and agentctl/server/process/. The process/
copies are the live ones (called by process/runner.go, manager.go,
vscode.go); the adapter/ copies have zero callers and are removed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore(agentctl): remove orphan helpers from transport/shared

Left over from the ACP refactor. All had zero callers:
- DebugEnabled(): internal debugMode var is checked directly by
  LogRawEvent/LogNormalizedEvent — the exported wrapper was unused.
- GetBool, GetMap, GetSlice: only GetString/GetInt are called in acp/normalize.go.
- NormalizeShellResult, NormalizeReadResult, NormalizeCodeSearchResult,
  NormalizeModifyResult: normalization moved to acp/normalize.go.
- TruncateIfNeeded, MaxContentLength, MaxFileCount: only used by the
  four Normalize* helpers above; removed transitively.
- TraceControlMessage: zero callers in the adapter transport layer.

GenerateUnifiedDiff, SplitLines, DetectLanguage, Tracer, ShutdownTracing,
TraceProtocolEvent, TraceProtocolRequest are alive and kept.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore(agentctl): remove unused tool_payload constructors

NewHttpRequest, NewCreateTask, NewSubagentTask, NewShowPlan,
NewManageTodos, and NewMisc have zero callers — ACP normalization
builds NormalizedPayload via struct literals and JSON unmarshaling,
not these factory functions. NewReadFile, NewModifyFile, NewShellExec,
NewCodeSearch, and NewGeneric are kept (live callers in acp/normalize.go).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore(agents/detect): remove dead fluent options and OSPaths type

WithFileExists, WithCommandOutput, WithEnvVar, expandHomePath (their
transitive helper), and OSPaths (struct + Resolve + Expanded) have
zero callers. Only WithCommand and WithNpxRunnable are used by agent
IsInstalled implementations. Imports trimmed to context and os/exec.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore(office): remove dead channel methods superseded by service/

ListComments, DeleteComment, and UpdateChannelStatus on ChannelService
have zero callers — the live implementations live in
internal/office/service/comments.go and service/channels.go.
CreateComment is kept: it is called by HandleChannelInbound.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore: remove scattered orphan helpers

Remove dead single-symbol exports verified to have zero callers:
- ParseCronExpression (office/shared/cron.go): NextCronTime covers
  all live usages; the validator wrapper was unused.
- IsValidPathComponent, MustJSON (office/shared/helpers.go): zero
  callers; GenerateSlug is kept.
- DiscoverUserSkillsAt (office/skills/userhome_discovery.go): exported
  wrapper over the private discoverUserSkillsAt; only the internal
  DiscoverUserSkills method is wired up.
- Manager.Get (agent/loginpty/manager.go): zero callers; GetByID and
  Stop are kept (Stop is tested in manager_test.go).
- DetachedWithValues (common/appctx/appctx.go): zero callers; Detached
  is kept.
- Repository.Close no-ops (task/share, terminal/repository): both are
  documented no-ops with zero callers.
- TaskBranchName (worktree/config.go): zero callers; the deterministic
  TaskBranchNameWithSuffix is kept.

Note: dialect.Now was initially targeted but is tested in
dialect_test.go (same-package call — missed by package-qualified grep).
It remains unchanged.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore(share): remove stale Close reference from provider.go comment

Repository.Close() was removed as a no-op in the round-4 cleanup.
The comment in provider.go still referenced it by name; updated to
reflect that the repo simply doesn't own its DB connections.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore(office/shared): delete helpers.go — GenerateSlug had zero callers

The triage step mistakenly identified skills/service.go as a caller,
but that package defines its own local GenerateSlug (different regex,
adds TrimSpace and a 'skill' fallback default). shared.GenerateSlug
was never called anywhere. helpers.go is now empty; delete the file.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore(worktree): fix stale TaskBranchName reference in TaskBranchNameWithSuffix doc

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
João Salavisa
2026-05-29 18:18:10 +01:00
committed by GitHub
parent a34ea719df
commit cab7b28ed9
20 changed files with 7 additions and 501 deletions
@@ -2,33 +2,12 @@ package agents
import (
"context"
"os"
"os/exec"
"path/filepath"
"regexp"
"runtime"
"strings"
)
// DetectOption is a detection strategy. Returns (found, matchedPath, err).
type DetectOption func(ctx context.Context) (bool, string, error)
// WithFileExists checks if any of the given paths exist (supports ~ expansion).
func WithFileExists(paths ...string) DetectOption {
return func(ctx context.Context) (bool, string, error) {
for _, p := range paths {
expanded := expandHomePath(p)
if expanded == "" {
continue
}
if _, err := os.Stat(expanded); err == nil {
return true, expanded, nil
}
}
return false, "", nil
}
}
// WithCommand checks if a command is in PATH (exec.LookPath).
func WithCommand(name string) DetectOption {
return func(ctx context.Context) (bool, string, error) {
@@ -69,36 +48,6 @@ func WithNpxRunnable(pkg string) DetectOption {
}
}
// WithCommandOutput runs a command and checks stdout matches regex.
func WithCommandOutput(pattern string, name string, args ...string) DetectOption {
return func(ctx context.Context) (bool, string, error) {
cmd := exec.CommandContext(ctx, name, args...)
out, err := cmd.Output()
if err != nil {
return false, "", nil
}
re, err := regexp.Compile(pattern)
if err != nil {
return false, "", err
}
if re.Match(out) {
return true, name, nil
}
return false, "", nil
}
}
// WithEnvVar checks if an environment variable is set and non-empty.
func WithEnvVar(name string) DetectOption {
return func(ctx context.Context) (bool, string, error) {
val := os.Getenv(name)
if val != "" {
return true, name, nil
}
return false, "", nil
}
}
// Detect runs options in order, returns first match.
// If none match, returns DiscoveryResult{Available: false}.
func Detect(ctx context.Context, opts ...DetectOption) (*DiscoveryResult, error) {
@@ -116,49 +65,3 @@ func Detect(ctx context.Context, opts ...DetectOption) (*DiscoveryResult, error)
}
return &DiscoveryResult{Available: false}, nil
}
// expandHomePath expands ~ to the user's home directory.
func expandHomePath(path string) string {
if path == "" {
return ""
}
if strings.HasPrefix(path, "~") {
home, err := os.UserHomeDir()
if err != nil {
return ""
}
path = filepath.Join(home, strings.TrimPrefix(path, "~"))
}
return filepath.Clean(filepath.FromSlash(path))
}
// OSPaths holds per-OS path lists. Use Resolve() to get the paths for the current OS.
type OSPaths struct {
Linux []string
MacOS []string
Windows []string
}
// Resolve returns the raw paths for the current operating system.
func (p OSPaths) Resolve() []string {
switch runtime.GOOS {
case "darwin":
return p.MacOS
case "windows":
return p.Windows
default:
return p.Linux
}
}
// Expanded returns the paths for the current OS with ~ expanded to the home directory.
func (p OSPaths) Expanded() []string {
paths := p.Resolve()
result := make([]string, 0, len(paths))
for _, path := range paths {
if expanded := expandHomePath(path); expanded != "" {
result = append(result, expanded)
}
}
return result
}
@@ -110,13 +110,6 @@ func (m *Manager) Start(agentID string, cmd []string, cols, rows uint16) (*Sessi
return sess, nil
}
// Get returns a session by agent ID (or nil).
func (m *Manager) Get(agentID string) *Session {
m.mu.Lock()
defer m.mu.Unlock()
return m.sessions[agentID]
}
// GetByID returns a session by session ID (or nil).
func (m *Manager) GetByID(sessionID string) *Session {
m.mu.Lock()
@@ -1,21 +0,0 @@
//go:build linux
package adapter
import (
"os/exec"
"syscall"
)
// setProcGroup configures the command to run in its own process group.
func setProcGroup(cmd *exec.Cmd) {
cmd.SysProcAttr = &syscall.SysProcAttr{
Setpgid: true,
Pdeathsig: syscall.SIGTERM,
}
}
// killProcessGroup kills the entire process group for the given PID.
func killProcessGroup(pid int) error {
return syscall.Kill(-pid, syscall.SIGKILL)
}
@@ -1,18 +0,0 @@
//go:build unix && !linux
package adapter
import (
"os/exec"
"syscall"
)
// setProcGroup configures the command to run in its own process group.
func setProcGroup(cmd *exec.Cmd) {
cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
}
// killProcessGroup kills the entire process group for the given PID.
func killProcessGroup(pid int) error {
return syscall.Kill(-pid, syscall.SIGKILL)
}
@@ -1,22 +0,0 @@
//go:build windows
package adapter
import (
"fmt"
"os/exec"
"syscall"
)
// setProcGroup configures the command to run in its own process group.
func setProcGroup(cmd *exec.Cmd) {
cmd.SysProcAttr = &syscall.SysProcAttr{
CreationFlags: syscall.CREATE_NEW_PROCESS_GROUP,
}
}
// killProcessGroup kills the entire process tree for the given PID.
func killProcessGroup(pid int) error {
kill := exec.Command("taskkill", "/F", "/T", "/PID", fmt.Sprintf("%d", pid))
return kill.Run()
}
@@ -38,9 +38,6 @@ const (
ProtocolCopilot = "copilot"
)
// DebugEnabled reports whether agent message debug mode is active.
func DebugEnabled() bool { return debugMode }
func resolveDebugLogDir() string {
if dir := os.Getenv("KANDEV_DEBUG_LOG_DIR"); dir != "" {
return dir
@@ -16,27 +16,3 @@ func GetInt(m map[string]any, key string) int {
}
return 0
}
// GetBool extracts a bool value from a map, returning false if not found or wrong type.
func GetBool(m map[string]any, key string) bool {
if v, ok := m[key].(bool); ok {
return v
}
return false
}
// GetMap extracts a nested map from a map, returning nil if not found or wrong type.
func GetMap(m map[string]any, key string) map[string]any {
if v, ok := m[key].(map[string]any); ok {
return v
}
return nil
}
// GetSlice extracts a slice from a map, returning nil if not found or wrong type.
func GetSlice(m map[string]any, key string) []any {
if v, ok := m[key].([]any); ok {
return v
}
return nil
}
@@ -3,10 +3,11 @@ package shared
import (
"fmt"
"strings"
"github.com/kandev/kandev/internal/agentctl/types/streams"
)
// langPlaintext is the fallback language identifier for files without a known extension.
const langPlaintext = "plaintext"
// GenerateUnifiedDiff creates a unified diff string from old and new content.
func GenerateUnifiedDiff(oldStr, newStr, path string, startLine int) string {
// If both empty or identical, no diff needed
@@ -57,97 +58,6 @@ func SplitLines(s string) []string {
return strings.Split(strings.ReplaceAll(s, "\r\n", "\n"), "\n")
}
// NormalizeShellResult updates a ShellExecPayload with result data.
func NormalizeShellResult(payload *streams.ShellExecPayload, result any) {
if payload.Output == nil {
payload.Output = &streams.ShellExecOutput{}
}
switch r := result.(type) {
case string:
payload.Output.Stdout = r
case map[string]any:
if stdout, ok := r["stdout"].(string); ok {
payload.Output.Stdout = stdout
}
if stderr, ok := r["stderr"].(string); ok {
payload.Output.Stderr = stderr
}
if exitCode, ok := r["exit_code"].(float64); ok {
payload.Output.ExitCode = int(exitCode)
}
}
}
// MaxContentLength is the maximum length for tool output content before truncation.
const MaxContentLength = 50000
// MaxFileCount is the maximum number of files to include in code search results.
const MaxFileCount = 500
// langPlaintext is the fallback language identifier for files without a known extension.
const langPlaintext = "plaintext"
// TruncateIfNeeded truncates a string if it exceeds maxLen.
func TruncateIfNeeded(s string, maxLen int) (string, bool) {
if len(s) <= maxLen {
return s, false
}
return s[:maxLen], true
}
// NormalizeReadResult populates ReadFilePayload.Output with result content.
func NormalizeReadResult(payload *streams.ReadFilePayload, result string) {
lines := SplitLines(result)
content, truncated := TruncateIfNeeded(result, MaxContentLength)
payload.Output = &streams.ReadFileOutput{
Content: content,
LineCount: len(lines),
Truncated: truncated,
Language: DetectLanguage(payload.FilePath),
}
}
// NormalizeCodeSearchResult populates CodeSearchPayload.Output with result content.
func NormalizeCodeSearchResult(payload *streams.CodeSearchPayload, result string) {
result = strings.TrimSpace(result)
if result == "" {
payload.Output = &streams.CodeSearchOutput{
Files: []string{},
FileCount: 0,
}
return
}
files := strings.Split(result, "\n")
truncated := false
if len(files) > MaxFileCount {
files = files[:MaxFileCount]
truncated = true
}
payload.Output = &streams.CodeSearchOutput{
Files: files,
FileCount: len(files),
Truncated: truncated,
}
}
// NormalizeModifyResult updates ModifyFilePayload with result content for Write operations.
func NormalizeModifyResult(payload *streams.ModifyFilePayload, result string) {
// For Write tool, store the written content confirmation
// The tool result confirms what was written
if len(payload.Mutations) > 0 {
mut := &payload.Mutations[0]
// If Content not already set from input args (for create), use result
if mut.Content == "" && mut.NewContent == "" && result != "" {
content, _ := TruncateIfNeeded(result, MaxContentLength)
mut.Content = content
}
}
}
// DetectLanguage maps file extension to language identifier.
// Used for syntax highlighting in diffs.
func DetectLanguage(path string) string {
@@ -100,41 +100,6 @@ func TraceProtocolRequest(
return ctx, span
}
// TraceControlMessage creates a span for a control-plane protocol message.
// Used for control_request, control_response, and control_cancel_request messages
// that bypass the regular sendUpdate() tracing path.
func TraceControlMessage(
ctx context.Context,
protocol, agentID string,
direction string,
eventType string,
rawData json.RawMessage,
) {
tracer := Tracer()
spanName := protocol + ".control." + eventType
kind := trace.SpanKindInternal
if direction == "send" {
kind = trace.SpanKindClient
}
_, span := tracer.Start(ctx, spanName, trace.WithSpanKind(kind))
defer span.End()
span.SetAttributes(
attribute.String("protocol", protocol),
attribute.String("agent_id", agentID),
attribute.String("direction", direction),
attribute.String("event_type", eventType),
)
if len(rawData) > 0 {
span.AddEvent("payload", trace.WithAttributes(
attribute.String("data", truncate(string(rawData), maxAttrValueLen)),
))
}
}
func truncate(s string, maxLen int) string {
if len(s) <= maxLen {
return s
@@ -334,17 +334,6 @@ func NewCodeSearch(query, pattern, path, glob string) *NormalizedPayload {
}
}
// NewHttpRequest creates a NormalizedPayload for HTTP request operations.
func NewHttpRequest(url, method string) *NormalizedPayload {
return &NormalizedPayload{
kind: ToolKindHttpRequest,
httpRequest: &HttpRequestPayload{
URL: url,
Method: method,
},
}
}
// NewGeneric creates a NormalizedPayload for unrecognized tools.
func NewGeneric(name string, input any) *NormalizedPayload {
return &NormalizedPayload{
@@ -355,59 +344,3 @@ func NewGeneric(name string, input any) *NormalizedPayload {
},
}
}
// NewCreateTask creates a NormalizedPayload for task creation operations.
func NewCreateTask(title, description string) *NormalizedPayload {
return &NormalizedPayload{
kind: ToolKindCreateTask,
createTask: &CreateTaskPayload{
Title: title,
Description: description,
},
}
}
// NewSubagentTask creates a NormalizedPayload for subagent task invocations.
func NewSubagentTask(description, prompt, subagentType string) *NormalizedPayload {
return &NormalizedPayload{
kind: ToolKindSubagentTask,
subagentTask: &SubagentTaskPayload{
Description: description,
Prompt: prompt,
SubagentType: subagentType,
},
}
}
// NewShowPlan creates a NormalizedPayload for plan display operations.
func NewShowPlan(summary string, steps []string) *NormalizedPayload {
return &NormalizedPayload{
kind: ToolKindShowPlan,
showPlan: &ShowPlanPayload{
Summary: summary,
Steps: steps,
},
}
}
// NewManageTodos creates a NormalizedPayload for todo management operations.
func NewManageTodos(operation string, items []TodoItem) *NormalizedPayload {
return &NormalizedPayload{
kind: ToolKindManageTodos,
manageTodos: &ManageTodosPayload{
Operation: operation,
Items: items,
},
}
}
// NewMisc creates a NormalizedPayload for miscellaneous operations.
func NewMisc(label string, details any) *NormalizedPayload {
return &NormalizedPayload{
kind: ToolKindMisc,
misc: &MiscPayload{
Label: label,
Details: details,
},
}
}
@@ -30,26 +30,3 @@ func Detached(parent context.Context, stopCh <-chan struct{}, timeout time.Durat
return ctx, cancel
}
// DetachedWithValues creates a detached context that copies deadline-insensitive values
// from the parent context while starting fresh with no deadline.
// Use sparingly - most values should be passed explicitly.
func DetachedWithValues(parent context.Context, stopCh <-chan struct{}, timeout time.Duration) (context.Context, context.CancelFunc) {
var ctx context.Context
var cancel context.CancelFunc
if timeout > 0 {
ctx, cancel = context.WithTimeout(context.Background(), timeout)
} else {
ctx, cancel = context.WithCancel(context.Background())
}
go func() {
select {
case <-stopCh:
cancel()
case <-ctx.Done():
}
}()
return ctx, cancel
}
@@ -29,16 +29,6 @@ func (s *ChannelService) CreateComment(ctx context.Context, comment *models.Task
return nil
}
// ListComments returns all comments for a task.
func (s *ChannelService) ListComments(ctx context.Context, taskID string) ([]*models.TaskComment, error) {
return s.repo.ListTaskComments(ctx, taskID)
}
// DeleteComment deletes a task comment by ID.
func (s *ChannelService) DeleteComment(ctx context.Context, id string) error {
return s.repo.DeleteTaskComment(ctx, id)
}
// publishCommentCreated emits an OfficeCommentCreated event.
// If no event bus is configured the call is silently skipped.
func (s *ChannelService) publishCommentCreated(ctx context.Context, comment *models.TaskComment) {
@@ -144,16 +144,6 @@ func (s *ChannelService) ListChannelsByAgent(ctx context.Context, agentInstanceI
return s.repo.ListChannelsByAgent(ctx, agentInstanceID)
}
// UpdateChannelStatus updates a channel's status.
func (s *ChannelService) UpdateChannelStatus(ctx context.Context, id, status string) error {
channel, err := s.repo.GetChannel(ctx, id)
if err != nil {
return err
}
channel.Status = models.ChannelStatus(status)
return s.repo.UpdateChannel(ctx, channel)
}
// DeleteChannel deletes a channel by ID.
func (s *ChannelService) DeleteChannel(ctx context.Context, id string) error {
return s.repo.DeleteChannel(ctx, id)
@@ -30,17 +30,6 @@ func NextCronTime(expression, timezone string, after time.Time) (time.Time, erro
return findNextMatch(spec, after.In(loc)), nil
}
// ParseCronExpression validates a cron expression string and returns an error
// if it is malformed.
func ParseCronExpression(expression string) error {
fields := strings.Fields(expression)
if len(fields) != 5 {
return fmt.Errorf("expected 5 cron fields, got %d", len(fields))
}
_, err := parseCronSpec(fields)
return err
}
type cronSpec struct {
minutes []int
hours []int
@@ -1,36 +0,0 @@
package shared
import (
"encoding/json"
"regexp"
"strings"
)
var slugRe = regexp.MustCompile(`[^a-z0-9]+`)
// IsValidPathComponent reports whether s is safe to use as a filesystem path
// component: non-empty, no slashes, no backslashes, no ".." traversal.
func IsValidPathComponent(s string) bool {
return s != "" &&
!strings.Contains(s, "/") &&
!strings.Contains(s, "\\") &&
!strings.Contains(s, "..")
}
// GenerateSlug converts a name to a lowercase hyphenated slug suitable for
// use as a URL or filesystem identifier.
func GenerateSlug(name string) string {
lower := strings.ToLower(name)
slug := slugRe.ReplaceAllString(lower, "-")
return strings.Trim(slug, "-")
}
// MustJSON marshals v to a JSON string, returning "{}" on any error.
// Intended for struct fields that must always contain valid JSON.
func MustJSON(v interface{}) string {
b, err := json.Marshal(v)
if err != nil {
return "{}"
}
return string(b)
}
@@ -163,13 +163,6 @@ func userHomeSourceLocator(provider, key string) string {
return strings.TrimSpace(provider) + ":" + strings.Trim(strings.TrimSpace(key), "/")
}
// DiscoverUserSkillsAt scans an already resolved provider user skill root.
func DiscoverUserSkillsAt(
ctx context.Context, provider, root string,
) ([]DiscoveredUserSkill, error) {
return discoverUserSkillsAt(ctx, provider, root, root)
}
func discoverUserSkillsAt(
ctx context.Context, provider, root, allowedBase string,
) ([]DiscoveredUserSkill, error) {
+1 -3
View File
@@ -36,8 +36,6 @@ func Provide(
svc := New(repo, taskReader, backend, log, cfg.KandevVersion)
h := NewHTTPHandlers(svc, ghClient, log)
// Cleanup is a true no-op — the repository doesn't own its database
// connection (the pool is owned by cmd/kandev), and Repository.Close()
// reflects that by returning nil. Returning an inline noop here keeps
// the contract obvious to readers without needing to chase Close.
// connection (the pool is owned by cmd/kandev).
return h, func() error { return nil }, nil
}
@@ -56,9 +56,6 @@ func NewRepository(writer, reader *sqlx.DB, log *logger.Logger) (*Repository, er
return r, nil
}
// Close is a no-op; the database connections are not owned by this repo.
func (r *Repository) Close() error { return nil }
func (r *Repository) initSchema() error {
mig := db.NewMigrateLogger(r.writer, r.log)
mig.Apply("table.task_shares", `CREATE TABLE IF NOT EXISTS task_shares (
@@ -37,9 +37,6 @@ func NewWithDB(writer, reader *sqlx.DB, log *logger.Logger) (*Repository, error)
return r, nil
}
// Close is a no-op — the writer/reader pools are owned by the caller.
func (r *Repository) Close() error { return nil }
func (r *Repository) initSchema() error {
stmts := []string{
`CREATE TABLE IF NOT EXISTS user_terminals (
+3 -8
View File
@@ -149,14 +149,9 @@ func NormalizeBranchPrefix(prefix string) string {
return trimmed
}
// TaskBranchName returns the standard per-task branch name used by isolated
// executors. The random suffix keeps similarly titled tasks from colliding.
func TaskBranchName(taskTitle, taskID, prefix string) string {
return TaskBranchNameWithSuffix(taskTitle, taskID, prefix, SmallSuffix(3))
}
// TaskBranchNameWithSuffix is the deterministic form of TaskBranchName used by
// worktree naming, where the caller already generated the suffix.
// TaskBranchNameWithSuffix builds the per-task branch name from a caller-supplied suffix.
// The caller is responsible for generating the suffix (e.g. SmallSuffix(3)) so that
// naming is deterministic when the suffix is derived externally.
func TaskBranchNameWithSuffix(taskTitle, taskID, prefix, suffix string) string {
sanitized := SanitizeForBranch(taskTitle, 20)
if sanitized == "" {