marketing-shibata50/multica
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(cli): address code review findings

Browse source 
1. Add Client.SendHeartbeat/Register methods — no more direct postJSON calls
2. Use url.Values for query params to prevent URL injection
3. Unexport helpers (envOrDefault, durationFromEnv, sleepWithContext)
4. CLI resolveWorkspaceID falls back to daemon.json
5. Implement agent stop (PUT /api/agents/{id} with status=offline)
6. Add --output flag to agent get for consistent UX
7. Add server/multica to .gitignore for stray builds
8. Inject version/commit via -ldflags in Makefile build target

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
yushen 2026-03-24 15:49:32 +08:00
parent 707b5ac6e7
commit 3293607bef
9 changed files with 130 additions and 34 deletions
Download patch file Download diff file Expand all files Collapse all files

1
.gitignore vendored
View file

@ -25,6 +25,7 @@ server/tmp/
server/migrate
server/daemon
server/multica-cli
server/multica
# Test artifacts
test-results/

5
Makefile
View file

@ -118,9 +118,12 @@ daemon:
cli:
cd server && go run ./cmd/multica $(ARGS)
VERSION ?= $(shell git describe --tags --always --dirty 2>/dev/null || echo dev)
COMMIT ?= $(shell git rev-parse --short HEAD 2>/dev/null || echo unknown)
build:
cd server && go build -o bin/server ./cmd/server
cd server && go build -o bin/multica-cli ./cmd/multica
cd server && go build -ldflags "-X main.version=$(VERSION) -X main.commit=$(COMMIT)" -o bin/multica-cli ./cmd/multica
test:
cd server && go test ./...

41
server/cmd/multica/cmd_agent.go
View file

