diff --git a/cli/cmd/albedo/main.go b/cli/cmd/albedo/main.go index d5412ede..2f76f9cc 100644 --- a/cli/cmd/albedo/main.go +++ b/cli/cmd/albedo/main.go @@ -1,9 +1,7 @@ +// Command albedo runs the terminal client and daemon management commands. package main import ( - "albedo/cli/internal/config" - "albedo/cli/internal/daemon" - "albedo/cli/internal/tui" "bufio" "context" "encoding/json" @@ -15,6 +13,10 @@ import ( "strings" "time" + "albedo/cli/internal/config" + "albedo/cli/internal/daemon" + "albedo/cli/internal/tui" + tea "charm.land/bubbletea/v2" "github.com/charmbracelet/x/ansi" "github.com/mattn/go-isatty" @@ -184,22 +186,22 @@ func open(id, workspace string, fresh bool, openBrowser func(string)) error { configured := profs.Active != "" if !configured && !isTTY() { - return errors.New("No model provider is configured. Run albedo login in a terminal to set one up.") + return errors.New("no model provider is configured; run albedo login in a terminal to set one up") } var initial *daemon.Session if selected != nil { initial = selected } else if configured && (fresh || len(sessions) == 0) { - created, err := daemon.Request[daemon.Session](ctx, conn, "/sessions", map[string]string{"workspace": absWorkspace}) - if err != nil { - return err + created, createErr := daemon.Request[daemon.Session](ctx, conn, "/sessions", map[string]string{"workspace": absWorkspace}) + if createErr != nil { + return createErr } initial = &created } if !isTTY() { - type NonTTYOutput struct { + type nonTTYOutput struct { Session *string `json:"session,omitempty"` Sessions []daemon.Session `json:"sessions"` } @@ -210,13 +212,13 @@ func open(id, workspace string, fresh bool, openBrowser func(string)) error { if sessions == nil { sessions = []daemon.Session{} } - out := NonTTYOutput{ + out := nonTTYOutput{ Session: sessID, Sessions: sessions, } - data, err := json.Marshal(out) - if err != nil { - return err + data, marshalErr := json.Marshal(out) + if marshalErr != nil { + return marshalErr } fmt.Println(string(data)) return nil @@ -268,13 +270,13 @@ func run(args []string) error { case "new": workspace := cwd - for i := 0; i < len(subArgs); i++ { - if subArgs[i] == "-h" || subArgs[i] == "--help" { + for _, arg := range subArgs { + if arg == "-h" || arg == "--help" { fmt.Println("Usage: albedo new [workspace]") return nil } - if !strings.HasPrefix(subArgs[i], "-") { - workspace = subArgs[i] + if !strings.HasPrefix(arg, "-") { + workspace = arg break } } @@ -286,7 +288,7 @@ func run(args []string) error { fmt.Println("Usage: albedo resume ") return nil } - return errors.New("Choose a session to reopen: albedo resume ") + return errors.New("choose a session to reopen: albedo resume ") } return open(subArgs[0], cwd, false, openBrowser) @@ -328,7 +330,7 @@ func run(args []string) error { return nil } if len(subArgs) < 2 { - return errors.New("Choose a session and a message to send: albedo send ") + return errors.New("choose a session and a message to send: albedo send ") } sessID := subArgs[0] prompt := subArgs[1] @@ -353,7 +355,7 @@ func run(args []string) error { return nil } if len(subArgs) < 1 { - return errors.New("Choose a session to interrupt: albedo stop ") + return errors.New("choose a session to interrupt: albedo stop ") } sessID := subArgs[0] homeDir := config.HomeDir() @@ -411,7 +413,7 @@ func run(args []string) error { return nil } if !isTTY() { - return errors.New("Run albedo login in a terminal so you can enter your API key without displaying it.") + return errors.New("run albedo login in a terminal so you can enter your API key without displaying it") } providerName := "" if len(subArgs) > 0 { @@ -435,6 +437,6 @@ func run(args []string) error { return err default: - return fmt.Errorf("Unknown command %q. Run albedo --help to see the available commands.", cmd) + return fmt.Errorf("unknown command %q; run albedo --help to see the available commands", cmd) } } diff --git a/cli/cmd/albedo/storage.go b/cli/cmd/albedo/storage.go index 991a9460..bfc520b7 100644 --- a/cli/cmd/albedo/storage.go +++ b/cli/cmd/albedo/storage.go @@ -1,9 +1,8 @@ package main import ( - "albedo/cli/internal/config" - "albedo/cli/internal/daemon" "bufio" + "cmp" "context" "encoding/json" "errors" @@ -13,11 +12,13 @@ import ( "os" "os/exec" "path/filepath" - "reflect" "regexp" - "sort" + "slices" "strings" "time" + + "albedo/cli/internal/config" + "albedo/cli/internal/daemon" ) const storageHelp = `Usage: albedo storage [--sessions | --json] @@ -112,10 +113,10 @@ func sqliteStorage(path string) (storageDB, error) { var db storageDB out, err := exec.Command("python3", "-c", inspectStorage, path).CombinedOutput() if err != nil { - return db, fmt.Errorf("Could not read disk usage from the database: %w: %s", err, strings.TrimSpace(string(out))) + return db, fmt.Errorf("read disk usage from the database: %w: %s", err, strings.TrimSpace(string(out))) } if err := json.Unmarshal(out, &db); err != nil { - return db, fmt.Errorf("Could not read the database disk usage report: %w", err) + return db, fmt.Errorf("decode the database disk usage report: %w", err) } return db, nil } @@ -129,7 +130,7 @@ func regularSize(path string) (int64, error) { return 0, err } if !info.Mode().IsRegular() { - return 0, fmt.Errorf("Expected a file, but found a different kind of entry: %s", path) + return 0, fmt.Errorf("expected a file, but found a different kind of entry: %s", path) } return info.Size(), nil } @@ -143,7 +144,7 @@ func storageFiles(dir string, eligible func(string, os.FileInfo) bool) (int64, [ return 0, nil, err } if !stat.IsDir() { - return 0, nil, fmt.Errorf("Expected a directory for storage: %s", dir) + return 0, nil, fmt.Errorf("expected a directory for storage: %s", dir) } entries, err := os.ReadDir(dir) if err != nil { @@ -184,9 +185,9 @@ func storageSnapshot(home string, now time.Time) (storagePreview, error) { return p, err } for _, suffix := range []string{"-wal", "-shm"} { - size, err := regularSize(dbPath + suffix) - if err != nil { - return p, err + size, sizeErr := regularSize(dbPath + suffix) + if sizeErr != nil { + return p, sizeErr } p.WAL += size } @@ -267,7 +268,7 @@ func storagePrint(w io.Writer, p storagePreview, sessions bool) { } fmt.Fprintf(w, "Available to clean up: %d unused Python state files (%s), %d old backups (%s)\n", len(p.OldKernels), storageSize(kernels), len(p.OldBackups), storageSize(backups)) if sessions { - sort.Slice(p.DB.Sessions, func(i, j int) bool { return p.DB.Sessions[i].Bytes > p.DB.Sessions[j].Bytes }) + slices.SortFunc(p.DB.Sessions, func(a, b storageSession) int { return cmp.Compare(b.Bytes, a.Bytes) }) for _, s := range p.DB.Sessions { fmt.Fprintf(w, " %s %s\n", storageSize(s.Bytes), s.ID) } @@ -286,7 +287,7 @@ func pruneFiles(files []storageFile) error { return err } if size != f.Bytes { - return fmt.Errorf("File changed after the preview. Cleanup stopped: %s", f.Path) + return fmt.Errorf("file changed after the preview; cleanup stopped: %s", f.Path) } if err := os.Remove(f.Path); err != nil { return err @@ -303,7 +304,7 @@ func storageCommand(args []string) error { } if len(args) == 0 || args[0] == "--json" || args[0] == "--sessions" { if len(args) > 1 { - return errors.New("Usage: albedo storage [--sessions | --json]") + return errors.New("usage: albedo storage [--sessions | --json]") } p, err := storageSnapshot(home, time.Now()) if err != nil { @@ -322,7 +323,7 @@ func storageCommand(args []string) error { return nil } if args[0] != "prune" { - return errors.New("Run albedo storage to see disk usage, or albedo storage prune --help for cleanup options.") + return errors.New("run albedo storage to see disk usage, or albedo storage prune --help for cleanup options") } if len(args) == 2 && (args[1] == "-h" || args[1] == "--help") { fmt.Print(storageHelp) @@ -341,19 +342,19 @@ func storageCommand(args []string) error { return err } if flags.NArg() != 0 { - return errors.New("Cleanup only accepts options. Run albedo storage prune --help for examples.") + return errors.New("cleanup only accepts options; run albedo storage prune --help for examples") } if *all { if len(sessions) > 0 || *kernels || *backups || *vacuum { - return errors.New("--all cannot be combined with other cleanup options. Use --all on its own or choose individual options.") + return errors.New("--all cannot be combined with other cleanup options; use --all on its own or choose individual options") } *kernels, *backups, *vacuum = true, true, true } if len(sessions) == 0 && !*kernels && !*backups && !*vacuum { - return errors.New("Choose what to clean up: --all, --session, --old-kernels, --backups or --vacuum.") + return errors.New("choose what to clean up: --all, --session, --old-kernels, --backups or --vacuum") } if len(sessions) > 0 && (*kernels || *backups || *vacuum) { - return errors.New("Delete sessions and clean up local files in separate commands. Session deletion needs Albedo running; file cleanup needs it stopped.") + return errors.New("delete sessions and clean up local files in separate commands; session deletion needs Albedo running, file cleanup needs it stopped") } p, err := storageSnapshot(home, time.Now()) if err != nil { @@ -361,14 +362,14 @@ func storageCommand(args []string) error { } if len(sessions) == 0 { if *kernels && p.Database == 0 { - return errors.New("Cannot identify unused Python state files without the SQLite database. No files have been removed.") + return errors.New("cannot identify unused Python state files without the SQLite database; no files have been removed") } - conn, err := daemon.Existing(home) - if err != nil { - return err + conn, existingErr := daemon.Existing(home) + if existingErr != nil { + return existingErr } if conn != nil { - return errors.New("Nothing has been removed. Cleanup needs Albedo stopped. When its work has finished, run albedo daemon --stop, then try again.") + return errors.New("cleanup needs Albedo stopped; no files have been removed; when its work has finished, run albedo daemon --stop, then try again") } } storagePrint(os.Stdout, p, false) @@ -379,7 +380,7 @@ func storageCommand(args []string) error { } for _, id := range sessions { if !known[id] { - return fmt.Errorf("No session has the ID %s. Use the full ID from albedo sessions.", id) + return fmt.Errorf("no session has the ID %s; use the full ID from albedo sessions", id) } fmt.Printf("Permanently delete session %s\n", id) } @@ -406,30 +407,30 @@ func storageCommand(args []string) error { } if !*yes { if !isTTY() { - return errors.New("Nothing has been removed. Run cleanup in a terminal to confirm, or add --yes to approve the changes shown above.") + return errors.New("no files have been removed; run cleanup in a terminal to confirm, or add --yes to approve the changes shown above") } if len(sessions) > 0 { fmt.Print("Permanently delete the sessions listed above? [y/N] ") } else { fmt.Print("Apply the cleanup shown above? [y/N] ") } - answer, err := bufio.NewReader(os.Stdin).ReadString('\n') - if err != nil && !errors.Is(err, io.EOF) { - return err + answer, readErr := bufio.NewReader(os.Stdin).ReadString('\n') + if readErr != nil && !errors.Is(readErr, io.EOF) { + return readErr } if !confirmed(answer) { return nil } } if len(sessions) > 0 { - conn, err := daemon.Ensure(home, findProjectRoot(), replaceStale) - if err != nil { - return err + conn, ensureErr := daemon.Ensure(home, findProjectRoot(), replaceStale) + if ensureErr != nil { + return ensureErr } for _, id := range sessions { path := "/sessions/" + id - if _, err := daemon.RequestMethod[json.RawMessage](context.Background(), conn, "DELETE", path, nil); err != nil { - return err + if _, deleteErr := daemon.RequestMethod[json.RawMessage](context.Background(), conn, "DELETE", path, nil); deleteErr != nil { + return deleteErr } } return nil @@ -440,23 +441,23 @@ func storageCommand(args []string) error { return err } if conn != nil { - return errors.New("Albedo started while you were confirming cleanup. Nothing has been removed. Stop Albedo and try again.") + return errors.New("daemon started while you were confirming cleanup; no files have been removed; stop Albedo and try again") } fresh, err := storageSnapshot(home, time.Now()) if err != nil { return err } if *kernels { - if !reflect.DeepEqual(p.OldKernels, fresh.OldKernels) { - return errors.New("The unused Python state files changed after the preview. Nothing has been removed. Run cleanup again to review the new list.") + if !slices.Equal(p.OldKernels, fresh.OldKernels) { + return errors.New("unused Python state files changed after the preview; no files have been removed; run cleanup again to review the new list") } if err := pruneFiles(p.OldKernels); err != nil { return err } } if *backups { - if !reflect.DeepEqual(p.OldBackups, fresh.OldBackups) { - return errors.New("The backups changed after the preview. No backups have been removed. Run cleanup again to review the new list.") + if !slices.Equal(p.OldBackups, fresh.OldBackups) { + return errors.New("backups changed after the preview; no backups have been removed; run cleanup again to review the new list") } if err := pruneFiles(p.OldBackups); err != nil { return err @@ -464,7 +465,7 @@ func storageCommand(args []string) error { } if *vacuum { if fresh.Database == 0 { - return errors.New("There is no SQLite database to shrink.") + return errors.New("no SQLite database to shrink") } if fresh.DB.FreePages == 0 { fmt.Println("The database has no unused space to reclaim. Skipping --vacuum.") @@ -472,7 +473,7 @@ func storageCommand(args []string) error { path := filepath.Join(home, "albedo.sqlite") out, err := exec.Command("python3", "-c", "import sqlite3,sys; db=sqlite3.connect(sys.argv[1]); db.execute('VACUUM'); db.close()", path).CombinedOutput() if err != nil { - return fmt.Errorf("Could not shrink the database: %w: %s", err, strings.TrimSpace(string(out))) + return fmt.Errorf("shrink the database: %w: %s", err, strings.TrimSpace(string(out))) } final, err := regularSize(path) if err != nil { @@ -492,12 +493,10 @@ var storageID = regexp.MustCompile(`^[0-9a-f]{32}$`) func (ids *sessionIDs) String() string { return strings.Join(*ids, ",") } func (ids *sessionIDs) Set(value string) error { if !storageID.MatchString(value) { - return errors.New("--session needs a full session ID. Find it with albedo sessions.") + return errors.New("--session needs a full session ID; find it with albedo sessions") } - for _, id := range *ids { - if id == value { - return fmt.Errorf("Session %s was selected more than once.", value) - } + if slices.Contains(*ids, value) { + return fmt.Errorf("session %s was selected more than once", value) } *ids = append(*ids, value) return nil diff --git a/cli/internal/config/config.go b/cli/internal/config/config.go index e738b361..afd8692e 100644 --- a/cli/internal/config/config.go +++ b/cli/internal/config/config.go @@ -1,3 +1,5 @@ +// Package config defines CLI settings and validates values before they are +// submitted to the daemon. The daemon owns persistence and credentials. package config import ( @@ -10,9 +12,7 @@ import ( "unicode" ) -var ( - providerNameRegex = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._-]{0,63}$`) -) +var providerNameRegex = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._-]{0,63}$`) // HomeDir returns the albedo configuration directory. func HomeDir() string { @@ -44,13 +44,8 @@ type Settings struct { func validateModel(value string) (string, error) { trimmed := strings.TrimSpace(value) - if trimmed == "" || len(trimmed) > 512 { - return "", errors.New("Model ID must be 1–512 characters and contain no control characters.") - } - for _, r := range trimmed { - if r < 0x20 || r == 0x7f { - return "", errors.New("Model ID must be 1–512 characters and contain no control characters.") - } + if trimmed == "" || len(trimmed) > 512 || strings.ContainsFunc(trimmed, func(r rune) bool { return r < 0x20 || r == 0x7f }) { + return "", errors.New("model ID must be 1–512 characters and contain no control characters") } return trimmed, nil } @@ -58,7 +53,7 @@ func validateModel(value string) (string, error) { // Validate checks whether the Settings instance is valid. func (s Settings) Validate() (Settings, error) { if s.Protocol != "responses" && s.Protocol != "chat_completions" { - return s, errors.New("Protocol must be responses or chat_completions.") + return s, errors.New("protocol must be responses or chat_completions") } model, err := validateModel(s.Model) @@ -80,11 +75,11 @@ func (s Settings) Validate() (Settings, error) { } if ext == "openai" && endpoint == "" { - return s, errors.New("OpenAI provider needs an API base URL.") + return s, errors.New("OpenAI provider needs an API base URL") } if strings.ContainsFunc(s.APIKey, func(r rune) bool { return unicode.IsSpace(r) || r < 0x20 || r == 0x7f }) { - return s, errors.New("API key must not contain spaces or control characters.") + return s, errors.New("API key must not contain spaces or control characters") } return Settings{ @@ -101,7 +96,7 @@ func (s Settings) Validate() (Settings, error) { func ValidateProviderName(value string) (string, error) { name := strings.TrimSpace(value) if !providerNameRegex.MatchString(name) { - return "", errors.New("Provider name must start with a letter or number and use only letters, numbers, dots, underscores, or hyphens (1–64 characters).") + return "", errors.New("provider name must start with a letter or number and use only letters, numbers, dots, underscores, or hyphens (1–64 characters)") } return name, nil } @@ -110,19 +105,15 @@ func ValidateProviderName(value string) (string, error) { func ValidateEndpoint(value string) (string, error) { u, err := url.Parse(strings.TrimSpace(value)) if err != nil || u.Scheme == "" || u.Host == "" { - return "", errors.New("Enter an HTTP or HTTPS API base URL.") + return "", errors.New("enter an HTTP or HTTPS API base URL") } if u.Scheme != "http" && u.Scheme != "https" { - return "", errors.New("Enter an HTTP or HTTPS API base URL.") + return "", errors.New("enter an HTTP or HTTPS API base URL") } if u.User != nil || u.RawQuery != "" || u.Fragment != "" { - return "", errors.New("Use an HTTP or HTTPS API base URL without credentials, a query, or a fragment.") - } - res := u.String() - for strings.HasSuffix(res, "/") { - res = strings.TrimSuffix(res, "/") + return "", errors.New("use an HTTP or HTTPS API base URL without credentials, a query, or a fragment") } - return res, nil + return strings.TrimRight(u.String(), "/"), nil } // Profiles represents the stored provider configuration. diff --git a/cli/internal/daemon/client.go b/cli/internal/daemon/client.go index 46d9db91..5e5b7df8 100644 --- a/cli/internal/daemon/client.go +++ b/cli/internal/daemon/client.go @@ -46,7 +46,7 @@ type WorkspaceMissingError struct { } func (e *WorkspaceMissingError) Error() string { - return "Workspace folder not found: " + e.Workspace + return "workspace folder not found: " + e.Workspace } type SendResult struct { @@ -175,9 +175,6 @@ func (c *ChatClient) Interrupt(ctx context.Context) (bool, error) { return data.Interrupted, nil } -// ContextWindow reads the model's context window from the session's last -// prepared request. It is nil when neither a catalog nor the configuration -// knows it, or when no request has been prepared yet. // HistoryPage is older transcript, rendered as the stream renders a reset. type HistoryPage struct { Events []StreamEvent @@ -228,6 +225,9 @@ func (c *ChatClient) History(ctx context.Context, before int64, rows int) (*Hist return page, nil } +// ContextWindow reads the model's context window from the session's last +// prepared request. It is nil when neither a catalog nor the configuration +// knows it, or when no request has been prepared yet. func (c *ChatClient) ContextWindow(ctx context.Context) (*int, error) { reqCtx, cancel := context.WithTimeout(ctx, 20*time.Second) defer cancel() @@ -698,7 +698,7 @@ func (c *ChatClient) Stream(ctx context.Context, tail int, onEvent func(StreamEv c.argumentsByCall = make(map[string]string) c.afterSeq = -1 c.mu.Unlock() - return errors.New("Too many tool argument previews for this client to display.") + return errors.New("too many tool argument previews for this client to display") } args := c.argumentsByCall[callID] + text diff --git a/cli/internal/daemon/client_test.go b/cli/internal/daemon/client_test.go index b9c2d386..d15c946d 100644 --- a/cli/internal/daemon/client_test.go +++ b/cli/internal/daemon/client_test.go @@ -27,9 +27,9 @@ func TestCompletedCallPreviewsReleaseState(t *testing.T) { a := string(aData) b := string(bData) - var reqCount int32 + var requestCount atomic.Int32 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - count := atomic.AddInt32(&reqCount, 1) + count := requestCount.Add(1) w.Header().Set("Content-Type", "text/event-stream") if count == 1 { _, _ = w.Write([]byte(formatPage(1, []any{ @@ -94,10 +94,10 @@ func TestCompletedCallPreviewsReleaseState(t *testing.T) { func TestUnfinishedArgumentsBounded(t *testing.T) { var requestedURLs []string - var serverCallCount int32 + var serverCallCount atomic.Int32 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { requestedURLs = append(requestedURLs, r.URL.String()) - count := atomic.AddInt32(&serverCallCount, 1) + count := serverCallCount.Add(1) w.Header().Set("Content-Type", "text/event-stream") if count <= 33 { _, _ = w.Write([]byte(formatPage(int(count), []any{ @@ -115,7 +115,7 @@ func TestUnfinishedArgumentsBounded(t *testing.T) { conn := NewConnection(ConnectionSnapshot{Port: server.Listener.Addr().(*net.TCPAddr).Port}, "") client := NewChatClient(conn, "session") - for i := 1; i < 33; i++ { + for range 32 { _ = client.Stream(context.Background(), 0, func(event StreamEvent) error { return nil }) } diff --git a/cli/internal/daemon/commands.go b/cli/internal/daemon/commands.go index 20a1220f..13f2f466 100644 --- a/cli/internal/daemon/commands.go +++ b/cli/internal/daemon/commands.go @@ -82,15 +82,14 @@ func ParseCommandCatalog(data []byte) ([]SessionCommand, error) { commands := make([]SessionCommand, 0, len(rawList)) for _, raw := range rawList { var cmd SessionCommand - if err := json.Unmarshal(raw, &cmd); err == nil { - if isValidCommand(cmd) { - commands = append(commands, cmd) - } + if err := json.Unmarshal(raw, &cmd); err != nil || !isValidCommand(cmd) { + continue } + commands = append(commands, cmd) } if len(rawList) > 0 && len(commands) == 0 { - return nil, errors.New("Albedo returned command details that this client could not read.") + return nil, errors.New("cannot read command details returned by Albedo") } return commands, nil @@ -114,9 +113,6 @@ func ParseCommandInvocation(value string, catalog []SessionCommand) (name, args return "", "", false } - rest := value[loc[1]:] - if strings.HasPrefix(rest, " ") { - rest = rest[1:] - } + rest := strings.TrimPrefix(value[loc[1]:], " ") return matched.Name, rest, true } diff --git a/cli/internal/daemon/detach_other.go b/cli/internal/daemon/detach_other.go index d1bbfd9c..c8242d2a 100644 --- a/cli/internal/daemon/detach_other.go +++ b/cli/internal/daemon/detach_other.go @@ -9,4 +9,4 @@ func detach(cmd *exec.Cmd) {} func passFileLimit() {} // processAlive cannot be checked portably here; assume the lock holder is live. -func processAlive(pid int) bool { return true } +func processAlive(int) bool { return true } diff --git a/cli/internal/daemon/errors.go b/cli/internal/daemon/errors.go index 0ed3c5e7..294c76a7 100644 --- a/cli/internal/daemon/errors.go +++ b/cli/internal/daemon/errors.go @@ -12,7 +12,7 @@ type UpgradeRequiredError struct { } func (e *UpgradeRequiredError) Error() string { - return "The running copy of Albedo needs an update " + e.Feature + ". Restarting will interrupt work in all sessions and clear their Python variables. When you are ready, run albedo daemon --stop, then launch your updated copy of Albedo." + return "the running copy of Albedo needs an update " + e.Feature + "; restarting will interrupt work in all sessions and clear their Python variables; when you are ready, run albedo daemon --stop, then launch your updated copy of Albedo" } // APIError retains the HTTP status and daemon error code independently of copy. diff --git a/cli/internal/daemon/events.go b/cli/internal/daemon/events.go index 7537621e..a1efa088 100644 --- a/cli/internal/daemon/events.go +++ b/cli/internal/daemon/events.go @@ -6,7 +6,7 @@ import ( "unicode" ) -// Display-only execution evidence, never appended to the model's tool output. +// ToolActivity records display-only execution evidence, not model-facing tool output. type ToolActivity struct { Kind string `json:"kind"` // "read" | "search" | "list" | "run" Target string `json:"target"` @@ -28,10 +28,6 @@ type ToolTrace struct { Truncated bool `json:"truncated,omitempty"` } -func isSafeText(s string, limit int) bool { - return len(s) <= limit -} - // ParseToolTrace validates and extracts a ToolTrace from untyped data. func ParseToolTrace(raw any) *ToolTrace { if raw == nil { @@ -58,24 +54,25 @@ func ParseToolTrace(raw any) *ToolTrace { default: return nil } - if !isSafeText(act.Target, 1000) { + if len(act.Target) > 1000 { return nil } } for _, ch := range trace.Changes { - if !isSafeText(ch.Path, 1000) { + if len(ch.Path) > 1000 { return nil } - if ch.Kind == "unavailable" { - if !isSafeText(ch.Reason, 1000) { + switch ch.Kind { + case "unavailable": + if len(ch.Reason) > 1000 { return nil } - } else if ch.Kind == "diff" { - if !isSafeText(ch.Diff, 16000) || ch.Added < 0 || ch.Removed < 0 { + case "diff": + if len(ch.Diff) > 16000 || ch.Added < 0 || ch.Removed < 0 { return nil } - } else { + default: return nil } } diff --git a/cli/internal/daemon/http.go b/cli/internal/daemon/http.go index 23a84785..8dbd97e4 100644 --- a/cli/internal/daemon/http.go +++ b/cli/internal/daemon/http.go @@ -89,8 +89,7 @@ func isConnectionError(err error) bool { errors.Is(err, net.ErrClosed) { return true } - var opErr *net.OpError - if errors.As(err, &opErr) { + if _, ok := errors.AsType[*net.OpError](err); ok { return true } msg := err.Error() @@ -148,7 +147,7 @@ func readBounded(r io.Reader, limit int64) ([]byte, error) { return nil, err } if int64(len(data)) > limit { - return nil, fmt.Errorf("Albedo returned more data than this client can read (limit: %d bytes).", limit) + return nil, fmt.Errorf("response exceeds this client's read limit (%d bytes)", limit) } return data, nil } @@ -164,7 +163,7 @@ func Request[T any](ctx context.Context, conn *Connection, path string, body any func RequestMethod[T any](ctx context.Context, conn *Connection, method, path string, body any) (T, error) { var zero T if conn == nil { - return zero, errors.New("Not connected to Albedo.") + return zero, errors.New("not connected to Albedo") } if ctx == nil { ctx = context.Background() diff --git a/cli/internal/daemon/image.go b/cli/internal/daemon/image.go index 25349e72..09fa27ba 100644 --- a/cli/internal/daemon/image.go +++ b/cli/internal/daemon/image.go @@ -3,7 +3,6 @@ package daemon import ( "encoding/json" "fmt" - "math" "strings" ) @@ -75,17 +74,12 @@ func ImageLabel(meta ImageMetadata) string { if meta.Bytes >= 1024*1024 { size = fmt.Sprintf("%.1f MB", float64(meta.Bytes)/1024.0/1024.0) } else if meta.Bytes >= 1024 { - size = fmt.Sprintf("%d KB", int(math.Ceil(float64(meta.Bytes)/1024.0))) + size = fmt.Sprintf("%d KB", (meta.Bytes+1023)/1024) } else { size = fmt.Sprintf("%d B", meta.Bytes) } - prefix := "" - if strings.HasPrefix(string(meta.MimeType), "image/") { - prefix = strings.ToUpper(string(meta.MimeType)[6:]) - } else { - prefix = strings.ToUpper(string(meta.MimeType)) - } + prefix := strings.ToUpper(strings.TrimPrefix(string(meta.MimeType), "image/")) return fmt.Sprintf("%s %d×%d · %s", prefix, meta.Width, meta.Height, size) } diff --git a/cli/internal/daemon/lifecycle.go b/cli/internal/daemon/lifecycle.go index 3cf61aa9..5b2e8e18 100644 --- a/cli/internal/daemon/lifecycle.go +++ b/cli/internal/daemon/lifecycle.go @@ -1,3 +1,4 @@ +// Package daemon discovers and starts the local daemon and provides its typed HTTP API client. package daemon import ( @@ -114,21 +115,21 @@ func (c *Connection) Update(other *Connection) { func (c *Connection) Refresh(ctx context.Context) error { if c == nil { - return errors.New("Not connected to Albedo.") + return errors.New("not connected to Albedo") } c.refreshMu.Lock() defer c.refreshMu.Unlock() homeDir := c.HomeDir() if homeDir == "" { - return errors.New("Cannot reconnect without a daemon home directory.") + return errors.New("cannot reconnect without a daemon home directory") } const attempts = 20 const interval = 100 * time.Millisecond var lastErr error - for i := 0; i < attempts; i++ { + for i := range attempts { if ctx != nil && ctx.Err() != nil { return ctx.Err() } @@ -153,7 +154,7 @@ func (c *Connection) Refresh(ctx context.Context) error { if lastErr != nil { return lastErr } - return fmt.Errorf("Could not reconnect to Albedo. Check whether it is running with ALBEDO_HOME=%s.", homeDir) + return fmt.Errorf("could not reconnect to Albedo; check whether it is running with ALBEDO_HOME=%s", homeDir) } // HTTPClient returns the reusable HTTP client owned by this Connection. @@ -170,8 +171,8 @@ func (c *Connection) MarshalJSON() ([]byte, error) { func (c *Connection) UnmarshalJSON(data []byte) error { var snap ConnectionSnapshot - if err := json.Unmarshal(data, &snap); err != nil { - return err + if decodeErr := json.Unmarshal(data, &snap); decodeErr != nil { + return decodeErr } c.snapshot.Store(&snap) return nil @@ -208,10 +209,7 @@ func AssistantAge(timestamp *int64, now time.Time) string { if timestamp == nil { return "time unknown" } - sec := now.Unix() - *timestamp - if sec < 0 { - sec = 0 - } + sec := max(0, now.Unix()-*timestamp) if sec < 60 { return "just now" } @@ -258,7 +256,7 @@ func Existing(homeDir string) (*Connection, error) { } var snap ConnectionSnapshot - if err := json.Unmarshal(data, &snap); err != nil { + if decodeErr := json.Unmarshal(data, &snap); decodeErr != nil { return nil, nil } @@ -312,7 +310,7 @@ func Existing(homeDir string) (*Connection, error) { func checkCompatible(conn *Connection) (*Connection, error) { if conn.Version() != 2 { - return nil, errors.New("This client cannot connect to the copy of Albedo already running. After its work has finished, run albedo daemon --stop, then launch Albedo again.") + return nil, errors.New("this client cannot connect to the copy of Albedo already running; after its work has finished, run albedo daemon --stop, then launch Albedo again") } return conn, nil } @@ -323,14 +321,14 @@ func resolveDaemonExecutable(path string) (string, error) { } resolved, err := exec.LookPath(path) if err != nil { - return "", fmt.Errorf("Cannot run the executable specified by ALBEDO_DAEMON (%q): %w", path, err) + return "", fmt.Errorf("cannot run the executable specified by ALBEDO_DAEMON (%q): %w", path, err) } fi, err := os.Stat(resolved) if err != nil { - return "", fmt.Errorf("Cannot run the executable specified by ALBEDO_DAEMON (%q): %w", path, err) + return "", fmt.Errorf("cannot run the executable specified by ALBEDO_DAEMON (%q): %w", path, err) } if fi.IsDir() { - return "", fmt.Errorf("ALBEDO_DAEMON points to a directory (%q). Set it to the executable file instead.", path) + return "", fmt.Errorf("ALBEDO_DAEMON points to a directory (%q); set it to the executable file instead", path) } return resolved, nil } @@ -360,19 +358,21 @@ func buildDaemonEnv(homeDir, tokenHex, build string) []string { "ALBEDO_PROTOCOL": true, "ALBEDO_BUILD": true, } - erlFlags := daemonErlFlags + var erlFlags strings.Builder + erlFlags.WriteString(daemonErlFlags) for _, kv := range os.Environ() { k, v, _ := strings.Cut(kv, "=") if k == "ERL_FLAGS" { // The operator's flags come last so they override the defaults. - erlFlags += " " + v + erlFlags.WriteByte(' ') + erlFlags.WriteString(v) continue } if !filteredVars[k] { env = append(env, kv) } } - env = append(env, "ERL_FLAGS="+erlFlags, "ALBEDO_HOME="+homeDir, "ALBEDO_TOKEN="+tokenHex) + env = append(env, "ERL_FLAGS="+erlFlags.String(), "ALBEDO_HOME="+homeDir, "ALBEDO_TOKEN="+tokenHex) if build != "" { env = append(env, "ALBEDO_BUILD="+build) } @@ -416,7 +416,7 @@ func daemonCommand(daemonExe, projectRoot string, env []string) (*exec.Cmd, erro // next daemon can take the home. func stop(homeDir string, conn *Connection) error { _, _ = Request[any](context.Background(), conn, "/shutdown", map[string]any{}) - for attempt := 0; attempt < pollAttempts; attempt++ { + for range pollAttempts { running, err := Existing(homeDir) if err != nil { return err @@ -426,7 +426,7 @@ func stop(homeDir string, conn *Connection) error { } time.Sleep(pollInterval) } - return fmt.Errorf("Albedo did not stop. Check %s for details. The running process ID is %d.", filepath.Join(homeDir, "daemon.log"), conn.Pid()) + return fmt.Errorf("daemon did not stop; check %s for details; the running process ID is %d", filepath.Join(homeDir, "daemon.log"), conn.Pid()) } // Ensure connects to the running daemon or starts one. When this client bundles @@ -444,19 +444,19 @@ func Ensure(homeDir, projectRoot string, replace Replace) (*Connection, error) { if build == "" || current.Build() == build || replace == nil || !replace(Stale{current, build}) { return checkCompatible(current) } - if err := stop(homeDir, current); err != nil { - return nil, err + if stopErr := stop(homeDir, current); stopErr != nil { + return nil, stopErr } } if daemonExe != "" { - if _, err := resolveDaemonExecutable(daemonExe); err != nil { - return nil, err + if _, executableErr := resolveDaemonExecutable(daemonExe); executableErr != nil { + return nil, executableErr } } - if err := os.MkdirAll(homeDir, 0700); err != nil { - return nil, err + if mkdirErr := os.MkdirAll(homeDir, 0700); mkdirErr != nil { + return nil, mkdirErr } _ = os.Chmod(homeDir, 0700) @@ -468,17 +468,17 @@ func Ensure(homeDir, projectRoot string, replace Replace) (*Connection, error) { } if err != nil { if os.IsExist(err) { - for attempt := 0; attempt < pollAttempts; attempt++ { - running, err := Existing(homeDir) - if err != nil { - return nil, err + for range pollAttempts { + running, discoveryErr := Existing(homeDir) + if discoveryErr != nil { + return nil, discoveryErr } if running != nil { return checkCompatible(running) } time.Sleep(pollInterval) } - return nil, fmt.Errorf("Albedo is taking too long to start. Check %s/daemon.log for details. Only remove %s if the process starting Albedo is no longer running.", homeDir, lockPath) + return nil, fmt.Errorf("timed out starting Albedo; check %s/daemon.log for details; only remove %s if the process starting Albedo is no longer running", homeDir, lockPath) } return nil, err } @@ -527,7 +527,7 @@ func Ensure(homeDir, projectRoot string, replace Replace) (*Connection, error) { exited := make(chan error, 1) go func() { exited <- cmd.Wait() }() - for attempt := 0; attempt < pollAttempts; attempt++ { + for range pollAttempts { running, err := Existing(homeDir) if err != nil { return nil, err @@ -546,7 +546,7 @@ func Ensure(homeDir, projectRoot string, replace Replace) (*Connection, error) { } } - return nil, fmt.Errorf("Albedo is taking too long to start. Check %s/daemon.log for details.", homeDir) + return nil, fmt.Errorf("timed out starting Albedo; check %s/daemon.log for details", homeDir) } // staleLock reports whether the startup lock was left by a starter that is no @@ -567,7 +567,7 @@ func staleLock(lockPath string) bool { } func startupExitError(waitErr error, logPath string, logStart int64, projectRoot string) error { - msg := "Albedo stopped before it finished starting" + msg := "daemon stopped before it finished starting" if f, err := os.Open(logPath); err == nil { defer f.Close() @@ -579,7 +579,7 @@ func startupExitError(waitErr error, logPath string, logStart int64, projectRoot } } if projectRoot != "" { - msg += fmt.Sprintf("\nSource checkout: %s. If this is not your Albedo checkout, set ALBEDO_ROOT to its full path.", projectRoot) + msg += fmt.Sprintf("\nsource checkout: %s; if this is not your Albedo checkout, set ALBEDO_ROOT to its full path", projectRoot) } if waitErr != nil { return fmt.Errorf("%s: %w", msg, waitErr) diff --git a/cli/internal/daemon/lifecycle_test.go b/cli/internal/daemon/lifecycle_test.go index ab5a6416..4bc0f594 100644 --- a/cli/internal/daemon/lifecycle_test.go +++ b/cli/internal/daemon/lifecycle_test.go @@ -65,10 +65,10 @@ func TestEnsureAsksOnlyAboutAnotherBundledBuild(t *testing.T) { running string // "" = no build recorded, "same" = the bundled build asked bool }{ - {"source client", false, "/nix/store/old", false}, - {"same build", true, "same", false}, - {"other build", true, "/nix/store/old", true}, - {"unrecorded build", true, "", true}, + {name: "source client", bundled: false, running: "/nix/store/old", asked: false}, + {name: "same build", bundled: true, running: "same", asked: false}, + {name: "other build", bundled: true, running: "/nix/store/old", asked: true}, + {name: "unrecorded build", bundled: true, running: "", asked: true}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { diff --git a/cli/internal/daemon/progress.go b/cli/internal/daemon/progress.go index 79e72235..e643aae0 100644 --- a/cli/internal/daemon/progress.go +++ b/cli/internal/daemon/progress.go @@ -1,7 +1,6 @@ package daemon import ( - "math" "regexp" "strconv" "strings" @@ -34,8 +33,7 @@ func ParsePythonIntent(code string) *ToolIntent { bindings := make(map[string]string) var intents []ToolIntent - lines := strings.Split(code, "\n") - for _, rawLine := range lines { + for rawLine := range strings.SplitSeq(code, "\n") { line := strings.TrimSpace(rawLine) if line == "" || strings.HasPrefix(line, "#") { continue @@ -52,14 +50,14 @@ func ParsePythonIntent(code string) *ToolIntent { } if m := pathMethodRegex.FindStringSubmatch(line); m != nil { - pLit := m[1] - pVar := m[2] + pathLiteral := m[1] + pathVariable := m[2] method := m[3] args := m[4] - target := pLit - if target == "" && pVar != "" { - target = bindings[pVar] + target := pathLiteral + if target == "" && pathVariable != "" { + target = bindings[pathVariable] } if target != "" { @@ -92,20 +90,20 @@ func ParsePythonIntent(code string) *ToolIntent { } if m := funcCallRegex.FindStringSubmatch(line); m != nil { - fnName := m[1] + functionName := m[1] args := m[2] if am := argExtractRegex.FindStringSubmatch(args); am != nil { - litVal := am[1] - varVal := am[2] - target := litVal - if target == "" && varVal != "" { - target = bindings[varVal] + literalValue := am[1] + variableName := am[2] + target := literalValue + if target == "" && variableName != "" { + target = bindings[variableName] } if target != "" { label := cleanLabel(target) - switch fnName { + switch functionName { case "edit": intents = append(intents, ToolIntent{Kind: "edit", Target: label}) case "read": @@ -147,15 +145,15 @@ func runWords(args string) []string { } func ExtractPartialJSONCode(s string) string { - idx := strings.Index(s, `"code"`) - if idx == -1 { + codeKeyIndex := strings.Index(s, `"code"`) + if codeKeyIndex == -1 { return "" } - colon := strings.Index(s[idx+6:], ":") + colon := strings.Index(s[codeKeyIndex+6:], ":") if colon == -1 { return "" } - colon += idx + 6 + colon += codeKeyIndex + 6 quote := strings.Index(s[colon+1:], `"`) if quote == -1 { @@ -163,56 +161,56 @@ func ExtractPartialJSONCode(s string) string { } start := colon + 1 + quote + 1 - var sb strings.Builder - bytes := []byte(s[start:]) + var decodedCode strings.Builder + code := s[start:] i := 0 - for i < len(bytes) { - b := bytes[i] + for i < len(code) { + b := code[i] if b == '"' { break } if b == '\\' { - if i+1 >= len(bytes) { + if i+1 >= len(code) { break } - nxt := bytes[i+1] - switch nxt { + escapedByte := code[i+1] + switch escapedByte { case 'n': - sb.WriteByte('\n') + decodedCode.WriteByte('\n') i += 2 case 'r': - sb.WriteByte('\r') + decodedCode.WriteByte('\r') i += 2 case 't': - sb.WriteByte('\t') + decodedCode.WriteByte('\t') i += 2 case '"': - sb.WriteByte('"') + decodedCode.WriteByte('"') i += 2 case '\\': - sb.WriteByte('\\') + decodedCode.WriteByte('\\') i += 2 case 'u': - if i+5 < len(bytes) { - hexVal := string(bytes[i+2 : i+6]) - if cp, err := strconv.ParseInt(hexVal, 16, 32); err == nil { - sb.WriteRune(rune(cp)) + if i+5 < len(code) { + hexDigits := code[i+2 : i+6] + if codePoint, err := strconv.ParseInt(hexDigits, 16, 32); err == nil { + decodedCode.WriteRune(rune(codePoint)) i += 6 continue } } - sb.WriteByte(nxt) + decodedCode.WriteByte(escapedByte) i += 2 default: - sb.WriteByte(nxt) + decodedCode.WriteByte(escapedByte) i += 2 } } else { - sb.WriteByte(b) + decodedCode.WriteByte(b) i++ } } - return sb.String() + return decodedCode.String() } type ToolCallAssembly struct { @@ -290,7 +288,7 @@ func (r *ToolProgressReporter) Report(call *ToolCallAssembly, phase string) erro var preview *ToolCodePreview if phase == "generating" && len(r.code) > 0 { codeRunes := []rune(r.code) - offset := int(math.Max(0, float64(len(codeRunes)-512))) + offset := max(0, len(codeRunes)-512) text := sanitizeControlRunes(string(codeRunes[offset:])) preview = &ToolCodePreview{ Offset: offset, diff --git a/cli/internal/daemon/settings.go b/cli/internal/daemon/settings.go index 07753473..7c7dc39c 100644 --- a/cli/internal/daemon/settings.go +++ b/cli/internal/daemon/settings.go @@ -9,17 +9,17 @@ import ( // Settings is a redacted snapshot of the daemon's shared persisted preferences. type Settings struct { + Credentials Credentials `json:"credentials"` Profiles config.Profiles `json:"profiles"` - MCP map[string]config.MCPServer `json:"mcp"` Capabilities config.CapabilityPrefs `json:"capabilities"` + MCP map[string]config.MCPServer `json:"mcp"` UI UIPreferences `json:"ui"` - Credentials Credentials `json:"credentials"` } type UIPreferences struct { + Opens map[string]int `json:"opens,omitempty"` Pinned []string `json:"pinned,omitempty"` Archived []string `json:"archived,omitempty"` - Opens map[string]int `json:"opens,omitempty"` Thinking bool `json:"thinking"` Tools bool `json:"tools"` } @@ -48,11 +48,11 @@ func DeleteProvider(ctx context.Context, conn *Connection, name string) error { func SetCapability(ctx context.Context, conn *Connection, session, kind, name, scope string, enabled *bool) error { _, err := RequestMethod[acknowledged](ctx, conn, http.MethodPost, "/sessions/"+url.PathEscape(session)+"/settings/capabilities", struct { + Enabled *bool `json:"enabled"` Kind string `json:"kind"` Name string `json:"name"` Scope string `json:"scope"` - Enabled *bool `json:"enabled"` - }{kind, name, scope, enabled}) + }{Kind: kind, Name: name, Scope: scope, Enabled: enabled}) return err }