fix(agentci): use log.E() error pattern, add Charm SSH TODOs

Replace fmt.Errorf() with structured log.E() errors in agentci, forge,
jobrunner packages. Update PipelineSignal comment to reflect dispatch
fields. Add TODO markers for charmbracelet/ssh migration across all
exec ssh call sites.

Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
Snider 2026-02-09 11:15:00 +00:00
parent ac587b0c53
commit b72ac61698
7 changed files with 21 additions and 12 deletions

View file

@ -52,6 +52,7 @@ func agentAddCmd() *cli.Command {
}
// Test SSH connectivity.
// TODO: Replace exec ssh with charmbracelet/ssh native Go client + keygen.
fmt.Printf("Testing SSH to %s... ", host)
out, err := exec.Command("ssh",
"-o", "StrictHostKeyChecking=accept-new",
@ -115,6 +116,7 @@ func agentListCmd() *cli.Command {
}
// Quick SSH check for queue depth.
// TODO: Replace exec ssh with charmbracelet/ssh native Go client.
queue := dimStyle.Render("-")
out, err := exec.Command("ssh",
"-o", "StrictHostKeyChecking=accept-new",
@ -180,6 +182,7 @@ func agentStatusCmd() *cli.Command {
fi
`
// TODO: Replace exec ssh with charmbracelet/ssh native Go client.
sshCmd := exec.Command("ssh",
"-o", "StrictHostKeyChecking=accept-new",
"-o", "ConnectTimeout=10",
@ -215,6 +218,7 @@ func agentLogsCmd() *cli.Command {
return fmt.Errorf("agent %q not found", name)
}
// TODO: Replace exec ssh with charmbracelet/ssh native Go client.
tailArgs := []string{
"-o", "StrictHostKeyChecking=accept-new",
"-o", "ConnectTimeout=10",

View file

@ -6,6 +6,7 @@ import (
"github.com/host-uk/core/pkg/config"
"github.com/host-uk/core/pkg/jobrunner/handlers"
"github.com/host-uk/core/pkg/log"
)
// AgentConfig represents a single agent machine in the config file.
@ -33,7 +34,7 @@ func LoadAgents(cfg *config.Config) (map[string]handlers.AgentTarget, error) {
continue
}
if ac.Host == "" {
return nil, fmt.Errorf("agent %q: host is required", name)
return nil, log.E("agentci.LoadAgents", fmt.Sprintf("agent %q: host is required", name), nil)
}
queueDir := ac.QueueDir
if queueDir == "" {
@ -80,10 +81,10 @@ func SaveAgent(cfg *config.Config, name string, ac AgentConfig) error {
func RemoveAgent(cfg *config.Config, name string) error {
var agents map[string]AgentConfig
if err := cfg.Get("agentci.agents", &agents); err != nil {
return fmt.Errorf("no agents configured")
return log.E("agentci.RemoveAgent", "no agents configured", err)
}
if _, ok := agents[name]; !ok {
return fmt.Errorf("agent %q not found", name)
return log.E("agentci.RemoveAgent", fmt.Sprintf("agent %q not found", name), nil)
}
delete(agents, name)
return cfg.Set("agentci.agents", agents)

View file

@ -29,7 +29,7 @@ func (c *Client) MergePullRequest(owner, repo string, index int64, method string
return log.E("forge.MergePullRequest", "failed to merge pull request", err)
}
if !merged {
return fmt.Errorf("forge.MergePullRequest: merge returned false for %s/%s#%d", owner, repo, index)
return log.E("forge.MergePullRequest", fmt.Sprintf("merge returned false for %s/%s#%d", owner, repo, index), nil)
}
return nil
}
@ -41,13 +41,13 @@ func (c *Client) SetPRDraft(owner, repo string, index int64, draft bool) error {
payload := map[string]bool{"draft": draft}
body, err := json.Marshal(payload)
if err != nil {
return fmt.Errorf("forge.SetPRDraft: marshal: %w", err)
return log.E("forge.SetPRDraft", "marshal payload", err)
}
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d", c.url, owner, repo, index)
req, err := http.NewRequest(http.MethodPatch, url, bytes.NewReader(body))
if err != nil {
return fmt.Errorf("forge.SetPRDraft: create request: %w", err)
return log.E("forge.SetPRDraft", "create request", err)
}
req.Header.Set("Content-Type", "application/json")
req.Header.Set("Authorization", "token "+c.token)
@ -59,7 +59,7 @@ func (c *Client) SetPRDraft(owner, repo string, index int64, draft bool) error {
defer func() { _ = resp.Body.Close() }()
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
return fmt.Errorf("forge.SetPRDraft: unexpected status %d", resp.StatusCode)
return log.E("forge.SetPRDraft", fmt.Sprintf("unexpected status %d", resp.StatusCode), nil)
}
return nil
}

View file

@ -82,7 +82,7 @@ func (s *ForgejoSource) pollRepo(_ context.Context, owner, repo string) ([]*jobr
// Fetch epic issues (label=epic, state=open).
issues, err := s.forge.ListIssues(owner, repo, forge.ListIssuesOpts{State: "open"})
if err != nil {
return nil, fmt.Errorf("fetch issues: %w", err)
return nil, log.E("forgejo.pollRepo", "fetch issues", err)
}
// Filter to epics only.
@ -106,7 +106,7 @@ func (s *ForgejoSource) pollRepo(_ context.Context, owner, repo string) ([]*jobr
// Fetch all open PRs (and also merged/closed to catch MERGED state).
prs, err := s.forge.ListPullRequests(owner, repo, "all")
if err != nil {
return nil, fmt.Errorf("fetch PRs: %w", err)
return nil, log.E("forgejo.pollRepo", "fetch PRs", err)
}
var signals []*jobrunner.PipelineSignal
@ -167,7 +167,7 @@ type epicInfo struct {
func splitRepo(full string) (string, string, error) {
parts := strings.SplitN(full, "/", 2)
if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
return "", "", fmt.Errorf("expected owner/repo format, got %q", full)
return "", "", log.E("forgejo.splitRepo", fmt.Sprintf("expected owner/repo format, got %q", full), nil)
}
return parts[0], parts[1], nil
}

View file

@ -79,7 +79,7 @@ func (h *DispatchHandler) Execute(ctx context.Context, signal *jobrunner.Pipelin
agent, ok := h.agents[signal.Assignee]
if !ok {
return nil, fmt.Errorf("unknown agent: %s", signal.Assignee)
return nil, log.E("dispatch.Execute", fmt.Sprintf("unknown agent: %s", signal.Assignee), nil)
}
// Determine target branch (default to repo default).
@ -166,8 +166,10 @@ func (h *DispatchHandler) Execute(ctx context.Context, signal *jobrunner.Pipelin
}
// scpTicket writes ticket data to a remote path via SSH.
// TODO: Replace exec ssh+cat with charmbracelet/ssh for native Go SSH.
func (h *DispatchHandler) scpTicket(ctx context.Context, host, remotePath string, data []byte) error {
// Use ssh + cat instead of scp for piping stdin.
// TODO: Use charmbracelet/keygen for key management, native Go SSH client for transport.
cmd := exec.CommandContext(ctx, "ssh",
"-o", "StrictHostKeyChecking=accept-new",
"-o", "ConnectTimeout=10",
@ -184,6 +186,7 @@ func (h *DispatchHandler) scpTicket(ctx context.Context, host, remotePath string
}
// ticketExists checks if a ticket file already exists in queue, active, or done.
// TODO: Replace exec ssh with native Go SSH client (charmbracelet/ssh).
func (h *DispatchHandler) ticketExists(agent AgentTarget, ticketName string) bool {
cmd := exec.Command("ssh",
"-o", "StrictHostKeyChecking=accept-new",

View file

@ -6,7 +6,7 @@ import (
)
// PipelineSignal is the structural snapshot of a child issue/PR.
// Never contains comment bodies or free text — structural signals only.
// Carries structural state plus issue title/body for dispatch prompts.
type PipelineSignal struct {
EpicNumber int
ChildNumber int

View file

@ -75,6 +75,7 @@ FORGEJO_USER=$(jq -r '.forgejo_user // empty' "$TICKET_FILE")
if [ -z "$FORGEJO_USER" ]; then
FORGEJO_USER="$(hostname -s)-$(whoami)"
fi
# TODO: Replace token-in-URL with git credential helper or SSH clone via charmbracelet/keygen.
CLONE_URL="https://${FORGEJO_USER}:${FORGE_TOKEN}@${FORGE_URL#https://}/${REPO_OWNER}/${REPO_NAME}.git"
if [ -d "$REPO_DIR/.git" ]; then