@ -3,6 +3,7 @@ package main
import (
"context"
"fmt"
"net/url"
"os"
"time"
@ -50,6 +51,7 @@ func init() {
agentCmd.AddCommand(agentStopCmd)
agentListCmd.Flags().String("output", "table", "Output format: table or json")
agentGetCmd.Flags().String("output", "json", "Output format: table or json")
}
func newAPIClient(cmd *cobra.Command) (*cli.APIClient, error) {
@ -84,7 +86,11 @@ func resolveWorkspaceID(cmd *cobra.Command) string {
return val
}
cfg, _ := cli.LoadCLIConfig()
return cfg.WorkspaceID
if cfg.WorkspaceID != "" {
return cfg.WorkspaceID
}
// Fallback: try daemon.json for workspace_id
return cli.LoadWorkspaceIDFromDaemonConfig()
}
func runAgentList(cmd *cobra.Command, _ []string) error {
@ -99,7 +105,7 @@ func runAgentList(cmd *cobra.Command, _ []string) error {
var agents []map[string]any
path := "/api/agents"
if client.WorkspaceID != "" {
path += "?workspace_id=" + client.WorkspaceID
path += "?" + url.Values{"workspace_id": {client.WorkspaceID}}.Encode()
}
if err := client.GetJSON(ctx, path, &agents); err != nil {
return fmt.Errorf("list agents: %w", err)
@ -138,6 +144,20 @@ func runAgentGet(cmd *cobra.Command, args []string) error {
return fmt.Errorf("get agent: %w", err)
}
output, _ := cmd.Flags().GetString("output")
if output == "table" {
headers := []string{"ID", "NAME", "STATUS", "RUNTIME", "DESCRIPTION"}
rows := [][]string{{
strVal(agent, "id"),
strVal(agent, "name"),
strVal(agent, "status"),
strVal(agent, "runtime_mode"),
strVal(agent, "description"),
}}
cli.PrintTable(os.Stdout, headers, rows)
return nil
}
return cli.PrintJSON(os.Stdout, agent)
}
@ -159,8 +179,21 @@ func runAgentDelete(cmd *cobra.Command, args []string) error {
}
func runAgentStop(cmd *cobra.Command, args []string) error {
// TODO: implement agent stop (PUT /api/agents/{id} with status=offline)
return fmt.Errorf("agent stop is not yet implemented")
client, err := newAPIClient(cmd)
if err != nil {
return err
}
ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
defer cancel()
body := map[string]any{"status": "offline"}
if err := client.PutJSON(ctx, "/api/agents/"+args[0], body, nil); err != nil {
return fmt.Errorf("stop agent: %w", err)
}
fmt.Fprintf(os.Stderr, "Agent %s stopped.\n", args[0])
return nil
}
func strVal(m map[string]any, key string) string {

33
server/internal/cli/client.go
View file

@ -1,6 +1,7 @@
package cli
import (
"bytes"
"context"
"encoding/json"
"fmt"
@ -76,6 +77,38 @@ func (c *APIClient) DeleteJSON(ctx context.Context, path string) error {
return nil
}
// PutJSON performs a PUT request with a JSON body.
func (c *APIClient) PutJSON(ctx context.Context, path string, body any, out any) error {
data, err := json.Marshal(body)
if err != nil {
return err
}
req, err := http.NewRequestWithContext(ctx, http.MethodPut, c.BaseURL+path, bytes.NewReader(data))
if err != nil {
return err
}
req.Header.Set("Content-Type", "application/json")
if c.WorkspaceID != "" {
req.Header.Set("X-Workspace-ID", c.WorkspaceID)
}
resp, err := c.HTTPClient.Do(req)
if err != nil {
return err
}
defer resp.Body.Close()
if resp.StatusCode >= 400 {
respData, _ := io.ReadAll(io.LimitReader(resp.Body, 4096))
return fmt.Errorf("PUT %s returned %d: %s", path, resp.StatusCode, strings.TrimSpace(string(respData)))
}
if out == nil {
return nil
}
return json.NewDecoder(resp.Body).Decode(out)
}
// HealthCheck hits the /health endpoint and returns the response body.
func (c *APIClient) HealthCheck(ctx context.Context) (string, error) {
req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.BaseURL+"/health", nil)

20
server/internal/cli/config.go
View file

@ -45,6 +45,26 @@ func LoadCLIConfig() (CLIConfig, error) {
return cfg, nil
}
// LoadWorkspaceIDFromDaemonConfig reads workspace_id from ~/.multica/daemon.json
// as a fallback when it's not set in the CLI config.
func LoadWorkspaceIDFromDaemonConfig() string {
home, err := os.UserHomeDir()
if err != nil {
return ""
}
data, err := os.ReadFile(filepath.Join(home, ".multica/daemon.json"))
if err != nil {
return ""
}
var cfg struct {
WorkspaceID string `json:"workspace_id"`
}
if json.Unmarshal(data, &cfg) != nil {
return ""
}
return cfg.WorkspaceID
}
// SaveCLIConfig writes the CLI config to disk.
func SaveCLIConfig(cfg CLIConfig) error {
path, err := CLIConfigPath()

16
server/internal/daemon/client.go
View file

@ -84,6 +84,22 @@ func (c *Client) FailTask(ctx context.Context, taskID, errMsg string) error {
}, nil)
}
func (c *Client) SendHeartbeat(ctx context.Context, runtimeID string) error {
return c.postJSON(ctx, "/api/daemon/heartbeat", map[string]string{
"runtime_id": runtimeID,
}, nil)
}
func (c *Client) Register(ctx context.Context, req map[string]any) ([]Runtime, error) {
var resp struct {
Runtimes []Runtime `json:"runtimes"`
}
if err := c.postJSON(ctx, "/api/daemon/register", req, &resp); err != nil {
return nil, err
}
return resp.Runtimes, nil
}
func (c *Client) postJSON(ctx context.Context, path string, reqBody any, respBody any) error {
var body io.Reader
if reqBody != nil {

18
server/internal/daemon/config.go
View file

@ -55,7 +55,7 @@ type Overrides struct {
// persisted config, and optional CLI flag overrides.
func LoadConfig(overrides Overrides) (Config, error) {
// Server URL: override > env > default
rawServerURL := EnvOrDefault("MULTICA_SERVER_URL", DefaultServerURL)
rawServerURL := envOrDefault("MULTICA_SERVER_URL", DefaultServerURL)
if overrides.ServerURL != "" {
rawServerURL = overrides.ServerURL
}
@ -91,14 +91,14 @@ func LoadConfig(overrides Overrides) (Config, error) {
// Probe available agent CLIs
agents := map[string]AgentEntry{}
claudePath := EnvOrDefault("MULTICA_CLAUDE_PATH", "claude")
claudePath := envOrDefault("MULTICA_CLAUDE_PATH", "claude")
if _, err := exec.LookPath(claudePath); err == nil {
agents["claude"] = AgentEntry{
Path: claudePath,
Model: strings.TrimSpace(os.Getenv("MULTICA_CLAUDE_MODEL")),
}
}
codexPath := EnvOrDefault("MULTICA_CODEX_PATH", "codex")
codexPath := envOrDefault("MULTICA_CODEX_PATH", "codex")
if _, err := exec.LookPath(codexPath); err == nil {
agents["codex"] = AgentEntry{
Path: codexPath,
@ -132,7 +132,7 @@ func LoadConfig(overrides Overrides) (Config, error) {
}
// Durations: override > env > default
pollInterval, err := DurationFromEnv("MULTICA_DAEMON_POLL_INTERVAL", DefaultPollInterval)
pollInterval, err := durationFromEnv("MULTICA_DAEMON_POLL_INTERVAL", DefaultPollInterval)
if err != nil {
return Config{}, err
}
@ -140,7 +140,7 @@ func LoadConfig(overrides Overrides) (Config, error) {
pollInterval = overrides.PollInterval
}
heartbeatInterval, err := DurationFromEnv("MULTICA_DAEMON_HEARTBEAT_INTERVAL", DefaultHeartbeatInterval)
heartbeatInterval, err := durationFromEnv("MULTICA_DAEMON_HEARTBEAT_INTERVAL", DefaultHeartbeatInterval)
if err != nil {
return Config{}, err
}
@ -148,7 +148,7 @@ func LoadConfig(overrides Overrides) (Config, error) {
heartbeatInterval = overrides.HeartbeatInterval
}
agentTimeout, err := DurationFromEnv("MULTICA_AGENT_TIMEOUT", DefaultAgentTimeout)
agentTimeout, err := durationFromEnv("MULTICA_AGENT_TIMEOUT", DefaultAgentTimeout)
if err != nil {
return Config{}, err
}
@ -157,17 +157,17 @@ func LoadConfig(overrides Overrides) (Config, error) {
}
// String overrides
daemonID := EnvOrDefault("MULTICA_DAEMON_ID", host)
daemonID := envOrDefault("MULTICA_DAEMON_ID", host)
if overrides.DaemonID != "" {
daemonID = overrides.DaemonID
}
deviceName := EnvOrDefault("MULTICA_DAEMON_DEVICE_NAME", host)
deviceName := envOrDefault("MULTICA_DAEMON_DEVICE_NAME", host)
if overrides.DeviceName != "" {
deviceName = overrides.DeviceName
}
runtimeName := EnvOrDefault("MULTICA_AGENT_RUNTIME_NAME", DefaultRuntimeName)
runtimeName := envOrDefault("MULTICA_AGENT_RUNTIME_NAME", DefaultRuntimeName)
if overrides.RuntimeName != "" {
runtimeName = overrides.RuntimeName
}

19
server/internal/daemon/daemon.go
View file

@ -84,16 +84,14 @@ func (d *Daemon) registerRuntimes(ctx context.Context) ([]Runtime, error) {
"runtimes": runtimes,
}
var resp struct {
Runtimes []Runtime `json:"runtimes"`
}
if err := d.client.postJSON(ctx, "/api/daemon/register", req, &resp); err != nil {
rts, err := d.client.Register(ctx, req)
if err != nil {
return nil, fmt.Errorf("register runtimes: %w", err)
}
if len(resp.Runtimes) == 0 {
if len(rts) == 0 {
return nil, fmt.Errorf("register runtimes: empty response")
}
return resp.Runtimes, nil
return rts, nil
}
func (d *Daemon) ensurePaired(ctx context.Context) (string, error) {
@ -160,7 +158,7 @@ func (d *Daemon) ensurePaired(ctx context.Context) (string, error) {
return "", fmt.Errorf("pairing session expired before approval")
}
if err := SleepWithContext(ctx, d.cfg.PollInterval); err != nil {
if err := sleepWithContext(ctx, d.cfg.PollInterval); err != nil {
return "", err
}
}
@ -176,10 +174,7 @@ func (d *Daemon) heartbeatLoop(ctx context.Context, runtimeIDs []string) {
return
case <-ticker.C:
for _, rid := range runtimeIDs {
err := d.client.postJSON(ctx, "/api/daemon/heartbeat", map[string]string{
"runtime_id": rid,
}, nil)
if err != nil {
if err := d.client.SendHeartbeat(ctx, rid); err != nil {
d.logger.Printf("heartbeat failed for runtime %s: %v", rid, err)
}
}
@ -216,7 +211,7 @@ func (d *Daemon) pollLoop(ctx context.Context, runtimeIDs []string) error {
if !claimed {
pollOffset = (pollOffset + 1) % n
if err := SleepWithContext(ctx, d.cfg.PollInterval); err != nil {
if err := sleepWithContext(ctx, d.cfg.PollInterval); err != nil {
return err
}
}

11
server/internal/daemon/helpers.go
View file

@ -8,9 +8,7 @@ import (
"time"
)
// EnvOrDefault returns the trimmed value of the environment variable key,
// falling back to fallback if empty.
func EnvOrDefault(key, fallback string) string {
func envOrDefault(key, fallback string) string {
value := strings.TrimSpace(os.Getenv(key))
if value == "" {
return fallback
@ -18,9 +16,7 @@ func EnvOrDefault(key, fallback string) string {
return value
}
// DurationFromEnv parses a duration from an environment variable,
// returning fallback if the variable is empty.
func DurationFromEnv(key string, fallback time.Duration) (time.Duration, error) {
func durationFromEnv(key string, fallback time.Duration) (time.Duration, error) {
value := strings.TrimSpace(os.Getenv(key))
if value == "" {
return fallback, nil
@ -32,8 +28,7 @@ func DurationFromEnv(key string, fallback time.Duration) (time.Duration, error)
return d, nil
}
// SleepWithContext blocks for the given duration or until the context is cancelled.
func SleepWithContext(ctx context.Context, d time.Duration) error {
func sleepWithContext(ctx context.Context, d time.Duration) error {
timer := time.NewTimer(d)
defer timer.Stop()