From eec7df9a66e664d3fbd1b2407f2074ed8053b744 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sun, 12 Jul 2026 08:53:56 +0000 Subject: [PATCH 01/18] perf(discovery): decouple PR enrichment --- internal/adapter/agent.go | 7 ++ internal/adapter/registry.go | 57 +++++++++- internal/adapter/registry_test.go | 30 ++++- internal/app/app.go | 28 ++++- internal/app/pr_refresh_test.go | 145 ++++++++++++++--------- internal/app/service.go | 183 ++++++++++++++++++++++-------- internal/cli/cli.go | 2 +- 7 files changed, 343 insertions(+), 109 deletions(-) diff --git a/internal/adapter/agent.go b/internal/adapter/agent.go index 4611f8b..5dc5a74 100644 --- a/internal/adapter/agent.go +++ b/internal/adapter/agent.go @@ -278,6 +278,13 @@ func (a *Agent) List(ctx context.Context) ([]Session, error) { if err != nil { return nil, fmt.Errorf("list %s sessions: %w", a.Name(), err) } + return a.ListFromSnapshot(ctx, infos) +} + +// ListFromSnapshot filters a shared backend snapshot for this provider. The +// registry uses it to scan the runtime directory once per refresh while custom +// adapters that only implement List retain their existing behavior. +func (a *Agent) ListFromSnapshot(ctx context.Context, infos []session.Info) ([]Session, error) { var out []Session prefix := "uam-" + a.Name() + "-" seen := make(map[string]struct{}, len(infos)) diff --git a/internal/adapter/registry.go b/internal/adapter/registry.go index dbf2751..e6c26bd 100644 --- a/internal/adapter/registry.go +++ b/internal/adapter/registry.go @@ -1,14 +1,31 @@ package adapter -import "sort" +import ( + "errors" + "fmt" + "sort" + + "github.com/RandomCodeSpace/unified-agent-manager/internal/session" +) type Registry struct { adapters map[string]AgentAdapter reasons map[string]string + backend Backend } func NewRegistry(adapters []AgentAdapter) *Registry { - r := &Registry{adapters: map[string]AgentAdapter{}, reasons: map[string]string{}} + return newRegistry(nil, adapters) +} + +// NewRegistryWithBackend records the backend shared by production adapters so +// ListAll can enumerate it once and fan the immutable snapshot out by provider. +func NewRegistryWithBackend(backend Backend, adapters []AgentAdapter) *Registry { + return newRegistry(backend, adapters) +} + +func newRegistry(backend Backend, adapters []AgentAdapter) *Registry { + r := &Registry{adapters: map[string]AgentAdapter{}, reasons: map[string]string{}, backend: backend} for _, a := range adapters { ok, reason := a.Available() if ok { @@ -20,6 +37,42 @@ func NewRegistry(adapters []AgentAdapter) *Registry { return r } +type snapshotListingAdapter interface { + ListFromSnapshot(ctx Context, infos []session.Info) ([]Session, error) +} + +// ListAll returns every enabled provider's sessions. Production adapters share +// one backend snapshot; custom adapters without snapshot support fall back to +// their ordinary List implementation. Partial results survive adapter errors. +func (r *Registry) ListAll(ctx Context) ([]Session, error) { + enabled := r.Enabled() + var infos []session.Info + if r.backend != nil { + var err error + infos, err = r.backend.List(ctx) + if err != nil { + return nil, fmt.Errorf("list shared session backend: %w", err) + } + } + var out []Session + var joined error + for _, a := range enabled { + var sessions []Session + var err error + if lister, ok := a.(snapshotListingAdapter); ok && r.backend != nil { + sessions, err = lister.ListFromSnapshot(ctx, infos) + } else { + sessions, err = a.List(ctx) + } + if err != nil { + joined = errors.Join(joined, fmt.Errorf("list %s sessions: %w", a.Name(), err)) + continue + } + out = append(out, sessions...) + } + return out, joined +} + func (r *Registry) Get(name string) (AgentAdapter, bool) { a, ok := r.adapters[name]; return a, ok } func (r *Registry) Enabled() []AgentAdapter { diff --git a/internal/adapter/registry_test.go b/internal/adapter/registry_test.go index 2071b50..c4dd1e7 100644 --- a/internal/adapter/registry_test.go +++ b/internal/adapter/registry_test.go @@ -1,6 +1,13 @@ package adapter -import "testing" +import ( + "context" + "testing" + "time" + + "github.com/RandomCodeSpace/unified-agent-manager/internal/adapter/adaptertest" + "github.com/RandomCodeSpace/unified-agent-manager/internal/session" +) func TestRegistryDefaultAndDisabledReasons(t *testing.T) { r := NewRegistry([]AgentAdapter{fakeAdapter{name: "b", available: true}, fakeAdapter{name: "a", available: true}, fakeAdapter{name: "x", available: false}}) @@ -29,6 +36,27 @@ func TestRegistrySkipsUnavailableAdapters(t *testing.T) { } } +func TestRegistryListAllUsesSingleBackendSnapshot(t *testing.T) { + backend := &adaptertest.Backend{Sessions: []session.Info{ + {Name: "uam-a-aaaa1111", CreatedUnix: time.Now().Unix(), Alive: true}, + {Name: "uam-b-bbbb2222", CreatedUnix: time.Now().Unix(), Alive: true}, + }} + a := NewAgent("a", "A", []CommandCandidate{{Display: "sh", Args: []string{"/bin/sh"}}}, nil, backend) + b := NewAgent("b", "B", []CommandCandidate{{Display: "sh", Args: []string{"/bin/sh"}}}, nil, backend) + r := NewRegistryWithBackend(backend, []AgentAdapter{a, b}) + + sessions, err := r.ListAll(context.Background()) + if err != nil { + t.Fatal(err) + } + if len(sessions) != 2 { + t.Fatalf("sessions = %d, want 2: %+v", len(sessions), sessions) + } + if got := len(backend.CallsOf("list")); got != 1 { + t.Fatalf("backend List calls = %d, want 1", got) + } +} + type fakeAdapter struct { name string available bool diff --git a/internal/app/app.go b/internal/app/app.go index da0f8c3..e78ac26 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -111,6 +111,8 @@ type attachSpecMsg struct { } type attachFinishedMsg struct{ err error } type refreshMsg time.Time +type prRefreshMsg time.Time +type prRefreshedMsg struct{ err error } // peekTickMsg is the peek-focus poll tick. When the peek panel is open it // re-captures the focused session's pane (rate-limited per id) so the panel @@ -143,7 +145,7 @@ func New() Model { // Build the registry from the single shared adapter list so the TUI and the // CLI service can never diverge (the old hand-rolled list here omitted // hermes — F14). - reg := adapter.NewRegistry(agents.Default(client)) + reg := adapter.NewRegistryWithBackend(client, agents.Default(client)) return NewWithDeps(st, reg) } @@ -180,13 +182,17 @@ func NewWizard(st *store.Store, reg *adapter.Registry) Model { } func (m Model) Init() tea.Cmd { - return tea.Batch(m.loadSessionsCmd(), refreshTick(), peekTick()) + return tea.Batch(m.loadSessionsCmd(), refreshTick(), peekTick(), prRefreshTick(100*time.Millisecond)) } func refreshTick() tea.Cmd { return tea.Tick(2*time.Second, func(t time.Time) tea.Msg { return refreshMsg(t) }) } +func prRefreshTick(after time.Duration) tea.Cmd { + return tea.Tick(after, func(t time.Time) tea.Msg { return prRefreshMsg(t) }) +} + func peekTick() tea.Cmd { return tea.Tick(peekTickInterval, func(t time.Time) tea.Msg { return peekTickMsg(t) }) } @@ -212,6 +218,12 @@ func (m Model) loadSessionsCmd() tea.Cmd { } } +func (m Model) refreshPRCmd() tea.Cmd { + return func() tea.Msg { + return prRefreshedMsg{err: m.service.RefreshPRStatuses(context.Background())} + } +} + func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { switch msg := msg.(type) { case tea.WindowSizeMsg: @@ -224,6 +236,18 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return next, tea.Batch(next.loadSessionsCmd(), refreshTick()) } return next, refreshTick() + case prRefreshMsg: + return m, tea.Batch(m.refreshPRCmd(), prRefreshTick(prRefreshAge)) + case prRefreshedMsg: + if msg.err != nil { + log.Warn("refresh pull-request statuses failed", "error", msg.err) + return m, nil + } + if m.loading { + return m, nil + } + m.loading = true + return m, m.loadSessionsCmd() case peekTickMsg: // Re-arm the peek ticker unconditionally; only re-capture the focused // session when the panel is open and the per-id rate limit allows it diff --git a/internal/app/pr_refresh_test.go b/internal/app/pr_refresh_test.go index e84cd79..ecaf4a9 100644 --- a/internal/app/pr_refresh_test.go +++ b/internal/app/pr_refresh_test.go @@ -2,42 +2,32 @@ package app import ( "context" - "os" - "path/filepath" + "errors" + "fmt" "sync" + "sync/atomic" "testing" "time" "github.com/RandomCodeSpace/unified-agent-manager/internal/adapter" + "github.com/RandomCodeSpace/unified-agent-manager/internal/pr" "github.com/RandomCodeSpace/unified-agent-manager/internal/store" ) -// writeFakeGH installs a fake `gh` binary at UAM_GH_BIN/PATH whose script body -// is given, returning nothing. Used to make pr.Check behavior deterministic. -func writeFakeGH(t *testing.T, body string) { - t.Helper() - dir := t.TempDir() - gh := filepath.Join(dir, "gh") - if err := os.WriteFile(gh, []byte(body), 0o755); err != nil { - t.Fatal(err) - } - t.Setenv("UAM_GH_BIN", gh) - t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) -} - -// F02 — when pr.Check times out (hung gh), updatePRRecord must still stamp -// LastChecked so the 60s guard arms and the next tick does NOT re-launch gh. -func TestUpdatePRRecordWritesLastCheckedOnTimeout(t *testing.T) { - // gh sleeps long enough to blow the per-check timeout. - writeFakeGH(t, "#!/bin/sh\nsleep 30\n") - +func TestLoadSessionsNeverRunsPRChecker(t *testing.T) { live := adapter.Session{ID: "aaaa1111", AgentType: "fake", DisplayName: "A", Cwd: "/tmp", SessionName: "uam-fake-aaaa1111", State: adapter.Active, ProcAlive: adapter.Alive, CreatedAt: time.Now(), PR: &adapter.PRRef{URL: "https://github.com/o/r/pull/1", Number: 1, Status: adapter.PROpen}} svc, st, _ := newLoadService(t, []adapter.Session{live}) - - before := time.Now() + var calls atomic.Int32 + svc.checkPR = func(context.Context, string) (pr.Status, error) { + calls.Add(1) + return pr.Open, nil + } if _, _, err := svc.LoadSessions(context.Background()); err != nil { t.Fatalf("LoadSessions: %v", err) } + if calls.Load() != 0 { + t.Fatalf("LoadSessions invoked PR checker %d times", calls.Load()) + } cfg, err := st.Load() if err != nil { t.Fatalf("Load: %v", err) @@ -46,48 +36,91 @@ func TestUpdatePRRecordWritesLastCheckedOnTimeout(t *testing.T) { if !ok || rec.PR == nil { t.Fatalf("PR record not persisted: %+v", cfg.Sessions) } - if !rec.PR.LastChecked.After(before.Add(-time.Second)) { - t.Fatalf("LastChecked not stamped on the timeout path: %v", rec.PR.LastChecked) + if !rec.PR.LastChecked.IsZero() { + t.Fatalf("discovery should not claim a network check: %v", rec.PR.LastChecked) } } -// F02 — concurrent LoadSessions must not stack overlapping pr.Check subprocesses. -// The fake gh uses an atomic mkdir lock to detect overlap: if a second gh runs -// while the first is still in flight, the lock fails and writes a sentinel. -func TestRefreshDoesNotStackConcurrentLoads(t *testing.T) { - dir := t.TempDir() - lockDir := filepath.Join(dir, "lock") - overlap := filepath.Join(dir, "overlap") - gh := filepath.Join(dir, "gh") - // mkdir is atomic: only one process can hold the lock at a time. If a second - // gh runs concurrently it can't create the dir, so it records overlap. - body := "#!/bin/sh\n" + - "if ! mkdir '" + lockDir + "' 2>/dev/null; then touch '" + overlap + "'; fi\n" + - "sleep 0.2\n" + - "rmdir '" + lockDir + "' 2>/dev/null\n" + - "echo '{\"state\":\"OPEN\",\"isDraft\":false,\"mergedAt\":null}'\n" - if err := os.WriteFile(gh, []byte(body), 0o755); err != nil { +func TestRefreshPRStatusesUsesBoundedConcurrencyAndPersistsResults(t *testing.T) { + sessions := make([]adapter.Session, 10) + for i := range sessions { + id := fmt.Sprintf("%08x", i+1) + sessions[i] = adapter.Session{ID: id, AgentType: "fake", DisplayName: id, Cwd: "/tmp", SessionName: "uam-fake-" + id, State: adapter.Active, ProcAlive: adapter.Alive, CreatedAt: time.Now(), PR: &adapter.PRRef{URL: fmt.Sprintf("https://github.com/o/r/pull/%d", i+1), Number: i + 1, Status: adapter.PROpen}} + } + svc, st, _ := newLoadService(t, sessions) + var active atomic.Int32 + var maximum atomic.Int32 + svc.checkPR = func(ctx context.Context, _ string) (pr.Status, error) { + n := active.Add(1) + defer active.Add(-1) + for { + old := maximum.Load() + if n <= old || maximum.CompareAndSwap(old, n) { + break + } + } + select { + case <-ctx.Done(): + return pr.None, ctx.Err() + case <-time.After(20 * time.Millisecond): + return pr.Merged, nil + } + } + if err := svc.RefreshPRStatuses(context.Background()); err != nil { + t.Fatal(err) + } + if got := maximum.Load(); got < 2 || got > prRefreshWorkers { + t.Fatalf("max concurrency = %d, want 2..%d", got, prRefreshWorkers) + } + cfg, err := st.Load() + if err != nil { t.Fatal(err) } - t.Setenv("UAM_GH_BIN", gh) - t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + for key, rec := range cfg.Sessions { + if rec.PR == nil || rec.PR.LastStatus != string(adapter.PRMerged) || rec.PR.LastChecked.IsZero() { + t.Fatalf("record %s not refreshed: %+v", key, rec) + } + } +} +func TestRefreshPRStatusesDoesNotOverlapPasses(t *testing.T) { live := adapter.Session{ID: "bbbb2222", AgentType: "fake", DisplayName: "B", Cwd: "/tmp", SessionName: "uam-fake-bbbb2222", State: adapter.Active, ProcAlive: adapter.Alive, CreatedAt: time.Now(), PR: &adapter.PRRef{URL: "https://github.com/o/r/pull/2", Number: 2, Status: adapter.PROpen}} svc, _, _ := newLoadService(t, []adapter.Session{live}) - - var wg sync.WaitGroup - for i := 0; i < 8; i++ { - wg.Add(1) - go func() { - defer wg.Done() - if _, _, err := svc.LoadSessions(context.Background()); err != nil { - t.Errorf("LoadSessions: %v", err) - } - }() + entered := make(chan struct{}) + release := make(chan struct{}) + var once sync.Once + svc.checkPR = func(context.Context, string) (pr.Status, error) { + once.Do(func() { close(entered) }) + <-release + return pr.Open, nil } - wg.Wait() + done := make(chan error, 1) + go func() { done <- svc.RefreshPRStatuses(context.Background()) }() + <-entered + if err := svc.RefreshPRStatuses(context.Background()); err != nil { + t.Fatalf("overlapping pass should skip cleanly: %v", err) + } + close(release) + if err := <-done; err != nil { + t.Fatal(err) + } +} - if _, err := os.Stat(overlap); err == nil { - t.Fatal("overlapping pr.Check subprocesses ran concurrently — in-flight guard missing") +func TestRefreshPRStatusesPreservesLastKnownStatusOnCheckerError(t *testing.T) { + live := adapter.Session{ID: "cccc3333", AgentType: "fake", DisplayName: "C", Cwd: "/tmp", SessionName: "uam-fake-cccc3333", State: adapter.Active, ProcAlive: adapter.Alive, CreatedAt: time.Now(), PR: &adapter.PRRef{URL: "https://github.com/o/r/pull/3", Number: 3, Status: adapter.PROpen}} + svc, st, _ := newLoadService(t, []adapter.Session{live}) + svc.checkPR = func(context.Context, string) (pr.Status, error) { + return pr.None, errors.New("checker unavailable") + } + if err := svc.RefreshPRStatuses(context.Background()); err != nil { + t.Fatal(err) + } + cfg, err := st.Load() + if err != nil { + t.Fatal(err) + } + rec := cfg.Sessions[store.Key("fake", live.ID)] + if rec.PR == nil || rec.PR.LastStatus != string(adapter.PROpen) || rec.PR.LastChecked.IsZero() { + t.Fatalf("last known PR state was not preserved: %+v", rec.PR) } } diff --git a/internal/app/service.go b/internal/app/service.go index 11736fe..2d130cc 100644 --- a/internal/app/service.go +++ b/internal/app/service.go @@ -8,6 +8,7 @@ import ( "os" "sort" "strings" + "sync" "sync/atomic" "time" @@ -26,6 +27,7 @@ type Service struct { // that fails to acquire it skips the network check and keeps the persisted // PR record; the holder clears it on completion, success or error (F02). prInFlight atomic.Bool + checkPR func(context.Context, string) (pr.Status, error) } const agentUnavailableFormat = "agent %q unavailable" @@ -43,13 +45,15 @@ const lastSeenRefresh = time.Minute // long (F20 Stage 2). const pruneMaxAge = 30 * 24 * time.Hour -// prCheckTimeout bounds a single `gh pr view` call so a hung gh cannot wedge the -// 2s refresh. It is comfortably under the 60s PR re-check interval, so a -// timed-out check just defers to the next tick (F02). -const prCheckTimeout = 4 * time.Second +const ( + prCheckTimeout = 4 * time.Second + prRefreshTimeout = 5 * time.Second + prRefreshAge = 60 * time.Second + prRefreshWorkers = 4 +) func NewService(st *store.Store, reg *adapter.Registry) *Service { - return &Service{Store: st, Registry: reg} + return &Service{Store: st, Registry: reg, checkPR: pr.Check} } func (s *Service) LoadSessions(ctx context.Context) ([]adapter.Session, store.Config, error) { @@ -59,10 +63,8 @@ func (s *Service) LoadSessions(ctx context.Context) ([]adapter.Session, store.Co } live := s.liveSessions(ctx) s.mergeStoredSessions(live, cfg, time.Now()) - // refreshSessionRecords runs pr.Check OUTSIDE any store lock and returns the - // records this refresh owns. persistRefresh re-reads inside the flock and - // writes only those keys, so a concurrent TogglePin/Rename on a different - // session is never clobbered by a stale whole-config Save (F01). + // Discovery and reconciliation are local-only. Network PR enrichment runs on + // the independent RefreshPRStatuses cadence and never delays this path. updates := s.refreshSessionRecords(ctx, live, &cfg) if err := s.persistRefresh(updates); err != nil { log.Warn("persist refreshed session metadata failed", "records", len(updates), "error", err) @@ -156,17 +158,14 @@ func (s *Service) liveSessions(ctx context.Context) map[string]adapter.Session { if s.Registry == nil { return live } - for _, a := range s.Registry.Enabled() { - sessions, err := a.List(ctx) - if err != nil { - // One adapter's List failure must not blank the dashboard for the - // others, but it shouldn't vanish silently either (F12). - log.Warn("listing sessions for adapter failed", "agent", a.Name(), "error", err) - continue - } - for _, sess := range sessions { - live[store.Key(sess.AgentType, sess.ID)] = sess - } + sessions, err := s.Registry.ListAll(ctx) + if err != nil { + // Partial custom-adapter results are retained; production's shared backend + // failure yields an empty snapshot and one actionable warning. + log.Warn("listing managed sessions failed", "error", err) + } + for _, sess := range sessions { + live[store.Key(sess.AgentType, sess.ID)] = sess } return live } @@ -224,27 +223,18 @@ func deadSessionFromRecord(rec store.SessionRecord, now time.Time) adapter.Sessi // refreshSessionRecords reconciles live sessions against the loaded config and // returns the records this refresh wants to persist (keyed by store key). It // also updates the in-memory cfg and live maps so the returned snapshot is -// consistent. pr.Check runs here, OUTSIDE any store lock; persistence happens -// later via persistRefresh's atomic re-read. -func (s *Service) refreshSessionRecords(ctx context.Context, live map[string]adapter.Session, cfg *store.Config) map[string]refreshPatch { +// consistent. It only records locally discovered PR metadata; status checks run +// independently in RefreshPRStatuses. +func (s *Service) refreshSessionRecords(_ context.Context, live map[string]adapter.Session, cfg *store.Config) map[string]refreshPatch { updates := map[string]refreshPatch{} now := time.Now() - // Acquire the PR-check guard for the duration of this refresh's network pass. - // If another refresh already holds it, skip the gh calls (the persisted PR - // record is kept) so stacked ticks never launch overlapping gh subprocesses. - // Released unconditionally on completion so a single pass never wedges the - // guard (F02). - doPR := s.prInFlight.CompareAndSwap(false, true) - if doPR { - defer s.prInFlight.Store(false) - } for key, sess := range live { before, existed := cfg.Sessions[key] rec, changed, keep := reconcileRefreshRecord(cfg, key, sess, now) if !keep { continue } - if doPR && updatePRRecord(ctx, key, &sess, &rec, live, now) { + if syncDiscoveredPRRecord(sess, &rec) { cfg.Sessions[key] = rec changed = true } @@ -319,24 +309,123 @@ func reconcileRefreshRecord(cfg *store.Config, key string, sess adapter.Session, return rec, changed, true } -func updatePRRecord(ctx context.Context, key string, sess *adapter.Session, rec *store.SessionRecord, live map[string]adapter.Session, now time.Time) bool { - if sess.PR == nil || (rec.PR != nil && rec.PR.URL == sess.PR.URL && time.Since(rec.PR.LastChecked) <= 60*time.Second) { +func syncDiscoveredPRRecord(sess adapter.Session, rec *store.SessionRecord) bool { + if sess.PR == nil || (rec.PR != nil && rec.PR.URL == sess.PR.URL) { return false } - status := string(sess.PR.Status) - // Bound the gh call so a hung process cannot wedge the refresh. On timeout - // (or any error) the status is left at the last-known value and LastChecked is - // stamped below, arming the 60s guard so the next tick does not immediately - // re-launch gh (F02). - checkCtx, cancel := context.WithTimeout(ctx, prCheckTimeout) + rec.PR = &store.PRRecord{URL: sess.PR.URL, Number: sess.PR.Number, LastStatus: string(sess.PR.Status)} + return true +} + +type prRefreshJob struct { + key string + ref adapter.PRRef +} + +type prRefreshResult struct { + key string + record store.PRRecord +} + +// RefreshPRStatuses enriches stale discovered PRs outside the session-discovery +// path. Only PR-owned fields are persisted, and overlapping passes coalesce. +func (s *Service) RefreshPRStatuses(ctx context.Context) error { + if s.Store == nil || !s.prInFlight.CompareAndSwap(false, true) { + return nil + } + defer s.prInFlight.Store(false) + ctx, cancel := context.WithTimeout(ctx, prRefreshTimeout) defer cancel() - if checked, err := pr.Check(checkCtx, sess.PR.URL); err == nil && checked != pr.None { - status = string(checked) - sess.PR.Status = adapter.PRStatus(status) - live[key] = *sess + sessions, cfg, err := s.LoadSessions(ctx) + if err != nil { + return err + } + now := time.Now() + jobs := make([]prRefreshJob, 0, len(sessions)) + for _, sess := range sessions { + if sess.PR == nil { + continue + } + rec := cfg.Sessions[store.Key(sess.AgentType, sess.ID)] + if rec.PR != nil && rec.PR.URL == sess.PR.URL && now.Sub(rec.PR.LastChecked) < prRefreshAge { + continue + } + jobs = append(jobs, prRefreshJob{key: store.Key(sess.AgentType, sess.ID), ref: *sess.PR}) + } + if len(jobs) == 0 { + return nil + } + checker := s.checkPR + if checker == nil { + checker = pr.Check + } + jobCh := make(chan prRefreshJob) + resultCh := make(chan prRefreshResult, len(jobs)) + workers := min(prRefreshWorkers, len(jobs)) + var wg sync.WaitGroup + for range workers { + wg.Add(1) + go func() { + defer wg.Done() + for job := range jobCh { + status := job.ref.Status + checkCtx, checkCancel := context.WithTimeout(ctx, prCheckTimeout) + checked, checkErr := checker(checkCtx, job.ref.URL) + checkCancel() + if checkErr == nil && checked != pr.None { + status = adapterStatus(checked) + } + resultCh <- prRefreshResult{key: job.key, record: store.PRRecord{URL: job.ref.URL, Number: job.ref.Number, LastStatus: string(status), LastChecked: time.Now()}} + } + }() + } + go func() { + defer close(jobCh) + for _, job := range jobs { + select { + case jobCh <- job: + case <-ctx.Done(): + return + } + } + }() + wg.Wait() + close(resultCh) + results := make([]prRefreshResult, 0, len(jobs)) + for result := range resultCh { + results = append(results, result) + } + updateErr := s.Store.Update(func(cfg *store.Config) error { + for _, result := range results { + rec, ok := cfg.Sessions[result.key] + if !ok || rec.PR == nil || rec.PR.URL != result.record.URL { + continue + } + record := result.record + rec.PR = &record + cfg.Sessions[result.key] = rec + } + return nil + }) + if updateErr != nil { + return updateErr + } + return ctx.Err() +} + +func adapterStatus(status pr.Status) adapter.PRStatus { + switch status { + case pr.Open: + return adapter.PROpen + case pr.Merged: + return adapter.PRMerged + case pr.Closed: + return adapter.PRClosed + case pr.Draft: + return adapter.PRDraft + default: + return adapter.PRNone } - rec.PR = &store.PRRecord{URL: sess.PR.URL, Number: sess.PR.Number, LastStatus: status, LastChecked: now} - return true } func sessionsFromMap(live map[string]adapter.Session) []adapter.Session { diff --git a/internal/cli/cli.go b/internal/cli/cli.go index a4b48fb..07012a2 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -242,7 +242,7 @@ func NewService(st *store.Store) *app.Service { }) // Build the registry from the single shared adapter list so the CLI service // and the TUI can never diverge on which providers exist (F14). - reg := adapter.NewRegistry(agents.Default(client)) + reg := adapter.NewRegistryWithBackend(client, agents.Default(client)) return app.NewService(st, reg) } From 995ccee21070ff240dd86de8809912cc0eb5e193 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sun, 12 Jul 2026 09:26:25 +0000 Subject: [PATCH 02/18] test(discovery): characterize PR refresh paths --- internal/app/app_more_test.go | 47 +++++++++++++++++++++++++++++++++ internal/app/pr_refresh_test.go | 44 ++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/internal/app/app_more_test.go b/internal/app/app_more_test.go index e0b0314..ac630f7 100644 --- a/internal/app/app_more_test.go +++ b/internal/app/app_more_test.go @@ -141,6 +141,53 @@ func TestModelUpdateMessages(t *testing.T) { } } +func TestPRRefreshCommandsAndMessages(t *testing.T) { + m := NewWithDeps(nil, nil) + msg, ok := m.refreshPRCmd()().(prRefreshedMsg) + if !ok || msg.err != nil { + t.Fatalf("refreshPRCmd message = %#v", msg) + } + + model, cmd := m.Update(prRefreshedMsg{}) + m = model.(Model) + if !m.loading || cmd == nil { + t.Fatalf("successful PR refresh did not schedule cached reload: loading=%v cmd=%v", m.loading, cmd) + } + model, cmd = m.Update(prRefreshedMsg{}) + if cmd != nil || !model.(Model).loading { + t.Fatal("PR refresh overlapped an in-flight cached reload") + } + + m.loading = false + model, cmd = m.Update(prRefreshedMsg{err: errors.New("refresh failed")}) + if cmd != nil || model.(Model).loading { + t.Fatal("failed PR refresh should keep cached sessions without scheduling a reload") + } +} + +func TestRenderPeekAndLongestCommonPrefix(t *testing.T) { + m := NewWithDeps(nil, nil) + m.peekText = "one\ntwo\nthree\nfour\nfive\nsix" + m.height = 9 + peek := m.renderPeek() + if !strings.Contains(peek, "PEEK") || strings.Contains(peek, "one") || !strings.Contains(peek, "six") { + t.Fatalf("renderPeek = %q", peek) + } + for _, tc := range []struct { + items []string + want string + }{ + {items: nil, want: ""}, + {items: []string{"alpha"}, want: "alpha"}, + {items: []string{"alpha", "alpine", "alps"}, want: "alp"}, + {items: []string{"one", "two"}, want: ""}, + } { + if got := longestCommonPrefix(tc.items); got != tc.want { + t.Fatalf("longestCommonPrefix(%q) = %q, want %q", tc.items, got, tc.want) + } + } +} + func TestDispatchedMessageAttachesNewSession(t *testing.T) { fake := &svcFakeAdapter{name: "fake", available: true} m := NewWithDeps(nil, adapter.NewRegistry([]adapter.AgentAdapter{fake})) diff --git a/internal/app/pr_refresh_test.go b/internal/app/pr_refresh_test.go index ecaf4a9..5cb8408 100644 --- a/internal/app/pr_refresh_test.go +++ b/internal/app/pr_refresh_test.go @@ -124,3 +124,47 @@ func TestRefreshPRStatusesPreservesLastKnownStatusOnCheckerError(t *testing.T) { t.Fatalf("last known PR state was not preserved: %+v", rec.PR) } } + +func TestRefreshPRStatusesSkipsFreshRecords(t *testing.T) { + live := adapter.Session{ID: "dddd4444", AgentType: "fake", DisplayName: "D", Cwd: "/tmp", SessionName: "uam-fake-dddd4444", State: adapter.Active, ProcAlive: adapter.Alive, CreatedAt: time.Now(), PR: &adapter.PRRef{URL: "https://github.com/o/r/pull/4", Number: 4, Status: adapter.PROpen}} + svc, st, _ := newLoadService(t, []adapter.Session{live}) + if _, _, err := svc.LoadSessions(context.Background()); err != nil { + t.Fatal(err) + } + if err := st.Update(func(cfg *store.Config) error { + rec := cfg.Sessions[store.Key("fake", live.ID)] + rec.PR.LastChecked = time.Now() + cfg.Sessions[store.Key("fake", live.ID)] = rec + return nil + }); err != nil { + t.Fatal(err) + } + var calls atomic.Int32 + svc.checkPR = func(context.Context, string) (pr.Status, error) { + calls.Add(1) + return pr.Closed, nil + } + if err := svc.RefreshPRStatuses(context.Background()); err != nil { + t.Fatal(err) + } + if calls.Load() != 0 { + t.Fatalf("fresh PR record triggered %d checks", calls.Load()) + } +} + +func TestAdapterStatusMapping(t *testing.T) { + for _, tc := range []struct { + in pr.Status + want adapter.PRStatus + }{ + {pr.Open, adapter.PROpen}, + {pr.Merged, adapter.PRMerged}, + {pr.Closed, adapter.PRClosed}, + {pr.Draft, adapter.PRDraft}, + {pr.None, adapter.PRNone}, + } { + if got := adapterStatus(tc.in); got != tc.want { + t.Fatalf("adapterStatus(%q) = %q, want %q", tc.in, got, tc.want) + } + } +} From 1beb57017797d0d284acc740671ca5cb61b0ad5c Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sun, 12 Jul 2026 08:48:20 +0000 Subject: [PATCH 03/18] fix: harden CLI startup and logging --- .claude/worktrees/plan-review-edits | 1 - .gitignore | 1 + internal/cli/cli.go | 63 ++++++++++++- internal/cli/cli_test.go | 82 ++++++++++++++++ internal/log/log.go | 139 +++++++++++++++++++++++++++- internal/log/log_test.go | 112 ++++++++++++++++++++++ 6 files changed, 390 insertions(+), 8 deletions(-) delete mode 160000 .claude/worktrees/plan-review-edits diff --git a/.claude/worktrees/plan-review-edits b/.claude/worktrees/plan-review-edits deleted file mode 160000 index 98a3861..0000000 --- a/.claude/worktrees/plan-review-edits +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 98a3861bdde9e53022ba1b1808b4d20ec084ab96 diff --git a/.gitignore b/.gitignore index 5882b61..8d40a06 100644 --- a/.gitignore +++ b/.gitignore @@ -6,5 +6,6 @@ coverage.* .DS_Store .idea/ .vscode/ +/.claude/worktrees/ /CLAUDE.md /PLAN.md diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 07012a2..1df5d85 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -26,16 +26,29 @@ import ( func Main() { flag.Usage = Usage flag.Parse() + args := flag.Args() + + // Help and version are deliberately independent from both the cache logger + // and the persistent store. They must remain usable when either location is + // unavailable (for example during installation diagnostics). + if handled, err := runBeforeLogger(args); handled { + if err != nil && !errors.Is(err, context.Canceled) { + fmt.Fprintf(os.Stderr, "uam: %v\n", err) + os.Exit(1) + } + return + } closer, err := log.Init() if err != nil { + log.UseStderr(os.Stderr) fmt.Fprintf(os.Stderr, "uam: failed to initialize logger: %v\n", err) - os.Exit(1) + } else { + defer func() { _ = closer.Close() }() } - defer func() { _ = closer.Close() }() ctx := context.Background() - if err := Run(ctx, flag.Args()); err != nil && !errors.Is(err, context.Canceled) { + if err := Run(ctx, args); err != nil && !errors.Is(err, context.Canceled) { log.Error("run exited with error", "err", err) fmt.Fprintf(os.Stderr, "uam: %v\n", err) os.Exit(1) @@ -69,6 +82,9 @@ func Run(ctx context.Context, args []string) error { // RunWithTUI executes the CLI with an injectable TUI runner for tests. func RunWithTUI(ctx context.Context, args []string, runTUI func(context.Context, tea.Model) error) error { + if handled, err := runWithoutStore(ctx, args); handled { + return err + } st, err := store.Open(store.DefaultPath()) if err != nil { return err @@ -86,6 +102,47 @@ func RunWithTUI(ctx context.Context, args []string, runTUI func(context.Context, return runCommand(ctx, svc, args, runTUI) } +// runBeforeLogger handles commands that have no cache, store, or session +// dependency. Main invokes it before log.Init; RunWithTUI reaches the same +// behavior through runWithoutStore when called directly by tests/embedders. +func runBeforeLogger(args []string) (bool, error) { + if len(args) == 0 { + return false, nil + } + switch args[0] { + case "-h", "--help", "help": + Usage() + return true, nil + case "version", "--version": + fmt.Println(version.String()) + return true, nil + default: + return false, nil + } +} + +// runWithoutStore handles commands whose behavior does not depend on +// sessions.json. Keeping these ahead of store.Open makes recovery commands +// available even when the config directory is damaged or unwritable. +func runWithoutStore(ctx context.Context, args []string) (bool, error) { + if handled, err := runBeforeLogger(args); handled { + return true, err + } + if len(args) == 0 { + return false, nil + } + switch args[0] { + case "kill-all": + return true, runKillAll(ctx, session.NewClient().KillAll) + case "__host": + return true, session.RunHost(args[1:]) + case "__attach": + return true, session.RunAttach(args[1:]) + default: + return false, nil + } +} + func runCommand(ctx context.Context, svc *app.Service, args []string, runTUI func(context.Context, tea.Model) error) error { switch args[0] { case "-h", "--help", "help": diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index c12addb..bee254c 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -4,8 +4,10 @@ import ( "bytes" "context" "errors" + "flag" "io" "os" + "os/exec" "path/filepath" "strings" "testing" @@ -147,6 +149,86 @@ func TestRunWithTUIHelpVersionAndDefault(t *testing.T) { } } +func TestRunWithTUIStateFreeCommandsDoNotOpenStore(t *testing.T) { + blocked := filepath.Join(t.TempDir(), "not-a-directory") + if err := os.WriteFile(blocked, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("UAM_CONFIG_DIR", blocked) + t.Setenv("UAM_SESSION_DIR", t.TempDir()) + + for _, args := range [][]string{{"help"}, {"version"}, {"kill-all"}} { + if err := RunWithTUI(context.Background(), args, noopRunTUI); err != nil { + t.Fatalf("RunWithTUI(%q) opened the store: %v", args, err) + } + } + if err := RunWithTUI(context.Background(), []string{"__host", "--name", "bad name", "--", "/bin/true"}, noopRunTUI); err == nil || !strings.Contains(err.Error(), "invalid session name") { + t.Fatalf("__host must be routed before the store and return its own validation error, got %v", err) + } + if err := RunWithTUI(context.Background(), []string{"__attach"}, noopRunTUI); err == nil || !strings.Contains(err.Error(), "attach requires a session name") { + t.Fatalf("__attach must be routed before the store and return its own validation error, got %v", err) + } +} + +func TestMainStatelessCommandsSkipLoggerAndStore(t *testing.T) { + blocked := filepath.Join(t.TempDir(), "not-a-directory") + if err := os.WriteFile(blocked, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + for _, command := range []string{"help", "version"} { + cmd := cliMainSubprocess(t, command, blocked, blocked) + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("uam %s: %v\n%s", command, err, output) + } + if strings.Contains(string(output), "failed to initialize logger") { + t.Fatalf("uam %s initialized the logger: %s", command, output) + } + } +} + +func TestMainFallsBackToStderrWhenFileLoggerFails(t *testing.T) { + blocked := filepath.Join(t.TempDir(), "not-a-directory") + if err := os.WriteFile(blocked, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + cmd := cliMainSubprocess(t, "kill-all", blocked, filepath.Join(t.TempDir(), "config")) + cmd.Env = append(cmd.Env, "UAM_SESSION_DIR="+t.TempDir()) + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("uam kill-all must continue with stderr logging: %v\n%s", err, output) + } + text := string(output) + if strings.Count(text, "failed to initialize logger") != 1 { + t.Fatalf("logger fallback warning count = %d, want 1: %s", strings.Count(text, "failed to initialize logger"), text) + } + if !strings.Contains(text, "all uam sessions stopped") { + t.Fatalf("kill-all did not run after logger fallback: %s", text) + } +} + +func TestCLIMainHelperProcess(t *testing.T) { + command := os.Getenv("UAM_TEST_MAIN_COMMAND") + if command == "" { + return + } + flag.CommandLine = flag.NewFlagSet("uam", flag.ContinueOnError) + os.Args = []string{"uam", command} + Main() + os.Exit(0) +} + +func cliMainSubprocess(t *testing.T, command, cacheDir, configDir string) *exec.Cmd { + t.Helper() + cmd := exec.Command(os.Args[0], "-test.run=^TestCLIMainHelperProcess$") + cmd.Env = append(os.Environ(), + "UAM_TEST_MAIN_COMMAND="+command, + "UAM_CACHE_DIR="+cacheDir, + "UAM_CONFIG_DIR="+configDir, + ) + return cmd +} + func TestRunCommandAttachLastAndNew(t *testing.T) { svc, fake := newCLITestService(t) id := dispatchAndCaptureID(t, svc, []string{"fake", "attachable"}) diff --git a/internal/log/log.go b/internal/log/log.go index fce104e..633ef5a 100644 --- a/internal/log/log.go +++ b/internal/log/log.go @@ -6,26 +6,157 @@ import ( "log/slog" "os" "path/filepath" + "sync" +) + +const ( + maxLogSize = int64(5 << 20) + maxBackups = 3 ) var current *slog.Logger = slog.New(slog.NewTextHandler(io.Discard, nil)) +type rotatingFile struct { + mu sync.Mutex + path string + file *os.File + size int64 +} + func Init() (io.Closer, error) { dir := cacheDir() if err := os.MkdirAll(dir, 0o700); err != nil { return nil, err } - f, err := os.OpenFile(filepath.Join(dir, "uam.log"), os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o600) // #nosec G304 -- UAM intentionally writes its own cache log path. + path := filepath.Join(dir, "uam.log") + if err := rotateIfNeeded(path); err != nil { + return nil, err + } + f, err := openLogFile(path) + if err != nil { + return nil, err + } + if err := enforcePrivateLogModes(path); err != nil { + _ = f.Close() + return nil, err + } + info, err := f.Stat() if err != nil { + _ = f.Close() return nil, err } + w := &rotatingFile{path: path, file: f, size: info.Size()} + current = newLogger(w) + current.Info("logger initialized", "path", f.Name()) + return w, nil +} + +func openLogFile(path string) (*os.File, error) { + return os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o600) // #nosec G304 -- UAM intentionally writes its own cache log path. +} + +func (w *rotatingFile) Write(p []byte) (int, error) { + w.mu.Lock() + defer w.mu.Unlock() + if w.file == nil { + return 0, os.ErrClosed + } + if w.size > 0 && w.size+int64(len(p)) > maxLogSize { + if err := w.file.Close(); err != nil { + return 0, err + } + w.file = nil + if err := rotateLogs(w.path); err != nil { + return 0, err + } + f, err := openLogFile(w.path) + if err != nil { + return 0, err + } + w.file = f + w.size = 0 + if err := enforcePrivateLogModes(w.path); err != nil { + _ = w.file.Close() + w.file = nil + return 0, err + } + } + n, err := w.file.Write(p) + w.size += int64(n) + return n, err +} + +func (w *rotatingFile) Close() error { + w.mu.Lock() + defer w.mu.Unlock() + if w.file == nil { + return nil + } + err := w.file.Close() + w.file = nil + return err +} + +// UseStderr installs the non-file fallback used when Init cannot create or +// open the cache log. The caller remains responsible for printing the single +// initialization warning that explains why the fallback was selected. +func UseStderr(w io.Writer) { + if w == nil { + w = os.Stderr + } + current = newLogger(w) +} + +func newLogger(w io.Writer) *slog.Logger { level := slog.LevelInfo if os.Getenv("UAM_DEBUG") != "" { level = slog.LevelDebug } - current = slog.New(slog.NewTextHandler(f, &slog.HandlerOptions{Level: level})) - current.Info("logger initialized", "path", f.Name()) - return f, nil + return slog.New(slog.NewTextHandler(w, &slog.HandlerOptions{Level: level})) +} + +func rotateIfNeeded(path string) error { + info, err := os.Stat(path) + if os.IsNotExist(err) { + return enforcePrivateLogModes(path) + } + if err != nil { + return err + } + if info.Size() < maxLogSize { + return enforcePrivateLogModes(path) + } + return rotateLogs(path) +} + +func rotateLogs(path string) error { + if err := os.Remove(path + ".3"); err != nil && !os.IsNotExist(err) { + return err + } + for i := maxBackups - 1; i >= 1; i-- { + oldPath := path + "." + string(rune('0'+i)) + newPath := path + "." + string(rune('1'+i)) + if err := os.Rename(oldPath, newPath); err != nil && !os.IsNotExist(err) { + return err + } + } + if err := os.Rename(path, path+".1"); err != nil { + return err + } + return enforcePrivateLogModes(path) +} + +func enforcePrivateLogModes(path string) error { + for i := 0; i <= maxBackups; i++ { + candidate := path + if i > 0 { + candidate += "." + string(rune('0'+i)) + } + if err := os.Chmod(candidate, 0o600); err != nil && !os.IsNotExist(err) { + return err + } + } + return nil } func L() *slog.Logger { return current } diff --git a/internal/log/log_test.go b/internal/log/log_test.go index 7daaaaa..9af8b5f 100644 --- a/internal/log/log_test.go +++ b/internal/log/log_test.go @@ -3,6 +3,7 @@ package log import ( "os" "path/filepath" + "strings" "testing" ) @@ -39,6 +40,117 @@ func TestInitError(t *testing.T) { } } +func TestInitEnforcesPrivateLogPermissions(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "uam.log") + if err := os.WriteFile(path, []byte("old log\n"), 0o644); err != nil { + t.Fatal(err) + } + t.Setenv("UAM_CACHE_DIR", dir) + c, err := Init() + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := c.Close(); err != nil { + t.Errorf("close logger: %v", err) + } + }) + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if got := info.Mode().Perm(); got != 0o600 { + t.Fatalf("uam.log mode = %o, want 600", got) + } +} + +func TestInitRotatesOversizedLogAndRetainsThreePrivateBackups(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "uam.log") + if err := os.WriteFile(path, nil, 0o644); err != nil { + t.Fatal(err) + } + if err := os.Truncate(path, maxLogSize); err != nil { + t.Fatal(err) + } + for i, body := range []string{"backup-one", "backup-two", "backup-three"} { + if err := os.WriteFile(path+"."+string(rune('1'+i)), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + t.Setenv("UAM_CACHE_DIR", dir) + c, err := Init() + if err != nil { + t.Fatal(err) + } + if err := c.Close(); err != nil { + t.Fatal(err) + } + + wantBodies := map[string]string{ + path + ".2": "backup-one", + path + ".3": "backup-two", + } + for _, candidate := range []string{path, path + ".1", path + ".2", path + ".3"} { + info, err := os.Stat(candidate) + if err != nil { + t.Fatalf("stat %s: %v", candidate, err) + } + if got := info.Mode().Perm(); got != 0o600 { + t.Fatalf("%s mode = %o, want 600", candidate, got) + } + if want, ok := wantBodies[candidate]; ok { + got, err := os.ReadFile(candidate) + if err != nil { + t.Fatal(err) + } + if string(got) != want { + t.Fatalf("%s = %q, want %q", candidate, got, want) + } + } + } + rotated, err := os.Stat(path + ".1") + if err != nil { + t.Fatal(err) + } + if rotated.Size() != maxLogSize { + t.Fatalf("rotated size = %d, want %d", rotated.Size(), maxLogSize) + } +} + +func TestLoggerRotatesWhenActiveLogCrossesSizeLimit(t *testing.T) { + dir := t.TempDir() + t.Setenv("UAM_CACHE_DIR", dir) + c, err := Init() + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := c.Close(); err != nil { + t.Errorf("close logger: %v", err) + } + }) + + Info("fill", "data", strings.Repeat("x", int(maxLogSize-1024))) + Info("after rotation", "marker", "new-log", "padding", strings.Repeat("y", 2048)) + + rotated, err := os.ReadFile(filepath.Join(dir, "uam.log.1")) + if err != nil { + t.Fatalf("read rotated log: %v", err) + } + if !strings.Contains(string(rotated), "fill") { + t.Fatal("rotated log does not contain the pre-limit entry") + } + current, err := os.ReadFile(filepath.Join(dir, "uam.log")) + if err != nil { + t.Fatalf("read current log: %v", err) + } + if !strings.Contains(string(current), "new-log") { + t.Fatal("current log does not contain the post-rotation entry") + } +} + func TestCacheDirFallbacks(t *testing.T) { dir := t.TempDir() t.Setenv("UAM_CACHE_DIR", "") From 83c9a31df7fdbcb10eea320e7feb59fdecfff1bc Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sun, 12 Jul 2026 09:00:14 +0000 Subject: [PATCH 04/18] fix(session): verify portable process identity --- internal/app/app.go | 24 -------------- internal/app/app_cmd_test.go | 5 +-- internal/app/app_more_test.go | 5 +-- internal/app/loadsessions_test.go | 4 ++- internal/app/render_cluster_test.go | 10 +++--- internal/log/log.go | 7 ---- internal/session/client.go | 15 ++++++--- internal/session/liveness_test.go | 35 ++++++++++++++++++++ internal/session/proc_start_darwin.go | 22 +++++++++++++ internal/session/proc_start_linux.go | 38 ++++++++++++++++++++++ internal/session/proc_start_other.go | 7 ++++ internal/session/session.go | 47 +++++++++------------------ internal/store/store.go | 8 +++-- 13 files changed, 142 insertions(+), 85 deletions(-) create mode 100644 internal/session/proc_start_darwin.go create mode 100644 internal/session/proc_start_linux.go create mode 100644 internal/session/proc_start_other.go diff --git a/internal/app/app.go b/internal/app/app.go index e78ac26..8bdea79 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -175,12 +175,6 @@ func (m Model) validateDefaultAgent(candidate string) string { } return candidate } -func NewWizard(st *store.Store, reg *adapter.Registry) Model { - m := NewWithDeps(st, reg) - m.wizard = true - return m -} - func (m Model) Init() tea.Cmd { return tea.Batch(m.loadSessionsCmd(), refreshTick(), peekTick(), prRefreshTick(100*time.Millisecond)) } @@ -1063,9 +1057,6 @@ func (m Model) resumeSelectedCmd() tea.Cmd { return m.loadSessionsCmd()() } } -func (m Model) stopSelectedCmd(remove bool) tea.Cmd { - return m.stopTargetCmd("", remove) -} // persistDefaultAgent persists the default-agent choice. On failure it surfaces // the error in the status line instead of swallowing it; on success it returns a @@ -1594,21 +1585,6 @@ func sessionGlyph(s adapter.Session) (string, lipgloss.Style) { } } -func stateGlyph(s adapter.State) (string, lipgloss.Style) { - switch s { - case adapter.Active: - return "⟳", liveGlyphStyle - case adapter.Failed: - return "✕", failGlyphStyle - default: - return "•", hintStyle - } -} - -func stateLabel(s adapter.State) string { - return strings.ToLower(string(s)) -} - // prStatusDot returns a distinct glyph per PR status (not color-only) so the PR // state survives a monochrome terminal or a screen scrape: open=hollow circle, // merged=filled circle, draft=half circle, closed=cross (F26). diff --git a/internal/app/app_cmd_test.go b/internal/app/app_cmd_test.go index 6a8f080..2aa67fb 100644 --- a/internal/app/app_cmd_test.go +++ b/internal/app/app_cmd_test.go @@ -80,9 +80,6 @@ func TestModelCommandFactories(t *testing.T) { if New().defaultAgent == "" { t.Fatal("New default empty") } - if !NewWizard(st, m.service.Registry).wizard { - t.Fatal("NewWizard not wizard") - } if m.Init() == nil { t.Fatal("Init returned nil") } @@ -98,7 +95,7 @@ func TestModelCommandFactories(t *testing.T) { if msg := m.pinSelectedCmd()(); msg.(sessionsLoadedMsg).err != nil { t.Fatalf("pin msg=%+v", msg) } - if msg := m.stopSelectedCmd(false)(); msg.(sessionsLoadedMsg).err != nil { + if msg := m.stopTargetCmd("", false)(); msg.(sessionsLoadedMsg).err != nil { t.Fatalf("stop msg=%+v", msg) } if msg := m.persistOrderCmd()(); msg.(sessionsLoadedMsg).err != nil { diff --git a/internal/app/app_more_test.go b/internal/app/app_more_test.go index ac630f7..f71dd74 100644 --- a/internal/app/app_more_test.go +++ b/internal/app/app_more_test.go @@ -114,7 +114,7 @@ func assertDispatchParsing(t *testing.T, m Model) { func assertViewHelpers(t *testing.T) { t.Helper() - if stateLabel(adapter.Active) != "active" || prStatusDot(adapter.PRMerged) == " " { + if prStatusDot(adapter.PRMerged) == " " { t.Fatal("status helpers bad") } if truncate("abcdef", 4) != "abc…" || trimLines("a\nb\nc", 2) != "b\nc" { @@ -431,9 +431,6 @@ func TestInputWindowAndStateBranches(t *testing.T) { if start < 0 || end < start || end > len(m.sessions) { t.Fatalf("bad window %d:%d", start, end) } - if stateLabel(adapter.Active) != "active" || stateLabel(adapter.Failed) != "failed" { - t.Fatal("state labels not covered") - } } func TestRenameEditingBranches(t *testing.T) { diff --git a/internal/app/loadsessions_test.go b/internal/app/loadsessions_test.go index 6754bda..bcd35e0 100644 --- a/internal/app/loadsessions_test.go +++ b/internal/app/loadsessions_test.go @@ -63,7 +63,9 @@ func TestLoadSessionsRefreshDoesNotClobberConcurrentPin(t *testing.T) { live := map[string]adapter.Session{store.Key("fake", liveA.ID): liveA} svc.mergeStoredSessions(live, cfgStale, time.Now()) updates := svc.refreshSessionRecords(context.Background(), live, &cfgStale) - svc.persistRefresh(updates) + if err := svc.persistRefresh(updates); err != nil { + t.Fatal(err) + } cfg, err := st.Load() if err != nil { diff --git a/internal/app/render_cluster_test.go b/internal/app/render_cluster_test.go index ef24bd0..75dbcac 100644 --- a/internal/app/render_cluster_test.go +++ b/internal/app/render_cluster_test.go @@ -178,9 +178,8 @@ func TestStateGlyphDistinguishesResumableFromFailed(t *testing.T) { if live == resumable { t.Fatalf("a live session and a resumable dead session must use distinct glyphs (both %q)", live) } - failGlyph, _ := stateGlyph(adapter.Failed) - if resumable == failGlyph { - t.Fatalf("a reboot-survivor resumable session must not use the red Failed glyph %q", failGlyph) + if resumable == "✕" { + t.Fatal("a reboot-survivor resumable session must not use the failure glyph") } _ = closed } @@ -192,9 +191,8 @@ func TestRenderRowResumableSessionNotMarkedFailed(t *testing.T) { if strings.Contains(strings.ToLower(row), "failed") { t.Fatalf("resumable row should not show the literal \"failed\": %q", row) } - failGlyph, _ := stateGlyph(adapter.Failed) - if strings.Contains(row, failGlyph) { - t.Fatalf("resumable row should not show the red Failed glyph %q: %q", failGlyph, row) + if strings.Contains(row, "✕") { + t.Fatalf("resumable row should not show the failure glyph: %q", row) } } diff --git a/internal/log/log.go b/internal/log/log.go index 633ef5a..f9571d6 100644 --- a/internal/log/log.go +++ b/internal/log/log.go @@ -1,7 +1,6 @@ package log import ( - "fmt" "io" "log/slog" "os" @@ -188,9 +187,3 @@ func cacheDir() string { } return filepath.Join(".uam", "cache") } - -// Fatal prints to stderr and exits with code 1. -func Fatal(format string, args ...any) { - fmt.Fprintf(os.Stderr, "uam: "+format+"\n", args...) - os.Exit(1) -} diff --git a/internal/session/client.go b/internal/session/client.go index d96b4c9..8b381cb 100644 --- a/internal/session/client.go +++ b/internal/session/client.go @@ -254,11 +254,18 @@ func (c *Client) Kill(ctx context.Context, name string) error { } // State exists but the socket path failed: the host is wedged, crashed, // or mid-shutdown. Escalate directly and wait for both processes to go. - // The start-time-verified probes matter most here: a recycled PID must - // never be signalled as if it were the session. - if st.hostAlive() { + // A live PID is not sufficient authority to signal: fallback control paths + // require a nonzero matching start identity so a recycled PID can never be + // signalled as if it were the session. + if ProcAlive(st.HostPID) { + if !procIdentityMatches(st.HostPID, st.HostStart) { + return fmt.Errorf("kill session %s: cannot verify process identity for host pid %d", name, st.HostPID) + } _ = syscall.Kill(st.HostPID, syscall.SIGTERM) - } else if st.childAlive() { + } else if ProcAlive(st.ChildPID) { + if !procIdentityMatches(st.ChildPID, st.ChildStart) { + return fmt.Errorf("kill session %s: cannot verify process identity for child pid %d", name, st.ChildPID) + } // Orphaned agent (host crashed): signal its process group directly. if err := syscall.Kill(-st.ChildPID, syscall.SIGTERM); err != nil { _ = syscall.Kill(st.ChildPID, syscall.SIGTERM) diff --git a/internal/session/liveness_test.go b/internal/session/liveness_test.go index 353ffd1..b311480 100644 --- a/internal/session/liveness_test.go +++ b/internal/session/liveness_test.go @@ -237,6 +237,41 @@ func TestProcAliveWithStartDetectsPIDReuse(t *testing.T) { } } +func TestProcIdentityMatchesRequiresRecordedAndCurrentIdentity(t *testing.T) { + pid := os.Getpid() + real := procStartTime(pid) + if real == 0 { + t.Skip("process start identity unavailable on this platform") + } + if !procIdentityMatches(pid, real) { + t.Fatal("matching nonzero process identity must be accepted") + } + if procIdentityMatches(pid, 0) { + t.Fatal("zero recorded identity must fail closed for signaling") + } + if procIdentityMatches(pid, real+1) { + t.Fatal("mismatched process identity must fail closed for signaling") + } +} + +func TestKillRefusesPIDFallbackWithoutVerifiedIdentity(t *testing.T) { + c := newTestClient(t) + if err := EnsureDir(c.Dir); err != nil { + t.Fatal(err) + } + name := "uam-fake-1d1d1d1d" + if err := writeState(c.Dir, State{Name: name, HostPID: os.Getpid(), HostStart: 0}); err != nil { + t.Fatal(err) + } + err := c.Kill(t.Context(), name) + if err == nil || !strings.Contains(err.Error(), "cannot verify process identity") { + t.Fatalf("Kill error = %v, want explicit process-identity safety error", err) + } + if !ProcAlive(os.Getpid()) { + t.Fatal("Kill signaled the test process despite missing identity") + } +} + // A stale state file whose PIDs were recycled by other processes (alive, but // with different start times) must be swept by List, not reported live. func TestListSweepsRecycledPIDState(t *testing.T) { diff --git a/internal/session/proc_start_darwin.go b/internal/session/proc_start_darwin.go new file mode 100644 index 0000000..2f2f1cd --- /dev/null +++ b/internal/session/proc_start_darwin.go @@ -0,0 +1,22 @@ +//go:build darwin + +package session + +import "golang.org/x/sys/unix" + +// procStartTime returns the process start timestamp in microseconds since the +// Unix epoch, or 0 when Darwin cannot provide a stable identity. +func procStartTime(pid int) int64 { + if pid <= 0 { + return 0 + } + info, err := unix.SysctlKinfoProc("kern.proc.pid", pid) + if err != nil || info == nil { + return 0 + } + started := info.Proc.P_starttime + if started.Sec <= 0 { + return 0 + } + return started.Sec*1_000_000 + int64(started.Usec) +} diff --git a/internal/session/proc_start_linux.go b/internal/session/proc_start_linux.go new file mode 100644 index 0000000..d23568b --- /dev/null +++ b/internal/session/proc_start_linux.go @@ -0,0 +1,38 @@ +//go:build linux + +package session + +import ( + "fmt" + "os" + "strconv" + "strings" +) + +// procStartTime returns /proc//stat field 22 (clock ticks since boot), +// or 0 when the process identity cannot be read. +func procStartTime(pid int) int64 { + if pid <= 0 { + return 0 + } + data, err := os.ReadFile(fmt.Sprintf("/proc/%d/stat", pid)) + if err != nil { + return 0 + } + // comm (field 2) is parenthesized and may itself contain spaces or ')', + // so split after the LAST ')'. starttime is overall field 22, i.e. index + // 19 of the fields that follow comm. + rest := string(data) + if i := strings.LastIndexByte(rest, ')'); i >= 0 { + rest = rest[i+1:] + } + fields := strings.Fields(rest) + if len(fields) < 20 { + return 0 + } + v, err := strconv.ParseInt(fields[19], 10, 64) + if err != nil { + return 0 + } + return v +} diff --git a/internal/session/proc_start_other.go b/internal/session/proc_start_other.go new file mode 100644 index 0000000..9907f7c --- /dev/null +++ b/internal/session/proc_start_other.go @@ -0,0 +1,7 @@ +//go:build !linux && !darwin + +package session + +// procStartTime reports that stable process identity is unavailable. Display +// liveness remains permissive, while PID-based fallback signaling fails closed. +func procStartTime(int) int64 { return 0 } diff --git a/internal/session/session.go b/internal/session/session.go index c4f08c4..7cd53b8 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -15,7 +15,6 @@ import ( "path/filepath" "regexp" "strconv" - "strings" "syscall" ) @@ -115,9 +114,9 @@ func verifyDirIdentity(dir string) error { type State struct { Name string `json:"name"` HostPID int `json:"host_pid"` - // HostStart / ChildStart are the processes' kernel start times - // (/proc//stat field 22, in clock ticks since boot; 0 where - // unavailable). They disambiguate a recycled PID from the original + // HostStart / ChildStart are platform-specific stable process identities + // derived from kernel start times (0 where unavailable). They disambiguate + // a recycled PID from the original // process, so a stale state file can never make uam treat — or worse, // signal — an unrelated process as a session. HostStart int64 `json:"host_start,omitempty"` @@ -226,9 +225,10 @@ func ProcAlive(pid int) bool { } // procAliveWithStart is ProcAlive hardened against PID reuse: when a start -// time was recorded AND the live process's start time is readable, they must -// match. Where /proc is unavailable (e.g. macOS) either side reads as 0 and -// the check degrades to the plain signal-0 probe. +// identity was recorded AND the live process's identity is readable, they +// must match. This intentionally remains permissive for display compatibility: +// missing identity degrades to the plain signal-0 probe. Signaling paths use +// procIdentityMatches instead. func procAliveWithStart(pid int, start int64) bool { if !ProcAlive(pid) { return false @@ -240,32 +240,15 @@ func procAliveWithStart(pid int, start int64) bool { return current == 0 || current == start } -// procStartTime returns the kernel start time of pid (clock ticks since boot, -// /proc//stat field 22), or 0 when unavailable. -func procStartTime(pid int) int64 { - if pid <= 0 { - return 0 - } - data, err := os.ReadFile(fmt.Sprintf("/proc/%d/stat", pid)) - if err != nil { - return 0 - } - // comm (field 2) is parenthesized and may itself contain spaces or ')', - // so split after the LAST ')'. starttime is overall field 22, i.e. index - // 19 of the fields that follow comm. - rest := string(data) - if i := strings.LastIndexByte(rest, ')'); i >= 0 { - rest = rest[i+1:] - } - fields := strings.Fields(rest) - if len(fields) < 20 { - return 0 - } - v, err := strconv.ParseInt(fields[19], 10, 64) - if err != nil { - return 0 +// procIdentityMatches is the fail-closed process identity check used before +// PID-based fallback signaling. Both recorded and live identities must be +// available and equal; liveness alone is never authorization to signal. +func procIdentityMatches(pid int, recorded int64) bool { + if pid <= 0 || recorded == 0 || !ProcAlive(pid) { + return false } - return v + current := procStartTime(pid) + return current != 0 && current == recorded } // procCwd returns the live working directory of pid via /proc (Linux). On diff --git a/internal/store/store.go b/internal/store/store.go index 2b143d0..732d825 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -147,9 +147,11 @@ func (c *Config) UnmarshalJSON(data []byte) error { } type UISettings struct { - GroupByDir bool `json:"group_by_dir"` - Sort string `json:"sort"` - PeekWidth int `json:"peek_width"` + GroupByDir bool `json:"group_by_dir"` + // Sort and PeekWidth are retained schema-v3 compatibility fields. They are + // normalized and round-tripped even when the TUI exposes no direct control. + Sort string `json:"sort"` + PeekWidth int `json:"peek_width"` } type SessionRecord struct { From 8650e168d0f50e58c8cbc52f2fb0e8f5e83e4f58 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sun, 12 Jul 2026 09:41:18 +0000 Subject: [PATCH 05/18] test(cli): create owner-only runtime fixtures --- internal/cli/cli_test.go | 15 ++++++++++++--- internal/cli/killall_test.go | 2 +- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index bee254c..8f712cf 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -155,7 +155,7 @@ func TestRunWithTUIStateFreeCommandsDoNotOpenStore(t *testing.T) { t.Fatal(err) } t.Setenv("UAM_CONFIG_DIR", blocked) - t.Setenv("UAM_SESSION_DIR", t.TempDir()) + t.Setenv("UAM_SESSION_DIR", secureSessionDir(t)) for _, args := range [][]string{{"help"}, {"version"}, {"kill-all"}} { if err := RunWithTUI(context.Background(), args, noopRunTUI); err != nil { @@ -193,7 +193,7 @@ func TestMainFallsBackToStderrWhenFileLoggerFails(t *testing.T) { t.Fatal(err) } cmd := cliMainSubprocess(t, "kill-all", blocked, filepath.Join(t.TempDir(), "config")) - cmd.Env = append(cmd.Env, "UAM_SESSION_DIR="+t.TempDir()) + cmd.Env = append(cmd.Env, "UAM_SESSION_DIR="+secureSessionDir(t)) output, err := cmd.CombinedOutput() if err != nil { t.Fatalf("uam kill-all must continue with stderr logging: %v\n%s", err, output) @@ -378,6 +378,15 @@ func writeCLIExecutable(t *testing.T, path string) { } } +func secureSessionDir(t *testing.T) string { + t.Helper() + dir := t.TempDir() + if err := os.Chmod(dir, 0o700); err != nil { + t.Fatal(err) + } + return dir +} + func must(t *testing.T, err error) { t.Helper() if err != nil { @@ -397,7 +406,7 @@ func firstNonEmpty(values ...string) string { // The internal __host/__attach subcommands are routed through runCommand; // invalid input must surface as errors rather than silently doing nothing. func TestRunCommandInternalSubcommands(t *testing.T) { - t.Setenv("UAM_SESSION_DIR", t.TempDir()) + t.Setenv("UAM_SESSION_DIR", secureSessionDir(t)) svc, _ := newCLITestService(t) noTUI := func(context.Context, tea.Model) error { return nil } if err := runCommand(context.Background(), svc, []string{"__host", "--name", "bad name", "--", "/bin/true"}, noTUI); err == nil { diff --git a/internal/cli/killall_test.go b/internal/cli/killall_test.go index b5bb76a..0ea7025 100644 --- a/internal/cli/killall_test.go +++ b/internal/cli/killall_test.go @@ -37,7 +37,7 @@ func TestRunKillAllPropagatesError(t *testing.T) { // real sessions: KillAll over zero sessions is an idempotent success. func TestRunCommandKillAllDispatches(t *testing.T) { svc, _ := newCLITestService(t) - t.Setenv("UAM_SESSION_DIR", t.TempDir()) + t.Setenv("UAM_SESSION_DIR", secureSessionDir(t)) out := captureCLIStdout(t, func() { if err := runCommand(context.Background(), svc, []string{"kill-all"}, func(context.Context, tea.Model) error { return nil }); err != nil { From 0f62d90a13771276a75263e033a17cf2b121b131 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sun, 12 Jul 2026 10:02:49 +0000 Subject: [PATCH 06/18] test: bound in-process host readiness --- internal/session/host_inprocess_test.go | 32 +++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/internal/session/host_inprocess_test.go b/internal/session/host_inprocess_test.go index 1a39ecb..3d824d4 100644 --- a/internal/session/host_inprocess_test.go +++ b/internal/session/host_inprocess_test.go @@ -3,6 +3,7 @@ package session import ( "bufio" "context" + "errors" "net" "os" "strings" @@ -20,14 +21,37 @@ func startInProcessHost(t *testing.T, c *Client, name, command string) chan erro if err != nil { t.Fatal(err) } + cwd := t.TempDir() done := make(chan error, 1) go func() { - done <- runHost(c.Dir, name, t.TempDir(), "label0", []string{"UAM_T=1"}, []string{"/bin/sh", "-c", command}, w) + defer func() { _ = w.Close() }() + done <- runHost(c.Dir, name, cwd, "label0", []string{"UAM_T=1"}, []string{"/bin/sh", "-c", command}, w) }() - line, err := bufio.NewReader(r).ReadString('\n') + type readyResult struct { + line string + err error + } + ready := make(chan readyResult, 1) + go func() { + line, err := bufio.NewReader(r).ReadString('\n') + ready <- readyResult{line: line, err: err} + }() + var result readyResult + select { + case result = <-ready: + case <-time.After(10 * time.Second): + _ = r.Close() + t.Fatal("timed out waiting for host readiness") + } _ = r.Close() - if err != nil || strings.TrimSpace(line) != "ok" { - t.Fatalf("host not ready: %q %v", line, err) + if result.err != nil || strings.TrimSpace(result.line) != "ok" { + var hostErr error + select { + case hostErr = <-done: + case <-time.After(time.Second): + hostErr = errors.New("host did not return after closing readiness pipe") + } + t.Fatalf("host not ready: %q (read: %v, host: %v)", result.line, result.err, hostErr) } return done } From a41c5283ae64b3b596c66821b26a3e11a58efd01 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sun, 12 Jul 2026 10:04:37 +0000 Subject: [PATCH 07/18] test: use short Darwin session paths --- internal/session/session_test.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/internal/session/session_test.go b/internal/session/session_test.go index e7251bb..5201d90 100644 --- a/internal/session/session_test.go +++ b/internal/session/session_test.go @@ -30,7 +30,15 @@ func TestMain(m *testing.M) { func newTestClient(t *testing.T) *Client { t.Helper() - dir := filepath.Join(t.TempDir(), "run") + dir, err := os.MkdirTemp("", "uam-test-") + if err != nil { + t.Fatal(err) + } + if err := os.Chmod(dir, 0o700); err != nil { + _ = os.RemoveAll(dir) + t.Fatal(err) + } + t.Cleanup(func() { _ = os.RemoveAll(dir) }) t.Setenv("UAM_CONFIG_DIR", filepath.Join(t.TempDir(), "cfg")) exe, err := os.Executable() if err != nil { From c352dfe5b8f79b60d71b9e8146d7ed627d94e09c Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sun, 12 Jul 2026 09:00:40 +0000 Subject: [PATCH 08/18] build: use Go 1.25.12 toolchain --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index aba81bd..dc63bdd 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module github.com/RandomCodeSpace/unified-agent-manager go 1.25.0 -toolchain go1.25.11 +toolchain go1.25.12 require ( github.com/charmbracelet/bubbletea v1.3.10 From 8c98d102e0bab51aaedc5eab98175550d84f4012 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sun, 12 Jul 2026 09:01:17 +0000 Subject: [PATCH 09/18] chore(deps): upgrade x/term to 0.2.2 --- go.mod | 4 ++-- go.sum | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index dc63bdd..c218439 100644 --- a/go.mod +++ b/go.mod @@ -7,9 +7,10 @@ toolchain go1.25.12 require ( github.com/charmbracelet/bubbletea v1.3.10 github.com/charmbracelet/lipgloss v1.1.0 - github.com/charmbracelet/x/term v0.2.1 + github.com/charmbracelet/x/term v0.2.2 github.com/creack/pty v1.1.24 github.com/mattn/go-runewidth v0.0.16 + golang.org/x/sys v0.36.0 ) require ( @@ -26,6 +27,5 @@ require ( github.com/muesli/termenv v0.16.0 // indirect github.com/rivo/uniseg v0.4.7 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect - golang.org/x/sys v0.36.0 // indirect golang.org/x/text v0.3.8 // indirect ) diff --git a/go.sum b/go.sum index c6551b4..58b52be 100644 --- a/go.sum +++ b/go.sum @@ -10,8 +10,8 @@ github.com/charmbracelet/x/ansi v0.10.1 h1:rL3Koar5XvX0pHGfovN03f5cxLbCF2YvLeyz7 github.com/charmbracelet/x/ansi v0.10.1/go.mod h1:3RQDQ6lDnROptfpWuUVIUG64bD2g2BgntdxH0Ya5TeE= github.com/charmbracelet/x/cellbuf v0.0.13-0.20250311204145-2c3ea96c31dd h1:vy0GVL4jeHEwG5YOXDmi86oYw2yuYUGqz6a8sLwg0X8= github.com/charmbracelet/x/cellbuf v0.0.13-0.20250311204145-2c3ea96c31dd/go.mod h1:xe0nKWGd3eJgtqZRaN9RjMtK7xUYchjzPr7q6kcvCCs= -github.com/charmbracelet/x/term v0.2.1 h1:AQeHeLZ1OqSXhrAWpYUtZyX1T3zVxfpZuEQMIQaGIAQ= -github.com/charmbracelet/x/term v0.2.1/go.mod h1:oQ4enTYFV7QN4m0i9mzHrViD7TQKvNEEkHUMCmsxdUg= +github.com/charmbracelet/x/term v0.2.2 h1:xVRT/S2ZcKdhhOuSP4t5cLi5o+JxklsoEObBSgfgZRk= +github.com/charmbracelet/x/term v0.2.2/go.mod h1:kF8CY5RddLWrsgVwpw4kAa6TESp6EB5y3uxGLeCqzAI= github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s= github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE= github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f h1:Y/CXytFA4m6baUTXGLOoWe4PQhGxaX0KpnayAqC48p4= From e3f2aaa5295113ce07cd8708ef5977bb7f8d6bd7 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sun, 12 Jul 2026 09:01:52 +0000 Subject: [PATCH 10/18] chore(deps): upgrade go-runewidth to 0.0.24 --- go.mod | 3 ++- go.sum | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index c218439..4d29612 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/charmbracelet/lipgloss v1.1.0 github.com/charmbracelet/x/term v0.2.2 github.com/creack/pty v1.1.24 - github.com/mattn/go-runewidth v0.0.16 + github.com/mattn/go-runewidth v0.0.24 golang.org/x/sys v0.36.0 ) @@ -18,6 +18,7 @@ require ( github.com/charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dc // indirect github.com/charmbracelet/x/ansi v0.10.1 // indirect github.com/charmbracelet/x/cellbuf v0.0.13-0.20250311204145-2c3ea96c31dd // indirect + github.com/clipperhouse/uax29/v2 v2.2.0 // indirect github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect github.com/lucasb-eyer/go-colorful v1.2.0 // indirect github.com/mattn/go-isatty v0.0.20 // indirect diff --git a/go.sum b/go.sum index 58b52be..b1e0044 100644 --- a/go.sum +++ b/go.sum @@ -12,6 +12,8 @@ github.com/charmbracelet/x/cellbuf v0.0.13-0.20250311204145-2c3ea96c31dd h1:vy0G github.com/charmbracelet/x/cellbuf v0.0.13-0.20250311204145-2c3ea96c31dd/go.mod h1:xe0nKWGd3eJgtqZRaN9RjMtK7xUYchjzPr7q6kcvCCs= github.com/charmbracelet/x/term v0.2.2 h1:xVRT/S2ZcKdhhOuSP4t5cLi5o+JxklsoEObBSgfgZRk= github.com/charmbracelet/x/term v0.2.2/go.mod h1:kF8CY5RddLWrsgVwpw4kAa6TESp6EB5y3uxGLeCqzAI= +github.com/clipperhouse/uax29/v2 v2.2.0 h1:ChwIKnQN3kcZteTXMgb1wztSgaU+ZemkgWdohwgs8tY= +github.com/clipperhouse/uax29/v2 v2.2.0/go.mod h1:EFJ2TJMRUaplDxHKj1qAEhCtQPW2tJSwu5BF98AuoVM= github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s= github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE= github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f h1:Y/CXytFA4m6baUTXGLOoWe4PQhGxaX0KpnayAqC48p4= @@ -22,15 +24,14 @@ github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWE github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/mattn/go-localereader v0.0.1 h1:ygSAOl7ZXTx4RdPYinUpg6W99U8jWvWi9Ye2JC/oIi4= github.com/mattn/go-localereader v0.0.1/go.mod h1:8fBrzywKY7BI3czFoHkuzRoWE9C+EiG4R1k4Cjx5p88= -github.com/mattn/go-runewidth v0.0.16 h1:E5ScNMtiwvlvB5paMFdw9p4kSQzbXFikJ5SQO6TULQc= -github.com/mattn/go-runewidth v0.0.16/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w= +github.com/mattn/go-runewidth v0.0.24 h1:cpokDiIn0MGnhdHwuWnJBITySJ20QyNGnY2kR/ay2DU= +github.com/mattn/go-runewidth v0.0.24/go.mod h1:XBkDxAl56ILZc9knddidhrOlY5R/pDhgLpndooCuJAs= github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 h1:ZK8zHtRHOkbHy6Mmr5D264iyp3TiX5OmNcI5cIARiQI= github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6/go.mod h1:CJlz5H+gyd6CUWT45Oy4q24RdLyn7Md9Vj2/ldJBSIo= github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELUXHmA= github.com/muesli/cancelreader v0.2.2/go.mod h1:3XuTXfFS2VjM+HTLZY9Ak0l6eUKfijIfMUZ4EgX0QYo= github.com/muesli/termenv v0.16.0 h1:S5AlUN9dENB57rsbnkPyfdGuWIlkmzJjbFf0Tf5FWUc= github.com/muesli/termenv v0.16.0/go.mod h1:ZRfOIKPFDYQoDFF4Olj7/QJbW60Ol/kL1pU3VfY/Cnk= -github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc= github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ= github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavMF/ppJZNG9ZpyihvCd0w101no= From d38383fb4e7d184ac5d2d2ae06b8c0a0f4b2e65e Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sun, 12 Jul 2026 09:08:53 +0000 Subject: [PATCH 11/18] test: add fuzz and release smoke coverage --- .github/workflows/fuzz.yml | 40 ++++++++++++++ .github/workflows/release-smoke.yml | 75 ++++++++++++++++++++++++++ internal/adapter/registry_test.go | 20 +++++++ internal/app/pr_refresh_test.go | 42 +++++++++++++++ internal/app/render_cluster_test.go | 21 ++++++++ internal/session/attach_filter_test.go | 13 +++++ internal/session/liveness_test.go | 29 ++++++++++ internal/session/proto_test.go | 25 +++++++++ internal/vterm/vterm_test.go | 30 +++++++++++ 9 files changed, 295 insertions(+) create mode 100644 .github/workflows/fuzz.yml create mode 100644 .github/workflows/release-smoke.yml diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml new file mode 100644 index 0000000..e1565b1 --- /dev/null +++ b/.github/workflows/fuzz.yml @@ -0,0 +1,40 @@ +name: Scheduled fuzzing + +on: + schedule: + - cron: "41 4 * * 2" + workflow_dispatch: + +permissions: + contents: read + +jobs: + fuzz: + name: fuzz-${{ matrix.target }} + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + include: + - package: ./internal/session + target: FuzzFrameDecoding + - package: ./internal/vterm + target: FuzzTerminalInput + - package: ./internal/displaytext + target: FuzzSanitize + - package: ./internal/session + target: FuzzAttachFiltering + - package: ./internal/session + target: FuzzStateDecoding + steps: + - name: Checkout + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + + - name: Set up Go + uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 + with: + go-version-file: go.mod + cache: true + + - name: Fuzz for 30 seconds + run: go test "${{ matrix.package }}" -run=^$ -fuzz="^${{ matrix.target }}$" -fuzztime=30s diff --git a/.github/workflows/release-smoke.yml b/.github/workflows/release-smoke.yml new file mode 100644 index 0000000..6af1e4a --- /dev/null +++ b/.github/workflows/release-smoke.yml @@ -0,0 +1,75 @@ +name: Release smoke + +on: + push: + branches: [main] + pull_request: + workflow_dispatch: + +permissions: + contents: read + +jobs: + snapshot: + name: GoReleaser snapshot + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + fetch-depth: 0 + + - name: Set up Go + uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 + with: + go-version-file: go.mod + cache: true + + - name: Validate GoReleaser configuration + uses: goreleaser/goreleaser-action@f06c13b6b1a9625abc9e6e439d9c05a8f2190e94 # v7.2.3 + with: + distribution: goreleaser + version: v2.17.0 + args: check + + - name: Build snapshot archives + uses: goreleaser/goreleaser-action@f06c13b6b1a9625abc9e6e439d9c05a8f2190e94 # v7.2.3 + with: + distribution: goreleaser + version: v2.17.0 + args: release --snapshot --clean --skip=sign,sbom + + - name: Verify archive names and contents + shell: bash + run: | + mapfile -t archives < <(find dist -maxdepth 1 -type f -name 'uam_*.tar.gz' -print | sort) + test "${#archives[@]}" -eq 4 + for archive in "${archives[@]}"; do + basename "${archive}" | grep -Eq '^uam_.+_(linux|darwin)_(amd64|arm64)\.tar\.gz$' + tar -tzf "${archive}" | grep -Eq '(^|/)uam$' + done + linux_archive="$(find dist -maxdepth 1 -type f -name 'uam_*_linux_amd64.tar.gz' -print -quit)" + test -n "${linux_archive}" + mkdir -p /tmp/uam-release-smoke + tar -xzf "${linux_archive}" -C /tmp/uam-release-smoke + binary="$(find /tmp/uam-release-smoke -type f -name uam -print -quit)" + test -n "${binary}" + "${binary}" version | grep -E '.+' + + darwin-native: + name: Darwin-native version smoke + runs-on: macos-14 + steps: + - name: Checkout + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + + - name: Set up Go + uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 + with: + go-version-file: go.mod + cache: true + + - name: Build and run version command + run: | + go build -o "${RUNNER_TEMP}/uam" ./cmd/uam + "${RUNNER_TEMP}/uam" version | grep -E '.+' diff --git a/internal/adapter/registry_test.go b/internal/adapter/registry_test.go index c4dd1e7..10fad55 100644 --- a/internal/adapter/registry_test.go +++ b/internal/adapter/registry_test.go @@ -2,6 +2,7 @@ package adapter import ( "context" + "fmt" "testing" "time" @@ -57,6 +58,25 @@ func TestRegistryListAllUsesSingleBackendSnapshot(t *testing.T) { } } +func BenchmarkRegistryListAllSharedSnapshot(b *testing.B) { + infos := make([]session.Info, 300) + for i := range infos { + infos[i] = session.Info{Name: fmt.Sprintf("uam-a-%08x", i+1), CreatedUnix: time.Now().Unix(), Alive: true} + } + backend := &adaptertest.Backend{Sessions: infos} + adapters := make([]AgentAdapter, 0, 6) + for _, name := range []string{"a", "b", "c", "d", "e", "f"} { + adapters = append(adapters, NewAgent(name, name, []CommandCandidate{{Display: "sh", Args: []string{"/bin/sh"}}}, nil, backend)) + } + registry := NewRegistryWithBackend(backend, adapters) + b.ResetTimer() + for b.Loop() { + if _, err := registry.ListAll(context.Background()); err != nil { + b.Fatal(err) + } + } +} + type fakeAdapter struct { name string available bool diff --git a/internal/app/pr_refresh_test.go b/internal/app/pr_refresh_test.go index 5cb8408..2a48e21 100644 --- a/internal/app/pr_refresh_test.go +++ b/internal/app/pr_refresh_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "path/filepath" "sync" "sync/atomic" "testing" @@ -168,3 +169,44 @@ func TestAdapterStatusMapping(t *testing.T) { } } } + +func BenchmarkRefreshPRStatuses(b *testing.B) { + sessions := make([]adapter.Session, 20) + for i := range sessions { + id := fmt.Sprintf("%08x", i+1) + sessions[i] = adapter.Session{ + ID: id, AgentType: "fake", DisplayName: id, Cwd: "/tmp", + SessionName: "uam-fake-" + id, State: adapter.Active, ProcAlive: adapter.Alive, + CreatedAt: time.Now(), PR: &adapter.PRRef{URL: fmt.Sprintf("https://github.com/o/r/pull/%d", i+1), Number: i + 1, Status: adapter.PROpen}, + } + } + st, err := store.Open(filepath.Join(b.TempDir(), "sessions.json")) + if err != nil { + b.Fatal(err) + } + fake := &svcFakeAdapter{name: "fake", available: true, sessions: sessions} + svc := NewService(st, adapter.NewRegistry([]adapter.AgentAdapter{fake})) + svc.checkPR = func(context.Context, string) (pr.Status, error) { return pr.Open, nil } + if _, _, err := svc.LoadSessions(context.Background()); err != nil { + b.Fatal(err) + } + b.ResetTimer() + for b.Loop() { + b.StopTimer() + if err := st.Update(func(cfg *store.Config) error { + for key, rec := range cfg.Sessions { + if rec.PR != nil { + rec.PR.LastChecked = time.Time{} + cfg.Sessions[key] = rec + } + } + return nil + }); err != nil { + b.Fatal(err) + } + b.StartTimer() + if err := svc.RefreshPRStatuses(context.Background()); err != nil { + b.Fatal(err) + } + } +} diff --git a/internal/app/render_cluster_test.go b/internal/app/render_cluster_test.go index 75dbcac..0a96f82 100644 --- a/internal/app/render_cluster_test.go +++ b/internal/app/render_cluster_test.go @@ -75,6 +75,27 @@ func TestTruncateIsWidthAware(t *testing.T) { } } +func TestTruncateUnicodeGolden(t *testing.T) { + for _, tc := range []struct { + name string + in string + cols int + want string + }{ + {name: "emoji", in: "🚀🚀🚀", cols: 5, want: "🚀🚀…"}, + {name: "combining", in: "e\u0301clair", cols: 3, want: "e\u0301c…"}, + {name: "cjk", in: "你好世界", cols: 5, want: "你好…"}, + {name: "narrow", in: "anything", cols: 1, want: "…"}, + {name: "pasted-multibyte-name", in: "部署 café 🚀", cols: 10, want: "部署 café…"}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := truncate(tc.in, tc.cols); got != tc.want { + t.Fatalf("truncate(%q, %d) = %q, want %q", tc.in, tc.cols, got, tc.want) + } + }) + } +} + // F28 — truncate must not chop a multibyte rune mid-sequence (the old byte-slice // path produced mojibake on the boundary). func TestTruncateNeverEmitsMojibake(t *testing.T) { diff --git a/internal/session/attach_filter_test.go b/internal/session/attach_filter_test.go index ecbae72..d1c7a3f 100644 --- a/internal/session/attach_filter_test.go +++ b/internal/session/attach_filter_test.go @@ -21,6 +21,19 @@ func runFilter(t *testing.T, f *stdinFilter, chunks ...string) (string, bool) { return out.String(), false } +func FuzzAttachFiltering(f *testing.F) { + for _, seed := range []string{"plain text", "世界🚀", "\x02d", "\x1b[D", "\x1b]11;rgb:ffff/ffff/ffff\x1b\\"} { + f.Add(seed, true) + } + f.Fuzz(func(t *testing.T, input string, backDetach bool) { + filter := &stdinFilter{backDetach: backDetach} + out, _ := filter.filter([]byte(input)) + if len(out) > 2*len(input)+2 { + t.Fatalf("filter expanded %d input bytes to %d bytes", len(input), len(out)) + } + }) +} + func TestLeftArrowDetachesWhenNothingTyped(t *testing.T) { f := &stdinFilter{backDetach: true} out, detach := runFilter(t, f, "\x1b[D") diff --git a/internal/session/liveness_test.go b/internal/session/liveness_test.go index b311480..48475aa 100644 --- a/internal/session/liveness_test.go +++ b/internal/session/liveness_test.go @@ -2,6 +2,7 @@ package session import ( "context" + "encoding/json" "errors" "os" "os/exec" @@ -12,6 +13,34 @@ import ( "testing" ) +func FuzzStateDecoding(f *testing.F) { + for _, seed := range [][]byte{ + []byte(`{"name":"uam-fake-aabbccdd","host_pid":1,"host_start":2}`), + []byte(`{"name":"../escape","host_pid":1}`), + []byte(`null`), + []byte(`{"unknown":{"nested":true}}`), + } { + f.Add(seed) + } + f.Fuzz(func(t *testing.T, data []byte) { + var state State + if err := json.Unmarshal(data, &state); err != nil { + return + } + encoded, err := json.Marshal(state) + if err != nil { + t.Fatal(err) + } + var decoded State + if err := json.Unmarshal(encoded, &decoded); err != nil { + t.Fatal(err) + } + if decoded.Name != state.Name || decoded.HostPID != state.HostPID || decoded.HostStart != state.HostStart { + t.Fatalf("state round trip changed identity: before=%+v after=%+v", state, decoded) + } + }) +} + func TestVerifyDirRejectsUnsafeRuntimeDirectories(t *testing.T) { parent := t.TempDir() diff --git a/internal/session/proto_test.go b/internal/session/proto_test.go index 87f9ac8..24f9882 100644 --- a/internal/session/proto_test.go +++ b/internal/session/proto_test.go @@ -84,3 +84,28 @@ func TestFrameWriterSerializesConcurrentFrames(t *testing.T) { t.Fatalf("trailing read error = %v, want EOF", err) } } + +func FuzzFrameDecoding(f *testing.F) { + for _, seed := range [][]byte{ + {frameDetach, 0, 0, 0, 0}, + {frameStdin, 0, 0, 0, 3, 'a', 'b', 'c'}, + {frameResize, 0, 0, 0, 4, 0, 80, 0, 24}, + {frameStdin, 0xff, 0xff, 0xff, 0xff}, + } { + f.Add(seed) + } + f.Fuzz(func(t *testing.T, data []byte) { + kind, payload, err := readFrame(bytes.NewReader(data)) + if err != nil { + return + } + var roundTrip bytes.Buffer + if err := writeFrame(&roundTrip, kind, payload); err != nil { + t.Fatal(err) + } + gotKind, gotPayload, err := readFrame(&roundTrip) + if err != nil || gotKind != kind || !bytes.Equal(gotPayload, payload) { + t.Fatalf("frame round trip = (%d, %x, %v), want (%d, %x)", gotKind, gotPayload, err, kind, payload) + } + }) +} diff --git a/internal/vterm/vterm_test.go b/internal/vterm/vterm_test.go index 16257d8..9824783 100644 --- a/internal/vterm/vterm_test.go +++ b/internal/vterm/vterm_test.go @@ -372,6 +372,36 @@ func BenchmarkHostileCSICounts(b *testing.B) { } } +func BenchmarkOrdinaryTerminalStream(b *testing.B) { + stream := []byte("\x1b[32mready\x1b[0m> go test ./...\r\nok package 0.123s\r\n") + term := New(120, 40, 500) + b.SetBytes(int64(len(stream))) + b.ResetTimer() + for b.Loop() { + _, _ = term.Write(stream) + } +} + +func FuzzTerminalInput(f *testing.F) { + for _, seed := range [][]byte{ + []byte("plain\r\ntext"), + []byte("café 世界 🚀"), + []byte("\x1b[999999999999999999999S"), + []byte("\x1b]0;title\x07visible"), + } { + f.Add(seed) + } + f.Fuzz(func(t *testing.T, input []byte) { + term := New(80, 24, 100) + if _, err := term.Write(input); err != nil { + t.Fatal(err) + } + if got := term.Capture(100); len(got) > 80*(24+100)*4 { + t.Fatalf("capture exceeded screen/history bound: %d bytes", len(got)) + } + }) +} + func TestScrollRegionReverseWrapAndKeypadIgnored(t *testing.T) { term := New(20, 4, 0) // Keypad mode escapes and charset designators are consumed silently. From 5b938d1e1237dcc96190143687a1da17437ceef9 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sun, 12 Jul 2026 09:54:59 +0000 Subject: [PATCH 12/18] ci: bound Darwin lifecycle smoke tests --- .github/workflows/release-smoke.yml | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/.github/workflows/release-smoke.yml b/.github/workflows/release-smoke.yml index 6af1e4a..4590d20 100644 --- a/.github/workflows/release-smoke.yml +++ b/.github/workflows/release-smoke.yml @@ -9,6 +9,10 @@ on: permissions: contents: read +concurrency: + group: release-smoke-${{ github.event.pull_request.head.ref || github.ref_name }} + cancel-in-progress: true + jobs: snapshot: name: GoReleaser snapshot @@ -57,8 +61,9 @@ jobs: "${binary}" version | grep -E '.+' darwin-native: - name: Darwin-native version smoke + name: Darwin-native lifecycle and version smoke runs-on: macos-14 + timeout-minutes: 8 steps: - name: Checkout uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 @@ -69,6 +74,12 @@ jobs: go-version-file: go.mod cache: true + - name: Run native session lifecycle tests + run: go test -race -count=1 -timeout=4m ./internal/session + + - name: Run stop, restart, and resume tests + run: go test -race -count=1 -timeout=2m ./internal/app + - name: Build and run version command run: | go build -o "${RUNNER_TEMP}/uam" ./cmd/uam From a0187b3cb5dfc2fe238bdf10592b83fc4190099f Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sun, 12 Jul 2026 09:14:19 +0000 Subject: [PATCH 13/18] ci: enforce least-privilege release gates --- .github/workflows/ci.yml | 69 ++++++++++++++++++++++++++-------- .github/workflows/release.yml | 57 ++++++++++++++++++++++++---- .github/workflows/security.yml | 46 ++++++++--------------- .github/workflows/sonar.yml | 21 +++-------- README.md | 10 +++-- 5 files changed, 131 insertions(+), 72 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0473cc4..559a197 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,44 +3,83 @@ name: CI on: push: pull_request: + workflow_call: + workflow_dispatch: permissions: contents: read jobs: - build-test-lint: - name: Build, test, and lint + build: + name: build runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: Set up Go - uses: actions/setup-go@v5 + uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 with: go-version-file: go.mod cache: true + - name: Verify modules + run: go mod verify + - name: Build run: go build ./... - - name: Vet - run: go vet ./... + test-race: + name: test-race + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - name: Set up Go + uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 + with: + go-version-file: go.mod + cache: true + + - name: Test with race detector and coverage + run: go test -race -count=1 -covermode=atomic -coverpkg=./... -coverprofile=coverage.out ./... + + - name: Enforce coverage floor + shell: bash + run: | + coverage="$(go tool cover -func=coverage.out | awk '$1 == "total:" {gsub("%", "", $3); print $3}')" + awk -v coverage="${coverage}" 'BEGIN { if (coverage + 0 < 88.1) { printf "coverage %.1f%% is below 88.1%%\n", coverage; exit 1 } }' + + lint: + name: lint + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - name: Set up Go + uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 + with: + go-version-file: go.mod + cache: true - name: Check gofmt + shell: bash run: | unformatted="$(gofmt -l . | grep -v '^vendor/' || true)" - if [ -n "$unformatted" ]; then - echo "The following files are not gofmt-clean:" - echo "$unformatted" - exit 1 - fi + test -z "${unformatted}" || { printf 'The following files are not gofmt-clean:\n%s\n' "${unformatted}"; exit 1; } + + - name: Vet + run: go vet ./... - - name: Test - run: go test -race -cover ./... + - name: Staticcheck + run: | + go install honnef.co/go/tools/cmd/staticcheck@2025.1.1 + staticcheck ./... - - name: Lint - uses: golangci/golangci-lint-action@9fae48acfc02a90574d7c304a1758ef9895495fa # v7 + - name: GolangCI-Lint + uses: golangci/golangci-lint-action@9fae48acfc02a90574d7c304a1758ef9895495fa # v7.0.1 with: version: v2.5.0 install-mode: goinstall diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c0d2cc6..bb8a8dc 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -6,22 +6,63 @@ on: - "v*" permissions: - contents: write - id-token: write - packages: read + contents: read jobs: - goreleaser: - name: GoReleaser + quality: + name: Quality verification + uses: ./.github/workflows/ci.yml + + security: + name: Security verification + uses: ./.github/workflows/security.yml + + ancestry: + name: Verify tag is on main + runs-on: ubuntu-latest + steps: + - name: Checkout tagged commit + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + fetch-depth: 0 + + - name: Verify tagged commit is an ancestor of main + run: | + git fetch --no-tags origin main + git merge-base --is-ancestor "${GITHUB_SHA}" origin/main + + release-config: + name: Validate release configuration + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + fetch-depth: 0 + + - name: Validate GoReleaser configuration + uses: goreleaser/goreleaser-action@e435ccd777264be153ace6237001ef4d979d3a7a # v6.4.0 + with: + version: "~> v2" + args: check + + publish: + name: Publish release + needs: [quality, security, ancestry, release-config] runs-on: ubuntu-latest + environment: release + permissions: + contents: write + id-token: write + packages: read steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: fetch-depth: 0 - name: Set up Go - uses: actions/setup-go@v5 + uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 with: go-version-file: go.mod cache: true @@ -33,7 +74,7 @@ jobs: uses: sigstore/cosign-installer@dc72c7d5c4d10cd6bcb8cf6e3fd625a9e5e537da # v3.7.0 - name: Run GoReleaser - uses: goreleaser/goreleaser-action@e435ccd777264be153ace6237001ef4d979d3a7a # v6 + uses: goreleaser/goreleaser-action@e435ccd777264be153ace6237001ef4d979d3a7a # v6.4.0 with: version: "~> v2" args: release --clean diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 8d43616..3af1054 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -4,72 +4,58 @@ on: push: branches: [main] pull_request: - branches: [main] schedule: - cron: "23 3 * * 1" + workflow_call: workflow_dispatch: permissions: contents: read - actions: read - security-events: write - pull-requests: read jobs: govulncheck: - name: Go vulnerability check + name: govulncheck runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: Set up Go - uses: actions/setup-go@v5 + uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 with: go-version-file: go.mod cache: true - - name: Install govulncheck - run: go install golang.org/x/vuln/cmd/govulncheck@v1.3.0 - - name: Run govulncheck - run: govulncheck ./... + run: | + go install golang.org/x/vuln/cmd/govulncheck@v1.3.0 + govulncheck ./... gosec: - name: Gosec static analysis + name: gosec runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: Set up Go - uses: actions/setup-go@v5 + uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 with: go-version-file: go.mod cache: true - - name: Install gosec - run: go install github.com/securego/gosec/v2/cmd/gosec@v2.26.1 - - name: Run gosec - run: gosec -exclude-dir=.claude -exclude-dir=vendor -fmt=sarif -out=gosec.sarif ./... - - - name: Upload gosec SARIF - if: always() - uses: github/codeql-action/upload-sarif@v3 - with: - sarif_file: gosec.sarif + run: | + go install github.com/securego/gosec/v2/cmd/gosec@v2.26.1 + gosec -exclude-dir=.claude -exclude-dir=vendor ./... dependency-review: - name: Dependency review + name: dependency-review if: github.event_name == 'pull_request' runs-on: ubuntu-latest - permissions: - contents: read - pull-requests: read steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: Review dependency changes - uses: actions/dependency-review-action@v4 + uses: actions/dependency-review-action@3c4e3dcb1aa7874d2c16be7d79418e9b7efd6261 # v4.8.2 diff --git a/.github/workflows/sonar.yml b/.github/workflows/sonar.yml index 2505e4b..5b39293 100644 --- a/.github/workflows/sonar.yml +++ b/.github/workflows/sonar.yml @@ -3,9 +3,8 @@ name: SonarCloud on: push: branches: [main] - pull_request: - branches: [main] - workflow_dispatch: + schedule: + - cron: "17 5 * * 3" permissions: contents: read @@ -15,16 +14,14 @@ jobs: sonarcloud: name: SonarCloud scan runs-on: ubuntu-latest - env: - SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: fetch-depth: 0 - name: Set up Go - uses: actions/setup-go@v5 + uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 with: go-version-file: go.mod cache: true @@ -34,15 +31,7 @@ jobs: - name: Run SonarCloud scan if: env.SONAR_TOKEN != '' - uses: SonarSource/sonarqube-scan-action@v6 + uses: SonarSource/sonarqube-scan-action@fd88b7d7ccbaefd23d8f36f73b59db7a3d246602 # v6.0.0 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} - - - name: Explain skipped SonarCloud scan - if: env.SONAR_TOKEN == '' - run: | - echo "SONAR_TOKEN is not configured, so the SonarCloud scan was skipped." - echo "Add a repository secret named SONAR_TOKEN to enable this check." - env: - SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} diff --git a/README.md b/README.md index 5b8e8af..02750eb 100644 --- a/README.md +++ b/README.md @@ -189,9 +189,13 @@ run `loginctl enable-linger`. ## Safety model -`uam` can launch providers in their full-access or auto-approve mode when the -provider supports it. Use `uam dispatch --safe ...` when you want the provider's -default approval behavior instead. +`uam` launches providers in their full-access or auto-approve ("yolo") mode by +default when the provider supports it. In that mode, treat the repository, +prompt, provider configuration, and any instructions the agent reads as trusted: +the provider may execute commands and change files without pausing for approval. +Use `uam dispatch --safe ...` when you want the provider's default approval +behavior instead. Safe mode changes provider arguments; it is not an operating- +system sandbox and does not reduce the permissions of the `uam` process itself. `uam` does not make git checkpoints, stash changes, or modify your repository on its own. It starts and manages agent sessions; the provider remains responsible From 3f2f08d11b3d889443b17ce473cbe86586839a60 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sun, 12 Jul 2026 09:21:47 +0000 Subject: [PATCH 14/18] test: restore coverage and remove duplicate routing --- internal/cli/cli.go | 15 --------------- internal/cli/cli_test.go | 10 ++++------ internal/cli/killall_test.go | 9 +++------ internal/log/log_test.go | 7 +++++++ internal/session/host.go | 9 ++------- internal/session/session_test.go | 8 ++++++++ internal/store/store_test.go | 3 +++ 7 files changed, 27 insertions(+), 34 deletions(-) diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 1df5d85..58c0c1d 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -145,14 +145,8 @@ func runWithoutStore(ctx context.Context, args []string) (bool, error) { func runCommand(ctx context.Context, svc *app.Service, args []string, runTUI func(context.Context, tea.Model) error) error { switch args[0] { - case "-h", "--help", "help": - Usage() - return nil case "new": return runNew(ctx, svc, runTUI) - case "version", "--version": - fmt.Println(version.String()) - return nil case "dispatch": return RunDispatch(ctx, svc, args[1:]) case "ls", "list": @@ -165,15 +159,6 @@ func runCommand(ctx context.Context, svc *app.Service, args []string, runTUI fun return runRestart(ctx, svc, args[1:]) case "notify-closed": return runNotifyClosed(svc, args[1:]) - case "kill-all": - return runKillAll(ctx, session.NewClient().KillAll) - case "__host": - // Internal: the detached per-session host process (see - // internal/session). Spawned by CreateSession, never typed by hand. - return session.RunHost(args[1:]) - case "__attach": - // Internal: the interactive attach client run by AttachSpec argv. - return session.RunAttach(args[1:]) case "attach": id, err := requireArg(args[1:], "attach requires ") if err != nil { diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 8f712cf..6bffe19 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -403,16 +403,14 @@ func firstNonEmpty(values ...string) string { return "" } -// The internal __host/__attach subcommands are routed through runCommand; +// The internal __host/__attach subcommands are routed before store access; // invalid input must surface as errors rather than silently doing nothing. -func TestRunCommandInternalSubcommands(t *testing.T) { +func TestRunWithoutStoreInternalSubcommands(t *testing.T) { t.Setenv("UAM_SESSION_DIR", secureSessionDir(t)) - svc, _ := newCLITestService(t) - noTUI := func(context.Context, tea.Model) error { return nil } - if err := runCommand(context.Background(), svc, []string{"__host", "--name", "bad name", "--", "/bin/true"}, noTUI); err == nil { + if handled, err := runWithoutStore(context.Background(), []string{"__host", "--name", "bad name", "--", "/bin/true"}); !handled || err == nil { t.Fatal("__host with an invalid name must fail") } - if err := runCommand(context.Background(), svc, []string{"__attach"}, noTUI); err == nil { + if handled, err := runWithoutStore(context.Background(), []string{"__attach"}); !handled || err == nil { t.Fatal("__attach without a session must fail") } } diff --git a/internal/cli/killall_test.go b/internal/cli/killall_test.go index 0ea7025..d28c512 100644 --- a/internal/cli/killall_test.go +++ b/internal/cli/killall_test.go @@ -4,8 +4,6 @@ import ( "context" "errors" "testing" - - tea "github.com/charmbracelet/bubbletea" ) // F24 — `uam kill-all` must invoke the session teardown exactly once. @@ -32,15 +30,14 @@ func TestRunKillAllPropagatesError(t *testing.T) { } } -// F24 — `kill-all` must be routed by runCommand to the default teardown path. +// F24 — `kill-all` must be routed before store access to the default teardown path. // An empty session runtime dir (via UAM_SESSION_DIR) keeps the test off any // real sessions: KillAll over zero sessions is an idempotent success. -func TestRunCommandKillAllDispatches(t *testing.T) { - svc, _ := newCLITestService(t) +func TestRunWithoutStoreKillAllDispatches(t *testing.T) { t.Setenv("UAM_SESSION_DIR", secureSessionDir(t)) out := captureCLIStdout(t, func() { - if err := runCommand(context.Background(), svc, []string{"kill-all"}, func(context.Context, tea.Model) error { return nil }); err != nil { + if handled, err := runWithoutStore(context.Background(), []string{"kill-all"}); !handled || err != nil { t.Fatalf("kill-all dispatch: %v", err) } }) diff --git a/internal/log/log_test.go b/internal/log/log_test.go index 9af8b5f..f644743 100644 --- a/internal/log/log_test.go +++ b/internal/log/log_test.go @@ -158,4 +158,11 @@ func TestCacheDirFallbacks(t *testing.T) { if got := cacheDir(); got != filepath.Join(dir, "uam") { t.Fatalf("%s", got) } + t.Setenv("XDG_CACHE_HOME", "") + t.Setenv("HOME", dir) + if got := cacheDir(); got != filepath.Join(dir, ".cache", "uam") { + t.Fatalf("home cache dir = %s", got) + } + UseStderr(nil) + Debug("stderr fallback debug path") } diff --git a/internal/session/host.go b/internal/session/host.go index 93dd274..971ac78 100644 --- a/internal/session/host.go +++ b/internal/session/host.go @@ -17,6 +17,7 @@ import ( "github.com/creack/pty" + "github.com/RandomCodeSpace/unified-agent-manager/internal/displaytext" "github.com/RandomCodeSpace/unified-agent-manager/internal/log" "github.com/RandomCodeSpace/unified-agent-manager/internal/store" "github.com/RandomCodeSpace/unified-agent-manager/internal/vterm" @@ -369,13 +370,7 @@ func (h *host) setLabel(label string) { // titleSequence sets the terminal title via OSC 0 — the native stand-in for // tmux's set-titles-string showing the user-facing session label. func titleSequence(label string) string { - clean := strings.Map(func(r rune) rune { - if r < 0x20 || r == 0x7f { - return -1 - } - return r - }, label) - return "\x1b]0;" + clean + "\x07" + return "\x1b]0;" + displaytext.Sanitize(label) + "\x07" } func validSize(cols, rows int) bool { diff --git a/internal/session/session_test.go b/internal/session/session_test.go index 5201d90..d16dcbf 100644 --- a/internal/session/session_test.go +++ b/internal/session/session_test.go @@ -309,6 +309,14 @@ func TestSetSessionLabelPersistsToState(t *testing.T) { }) } +func TestTitleSequenceSanitizesTerminalControls(t *testing.T) { + got := titleSequence("safe\u009d0;forged\a red\nnow") + want := "\x1b]0;safe red now\x07" + if got != want { + t.Fatalf("titleSequence = %q, want %q", got, want) + } +} + func TestListSweepsStaleStateFiles(t *testing.T) { c := newTestClient(t) if err := EnsureDir(c.Dir); err != nil { diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 026a420..55be9a1 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -104,6 +104,9 @@ func TestTryMarkSessionClosedReportsWhetherRecordMatched(t *testing.T) { if rec.Status != StatusClosedByUser || rec.LastExitCode == nil || *rec.LastExitCode != 7 { t.Fatalf("record not closed: %+v", rec) } + if err := s.MarkSessionClosed("uam-fake-deadbeef", 8); err != nil { + t.Fatalf("idempotent MarkSessionClosed: %v", err) + } } func TestSyncDirAcceptsStoreDirectoryAndRejectsMissingDirectory(t *testing.T) { From 071050e5c439a57abd1cf05dc6ea6f0bf65cbecd Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sun, 12 Jul 2026 09:23:34 +0000 Subject: [PATCH 15/18] fix(pr): enforce refresh pass deadlines --- internal/app/pr_refresh_test.go | 22 ++++++++++++++++++++++ internal/app/service.go | 24 +++++++++++++++++++++++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/internal/app/pr_refresh_test.go b/internal/app/pr_refresh_test.go index 2a48e21..2601f36 100644 --- a/internal/app/pr_refresh_test.go +++ b/internal/app/pr_refresh_test.go @@ -170,6 +170,28 @@ func TestAdapterStatusMapping(t *testing.T) { } } +func TestCheckPRWithContextReturnsWhenCheckerIgnoresCancellation(t *testing.T) { + release := make(chan struct{}) + checkerDone := make(chan struct{}) + checker := func(context.Context, string) (pr.Status, error) { + defer close(checkerDone) + <-release + return pr.Merged, nil + } + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Millisecond) + defer cancel() + started := time.Now() + status, err := checkPRWithContext(ctx, checker, "https://github.com/o/r/pull/5") + if !errors.Is(err, context.DeadlineExceeded) || status != pr.None { + t.Fatalf("checkPRWithContext = (%q, %v), want deadline", status, err) + } + if elapsed := time.Since(started); elapsed > 250*time.Millisecond { + t.Fatalf("cancellation-ignoring checker blocked for %v", elapsed) + } + close(release) + <-checkerDone +} + func BenchmarkRefreshPRStatuses(b *testing.B) { sessions := make([]adapter.Session, 20) for i := range sessions { diff --git a/internal/app/service.go b/internal/app/service.go index 2d130cc..0869ffb 100644 --- a/internal/app/service.go +++ b/internal/app/service.go @@ -327,6 +327,28 @@ type prRefreshResult struct { record store.PRRecord } +type prCheckResult struct { + status pr.Status + err error +} + +// checkPRWithContext enforces the caller's deadline even when a custom checker +// fails to observe context cancellation. The result channel is buffered so a +// late checker can finish without retaining the refresh worker. +func checkPRWithContext(ctx context.Context, checker func(context.Context, string) (pr.Status, error), url string) (pr.Status, error) { + resultCh := make(chan prCheckResult, 1) + go func() { + status, err := checker(ctx, url) + resultCh <- prCheckResult{status: status, err: err} + }() + select { + case result := <-resultCh: + return result.status, result.err + case <-ctx.Done(): + return pr.None, ctx.Err() + } +} + // RefreshPRStatuses enriches stale discovered PRs outside the session-discovery // path. Only PR-owned fields are persisted, and overlapping passes coalesce. func (s *Service) RefreshPRStatuses(ctx context.Context) error { @@ -370,7 +392,7 @@ func (s *Service) RefreshPRStatuses(ctx context.Context) error { for job := range jobCh { status := job.ref.Status checkCtx, checkCancel := context.WithTimeout(ctx, prCheckTimeout) - checked, checkErr := checker(checkCtx, job.ref.URL) + checked, checkErr := checkPRWithContext(checkCtx, checker, job.ref.URL) checkCancel() if checkErr == nil && checked != pr.None { status = adapterStatus(checked) From ae1f883b1533dda08e61aa1cdc0015bd2462afc8 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sun, 12 Jul 2026 09:32:37 +0000 Subject: [PATCH 16/18] refactor: simplify hardened control paths --- internal/adapter/agent.go | 25 +++++--- internal/app/service.go | 102 +++++++++++++++++++------------ internal/displaytext/sanitize.go | 44 +++++++------ internal/session/client.go | 80 ++++++++++++++---------- internal/session/host.go | 14 +++-- 5 files changed, 160 insertions(+), 105 deletions(-) diff --git a/internal/adapter/agent.go b/internal/adapter/agent.go index 5dc5a74..76036f4 100644 --- a/internal/adapter/agent.go +++ b/internal/adapter/agent.go @@ -219,16 +219,7 @@ func (a *Agent) startSession(ctx context.Context, req ResumeRequest, activity st if err := a.Backend.CreateSession(ctx, sessionName, cwd, env, cmd); err != nil { return Session{}, fmt.Errorf("create session %s: %w", sessionName, err) } - // Surface the user-facing name in attached terminals' titles (the - // canonical uam-- stays the machine-parseable session name). - // Cosmetic and best-effort — a failure never affects the session. - displayName := req.Name - if displayName == "" { - displayName = displayNameFromDir(cwd) - } - if err := a.Backend.SetSessionLabel(ctx, sessionName, displaytext.Sanitize(displayName+" · "+a.Name())); err != nil { - log.Debug("set session label failed", "session", sessionName, "error", err) - } + displayName := a.setSessionDisplayLabel(ctx, sessionName, req.Name, cwd) shouldSendPrompt := strings.TrimSpace(req.Prompt) != "" && (activity != "resumed" || !a.SkipPromptOnResume) if shouldSendPrompt { if err := a.Backend.SendLine(ctx, sessionName, req.Prompt); err != nil { @@ -254,6 +245,20 @@ func (a *Agent) startSession(ctx context.Context, req ResumeRequest, activity st return Session{ID: req.ID, AgentType: a.Name(), CommandAlias: req.CommandAlias, DisplayName: displayName, Prompt: req.Prompt, Cwd: cwd, SessionName: sessionName, ProviderSessionID: providerID, State: Active, ProcAlive: Alive, CreatedAt: created, LastChange: now}, nil } +// setSessionDisplayLabel surfaces the user-facing name in attached terminal +// titles while keeping the canonical session name machine-parseable. It is +// cosmetic and best-effort: failure never changes session startup. +func (a *Agent) setSessionDisplayLabel(ctx context.Context, sessionName, requestedName, cwd string) string { + displayName := requestedName + if displayName == "" { + displayName = displayNameFromDir(cwd) + } + if err := a.Backend.SetSessionLabel(ctx, sessionName, displaytext.Sanitize(displayName+" · "+a.Name())); err != nil { + log.Debug("set session label failed", "session", sessionName, "error", err) + } + return displayName +} + func resolveSessionCwd(cwd string) (string, error) { if cwd == "" { var err error diff --git a/internal/app/service.go b/internal/app/service.go index 0869ffb..3b66326 100644 --- a/internal/app/service.go +++ b/internal/app/service.go @@ -123,28 +123,32 @@ func (s *Service) persistRefresh(updates map[string]refreshPatch) error { } return s.Store.Update(func(cfg *store.Config) error { for key, patch := range updates { - rec, exists := cfg.Sessions[key] - if (!exists || rec.ID == "") && patch.create != nil { - rec = *patch.create - } - if patch.status != nil { - rec.Status = *patch.status - } - if patch.lastSeenAt != nil { - rec.LastSeenAt = *patch.lastSeenAt - } - if patch.clearPR { - rec.PR = nil - } else if patch.pr != nil { - pr := *patch.pr - rec.PR = &pr - } - cfg.Sessions[key] = rec + applyRefreshPatch(cfg, key, patch) } return nil }) } +func applyRefreshPatch(cfg *store.Config, key string, patch refreshPatch) { + rec, exists := cfg.Sessions[key] + if (!exists || rec.ID == "") && patch.create != nil { + rec = *patch.create + } + if patch.status != nil { + rec.Status = *patch.status + } + if patch.lastSeenAt != nil { + rec.LastSeenAt = *patch.lastSeenAt + } + if patch.clearPR { + rec.PR = nil + } else if patch.pr != nil { + pr := *patch.pr + rec.PR = &pr + } + cfg.Sessions[key] = rec +} + func (s *Service) loadConfig() (store.Config, error) { cfg := store.DefaultConfig() if s.Store == nil { @@ -362,25 +366,41 @@ func (s *Service) RefreshPRStatuses(ctx context.Context) error { if err != nil { return err } - now := time.Now() + jobs := stalePRRefreshJobs(sessions, cfg, time.Now()) + if len(jobs) == 0 { + return nil + } + results := runPRRefreshJobs(ctx, jobs, firstPRChecker(s.checkPR)) + if err := s.persistPRRefreshResults(results); err != nil { + return err + } + return ctx.Err() +} + +func stalePRRefreshJobs(sessions []adapter.Session, cfg store.Config, now time.Time) []prRefreshJob { jobs := make([]prRefreshJob, 0, len(sessions)) for _, sess := range sessions { if sess.PR == nil { continue } - rec := cfg.Sessions[store.Key(sess.AgentType, sess.ID)] + key := store.Key(sess.AgentType, sess.ID) + rec := cfg.Sessions[key] if rec.PR != nil && rec.PR.URL == sess.PR.URL && now.Sub(rec.PR.LastChecked) < prRefreshAge { continue } - jobs = append(jobs, prRefreshJob{key: store.Key(sess.AgentType, sess.ID), ref: *sess.PR}) + jobs = append(jobs, prRefreshJob{key: key, ref: *sess.PR}) } - if len(jobs) == 0 { - return nil - } - checker := s.checkPR - if checker == nil { - checker = pr.Check + return jobs +} + +func firstPRChecker(checker func(context.Context, string) (pr.Status, error)) func(context.Context, string) (pr.Status, error) { + if checker != nil { + return checker } + return pr.Check +} + +func runPRRefreshJobs(ctx context.Context, jobs []prRefreshJob, checker func(context.Context, string) (pr.Status, error)) []prRefreshResult { jobCh := make(chan prRefreshJob) resultCh := make(chan prRefreshResult, len(jobs)) workers := min(prRefreshWorkers, len(jobs)) @@ -390,14 +410,7 @@ func (s *Service) RefreshPRStatuses(ctx context.Context) error { go func() { defer wg.Done() for job := range jobCh { - status := job.ref.Status - checkCtx, checkCancel := context.WithTimeout(ctx, prCheckTimeout) - checked, checkErr := checkPRWithContext(checkCtx, checker, job.ref.URL) - checkCancel() - if checkErr == nil && checked != pr.None { - status = adapterStatus(checked) - } - resultCh <- prRefreshResult{key: job.key, record: store.PRRecord{URL: job.ref.URL, Number: job.ref.Number, LastStatus: string(status), LastChecked: time.Now()}} + resultCh <- runPRRefreshJob(ctx, job, checker) } }() } @@ -417,7 +430,22 @@ func (s *Service) RefreshPRStatuses(ctx context.Context) error { for result := range resultCh { results = append(results, result) } - updateErr := s.Store.Update(func(cfg *store.Config) error { + return results +} + +func runPRRefreshJob(ctx context.Context, job prRefreshJob, checker func(context.Context, string) (pr.Status, error)) prRefreshResult { + status := job.ref.Status + checkCtx, checkCancel := context.WithTimeout(ctx, prCheckTimeout) + checked, checkErr := checkPRWithContext(checkCtx, checker, job.ref.URL) + checkCancel() + if checkErr == nil && checked != pr.None { + status = adapterStatus(checked) + } + return prRefreshResult{key: job.key, record: store.PRRecord{URL: job.ref.URL, Number: job.ref.Number, LastStatus: string(status), LastChecked: time.Now()}} +} + +func (s *Service) persistPRRefreshResults(results []prRefreshResult) error { + return s.Store.Update(func(cfg *store.Config) error { for _, result := range results { rec, ok := cfg.Sessions[result.key] if !ok || rec.PR == nil || rec.PR.URL != result.record.URL { @@ -429,10 +457,6 @@ func (s *Service) RefreshPRStatuses(ctx context.Context) error { } return nil }) - if updateErr != nil { - return updateErr - } - return ctx.Err() } func adapterStatus(status pr.Status) adapter.PRStatus { diff --git a/internal/displaytext/sanitize.go b/internal/displaytext/sanitize.go index 78116c8..76a600f 100644 --- a/internal/displaytext/sanitize.go +++ b/internal/displaytext/sanitize.go @@ -45,16 +45,7 @@ func Sanitize(s string) string { func step(state parseState, r rune, b *strings.Builder) parseState { switch state { case escape: - switch r { - case '[': - return csi - case ']', 'P', 'X', '^', '_': - return controlString - case 0x1b: - return escape - default: - return appendGround(r, b) - } + return stepEscape(r, b) case csi: if r == 0x1b { return escape @@ -64,14 +55,7 @@ func step(state parseState, r rune, b *strings.Builder) parseState { } return csi case controlString: - switch r { - case 0x07, 0x9c: - return ground - case 0x1b: - return controlStringEscape - default: - return controlString - } + return stepControlString(r) case controlStringEscape: if r == '\\' || r == 0x9c || r == 0x07 { return ground @@ -85,6 +69,30 @@ func step(state parseState, r rune, b *strings.Builder) parseState { } } +func stepEscape(r rune, b *strings.Builder) parseState { + switch r { + case '[': + return csi + case ']', 'P', 'X', '^', '_': + return controlString + case 0x1b: + return escape + default: + return appendGround(r, b) + } +} + +func stepControlString(r rune) parseState { + switch r { + case 0x07, 0x9c: + return ground + case 0x1b: + return controlStringEscape + default: + return controlString + } +} + func appendGround(r rune, b *strings.Builder) parseState { switch r { case 0x1b: diff --git a/internal/session/client.go b/internal/session/client.go index 8b381cb..dd7e242 100644 --- a/internal/session/client.go +++ b/internal/session/client.go @@ -168,32 +168,36 @@ func (c *Client) List(_ context.Context) ([]Info, error) { } var out []Info for _, e := range entries { - name, isState := strings.CutSuffix(e.Name(), ".json") - if !isState || ValidateName(name) != nil { - continue + if info, keep := c.infoFromStateEntry(e); keep { + out = append(out, info) } - st, err := readState(c.Dir, name) - if err != nil { - log.Warn("skipping unreadable session state", "file", e.Name(), "error", err) - continue - } - if !st.hostAlive() { - if st.childAlive() { - // Host crashed but the agent is still winding down (it gets - // SIGHUP when the PTY master closed). Keep it visible and - // leave the files for the next sweep. - out = append(out, infoFromState(st)) - continue - } - _ = removeSessionFiles(c.Dir, name) - continue - } - out = append(out, infoFromState(st)) } sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) return out, nil } +func (c *Client) infoFromStateEntry(entry os.DirEntry) (Info, bool) { + name, isState := strings.CutSuffix(entry.Name(), ".json") + if !isState || ValidateName(name) != nil { + return Info{}, false + } + st, err := readState(c.Dir, name) + if err != nil { + log.Warn("skipping unreadable session state", "file", entry.Name(), "error", err) + return Info{}, false + } + if st.hostAlive() { + return infoFromState(st), true + } + if st.childAlive() { + // Host crashed but the agent is still winding down. Keep it visible + // and leave the runtime files for the next sweep. + return infoFromState(st), true + } + _ = removeSessionFiles(c.Dir, name) + return Info{}, false +} + func infoFromState(st State) Info { cwd := procCwd(st.ChildPID) if cwd == "" { @@ -257,19 +261,8 @@ func (c *Client) Kill(ctx context.Context, name string) error { // A live PID is not sufficient authority to signal: fallback control paths // require a nonzero matching start identity so a recycled PID can never be // signalled as if it were the session. - if ProcAlive(st.HostPID) { - if !procIdentityMatches(st.HostPID, st.HostStart) { - return fmt.Errorf("kill session %s: cannot verify process identity for host pid %d", name, st.HostPID) - } - _ = syscall.Kill(st.HostPID, syscall.SIGTERM) - } else if ProcAlive(st.ChildPID) { - if !procIdentityMatches(st.ChildPID, st.ChildStart) { - return fmt.Errorf("kill session %s: cannot verify process identity for child pid %d", name, st.ChildPID) - } - // Orphaned agent (host crashed): signal its process group directly. - if err := syscall.Kill(-st.ChildPID, syscall.SIGTERM); err != nil { - _ = syscall.Kill(st.ChildPID, syscall.SIGTERM) - } + if err := signalVerifiedFallback(name, st); err != nil { + return err } deadline := time.Now().Add(callTimeout) for time.Now().Before(deadline) { @@ -286,6 +279,27 @@ func (c *Client) Kill(ctx context.Context, name string) error { return fmt.Errorf("kill session %s: still running", name) } +func signalVerifiedFallback(name string, st State) error { + if ProcAlive(st.HostPID) { + if !procIdentityMatches(st.HostPID, st.HostStart) { + return fmt.Errorf("kill session %s: cannot verify process identity for host pid %d", name, st.HostPID) + } + _ = syscall.Kill(st.HostPID, syscall.SIGTERM) + return nil + } + if !ProcAlive(st.ChildPID) { + return nil + } + if !procIdentityMatches(st.ChildPID, st.ChildStart) { + return fmt.Errorf("kill session %s: cannot verify process identity for child pid %d", name, st.ChildPID) + } + // Orphaned agent (host crashed): signal its process group directly. + if err := syscall.Kill(-st.ChildPID, syscall.SIGTERM); err != nil { + _ = syscall.Kill(st.ChildPID, syscall.SIGTERM) + } + return nil +} + // KillAll terminates every managed session. It replaces `tmux kill-server` // and is idempotent: an empty (or missing) runtime directory is success. func (c *Client) KillAll(ctx context.Context) error { diff --git a/internal/session/host.go b/internal/session/host.go index 971ac78..52ca71c 100644 --- a/internal/session/host.go +++ b/internal/session/host.go @@ -550,14 +550,18 @@ func (h *host) markClosed(exitCode int) { lastErr = err remaining := time.Until(deadline) if remaining <= 0 { - if lastErr != nil { - log.Warn("mark session closed failed after retry", "session", h.name, "error", lastErr) - } else { - log.Warn("mark session closed record not found before retry deadline", "session", h.name) - } + h.logMarkClosedFailure(lastErr) return } time.Sleep(min(delay, remaining)) delay = min(delay*2, markClosedRetryMax) } } + +func (h *host) logMarkClosedFailure(err error) { + if err != nil { + log.Warn("mark session closed failed after retry", "session", h.name, "error", err) + return + } + log.Warn("mark session closed record not found before retry deadline", "session", h.name) +} From 5b94e78a1f416054066e08a3de86a3b6160e210d Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sun, 12 Jul 2026 09:35:03 +0000 Subject: [PATCH 17/18] chore(security): document validated filesystem boundaries --- internal/session/session.go | 4 +++- internal/store/store.go | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/session/session.go b/internal/session/session.go index 7cd53b8..543d4c4 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -186,7 +186,9 @@ func readState(dir, name string) (State, error) { if !info.Mode().IsRegular() { return State{}, fmt.Errorf("session state %s is not a regular file", name) } - data, err := os.ReadFile(path) + // name has passed the strict canonical allow-list and dir has passed the + // owner-only identity check above, so path cannot escape the runtime dir. + data, err := os.ReadFile(path) // #nosec G304 -- validated name within a verified owner-only directory. if err != nil { return State{}, err } diff --git a/internal/store/store.go b/internal/store/store.go index 732d825..0d39ba0 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -583,7 +583,9 @@ func (s *Store) saveNoLock(cfg Config) error { } func syncDir(dir string) error { - f, err := os.Open(dir) + // dir is the containing directory of Store.path; opening that configured + // path is the intended durability operation, not user-controlled inclusion. + f, err := os.Open(dir) // #nosec G304 -- Store intentionally fsyncs its configured parent directory. if err != nil { return fmt.Errorf("open store directory for sync: %w", err) } From 5810dea66cc3abbbf5a08d67d9cf8c8d20e386d5 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sun, 12 Jul 2026 10:06:08 +0000 Subject: [PATCH 18/18] docs: define Windows support via WSL2 --- README.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/README.md b/README.md index 02750eb..384e0cb 100644 --- a/README.md +++ b/README.md @@ -46,6 +46,19 @@ there is nothing else to install. Providers are capability-probed at runtime. If a CLI is missing, `uam` hides it from the dispatch UI instead of failing the whole app. +## Supported platforms + +- Linux, including Ubuntu, on amd64 and arm64 +- macOS on Intel and Apple silicon +- Windows 10/11 through WSL2 with an Ubuntu distribution + +On Windows, install and run `uam` and the provider CLIs inside the same WSL2 +distribution. Sessions and paths then live inside that Linux environment. +Native Windows processes are not supported yet: the session host depends on a +Unix PTY, Unix process groups, and owner-only Unix runtime directories. A native +port requires a ConPTY and Job Object backend and will not be advertised until +its full create/list/attach/stop/restart lifecycle passes on Windows. + ## Install Install the `uam` binary directly: