diff --git a/backend/internal/adapters/tracker/github/tracker.go b/backend/internal/adapters/tracker/github/tracker.go index 1d5d6c5df..df0524a0f 100644 --- a/backend/internal/adapters/tracker/github/tracker.go +++ b/backend/internal/adapters/tracker/github/tracker.go @@ -33,10 +33,12 @@ const ( stateClosedGH = "closed" reasonNotPlan = "not_planned" - // List pagination — GitHub's per_page maxes at 100. We default to 30 - // (matching the legacy gh CLI default) when the caller passes 0. - defaultListLimit = 30 - maxListLimit = 100 + // List pagination — GitHub's per_page maxes at 100. ListFilter.Limit is + // an optional total-result cap; page size stays at the provider max. + listPageSize = 100 + // Guard against a pathological Link cycle. At GitHub's max page size this + // still permits a 5k-issue intake sweep before failing loud. + maxListPages = 50 ) // Sentinel errors. Adapter-level callers should match on these via @@ -96,6 +98,12 @@ type Tracker struct { baseURL string userAgent string + // listCache stores one entry per distinct List request path. The key + // space is naturally bounded by intake-enabled repo/filter pairs, so no + // eviction is needed here. + listCacheMu sync.Mutex + listCache map[string]listCacheEntry + // preflightOK is the fast-path: once a Preflight succeeds, every // subsequent call short-circuits via atomic.Load without touching the // mutex. preflightMu serializes the one-time network call so concurrent @@ -104,6 +112,12 @@ type Tracker struct { preflightMu sync.Mutex } +type listCacheEntry struct { + etag string + issues []domain.Issue + nextPath string +} + // New returns a Tracker. It fails fast when no token can be obtained so // daemons crash at startup rather than at first issue lookup. func New(opts Options) (*Tracker, error) { @@ -119,6 +133,7 @@ func New(opts Options) (*Tracker, error) { tokens: src, baseURL: opts.BaseURL, userAgent: opts.UserAgent, + listCache: map[string]listCacheEntry{}, } if t.http == nil { t.http = &http.Client{Timeout: 30 * time.Second} @@ -253,8 +268,8 @@ func mapStateFromGitHub(state, reason string, labels []string) domain.Normalized // List returns issues for a repo, filtered by state/labels/assignee. PRs // that GitHub's /issues endpoint conflates into the response are filtered -// out client-side. Pagination is intentionally NOT implemented in v1 — -// callers get one page bounded by ListFilter.Limit (default 30, max 100). +// out client-side. GitHub pagination is followed until no next link remains; +// ListFilter.Limit, when set, caps the total accumulated issue count. func (t *Tracker) List(ctx context.Context, repo domain.TrackerRepo, filter domain.ListFilter) ([]domain.Issue, error) { if repo.Provider != domain.TrackerProviderGitHub { return nil, fmt.Errorf("%w: provider=%q", ErrWrongProvider, repo.Provider) @@ -279,34 +294,101 @@ func (t *Tracker) List(ctx context.Context, repo domain.TrackerRepo, filter doma if filter.Assignee != "" { q.Set("assignee", filter.Assignee) } - limit := filter.Limit - if limit <= 0 { - limit = defaultListLimit - } - if limit > maxListLimit { - limit = maxListLimit - } - q.Set("per_page", strconv.Itoa(limit)) + q.Set("per_page", strconv.Itoa(listPageSize)) path := fmt.Sprintf("/repos/%s/%s/issues?%s", owner, repoName, q.Encode()) - resp, err := t.do(ctx, http.MethodGet, path, nil) - if err != nil { - return nil, err + out := make([]domain.Issue, 0) + if filter.Limit > 0 { + out = make([]domain.Issue, 0, filter.Limit) } - var raw []ghIssue - if err := json.Unmarshal(resp, &raw); err != nil { - return nil, fmt.Errorf("github tracker: decode list: %w", err) - } - out := make([]domain.Issue, 0, len(raw)) - for _, r := range raw { - if r.PullRequest != nil { - continue + for page := 0; path != ""; page++ { + if page >= maxListPages { + return nil, fmt.Errorf("github tracker: list pagination exceeded %d pages", maxListPages) } - out = append(out, issueFromGH(owner, repoName, r)) + t.listCacheMu.Lock() + cached, hasCached := t.listCache[path] + t.listCacheMu.Unlock() + + resp, etag, nextPath, notModified, err := t.roundTrip(ctx, http.MethodGet, path, nil, cached.etag) + if err != nil { + return nil, err + } + if notModified { + if hasCached { + if etag != "" && etag != cached.etag { + t.listCacheMu.Lock() + t.listCache[path] = listCacheEntry{etag: etag, issues: cached.issues, nextPath: cached.nextPath} + t.listCacheMu.Unlock() + } + var done bool + out, done = appendIssuesWithLimit(out, cloneIssues(cached.issues), filter.Limit) + if done { + break + } + path = cached.nextPath + continue + } + // A 304 requires a prior validator, but if a server violates that + // contract, retry unconditionally rather than returning no data. + resp, etag, nextPath, notModified, err = t.roundTrip(ctx, http.MethodGet, path, nil, "") + if err != nil { + return nil, err + } + if notModified { + return nil, fmt.Errorf("github tracker: unexpected 304 for uncached list") + } + } + var raw []ghIssue + if err := json.Unmarshal(resp, &raw); err != nil { + return nil, fmt.Errorf("github tracker: decode list: %w", err) + } + pageIssues := make([]domain.Issue, 0, len(raw)) + for _, r := range raw { + if r.PullRequest != nil { + continue + } + pageIssues = append(pageIssues, issueFromGH(owner, repoName, r)) + } + if etag != "" { + t.listCacheMu.Lock() + t.listCache[path] = listCacheEntry{etag: etag, issues: cloneIssues(pageIssues), nextPath: nextPath} + t.listCacheMu.Unlock() + } else if hasCached { + t.listCacheMu.Lock() + delete(t.listCache, path) + t.listCacheMu.Unlock() + } + var done bool + out, done = appendIssuesWithLimit(out, pageIssues, filter.Limit) + if done { + break + } + path = nextPath } return out, nil } +func appendIssuesWithLimit(dst, src []domain.Issue, limit int) ([]domain.Issue, bool) { + if limit <= 0 { + return append(dst, src...), false + } + remaining := limit - len(dst) + if remaining <= 0 { + return dst, true + } + if len(src) > remaining { + return append(dst, src[:remaining]...), true + } + dst = append(dst, src...) + return dst, len(dst) >= limit +} + +func cloneIssues(in []domain.Issue) []domain.Issue { + out := make([]domain.Issue, len(in)) + copy(out, in) + return out +} + // --------------------------------------------------------------------------- // Preflight // --------------------------------------------------------------------------- @@ -347,43 +429,116 @@ func (t *Tracker) Preflight(ctx context.Context) error { // --------------------------------------------------------------------------- func (t *Tracker) do(ctx context.Context, method, path string, body any) ([]byte, error) { + respBody, _, _, _, err := t.roundTrip(ctx, method, path, body, "") + return respBody, err +} + +func (t *Tracker) roundTrip(ctx context.Context, method, path string, body any, ifNoneMatch string) ([]byte, string, string, bool, error) { var rdr io.Reader if body != nil { b, err := json.Marshal(body) if err != nil { - return nil, fmt.Errorf("github tracker: encode body: %w", err) + return nil, "", "", false, fmt.Errorf("github tracker: encode body: %w", err) } rdr = bytes.NewReader(b) } req, err := http.NewRequestWithContext(ctx, method, t.baseURL+path, rdr) if err != nil { - return nil, fmt.Errorf("github tracker: build request: %w", err) + return nil, "", "", false, fmt.Errorf("github tracker: build request: %w", err) } if body != nil { req.Header.Set("Content-Type", "application/json") } + if ifNoneMatch != "" { + req.Header.Set("If-None-Match", ifNoneMatch) + } req.Header.Set("Accept", "application/vnd.github+json") req.Header.Set("X-GitHub-Api-Version", "2022-11-28") req.Header.Set("User-Agent", t.userAgent) tok, err := t.tokens.Token(ctx) if err != nil { - return nil, err + return nil, "", "", false, err } req.Header.Set("Authorization", "Bearer "+tok) resp, err := t.http.Do(req) if err != nil { - return nil, fmt.Errorf("github tracker: %s %s: %w", method, path, err) + return nil, "", "", false, fmt.Errorf("github tracker: %s %s: %w", method, path, err) } defer func() { _ = resp.Body.Close() }() + etag := resp.Header.Get("ETag") + nextPath := parseLinkNext(resp.Header.Get("Link"), t.baseURL) + if resp.StatusCode == http.StatusNotModified { + return nil, etag, nextPath, true, nil + } respBody, readErr := io.ReadAll(resp.Body) if readErr != nil { - return nil, fmt.Errorf("github tracker: read response body: %w", readErr) + return nil, "", "", false, fmt.Errorf("github tracker: read response body: %w", readErr) } if resp.StatusCode >= 200 && resp.StatusCode < 300 { - return respBody, nil + return respBody, etag, nextPath, false, nil } - return respBody, classifyError(resp, respBody) + return respBody, etag, nextPath, false, classifyError(resp, respBody) +} + +func parseLinkNext(linkHeader, baseURL string) string { + for _, part := range strings.Split(linkHeader, ",") { + part = strings.TrimSpace(part) + if part == "" || !strings.HasPrefix(part, "<") { + continue + } + urlEnd := strings.Index(part, ">") + if urlEnd < 0 { + continue + } + rawURL := part[1:urlEnd] + if !linkHasRelNext(part[urlEnd+1:]) { + continue + } + return relativePathForLink(rawURL, baseURL) + } + return "" +} + +func linkHasRelNext(params string) bool { + for _, param := range strings.Split(params, ";") { + name, value, ok := strings.Cut(strings.TrimSpace(param), "=") + if !ok || !strings.EqualFold(name, "rel") { + continue + } + value = strings.Trim(value, `"`) + for _, rel := range strings.Fields(value) { + if strings.EqualFold(rel, "next") { + return true + } + } + } + return false +} + +func relativePathForLink(rawLink, baseURL string) string { + if strings.HasPrefix(rawLink, "/") { + return rawLink + } + base, err := url.Parse(baseURL) + if err != nil { + return "" + } + u, err := url.Parse(rawLink) + if err != nil { + return "" + } + if !u.IsAbs() { + u = base.ResolveReference(u) + } + if u.Path == "" { + return "" + } + path := u.EscapedPath() + if u.RawQuery != "" { + path += "?" + u.RawQuery + } + return path } func classifyError(resp *http.Response, body []byte) error { diff --git a/backend/internal/adapters/tracker/github/tracker_test.go b/backend/internal/adapters/tracker/github/tracker_test.go index 57585b743..23fae915a 100644 --- a/backend/internal/adapters/tracker/github/tracker_test.go +++ b/backend/internal/adapters/tracker/github/tracker_test.go @@ -20,9 +20,10 @@ import ( // recordedReq captures one inbound HTTP request so tests can assert against // the exact GitHub API surface the adapter touched. type recordedReq struct { - Method string - Path string - Body string + Method string + Path string + Body string + IfNoneMatch string } // fakeGH is a programmable httptest.Server that matches requests by @@ -54,7 +55,7 @@ func (f *fakeGH) serve(w http.ResponseWriter, r *http.Request) { body, _ := io.ReadAll(r.Body) key := r.Method + " " + r.URL.Path f.mu.Lock() - f.requests = append(f.requests, recordedReq{Method: r.Method, Path: r.URL.Path, Body: string(body)}) + f.requests = append(f.requests, recordedReq{Method: r.Method, Path: r.URL.Path, Body: string(body), IfNoneMatch: r.Header.Get("If-None-Match")}) h, ok := f.handlers[key] f.mu.Unlock() if !ok { @@ -427,8 +428,8 @@ func TestList_HappyPathAndDefaults(t *testing.T) { if got := q.Get("state"); got != "all" { t.Errorf("state = %q, want all (default)", got) } - if got := q.Get("per_page"); got != "30" { - t.Errorf("per_page = %q, want 30 (default)", got) + if got := q.Get("per_page"); got != "100" { + t.Errorf("per_page = %q, want 100 (default)", got) } _, _ = w.Write([]byte(`[ {"number":1,"title":"first","body":"b1","state":"open","html_url":"https://github.com/o/r/issues/1","labels":[{"name":"bug"}],"assignees":[]}, @@ -484,15 +485,15 @@ func TestList_QueryEncoding(t *testing.T) { { name: "open + labels + assignee + limit", filter: domain.ListFilter{State: domain.ListOpen, Labels: []string{"bug", "help wanted"}, Assignee: "alice", Limit: 50}, - wantQ: map[string]string{"state": "open", "labels": "bug,help wanted", "assignee": "alice", "per_page": "50"}, + wantQ: map[string]string{"state": "open", "labels": "bug,help wanted", "assignee": "alice", "per_page": "100"}, }, { name: "closed only", filter: domain.ListFilter{State: domain.ListClosed}, - wantQ: map[string]string{"state": "closed", "per_page": "30"}, + wantQ: map[string]string{"state": "closed", "per_page": "100"}, }, { - name: "limit capped at 100", + name: "large total limit still uses max page size", filter: domain.ListFilter{Limit: 9999}, wantQ: map[string]string{"state": "all", "per_page": "100"}, }, @@ -517,6 +518,324 @@ func TestList_QueryEncoding(t *testing.T) { } } +func TestList_PaginatesAcrossLinkNext(t *testing.T) { + f := newFakeGH(t) + f.on("GET", "/repos/o/r/issues", func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Query().Get("page") { + case "": + w.Header().Set("Link", `<`+f.server.URL+`/repos/o/r/issues?state=all&per_page=100&page=2>; rel="next"`) + _, _ = w.Write([]byte(`[{"number":1,"title":"first","state":"open","html_url":"https://github.com/o/r/issues/1"}]`)) + case "2": + _, _ = w.Write([]byte(`[{"number":2,"title":"second","state":"open","html_url":"https://github.com/o/r/issues/2"}]`)) + default: + t.Fatalf("unexpected page %q", r.URL.Query().Get("page")) + } + }) + tr := newTrackerForTest(t, f) + + issues, err := tr.List(ctx(), domain.TrackerRepo{Provider: domain.TrackerProviderGitHub, Native: "o/r"}, domain.ListFilter{}) + if err != nil { + t.Fatalf("List: %v", err) + } + if len(issues) != 2 || issues[0].ID.Native != "o/r#1" || issues[1].ID.Native != "o/r#2" { + t.Fatalf("issues = %#v, want both pages in order", issues) + } +} + +func TestList_ConditionalRevalidationContinuesCachedPageChain(t *testing.T) { + f := newFakeGH(t) + pageCalls := map[string]int{} + f.on("GET", "/repos/o/r/issues", func(w http.ResponseWriter, r *http.Request) { + page := r.URL.Query().Get("page") + pageCalls[page]++ + switch { + case page == "" && pageCalls[page] == 1: + if got := r.Header.Get("If-None-Match"); got != "" { + t.Errorf("first page1 If-None-Match = %q, want empty", got) + } + w.Header().Set("ETag", `"e1"`) + w.Header().Set("Link", `<`+f.server.URL+`/repos/o/r/issues?state=all&per_page=100&page=2>; rel="next"`) + _, _ = w.Write([]byte(`[{"number":1,"title":"first","state":"open","html_url":"https://github.com/o/r/issues/1"}]`)) + case page == "2" && pageCalls[page] == 1: + if got := r.Header.Get("If-None-Match"); got != "" { + t.Errorf("first page2 If-None-Match = %q, want empty", got) + } + w.Header().Set("ETag", `"e2"`) + _, _ = w.Write([]byte(`[{"number":2,"title":"second","state":"open","html_url":"https://github.com/o/r/issues/2"}]`)) + case page == "" && pageCalls[page] == 2: + if got := r.Header.Get("If-None-Match"); got != `"e1"` { + t.Errorf("second page1 If-None-Match = %q, want \"e1\"", got) + } + w.WriteHeader(http.StatusNotModified) + case page == "2" && pageCalls[page] == 2: + if got := r.Header.Get("If-None-Match"); got != `"e2"` { + t.Errorf("second page2 If-None-Match = %q, want \"e2\"", got) + } + w.WriteHeader(http.StatusNotModified) + default: + t.Fatalf("unexpected page=%q call=%d", page, pageCalls[page]) + } + }) + tr := newTrackerForTest(t, f) + repo := domain.TrackerRepo{Provider: domain.TrackerProviderGitHub, Native: "o/r"} + + first, err := tr.List(ctx(), repo, domain.ListFilter{}) + if err != nil { + t.Fatalf("first List: %v", err) + } + second, err := tr.List(ctx(), repo, domain.ListFilter{}) + if err != nil { + t.Fatalf("second List: %v", err) + } + if !reflect.DeepEqual(second, first) { + t.Fatalf("second issues = %#v\nwant %#v", second, first) + } + if len(second) != 2 { + t.Fatalf("second len = %d, want both cached pages", len(second)) + } +} + +func TestList_PageCountShrinkIgnoresOrphanedCachedPage(t *testing.T) { + f := newFakeGH(t) + var page1Calls int + f.on("GET", "/repos/o/r/issues", func(w http.ResponseWriter, r *http.Request) { + page := r.URL.Query().Get("page") + if page == "2" { + w.Header().Set("ETag", `"old-page-2"`) + _, _ = w.Write([]byte(`[{"number":2,"title":"old second","state":"open","html_url":"https://github.com/o/r/issues/2"}]`)) + return + } + page1Calls++ + switch page1Calls { + case 1: + w.Header().Set("ETag", `"old-page-1"`) + w.Header().Set("Link", `<`+f.server.URL+`/repos/o/r/issues?state=all&per_page=100&page=2>; rel="next"`) + _, _ = w.Write([]byte(`[{"number":1,"title":"old first","state":"open","html_url":"https://github.com/o/r/issues/1"}]`)) + case 2: + if got := r.Header.Get("If-None-Match"); got != `"old-page-1"` { + t.Errorf("second page1 If-None-Match = %q, want \"old-page-1\"", got) + } + w.Header().Set("ETag", `"new-page-1"`) + _, _ = w.Write([]byte(`[{"number":3,"title":"only remaining","state":"open","html_url":"https://github.com/o/r/issues/3"}]`)) + default: + t.Fatalf("unexpected page1 call #%d", page1Calls) + } + }) + tr := newTrackerForTest(t, f) + repo := domain.TrackerRepo{Provider: domain.TrackerProviderGitHub, Native: "o/r"} + + first, err := tr.List(ctx(), repo, domain.ListFilter{}) + if err != nil { + t.Fatalf("first List: %v", err) + } + if len(first) != 2 { + t.Fatalf("first len = %d, want two cached pages", len(first)) + } + second, err := tr.List(ctx(), repo, domain.ListFilter{}) + if err != nil { + t.Fatalf("second List: %v", err) + } + if len(second) != 1 || second[0].ID.Native != "o/r#3" { + t.Fatalf("second issues = %#v, want only refreshed page 1", second) + } +} + +func TestParseLinkNext(t *testing.T) { + baseURL := "https://api.github.com" + cases := []struct { + name string + link string + want string + }{ + { + name: "quoted next strips absolute host", + link: `; rel="next"`, + want: "/repos/o/r/issues?state=all&per_page=100&page=2", + }, + { + name: "unquoted next among multiple links", + link: `; rel=prev, ; rel=next`, + want: "/repos/o/r/issues?page=3", + }, + { + name: "multiple rel values", + link: `; rel="last next"`, + want: "/repos/o/r/issues?page=4", + }, + { + name: "relative path", + link: `; rel="next"`, + want: "/repos/o/r/issues?page=2", + }, + { + name: "no next", + link: `; rel="prev"`, + want: "", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := parseLinkNext(tc.link, baseURL); got != tc.want { + t.Fatalf("parseLinkNext() = %q, want %q", got, tc.want) + } + }) + } +} + +func TestList_ConditionalRevalidationReturns304CachedIssues(t *testing.T) { + f := newFakeGH(t) + var calls int + f.on("GET", "/repos/o/r/issues", func(w http.ResponseWriter, r *http.Request) { + calls++ + switch calls { + case 1: + if got := r.Header.Get("If-None-Match"); got != "" { + t.Errorf("first If-None-Match = %q, want empty", got) + } + w.Header().Set("ETag", `"abc"`) + _, _ = w.Write([]byte(`[ + {"number":1,"title":"first","body":"b1","state":"open","html_url":"https://github.com/o/r/issues/1"}, + {"number":2,"title":"second","body":"b2","state":"closed","state_reason":"completed","html_url":"https://github.com/o/r/issues/2"} + ]`)) + case 2: + if got := r.Header.Get("If-None-Match"); got != `"abc"` { + t.Errorf("second If-None-Match = %q, want \"abc\"", got) + } + w.WriteHeader(http.StatusNotModified) + default: + t.Fatalf("unexpected List call #%d", calls) + } + }) + tr := newTrackerForTest(t, f) + repo := domain.TrackerRepo{Provider: domain.TrackerProviderGitHub, Native: "o/r"} + + first, err := tr.List(ctx(), repo, domain.ListFilter{}) + if err != nil { + t.Fatalf("first List: %v", err) + } + second, err := tr.List(ctx(), repo, domain.ListFilter{}) + if err != nil { + t.Fatalf("second List: %v", err) + } + if !reflect.DeepEqual(second, first) { + t.Fatalf("second issues = %#v\nwant %#v", second, first) + } + reqs := f.calls() + if len(reqs) != 2 { + t.Fatalf("HTTP calls = %d, want 2", len(reqs)) + } + if got := reqs[1].IfNoneMatch; got != `"abc"` { + t.Fatalf("recorded If-None-Match = %q, want \"abc\"", got) + } +} + +func TestList_ETagUpdatesWhenIssuesChange(t *testing.T) { + f := newFakeGH(t) + var calls int + f.on("GET", "/repos/o/r/issues", func(w http.ResponseWriter, r *http.Request) { + calls++ + switch calls { + case 1: + w.Header().Set("ETag", `"v1"`) + _, _ = w.Write([]byte(`[{"number":1,"title":"old","state":"open","html_url":"https://github.com/o/r/issues/1"}]`)) + case 2: + if got := r.Header.Get("If-None-Match"); got != `"v1"` { + t.Errorf("second If-None-Match = %q, want \"v1\"", got) + } + w.Header().Set("ETag", `"v2"`) + _, _ = w.Write([]byte(`[{"number":2,"title":"new","state":"open","html_url":"https://github.com/o/r/issues/2"}]`)) + case 3: + if got := r.Header.Get("If-None-Match"); got != `"v2"` { + t.Errorf("third If-None-Match = %q, want \"v2\"", got) + } + w.WriteHeader(http.StatusNotModified) + default: + t.Fatalf("unexpected List call #%d", calls) + } + }) + tr := newTrackerForTest(t, f) + repo := domain.TrackerRepo{Provider: domain.TrackerProviderGitHub, Native: "o/r"} + + if _, err := tr.List(ctx(), repo, domain.ListFilter{}); err != nil { + t.Fatalf("first List: %v", err) + } + second, err := tr.List(ctx(), repo, domain.ListFilter{}) + if err != nil { + t.Fatalf("second List: %v", err) + } + if len(second) != 1 || second[0].ID.Native != "o/r#2" || second[0].Title != "new" { + t.Fatalf("second issues = %#v, want new issue", second) + } + if _, err := tr.List(ctx(), repo, domain.ListFilter{}); err != nil { + t.Fatalf("third List: %v", err) + } +} + +func TestList_SeparateCacheKeyPerFilter(t *testing.T) { + f := newFakeGH(t) + var calls int + f.on("GET", "/repos/o/r/issues", func(w http.ResponseWriter, r *http.Request) { + calls++ + switch calls { + case 1: + if got := r.Header.Get("If-None-Match"); got != "" { + t.Errorf("first If-None-Match = %q, want empty", got) + } + w.Header().Set("ETag", `"bug-etag"`) + _, _ = w.Write([]byte(`[{"number":1,"title":"bug","state":"open","html_url":"https://github.com/o/r/issues/1"}]`)) + case 2: + if got := r.Header.Get("If-None-Match"); got != "" { + t.Errorf("second If-None-Match = %q, want empty for different filter", got) + } + w.Header().Set("ETag", `"docs-etag"`) + _, _ = w.Write([]byte(`[{"number":2,"title":"docs","state":"open","html_url":"https://github.com/o/r/issues/2"}]`)) + default: + t.Fatalf("unexpected List call #%d", calls) + } + }) + tr := newTrackerForTest(t, f) + repo := domain.TrackerRepo{Provider: domain.TrackerProviderGitHub, Native: "o/r"} + + first, err := tr.List(ctx(), repo, domain.ListFilter{Labels: []string{"bug"}}) + if err != nil { + t.Fatalf("first List: %v", err) + } + second, err := tr.List(ctx(), repo, domain.ListFilter{Labels: []string{"docs"}}) + if err != nil { + t.Fatalf("second List: %v", err) + } + if len(first) != 1 || first[0].Title != "bug" { + t.Fatalf("first issues = %#v, want bug", first) + } + if len(second) != 1 || second[0].Title != "docs" { + t.Fatalf("second issues = %#v, want docs", second) + } +} + +func TestList_NoETagHeaderNotCached(t *testing.T) { + f := newFakeGH(t) + var calls int + f.on("GET", "/repos/o/r/issues", func(w http.ResponseWriter, r *http.Request) { + calls++ + if got := r.Header.Get("If-None-Match"); got != "" { + t.Errorf("call #%d If-None-Match = %q, want empty", calls, got) + } + _, _ = w.Write([]byte(`[{"number":1,"title":"uncached","state":"open","html_url":"https://github.com/o/r/issues/1"}]`)) + }) + tr := newTrackerForTest(t, f) + repo := domain.TrackerRepo{Provider: domain.TrackerProviderGitHub, Native: "o/r"} + + if _, err := tr.List(ctx(), repo, domain.ListFilter{}); err != nil { + t.Fatalf("first List: %v", err) + } + if _, err := tr.List(ctx(), repo, domain.ListFilter{}); err != nil { + t.Fatalf("second List: %v", err) + } + if got := len(f.calls()); got != 2 { + t.Fatalf("HTTP calls = %d, want 2", got) + } +} + func TestList_RejectsWrongProvider(t *testing.T) { f := newFakeGH(t) tr := newTrackerForTest(t, f) diff --git a/backend/internal/cli/project.go b/backend/internal/cli/project.go index ff74dec8e..08e9a7439 100644 --- a/backend/internal/cli/project.go +++ b/backend/internal/cli/project.go @@ -85,18 +85,27 @@ type roleOverride struct { AgentConfig agentConfig `json:"agentConfig,omitempty"` } +// trackerIntakeConfig mirrors domain.TrackerIntakeConfig. +type trackerIntakeConfig struct { + Enabled bool `json:"enabled,omitempty"` + Provider string `json:"provider,omitempty"` + Repo string `json:"repo,omitempty"` + Assignee string `json:"assignee,omitempty"` +} + // projectConfig mirrors the daemon's typed domain.ProjectConfig for the CLI // client. The CLI sets common fields via flags and the whole object via // --config-json. type projectConfig struct { - DefaultBranch string `json:"defaultBranch,omitempty"` - SessionPrefix string `json:"sessionPrefix,omitempty"` - Env map[string]string `json:"env,omitempty"` - Symlinks []string `json:"symlinks,omitempty"` - PostCreate []string `json:"postCreate,omitempty"` - AgentConfig agentConfig `json:"agentConfig,omitempty"` - Worker roleOverride `json:"worker,omitempty"` - Orchestrator roleOverride `json:"orchestrator,omitempty"` + DefaultBranch string `json:"defaultBranch,omitempty"` + SessionPrefix string `json:"sessionPrefix,omitempty"` + Env map[string]string `json:"env,omitempty"` + Symlinks []string `json:"symlinks,omitempty"` + PostCreate []string `json:"postCreate,omitempty"` + AgentConfig agentConfig `json:"agentConfig,omitempty"` + Worker roleOverride `json:"worker,omitempty"` + Orchestrator roleOverride `json:"orchestrator,omitempty"` + TrackerIntake trackerIntakeConfig `json:"trackerIntake,omitempty"` } // setConfigRequest mirrors the daemon's SetConfigInput body for @@ -115,6 +124,9 @@ type projectSetConfigOptions struct { env []string symlink []string postCreate []string + trackerIntake bool + trackerRepo string + trackerAssignee string configJSON string clear bool json bool @@ -259,7 +271,7 @@ func newProjectSetConfigCommand(ctx *commandContext) *cobra.Command { Use: "set-config ", Short: "Set the per-project config", Long: "Replace a project's per-project config (branch, session prefix, env, " + - "symlinks, post-create, agent model/permissions, role overrides). The config " + + "symlinks, post-create, agent model/permissions, role overrides, tracker intake). The config " + "is resolved when a session spawns.\n\n" + "Set fields via flags, pass the whole object with --config-json, or --clear " + "to remove all config.", @@ -300,6 +312,9 @@ func newProjectSetConfigCommand(ctx *commandContext) *cobra.Command { f.StringArrayVar(&opts.env, "env", nil, "Env var KEY=VALUE forwarded into sessions (repeatable)") f.StringArrayVar(&opts.symlink, "symlink", nil, "Repo-relative path to symlink into workspaces (repeatable)") f.StringArrayVar(&opts.postCreate, "post-create", nil, "Command to run after workspace creation (repeatable)") + f.BoolVar(&opts.trackerIntake, "tracker-intake", false, "Enable GitHub issue intake for matching issues") + f.StringVar(&opts.trackerRepo, "tracker-repo", "", "GitHub repo for issue intake (owner/repo; default: derive from git origin)") + f.StringVar(&opts.trackerAssignee, "tracker-assignee", "", "GitHub issue assignee required for intake eligibility") f.StringVar(&opts.configJSON, "config-json", "", "Full config as a JSON object (overrides field flags)") f.BoolVar(&opts.clear, "clear", false, "Clear all config") f.BoolVar(&opts.json, "json", false, "Output the updated project as JSON") @@ -335,6 +350,12 @@ func buildProjectConfig(opts projectSetConfigOptions) (projectConfig, error) { AgentConfig: agentConfig{Model: opts.model, Permissions: opts.permission}, Worker: roleOverride{Agent: opts.workerAgent}, Orchestrator: roleOverride{Agent: opts.orchestratorAgent}, + TrackerIntake: trackerIntakeConfig{ + Enabled: opts.trackerIntake, + Provider: trackerProviderForFlags(opts), + Repo: opts.trackerRepo, + Assignee: opts.trackerAssignee, + }, } if reflect.DeepEqual(cfg, projectConfig{}) { return projectConfig{}, usageError{errors.New("usage: provide at least one config flag, --config-json, or --clear")} @@ -342,6 +363,13 @@ func buildProjectConfig(opts projectSetConfigOptions) (projectConfig, error) { return cfg, nil } +func trackerProviderForFlags(opts projectSetConfigOptions) string { + if opts.trackerIntake || opts.trackerRepo != "" || opts.trackerAssignee != "" { + return "github" + } + return "" +} + // parseEnvPairs turns repeated KEY=VALUE flags into a map. func parseEnvPairs(pairs []string) (map[string]string, error) { if len(pairs) == 0 { diff --git a/backend/internal/cli/project_test.go b/backend/internal/cli/project_test.go index eea7c924a..2099d5ad5 100644 --- a/backend/internal/cli/project_test.go +++ b/backend/internal/cli/project_test.go @@ -12,6 +12,7 @@ import ( type projectCapture struct { method string path string + body []byte } func projectServer(t *testing.T, status int, respBody string) (*httptest.Server, *projectCapture) { @@ -20,6 +21,7 @@ func projectServer(t *testing.T, status int, respBody string) (*httptest.Server, srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { capture.method = r.Method capture.path = r.URL.Path + capture.body, _ = io.ReadAll(r.Body) if !strings.HasPrefix(r.URL.Path, "/api/v1/projects") { http.NotFound(w, r) return @@ -32,6 +34,63 @@ func projectServer(t *testing.T, status int, respBody string) (*httptest.Server, return srv, capture } +func TestProjectSetConfig_TrackerIntakeFlags(t *testing.T) { + cfg := setConfigEnv(t) + srv, capture := projectServer(t, http.StatusOK, `{"project":{"id":"demo","path":"/repo/demo"}}`) + writeRunFileFor(t, cfg, srv) + + _, errOut, err := executeCLI(t, Deps{ + ProcessAlive: func(int) bool { return true }, + }, "project", "set-config", "demo", "--tracker-intake", "--tracker-repo", "acme/demo", "--tracker-assignee", "alice") + if err != nil { + t.Fatalf("unexpected error: %v\nstderr=%s", err, errOut) + } + if capture.method != http.MethodPut || capture.path != "/api/v1/projects/demo/config" { + t.Fatalf("request = %s %s, want PUT /api/v1/projects/demo/config", capture.method, capture.path) + } + var got setConfigRequest + if err := json.Unmarshal(capture.body, &got); err != nil { + t.Fatalf("decode request: %v\nbody=%s", err, capture.body) + } + if !got.Config.TrackerIntake.Enabled || got.Config.TrackerIntake.Provider != "github" || got.Config.TrackerIntake.Repo != "acme/demo" || got.Config.TrackerIntake.Assignee != "alice" { + t.Fatalf("tracker intake request = %#v", got.Config.TrackerIntake) + } +} + +func TestProjectSetConfig_TrackerIntakeJSON(t *testing.T) { + cfg := setConfigEnv(t) + srv, capture := projectServer(t, http.StatusOK, `{"project":{"id":"demo","path":"/repo/demo"}}`) + writeRunFileFor(t, cfg, srv) + + _, errOut, err := executeCLI(t, Deps{ + ProcessAlive: func(int) bool { return true }, + }, "project", "set-config", "demo", "--config-json", `{"trackerIntake":{"enabled":true,"provider":"github","assignee":"alice"}}`) + if err != nil { + t.Fatalf("unexpected error: %v\nstderr=%s", err, errOut) + } + var got setConfigRequest + if err := json.Unmarshal(capture.body, &got); err != nil { + t.Fatalf("decode request: %v\nbody=%s", err, capture.body) + } + if !got.Config.TrackerIntake.Enabled || got.Config.TrackerIntake.Provider != "github" || got.Config.TrackerIntake.Assignee != "alice" { + t.Fatalf("tracker intake request = %#v", got.Config.TrackerIntake) + } +} + +func TestBuildProjectConfigTrackerIntakeFlags(t *testing.T) { + got, err := buildProjectConfig(projectSetConfigOptions{ + trackerIntake: true, + trackerRepo: "acme/demo", + trackerAssignee: "alice", + }) + if err != nil { + t.Fatal(err) + } + if !got.TrackerIntake.Enabled || got.TrackerIntake.Provider != "github" || got.TrackerIntake.Repo != "acme/demo" || got.TrackerIntake.Assignee != "alice" { + t.Fatalf("tracker intake config = %#v", got.TrackerIntake) + } +} + func TestProjectList_Success(t *testing.T) { cfg := setConfigEnv(t) srv, capture := projectServer(t, http.StatusOK, `{"projects":[{"id":"zeta","name":"Zeta","sessionPrefix":"zeta"},{"id":"alpha","name":"Alpha","sessionPrefix":"alpha","resolveError":"config missing"}]}`) diff --git a/backend/internal/daemon/daemon.go b/backend/internal/daemon/daemon.go index 4f798cb7c..ab7c2b2eb 100644 --- a/backend/internal/daemon/daemon.go +++ b/backend/internal/daemon/daemon.go @@ -128,6 +128,7 @@ func Run() error { } return fmt.Errorf("wire session service: %w", err) } + lcStack.trackerDone = startTrackerIntake(ctx, store, sessionSvc, log) previewDone := preview.NewPoller(store, sessionSvc, "http://"+cfg.Addr(), preview.PollerConfig{Logger: log}).Start(ctx) srv, err := httpd.NewWithDeps(cfg, log, termMgr, httpd.APIDeps{ diff --git a/backend/internal/daemon/lifecycle_wiring.go b/backend/internal/daemon/lifecycle_wiring.go index c5391fe19..5113d9dcb 100644 --- a/backend/internal/daemon/lifecycle_wiring.go +++ b/backend/internal/daemon/lifecycle_wiring.go @@ -34,9 +34,10 @@ type lifecycleStack struct { // LCM is the Lifecycle Manager (the canonical write path). It is exposed so // startSession can share the same reducer the reaper drives, rather than // standing up a second store+LCM pair that would diverge under writes. - LCM *lifecycle.Manager - reaperDone <-chan struct{} - scmDone <-chan struct{} + LCM *lifecycle.Manager + reaperDone <-chan struct{} + scmDone <-chan struct{} + trackerDone <-chan struct{} } // startLifecycle constructs the Lifecycle Manager over the store and starts the @@ -56,6 +57,9 @@ func (l *lifecycleStack) Stop() { if l.scmDone != nil { <-l.scmDone } + if l.trackerDone != nil { + <-l.trackerDone + } } // sessionLifecycle is the narrow surface of sessionmanager.Manager used for diff --git a/backend/internal/daemon/tracker_intake_wiring.go b/backend/internal/daemon/tracker_intake_wiring.go new file mode 100644 index 000000000..cfdd15576 --- /dev/null +++ b/backend/internal/daemon/tracker_intake_wiring.go @@ -0,0 +1,132 @@ +package daemon + +import ( + "context" + "errors" + "log/slog" + "strings" + "sync" + "time" + + trackergithub "github.com/aoagents/agent-orchestrator/backend/internal/adapters/tracker/github" + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + trackerintake "github.com/aoagents/agent-orchestrator/backend/internal/observe/trackerintake" + "github.com/aoagents/agent-orchestrator/backend/internal/ports" + aoprocess "github.com/aoagents/agent-orchestrator/backend/internal/process" + sessionsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/session" + "github.com/aoagents/agent-orchestrator/backend/internal/storage/sqlite" +) + +// startTrackerIntake wires the opt-in GitHub issue-intake loop. The observer +// always runs — Poll re-reads each project's config on every tick and skips +// projects with intake disabled, so a project enabling intake after daemon +// boot is picked up on the next tick without a restart. The adapter itself +// stays lazy so daemon readiness is not blocked by credential probing or a gh +// CLI call, and no token is resolved until some enabled project is actually +// polled. +func startTrackerIntake(ctx context.Context, store *sqlite.Store, sessions *sessionsvc.Service, logger *slog.Logger) <-chan struct{} { + resolver := trackerintake.SingleTrackerResolver{ + Provider: domain.TrackerProviderGitHub, + Adapter: newLazyGitHubTracker(logger), + } + observer := trackerintake.New(resolver, store, sessions, trackerintake.Config{Logger: logger}) + return observer.Start(ctx) +} + +// --------------------------------------------------------------------------- +// GitHub lazy adapter (token sourced from env or gh CLI fallback) +// --------------------------------------------------------------------------- + +type lazyGitHubTracker struct { + logger *slog.Logger + tokens *trackerTokenSource + mu sync.Mutex + tracker ports.Tracker +} + +func newLazyGitHubTracker(logger *slog.Logger) *lazyGitHubTracker { + return &lazyGitHubTracker{logger: logger, tokens: &trackerTokenSource{}} +} + +func (t *lazyGitHubTracker) Get(ctx context.Context, id domain.TrackerID) (domain.Issue, error) { + tracker, err := t.resolve() + if err != nil { + return domain.Issue{}, err + } + return tracker.Get(ctx, id) +} + +func (t *lazyGitHubTracker) List(ctx context.Context, repo domain.TrackerRepo, filter domain.ListFilter) ([]domain.Issue, error) { + tracker, err := t.resolve() + if err != nil { + return nil, err + } + return tracker.List(ctx, repo, filter) +} + +func (t *lazyGitHubTracker) Preflight(ctx context.Context) error { + tracker, err := t.resolve() + if err != nil { + return err + } + return tracker.Preflight(ctx) +} + +func (t *lazyGitHubTracker) resolve() (ports.Tracker, error) { + t.mu.Lock() + defer t.mu.Unlock() + if t.tracker != nil { + return t.tracker, nil + } + tracker, err := trackergithub.New(trackergithub.Options{Token: t.tokens}) + if err != nil { + if errors.Is(err, trackergithub.ErrNoToken) && t.logger != nil { + t.logger.Warn("tracker intake disabled: no usable GitHub token", "err", err) + } + return nil, err + } + t.tracker = tracker + return tracker, nil +} + +const ( + trackerTokenCacheTTL = 5 * time.Minute + trackerTokenCommandTimeout = 5 * time.Second +) + +// trackerTokenSource mirrors the SCM credential precedence while returning the +// tracker adapter's own ErrNoToken sentinel. +type trackerTokenSource struct { + mu sync.Mutex + token string + expiresAt time.Time +} + +func (s *trackerTokenSource) Token(ctx context.Context) (string, error) { + env := trackergithub.EnvTokenSource{EnvVars: []string{"AO_GITHUB_TOKEN"}} + if tok, err := env.Token(ctx); err == nil { + return tok, nil + } else if !errors.Is(err, trackergithub.ErrNoToken) { + return "", err + } + + s.mu.Lock() + defer s.mu.Unlock() + now := time.Now() + if s.token != "" && now.Before(s.expiresAt) { + return s.token, nil + } + cmdCtx, cancel := context.WithTimeout(ctx, trackerTokenCommandTimeout) + defer cancel() + out, err := aoprocess.CommandContext(cmdCtx, "gh", "auth", "token").Output() + if err != nil { + return "", err + } + token := strings.TrimSpace(string(out)) + if token == "" { + return "", trackergithub.ErrNoToken + } + s.token = token + s.expiresAt = now.Add(trackerTokenCacheTTL) + return token, nil +} diff --git a/backend/internal/daemon/wiring_test.go b/backend/internal/daemon/wiring_test.go index c5a24402d..9a3eb2058 100644 --- a/backend/internal/daemon/wiring_test.go +++ b/backend/internal/daemon/wiring_test.go @@ -167,6 +167,58 @@ func TestWiring_StartSessionBuildsSessionService(t *testing.T) { } } +// TestStartTrackerIntake_RunsEvenWithoutEnabledProjects is a regression test: +// startTrackerIntake used to scan projects once at call time and skip starting +// the observer loop entirely when none had intake enabled yet. Poll() itself +// already re-reads project config on every tick, so a project enabling +// intake after daemon boot was silently never picked up until a restart. The +// loop must always start; Poll is what decides whether there's work to do. +func TestStartTrackerIntake_RunsEvenWithoutEnabledProjects(t *testing.T) { + store, err := sqlite.Open(t.TempDir()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = store.Close() }) + + log := slog.New(slog.NewTextHandler(io.Discard, nil)) + lcm := lifecycle.New(store, nil) + cfg := config.Config{DataDir: t.TempDir()} + rt := runtimeselect.New(nil) + messenger := newSessionMessenger(store, rt, log) + svc, _, _, err := startSession(cfg, rt, store, lcm, messenger, telemetryadapter.NoopSink{}, log) + if err != nil { + t.Fatalf("startSession: %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + done := startTrackerIntake(ctx, store, svc, log) + + select { + case <-done: + t.Fatal("startTrackerIntake returned an already-closed channel; observer loop did not start") + case <-time.After(20 * time.Millisecond): + } + + cancel() + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("observer did not stop after context cancellation") + } +} + +func TestTrackerTokenSourcePrefersAOGitHubToken(t *testing.T) { + t.Setenv("AO_GITHUB_TOKEN", "ao-token") + t.Setenv("GITHUB_TOKEN", "github-token") + token, err := (&trackerTokenSource{}).Token(context.Background()) + if err != nil { + t.Fatal(err) + } + if token != "ao-token" { + t.Fatalf("token = %q, want AO_GITHUB_TOKEN", token) + } +} + type captureRuntimeSender struct { handle ports.RuntimeHandle message string diff --git a/backend/internal/domain/projectconfig.go b/backend/internal/domain/projectconfig.go index 828890e39..15e1c918a 100644 --- a/backend/internal/domain/projectconfig.go +++ b/backend/internal/domain/projectconfig.go @@ -14,9 +14,8 @@ import ( // // Only fields with a live consumer are modeled: DefaultBranch, Env, Symlinks, // PostCreate, AgentConfig, and the role overrides are consumed at spawn; -// SessionPrefix feeds the display prefix. Settings whose consumers do not yet -// exist (tracker/SCM per-project config, prompt rules) are intentionally absent -// and land in focused follow-up PRs alongside the code that reads them. +// SessionPrefix feeds the display prefix. TrackerIntake feeds the background +// issue-intake loop. type ProjectConfig struct { // DefaultBranch is the base branch new session worktrees are created from. DefaultBranch string `json:"defaultBranch,omitempty"` @@ -41,6 +40,11 @@ type ProjectConfig struct { // triggered. It is configured independently of the Worker override; an empty // list falls back to claude-code (see ResolveReviewerHarness). Reviewers []ReviewerConfig `json:"reviewers,omitempty"` + + // TrackerIntake controls issue-driven worker spawning. It is opt-in and + // read-only toward the tracker in v1: matching issues spawn sessions, but the + // tracker is not commented on or transitioned. + TrackerIntake TrackerIntakeConfig `json:"trackerIntake,omitempty"` } // ReviewerConfig names one reviewer agent by harness. The harness is drawn from @@ -88,6 +92,7 @@ func (c ProjectConfig) WithDefaults() ProjectConfig { if c.DefaultBranch == "" { c.DefaultBranch = def.DefaultBranch } + c.TrackerIntake = c.TrackerIntake.WithDefaults() return c } @@ -124,6 +129,19 @@ func (c ProjectConfig) Validate() error { return fmt.Errorf("reviewers[%d].harness: unknown harness %q", i, rv.Harness) } } + if err := c.TrackerIntake.Validate(); err != nil { + return err + } + return nil +} + +func validateNoWhitespaceField(name, value string) error { + if value == "" { + return nil + } + if strings.TrimSpace(value) != value { + return fmt.Errorf("%s: must not have leading or trailing whitespace", name) + } return nil } diff --git a/backend/internal/domain/projectconfig_test.go b/backend/internal/domain/projectconfig_test.go index 2542c5653..d100708f6 100644 --- a/backend/internal/domain/projectconfig_test.go +++ b/backend/internal/domain/projectconfig_test.go @@ -29,6 +29,12 @@ func TestProjectConfigValidate(t *testing.T) { {"unknown reviewer harness", ProjectConfig{Reviewers: []ReviewerConfig{{Harness: "nope"}}}, true}, {"worker-only harness is not auto a reviewer", ProjectConfig{Reviewers: []ReviewerConfig{{Harness: ReviewerHarness(HarnessAider)}}}, true}, {"empty reviewer harness", ProjectConfig{Reviewers: []ReviewerConfig{{Harness: ""}}}, true}, + {"tracker intake assignee rule", ProjectConfig{TrackerIntake: TrackerIntakeConfig{Enabled: true, Assignee: "alice"}}, false}, + {"tracker intake explicit github", ProjectConfig{TrackerIntake: TrackerIntakeConfig{Enabled: true, Provider: TrackerProviderGitHub, Assignee: "alice"}}, false}, + {"tracker intake no rule", ProjectConfig{TrackerIntake: TrackerIntakeConfig{Enabled: true}}, true}, + {"tracker intake unknown provider", ProjectConfig{TrackerIntake: TrackerIntakeConfig{Enabled: true, Provider: "linear", Assignee: "alice"}}, true}, + {"tracker intake repo with whitespace", ProjectConfig{TrackerIntake: TrackerIntakeConfig{Enabled: true, Repo: " acme/demo", Assignee: "alice"}}, true}, + {"tracker intake assignee with whitespace", ProjectConfig{TrackerIntake: TrackerIntakeConfig{Enabled: true, Assignee: " alice"}}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -73,6 +79,16 @@ func TestProjectConfigWithDefaults(t *testing.T) { if got.AgentConfig.Model != "m" { t.Fatalf("WithDefaults dropped a set field: %#v", got.AgentConfig) } + + got = (ProjectConfig{TrackerIntake: TrackerIntakeConfig{Enabled: true, Assignee: "alice"}}).WithDefaults() + if got.TrackerIntake.Provider != TrackerProviderGitHub { + t.Fatalf("TrackerIntake.Provider = %q, want %q", got.TrackerIntake.Provider, TrackerProviderGitHub) + } + + got = (ProjectConfig{}).WithDefaults() + if got.TrackerIntake.Provider != "" { + t.Fatalf("disabled TrackerIntake.Provider = %q, want empty", got.TrackerIntake.Provider) + } } func TestResolveReviewerHarness(t *testing.T) { diff --git a/backend/internal/domain/tracker.go b/backend/internal/domain/tracker.go index fde1631b3..289d4347f 100644 --- a/backend/internal/domain/tracker.go +++ b/backend/internal/domain/tracker.go @@ -1,5 +1,10 @@ package domain +import ( + "fmt" + "strings" +) + // TrackerProvider identifies an issue-tracker provider implementation. type TrackerProvider string @@ -64,11 +69,56 @@ const ( // ListFilter is the query the Session Manager passes to Tracker.List. // Empty / zero values mean "no filter on this dimension". // -// Limit is the requested page size. The adapter applies its own default when -// zero and caps at the provider's per-page maximum. +// Limit is an optional total-result cap. Adapters choose their own provider +// page size. type ListFilter struct { State ListStateFilter `json:"state,omitempty"` Labels []string `json:"labels,omitempty"` Assignee string `json:"assignee,omitempty"` Limit int `json:"limit,omitempty"` } + +// TrackerIntakeConfig controls issue-driven worker spawning for a project. +// Enabled requires an explicit assignee eligibility rule so turning intake on +// cannot accidentally drain an entire issue backlog. +type TrackerIntakeConfig struct { + Enabled bool `json:"enabled,omitempty"` + // Provider defaults to github when Enabled is true. + Provider TrackerProvider `json:"provider,omitempty" enum:"github"` + // Repo is the GitHub-native repository key ("owner/repo"). When empty, the + // intake loop derives it from the project's repo origin URL. GitHub only. + Repo string `json:"repo,omitempty"` + // Assignee narrows eligible issues to one assignee. Provider-specific values + // such as "*" are passed through unchanged. + Assignee string `json:"assignee,omitempty"` +} + +// WithDefaults fills the provider only when intake is enabled. Disabled intake +// leaves the zero value untouched so empty project configs still store as NULL. +func (c TrackerIntakeConfig) WithDefaults() TrackerIntakeConfig { + if c.Enabled && c.Provider == "" { + c.Provider = TrackerProviderGitHub + } + return c +} + +// Validate rejects accidental broad intake and unknown providers. +func (c TrackerIntakeConfig) Validate() error { + if !c.Enabled { + return nil + } + c = c.WithDefaults() + if c.Enabled && c.Provider != TrackerProviderGitHub { + return fmt.Errorf("trackerIntake.provider: unsupported provider %q", c.Provider) + } + if err := validateNoWhitespaceField("trackerIntake.repo", c.Repo); err != nil { + return err + } + if err := validateNoWhitespaceField("trackerIntake.assignee", c.Assignee); err != nil { + return err + } + if strings.TrimSpace(c.Assignee) == "" { + return fmt.Errorf("trackerIntake: assignee is required when enabled") + } + return nil +} diff --git a/backend/internal/httpd/apispec/openapi.yaml b/backend/internal/httpd/apispec/openapi.yaml index c194ffa0b..2d7e7bf46 100644 --- a/backend/internal/httpd/apispec/openapi.yaml +++ b/backend/internal/httpd/apispec/openapi.yaml @@ -1938,6 +1938,8 @@ components: items: type: string type: array + trackerIntake: + $ref: '#/components/schemas/TrackerIntakeConfig' worker: $ref: '#/components/schemas/RoleOverride' type: object @@ -2546,6 +2548,19 @@ components: - runId - verdict type: object + TrackerIntakeConfig: + properties: + assignee: + type: string + enabled: + type: boolean + provider: + enum: + - github + type: string + repo: + type: string + type: object TriggerReviewResponse: properties: reviewerHandleId: diff --git a/backend/internal/httpd/apispec/specgen/build.go b/backend/internal/httpd/apispec/specgen/build.go index 21570e358..564328684 100644 --- a/backend/internal/httpd/apispec/specgen/build.go +++ b/backend/internal/httpd/apispec/specgen/build.go @@ -124,13 +124,14 @@ var schemaNames = map[string]string{ // httpd/envelope "EnvelopeAPIError": "APIError", // domain - "DomainProjectID": "ProjectID", - "DomainSessionID": "SessionID", - "DomainIssueID": "IssueID", - "DomainSession": "Session", - "DomainProjectConfig": "ProjectConfig", - "DomainAgentConfig": "AgentConfig", - "DomainRoleOverride": "RoleOverride", + "DomainProjectID": "ProjectID", + "DomainSessionID": "SessionID", + "DomainIssueID": "IssueID", + "DomainSession": "Session", + "DomainProjectConfig": "ProjectConfig", + "DomainTrackerIntakeConfig": "TrackerIntakeConfig", + "DomainAgentConfig": "AgentConfig", + "DomainRoleOverride": "RoleOverride", // httpd/controllers (wire envelopes) "ControllersListProjectsResponse": "ListProjectsResponse", "ControllersProjectResponse": "ProjectResponse", diff --git a/backend/internal/observe/trackerintake/observer.go b/backend/internal/observe/trackerintake/observer.go new file mode 100644 index 000000000..d3e46964b --- /dev/null +++ b/backend/internal/observe/trackerintake/observer.go @@ -0,0 +1,369 @@ +// Package trackerintake implements the opt-in issue-intake observer. It polls a +// project's configured tracker for eligible issues and starts one worker session +// per issue, leaving PR/lifecycle handling to the existing observers. +package trackerintake + +import ( + "context" + "fmt" + "log/slog" + "net/url" + "strings" + "time" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + "github.com/aoagents/agent-orchestrator/backend/internal/observe" + "github.com/aoagents/agent-orchestrator/backend/internal/ports" +) + +const ( + // DefaultTickInterval is intentionally slower than runtime liveness checks: + // intake is a backlog sweep, not an interactive status surface. + DefaultTickInterval = time.Minute + // DefaultFailureBackoff suppresses repeated polls for a project after an + // intake failure. The observer retries automatically after this window. + DefaultFailureBackoff = 5 * time.Minute + // maxIntakePromptLen mirrors the session HTTP prompt limit. Intake uses the + // session service directly, so it must enforce the same boundary itself. + maxIntakePromptLen = 4096 + + intakePromptTruncationNotice = "\n\n[Issue content truncated to fit the session prompt limit. Open the linked issue for the full details.]\n" + intakePromptFooter = "\nImplement the requested change in this repository, run the relevant checks, and open or update a pull request when ready." +) + +// Store is the durable read surface the observer needs. +type Store interface { + ListProjects(ctx context.Context) ([]domain.ProjectRecord, error) + ListAllSessions(ctx context.Context) ([]domain.SessionRecord, error) +} + +// Spawner is the session creation surface used by intake. +type Spawner interface { + Spawn(ctx context.Context, cfg ports.SpawnConfig) (domain.Session, error) +} + +// TrackerResolver picks the tracker adapter for a project's configured +// provider. +type TrackerResolver interface { + Resolve(provider domain.TrackerProvider) (ports.Tracker, error) +} + +// SingleTrackerResolver returns the same tracker for one specific provider and +// refuses every other provider. It exists so single-provider deployments don't +// need to construct a map. +type SingleTrackerResolver struct { + Provider domain.TrackerProvider + Adapter ports.Tracker +} + +// Resolve returns the wrapped adapter when the requested provider matches, or +// when the resolver was constructed without a provider pin. +func (s SingleTrackerResolver) Resolve(provider domain.TrackerProvider) (ports.Tracker, error) { + if s.Adapter == nil { + return nil, fmt.Errorf("tracker intake: no adapter for provider %q", provider) + } + if s.Provider == "" || provider == "" || provider == s.Provider { + return s.Adapter, nil + } + return nil, fmt.Errorf("tracker intake: no adapter for provider %q", provider) +} + +// Config holds optional observer knobs. Zero values use production defaults. +type Config struct { + Tick time.Duration + FailureBackoff time.Duration + Clock func() time.Time + Logger *slog.Logger +} + +// Observer polls configured projects and starts sessions for eligible issues. +type Observer struct { + resolver TrackerResolver + store Store + spawner Spawner + tick time.Duration + failureBackoff time.Duration + clock func() time.Time + logger *slog.Logger + backoffUntil map[string]time.Time +} + +// New constructs an Observer with safe defaults. +func New(resolver TrackerResolver, store Store, spawner Spawner, cfg Config) *Observer { + o := &Observer{resolver: resolver, store: store, spawner: spawner, tick: cfg.Tick, failureBackoff: cfg.FailureBackoff, clock: cfg.Clock, logger: cfg.Logger, backoffUntil: map[string]time.Time{}} + if o.tick <= 0 { + o.tick = DefaultTickInterval + } + if o.failureBackoff <= 0 { + o.failureBackoff = DefaultFailureBackoff + } + if o.clock == nil { + o.clock = time.Now + } + if o.logger == nil { + o.logger = slog.Default() + } + return o +} + +// Start launches the observer loop. The first poll runs immediately inside the +// goroutine, keeping daemon startup non-blocking. +func (o *Observer) Start(ctx context.Context) <-chan struct{} { + return observe.StartPollLoop(ctx, o.tick, o.Poll, o.logger, "tracker intake") +} + +// Poll runs one synchronous intake pass. Store discovery failures are returned +// because they prevent the pass from knowing the current world; provider and +// spawn failures are logged and skipped so one bad issue/project does not block +// the rest of the daemon. +func (o *Observer) Poll(ctx context.Context) error { + if err := ctx.Err(); err != nil { + return err + } + if o.resolver == nil || o.store == nil || o.spawner == nil { + return nil + } + now := o.clock().UTC() + projects, err := o.store.ListProjects(ctx) + if err != nil { + return err + } + enabledProjects := make([]domain.ProjectRecord, 0, len(projects)) + for _, project := range projects { + if project.Config.TrackerIntake.Enabled { + enabledProjects = append(enabledProjects, project) + } + } + if len(enabledProjects) == 0 { + return nil + } + sessions, err := o.store.ListAllSessions(ctx) + if err != nil { + return err + } + seen := seenIssueIDs(sessions) + for _, project := range enabledProjects { + if err := ctx.Err(); err != nil { + return err + } + if until, ok := o.backoffUntil[project.ID]; ok && now.Before(until) { + o.logger.Debug("tracker intake: project in failure backoff", "project", project.ID, "until", until) + continue + } + if failed := o.pollProject(ctx, project, seen); failed { + o.backoffUntil[project.ID] = now.Add(o.failureBackoff) + } else { + delete(o.backoffUntil, project.ID) + } + } + return nil +} + +// pollProject returns failed=true for conditions that should be retried after a +// backoff window rather than logged on every poll. +func (o *Observer) pollProject(ctx context.Context, project domain.ProjectRecord, seen map[domain.IssueID]bool) (failed bool) { + cfg := project.Config.TrackerIntake.WithDefaults() + if !cfg.Enabled { + return false + } + if err := cfg.Validate(); err != nil { + o.logger.Warn("tracker intake: skipping project with invalid config", "project", project.ID, "err", err) + return true + } + repo, ok := trackerRepo(project, cfg) + if !ok { + o.logger.Warn("tracker intake: skipping project without tracker scope", "project", project.ID, "provider", cfg.Provider, "origin", project.RepoOriginURL) + return true + } + tracker, err := o.resolver.Resolve(cfg.Provider) + if err != nil { + o.logger.Warn("tracker intake: no adapter for provider", "project", project.ID, "provider", cfg.Provider, "err", err) + return true + } + issues, err := tracker.List(ctx, repo, domain.ListFilter{ + State: domain.ListOpen, + Assignee: cfg.Assignee, + }) + if err != nil { + o.logger.Error("tracker intake: list issues failed", "project", project.ID, "repo", repo.Native, "err", err) + return true + } + var spawnFailed bool + for _, issue := range issues { + if ctx.Err() != nil { + return true + } + if issue.State != domain.IssueOpen { + continue + } + if !issueMatchesConfig(issue, cfg) { + continue + } + issueID := CanonicalIssueID(issue.ID) + if issueID == "" || seen[issueID] { + continue + } + if _, err := o.spawner.Spawn(ctx, ports.SpawnConfig{ + ProjectID: domain.ProjectID(project.ID), + IssueID: issueID, + Kind: domain.KindWorker, + Prompt: BuildIssuePrompt(issue), + }); err != nil { + o.logger.Error("tracker intake: spawn issue session failed", "project", project.ID, "issue", issueID, "err", err) + spawnFailed = true + continue + } + seen[issueID] = true + } + return spawnFailed +} + +func issueMatchesConfig(issue domain.Issue, cfg domain.TrackerIntakeConfig) bool { + assignee := strings.TrimSpace(cfg.Assignee) + switch { + case assignee == "": + return true + case assignee == "*": + return len(issue.Assignees) > 0 + case strings.EqualFold(assignee, "none"): + return len(issue.Assignees) == 0 + default: + return containsFold(issue.Assignees, assignee) + } +} + +func containsFold(values []string, needle string) bool { + for _, value := range values { + if strings.EqualFold(strings.TrimSpace(value), needle) { + return true + } + } + return false +} + +func seenIssueIDs(sessions []domain.SessionRecord) map[domain.IssueID]bool { + seen := make(map[domain.IssueID]bool, len(sessions)) + for _, sess := range sessions { + if sess.IssueID != "" { + seen[sess.IssueID] = true + } + } + return seen +} + +// CanonicalIssueID stores tracker issue ids in sessions.issue_id with the +// provider included, so future providers cannot collide on native ids. +func CanonicalIssueID(id domain.TrackerID) domain.IssueID { + provider := id.Provider + if provider == "" { + provider = domain.TrackerProviderGitHub + } + native := strings.TrimSpace(id.Native) + if native == "" { + return "" + } + return domain.IssueID(string(provider) + ":" + native) +} + +// BuildIssuePrompt turns normalized issue facts into the worker's initial task. +func BuildIssuePrompt(issue domain.Issue) string { + var b strings.Builder + fmt.Fprintf(&b, "Work on tracker issue %s.\n\n", CanonicalIssueID(issue.ID)) + if issue.Title != "" { + fmt.Fprintf(&b, "Title: %s\n", issue.Title) + } + if issue.URL != "" { + fmt.Fprintf(&b, "URL: %s\n", issue.URL) + } + if len(issue.Labels) > 0 { + fmt.Fprintf(&b, "Labels: %s\n", strings.Join(issue.Labels, ", ")) + } + if len(issue.Assignees) > 0 { + fmt.Fprintf(&b, "Assignees: %s\n", strings.Join(issue.Assignees, ", ")) + } + body := strings.TrimSpace(issue.Body) + if body != "" { + fmt.Fprintf(&b, "\nBody:\n%s\n", body) + } + b.WriteString(intakePromptFooter) + return capIntakePrompt(b.String()) +} + +func capIntakePrompt(prompt string) string { + if len(prompt) <= maxIntakePromptLen { + return prompt + } + prefix := strings.TrimSuffix(prompt, intakePromptFooter) + prefixBudget := maxIntakePromptLen - len(intakePromptTruncationNotice) - len(intakePromptFooter) + if prefixBudget <= 0 { + return truncateUTF8(prompt, maxIntakePromptLen) + } + return truncateUTF8(prefix, prefixBudget) + intakePromptTruncationNotice + intakePromptFooter +} + +func truncateUTF8(s string, maxBytes int) string { + if len(s) <= maxBytes { + return s + } + cut := 0 + for i := range s { + if i > maxBytes { + break + } + cut = i + } + return s[:cut] +} + +func trackerRepo(project domain.ProjectRecord, cfg domain.TrackerIntakeConfig) (domain.TrackerRepo, bool) { + provider := cfg.Provider + if provider == "" { + provider = domain.TrackerProviderGitHub + } + if provider != domain.TrackerProviderGitHub { + return domain.TrackerRepo{}, false + } + native := strings.TrimSpace(cfg.Repo) + if native == "" { + native = parseGitHubRepoNative(project.RepoOriginURL) + } + if native == "" { + return domain.TrackerRepo{}, false + } + return domain.TrackerRepo{Provider: provider, Native: native}, true +} + +func parseGitHubRepoNative(remote string) string { + remote = strings.TrimSpace(remote) + if remote == "" { + return "" + } + if strings.HasPrefix(remote, "git@") { + if _, rest, ok := strings.Cut(remote, ":"); ok { + return cleanRepoPath(rest) + } + } + if u, err := url.Parse(remote); err == nil && u.Host != "" { + host := strings.TrimPrefix(strings.ToLower(u.Host), "www.") + if host == "github.com" || strings.HasSuffix(host, ".github.com") || strings.HasSuffix(host, ".ghe.io") { + return cleanRepoPath(u.Path) + } + return "" + } + return cleanRepoPath(remote) +} + +func cleanRepoPath(path string) string { + path = strings.Trim(strings.TrimSpace(path), "/") + path = strings.TrimSuffix(path, ".git") + parts := strings.Split(path, "/") + if len(parts) < 2 { + return "" + } + owner := strings.TrimSpace(parts[len(parts)-2]) + repo := strings.TrimSpace(parts[len(parts)-1]) + if owner == "" || repo == "" { + return "" + } + return owner + "/" + repo +} diff --git a/backend/internal/observe/trackerintake/observer_test.go b/backend/internal/observe/trackerintake/observer_test.go new file mode 100644 index 000000000..08ef3d8c2 --- /dev/null +++ b/backend/internal/observe/trackerintake/observer_test.go @@ -0,0 +1,345 @@ +package trackerintake + +import ( + "context" + "errors" + "io" + "log/slog" + "strings" + "testing" + "time" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + "github.com/aoagents/agent-orchestrator/backend/internal/ports" +) + +func TestPollSpawnsWorkerForEligibleIssue(t *testing.T) { + store := &fakeStore{ + projects: []domain.ProjectRecord{{ + ID: "demo", + RepoOriginURL: "https://github.com/acme/demo.git", + Config: domain.ProjectConfig{TrackerIntake: domain.TrackerIntakeConfig{ + Enabled: true, + Assignee: "alice", + }}, + }}, + } + tracker := &fakeTracker{issues: []domain.Issue{{ + ID: domain.TrackerID{Provider: domain.TrackerProviderGitHub, Native: "acme/demo#12"}, + Title: "Fix login", + Body: "The login form submits twice.", + State: domain.IssueOpen, + URL: "https://github.com/acme/demo/issues/12", + Labels: []string{"agent-ready"}, + Assignees: []string{"alice"}, + }}} + spawner := &fakeSpawner{} + + if err := New(singleResolver(tracker), store, spawner, Config{Logger: discardLogger()}).Poll(context.Background()); err != nil { + t.Fatalf("Poll() error = %v", err) + } + if len(spawner.calls) != 1 { + t.Fatalf("spawn calls = %d, want 1", len(spawner.calls)) + } + call := spawner.calls[0] + if call.ProjectID != "demo" || call.Kind != domain.KindWorker { + t.Fatalf("spawn config = %+v", call) + } + if call.IssueID != "github:acme/demo#12" { + t.Fatalf("IssueID = %q, want canonical github id", call.IssueID) + } + if !strings.Contains(call.Prompt, "Fix login") || !strings.Contains(call.Prompt, "The login form submits twice.") { + t.Fatalf("prompt missing issue context:\n%s", call.Prompt) + } + if len(tracker.filters) != 1 { + t.Fatalf("tracker filters = %d, want 1", len(tracker.filters)) + } + if got := tracker.filters[0]; got.State != domain.ListOpen || got.Assignee != "alice" || len(got.Labels) != 0 { + t.Fatalf("tracker filter = %+v", got) + } +} + +func TestPollSkipsExistingIssueSessionsAfterRestart(t *testing.T) { + store := &fakeStore{ + projects: []domain.ProjectRecord{{ + ID: "demo", + RepoOriginURL: "https://github.com/acme/demo.git", + Config: domain.ProjectConfig{TrackerIntake: domain.TrackerIntakeConfig{Enabled: true, Assignee: "alice"}}, + }}, + sessions: []domain.SessionRecord{{ID: "demo-1", ProjectID: "demo", IssueID: "github:acme/demo#12"}}, + } + tracker := &fakeTracker{issues: []domain.Issue{{ + ID: domain.TrackerID{Provider: domain.TrackerProviderGitHub, Native: "acme/demo#12"}, + Title: "Already running", + State: domain.IssueOpen, + Assignees: []string{"alice"}, + }}} + spawner := &fakeSpawner{} + + if err := New(singleResolver(tracker), store, spawner, Config{Logger: discardLogger()}).Poll(context.Background()); err != nil { + t.Fatalf("Poll() error = %v", err) + } + if len(spawner.calls) != 0 { + t.Fatalf("spawn calls = %d, want 0", len(spawner.calls)) + } +} + +func TestPollSkipsSessionScanWhenIntakeDisabled(t *testing.T) { + store := &fakeStore{ + projects: []domain.ProjectRecord{{ID: "demo"}}, + sessionsErr: errors.New("session scan should not run"), + } + + if err := New(singleResolver(&fakeTracker{}), store, &fakeSpawner{}, Config{Logger: discardLogger()}).Poll(context.Background()); err != nil { + t.Fatalf("Poll() error = %v, want nil", err) + } +} + +func TestPollSkipsIneligibleAndInvalidProjects(t *testing.T) { + store := &fakeStore{ + projects: []domain.ProjectRecord{ + {ID: "off", RepoOriginURL: "https://github.com/acme/off.git"}, + {ID: "broad", RepoOriginURL: "https://github.com/acme/broad.git", Config: domain.ProjectConfig{TrackerIntake: domain.TrackerIntakeConfig{Enabled: true}}}, + {ID: "missing-origin", Config: domain.ProjectConfig{TrackerIntake: domain.TrackerIntakeConfig{Enabled: true, Assignee: "alice"}}}, + }, + } + tracker := &fakeTracker{issues: []domain.Issue{{ + ID: domain.TrackerID{Provider: domain.TrackerProviderGitHub, Native: "acme/off#1"}, + Title: "ignored", + State: domain.IssueOpen, + }}} + spawner := &fakeSpawner{} + + if err := New(singleResolver(tracker), store, spawner, Config{Logger: discardLogger()}).Poll(context.Background()); err != nil { + t.Fatalf("Poll() error = %v", err) + } + if len(tracker.repos) != 0 { + t.Fatalf("tracker was called for invalid/off projects: %+v", tracker.repos) + } + if len(spawner.calls) != 0 { + t.Fatalf("spawn calls = %d, want 0", len(spawner.calls)) + } +} + +func TestPollContinuesAfterTrackerAndSpawnFailures(t *testing.T) { + store := &fakeStore{projects: []domain.ProjectRecord{ + {ID: "bad", RepoOriginURL: "https://github.com/acme/bad.git", Config: domain.ProjectConfig{TrackerIntake: domain.TrackerIntakeConfig{Enabled: true, Assignee: "alice"}}}, + {ID: "good", RepoOriginURL: "https://github.com/acme/good.git", Config: domain.ProjectConfig{TrackerIntake: domain.TrackerIntakeConfig{Enabled: true, Assignee: "alice"}}}, + }} + tracker := &fakeTracker{ + failRepos: map[string]error{"acme/bad": errors.New("rate limited")}, + issuesByRepo: map[string][]domain.Issue{ + "acme/good": { + {ID: domain.TrackerID{Provider: domain.TrackerProviderGitHub, Native: "acme/good#1"}, Title: "first", State: domain.IssueOpen, Assignees: []string{"alice"}}, + {ID: domain.TrackerID{Provider: domain.TrackerProviderGitHub, Native: "acme/good#2"}, Title: "second", State: domain.IssueOpen, Assignees: []string{"alice"}}, + }, + }, + } + spawner := &fakeSpawner{failIssue: domain.IssueID("github:acme/good#1")} + + if err := New(singleResolver(tracker), store, spawner, Config{Logger: discardLogger()}).Poll(context.Background()); err != nil { + t.Fatalf("Poll() error = %v", err) + } + if len(spawner.calls) != 2 { + t.Fatalf("spawn attempts = %d, want 2", len(spawner.calls)) + } + if spawner.calls[1].IssueID != "github:acme/good#2" { + t.Fatalf("second spawn issue = %q", spawner.calls[1].IssueID) + } +} + +func TestPollBacksOffProjectAfterFailure(t *testing.T) { + now := time.Date(2026, 6, 27, 10, 0, 0, 0, time.UTC) + store := &fakeStore{projects: []domain.ProjectRecord{{ + ID: "demo", + RepoOriginURL: "https://github.com/acme/demo.git", + Config: domain.ProjectConfig{TrackerIntake: domain.TrackerIntakeConfig{Enabled: true, Assignee: "alice"}}, + }}} + tracker := &fakeTracker{failRepos: map[string]error{"acme/demo": errors.New("rate limited")}} + observer := New(singleResolver(tracker), store, &fakeSpawner{}, Config{ + Clock: func() time.Time { return now }, + FailureBackoff: time.Minute, + Logger: discardLogger(), + }) + + if err := observer.Poll(context.Background()); err != nil { + t.Fatalf("first Poll() error = %v", err) + } + if len(tracker.repos) != 1 { + t.Fatalf("tracker calls after first poll = %d, want 1", len(tracker.repos)) + } + + if err := observer.Poll(context.Background()); err != nil { + t.Fatalf("second Poll() error = %v", err) + } + if len(tracker.repos) != 1 { + t.Fatalf("tracker calls during backoff = %d, want still 1", len(tracker.repos)) + } + + now = now.Add(time.Minute + time.Nanosecond) + if err := observer.Poll(context.Background()); err != nil { + t.Fatalf("third Poll() error = %v", err) + } + if len(tracker.repos) != 2 { + t.Fatalf("tracker calls after backoff = %d, want 2", len(tracker.repos)) + } +} + +func TestPollSkipsNonOpenIssueStates(t *testing.T) { + store := &fakeStore{projects: []domain.ProjectRecord{{ + ID: "demo", + RepoOriginURL: "https://github.com/acme/demo.git", + Config: domain.ProjectConfig{TrackerIntake: domain.TrackerIntakeConfig{Enabled: true, Assignee: "alice"}}, + }}} + tracker := &fakeTracker{issues: []domain.Issue{ + {ID: domain.TrackerID{Provider: domain.TrackerProviderGitHub, Native: "acme/demo#1"}, Title: "already active", State: domain.IssueInProgress, Assignees: []string{"alice"}}, + {ID: domain.TrackerID{Provider: domain.TrackerProviderGitHub, Native: "acme/demo#2"}, Title: "ready", State: domain.IssueOpen, Assignees: []string{"alice"}}, + }} + spawner := &fakeSpawner{} + + if err := New(singleResolver(tracker), store, spawner, Config{Logger: discardLogger()}).Poll(context.Background()); err != nil { + t.Fatalf("Poll() error = %v", err) + } + if len(spawner.calls) != 1 || spawner.calls[0].IssueID != "github:acme/demo#2" { + t.Fatalf("spawn calls = %+v, want only open issue #2", spawner.calls) + } +} + +func TestPollAppliesLocalEligibilityFilter(t *testing.T) { + store := &fakeStore{projects: []domain.ProjectRecord{{ + ID: "demo", + RepoOriginURL: "https://github.com/acme/demo.git", + Config: domain.ProjectConfig{TrackerIntake: domain.TrackerIntakeConfig{Enabled: true, Assignee: "alice"}}, + }}} + tracker := &fakeTracker{issues: []domain.Issue{ + {ID: domain.TrackerID{Provider: domain.TrackerProviderGitHub, Native: "acme/demo#1"}, Title: "unassigned", State: domain.IssueOpen}, + {ID: domain.TrackerID{Provider: domain.TrackerProviderGitHub, Native: "acme/demo#2"}, Title: "wrong assignee", State: domain.IssueOpen, Assignees: []string{"bob"}}, + {ID: domain.TrackerID{Provider: domain.TrackerProviderGitHub, Native: "acme/demo#3"}, Title: "eligible", State: domain.IssueOpen, Labels: []string{"Agent-Ready"}, Assignees: []string{"Alice"}}, + }} + spawner := &fakeSpawner{} + + if err := New(singleResolver(tracker), store, spawner, Config{Logger: discardLogger()}).Poll(context.Background()); err != nil { + t.Fatalf("Poll() error = %v", err) + } + if len(spawner.calls) != 1 || spawner.calls[0].IssueID != "github:acme/demo#3" { + t.Fatalf("spawn calls = %+v, want only eligible issue #3", spawner.calls) + } +} + +func TestIssueMatchesConfigAssigneeSpecialValues(t *testing.T) { + assigned := domain.Issue{Assignees: []string{"alice"}} + unassigned := domain.Issue{} + if !issueMatchesConfig(assigned, domain.TrackerIntakeConfig{Assignee: "*"}) { + t.Fatal("assigned issue should match assignee=*") + } + if issueMatchesConfig(unassigned, domain.TrackerIntakeConfig{Assignee: "*"}) { + t.Fatal("unassigned issue should not match assignee=*") + } + if !issueMatchesConfig(unassigned, domain.TrackerIntakeConfig{Assignee: "none"}) { + t.Fatal("unassigned issue should match assignee=none") + } + if issueMatchesConfig(assigned, domain.TrackerIntakeConfig{Assignee: "none"}) { + t.Fatal("assigned issue should not match assignee=none") + } +} + +func TestBuildIssuePromptCapsLargeIssueBody(t *testing.T) { + prompt := BuildIssuePrompt(domain.Issue{ + ID: domain.TrackerID{Provider: domain.TrackerProviderGitHub, Native: "acme/demo#99"}, + Title: "Large issue", + URL: "https://github.com/acme/demo/issues/99", + Body: strings.Repeat("body ", 2000), + }) + if len(prompt) > maxIntakePromptLen { + t.Fatalf("prompt length = %d, want <= %d", len(prompt), maxIntakePromptLen) + } + if !strings.Contains(prompt, "Issue content truncated") { + t.Fatalf("prompt missing truncation notice:\n%s", prompt) + } + if !strings.Contains(prompt, "https://github.com/acme/demo/issues/99") { + t.Fatalf("prompt missing issue URL:\n%s", prompt) + } + if !strings.HasSuffix(prompt, intakePromptFooter) { + t.Fatalf("prompt missing footer:\n%s", prompt) + } +} + +func TestTrackerRepoUsesConfiguredRepo(t *testing.T) { + project := domain.ProjectRecord{ + ID: "demo", + RepoOriginURL: "https://github.com/wrong/repo.git", + Config: domain.ProjectConfig{TrackerIntake: domain.TrackerIntakeConfig{ + Enabled: true, + Repo: "acme/demo", + Assignee: "alice", + }}, + } + repo, ok := trackerRepo(project, project.Config.TrackerIntake.WithDefaults()) + if !ok { + t.Fatal("trackerRepo ok = false") + } + if repo.Native != "acme/demo" { + t.Fatalf("repo.Native = %q, want acme/demo", repo.Native) + } +} + +func singleResolver(tracker ports.Tracker) TrackerResolver { + return SingleTrackerResolver{Provider: domain.TrackerProviderGitHub, Adapter: tracker} +} + +type fakeStore struct { + projects []domain.ProjectRecord + sessions []domain.SessionRecord + sessionsErr error +} + +func (f *fakeStore) ListProjects(context.Context) ([]domain.ProjectRecord, error) { + return append([]domain.ProjectRecord(nil), f.projects...), nil +} + +func (f *fakeStore) ListAllSessions(context.Context) ([]domain.SessionRecord, error) { + return append([]domain.SessionRecord(nil), f.sessions...), f.sessionsErr +} + +type fakeTracker struct { + issues []domain.Issue + issuesByRepo map[string][]domain.Issue + failRepos map[string]error + repos []domain.TrackerRepo + filters []domain.ListFilter +} + +func (f *fakeTracker) Get(context.Context, domain.TrackerID) (domain.Issue, error) { + return domain.Issue{}, nil +} + +func (f *fakeTracker) List(_ context.Context, repo domain.TrackerRepo, filter domain.ListFilter) ([]domain.Issue, error) { + f.repos = append(f.repos, repo) + f.filters = append(f.filters, filter) + if err := f.failRepos[repo.Native]; err != nil { + return nil, err + } + if f.issuesByRepo != nil { + return append([]domain.Issue(nil), f.issuesByRepo[repo.Native]...), nil + } + return append([]domain.Issue(nil), f.issues...), nil +} + +func (f *fakeTracker) Preflight(context.Context) error { return nil } + +type fakeSpawner struct { + calls []ports.SpawnConfig + failIssue domain.IssueID +} + +func (f *fakeSpawner) Spawn(_ context.Context, cfg ports.SpawnConfig) (domain.Session, error) { + f.calls = append(f.calls, cfg) + if cfg.IssueID == f.failIssue { + return domain.Session{}, errors.New("spawn failed") + } + return domain.Session{SessionRecord: domain.SessionRecord{ID: domain.SessionID(string(cfg.ProjectID) + "-1"), ProjectID: cfg.ProjectID, IssueID: cfg.IssueID, Kind: cfg.Kind}}, nil +} + +func discardLogger() *slog.Logger { + return slog.New(slog.NewTextHandler(io.Discard, nil)) +} diff --git a/frontend/src/api/schema.ts b/frontend/src/api/schema.ts index 9ee643dad..8bef18801 100644 --- a/frontend/src/api/schema.ts +++ b/frontend/src/api/schema.ts @@ -679,6 +679,7 @@ export interface components { reviewers?: components["schemas"]["DomainReviewerConfig"][]; sessionPrefix?: string; symlinks?: string[]; + trackerIntake?: components["schemas"]["TrackerIntakeConfig"]; worker?: components["schemas"]["RoleOverride"]; }; ProjectGetResponse: { @@ -911,6 +912,13 @@ export interface components { /** @description Review verdict: approved or changes_requested. */ verdict: string; }; + TrackerIntakeConfig: { + assignee?: string; + enabled?: boolean; + /** @enum {string} */ + provider?: "github"; + repo?: string; + }; TriggerReviewResponse: { reviewerHandleId: string; reviews: components["schemas"]["PRReviewState"][]; diff --git a/frontend/src/renderer/components/CreateProjectAgentSheet.test.tsx b/frontend/src/renderer/components/CreateProjectAgentSheet.test.tsx new file mode 100644 index 000000000..6101d11e0 --- /dev/null +++ b/frontend/src/renderer/components/CreateProjectAgentSheet.test.tsx @@ -0,0 +1,71 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, expect, it, vi } from "vitest"; +import { CreateProjectAgentSheet } from "./CreateProjectAgentSheet"; + +function renderSheet(onSubmit = vi.fn().mockResolvedValue(undefined)) { + render( + undefined} + onSubmit={onSubmit} + open={true} + path="/repo/new-project" + />, + ); + return onSubmit; +} + +async function chooseOption(trigger: HTMLElement, optionName: string) { + await userEvent.click(trigger); + await userEvent.click(await screen.findByRole("option", { name: optionName })); +} + +describe("CreateProjectAgentSheet", () => { + it("creates without intake when the toggle is left off", async () => { + const onSubmit = renderSheet(); + await chooseOption(screen.getByLabelText("Worker agent"), "claude-code"); + await chooseOption(screen.getByLabelText("Orchestrator agent"), "codex"); + + await userEvent.click(screen.getByRole("button", { name: "Create and start" })); + + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)); + expect(onSubmit).toHaveBeenCalledWith({ + workerAgent: "claude-code", + orchestratorAgent: "codex", + trackerIntake: undefined, + }); + }); + + it("blocks submit when intake is enabled with no assignee, then passes the intake payload once one is set", async () => { + const onSubmit = renderSheet(); + await chooseOption(screen.getByLabelText("Worker agent"), "claude-code"); + await chooseOption(screen.getByLabelText("Orchestrator agent"), "codex"); + + await userEvent.click(screen.getByLabelText("Enable issue intake")); + // Enabled with no eligibility rule → submit stays disabled (compact sheet + // carries no inline guard prose; gating is the disabled button). + expect(screen.getByRole("button", { name: "Create and start" })).toBeDisabled(); + + await userEvent.type(screen.getByLabelText("Assignee"), "octocat"); + await userEvent.click(screen.getByRole("button", { name: "Create and start" })); + + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)); + expect(onSubmit).toHaveBeenCalledWith({ + workerAgent: "claude-code", + orchestratorAgent: "codex", + trackerIntake: { enabled: true, provider: "github", assignee: "octocat" }, + }); + }); + + it("keeps the create sheet minimal: info tooltip instead of prose, no repo row or credential hint", async () => { + renderSheet(); + // Info affordance is present even before enabling; the descriptive prose is not. + expect(screen.getByLabelText("What does enabling issue intake do?")).toBeInTheDocument(); + expect(screen.queryByText(/Auto-spawn worker sessions from matching tracker issues/)).not.toBeInTheDocument(); + + await userEvent.click(screen.getByLabelText("Enable issue intake")); + expect(screen.queryByText("Repository")).not.toBeInTheDocument(); + expect(screen.queryByText(/Reads credentials from/)).not.toBeInTheDocument(); + }); +}); diff --git a/frontend/src/renderer/components/CreateProjectAgentSheet.tsx b/frontend/src/renderer/components/CreateProjectAgentSheet.tsx index a2c52fe73..5f9e1a67a 100644 --- a/frontend/src/renderer/components/CreateProjectAgentSheet.tsx +++ b/frontend/src/renderer/components/CreateProjectAgentSheet.tsx @@ -2,15 +2,22 @@ import * as Dialog from "@radix-ui/react-dialog"; import { X } from "lucide-react"; import { memo, useEffect, useState } from "react"; import { AGENT_OPTIONS, DEFAULT_PROJECT_AGENT } from "../lib/agent-options"; +import { buildIntake, type IntakeForm, IntakeFields, intakeNeedsRule } from "./IntakeFields"; import { Button } from "./ui/button"; import { Label } from "./ui/label"; import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "./ui/select"; +import type { components } from "../../api/schema"; + +type TrackerIntakeConfig = components["schemas"]["TrackerIntakeConfig"]; export type CreateProjectAgentSelection = { workerAgent: string; orchestratorAgent: string; + trackerIntake?: TrackerIntakeConfig; }; +const EMPTY_INTAKE: IntakeForm = { enabled: false, repo: "", assignee: "" }; + type CreateProjectAgentSheetProps = { error?: string | null; isCreating: boolean; @@ -30,12 +37,15 @@ export function CreateProjectAgentSheet({ }: CreateProjectAgentSheetProps) { const [workerAgent, setWorkerAgent] = useState(DEFAULT_PROJECT_AGENT); const [orchestratorAgent, setOrchestratorAgent] = useState(DEFAULT_PROJECT_AGENT); - const canSubmit = workerAgent !== "" && orchestratorAgent !== "" && !isCreating; + const [intake, setIntake] = useState(EMPTY_INTAKE); + const intakeIncomplete = intakeNeedsRule(intake); + const canSubmit = workerAgent !== "" && orchestratorAgent !== "" && !intakeIncomplete && !isCreating; useEffect(() => { if (!open) { setWorkerAgent(DEFAULT_PROJECT_AGENT); setOrchestratorAgent(DEFAULT_PROJECT_AGENT); + setIntake(EMPTY_INTAKE); } }, [open, path]); @@ -67,7 +77,7 @@ export function CreateProjectAgentSheet({ onSubmit={(event) => { event.preventDefault(); if (!canSubmit) return; - void onSubmit({ workerAgent, orchestratorAgent }); + void onSubmit({ workerAgent, orchestratorAgent, trackerIntake: buildIntake(intake) }); }} >
@@ -87,6 +97,10 @@ export function CreateProjectAgentSheet({ />
+
+ setIntake((f) => ({ ...f, ...patch }))} compact /> +
+ {error && (
{error} diff --git a/frontend/src/renderer/components/IntakeFields.tsx b/frontend/src/renderer/components/IntakeFields.tsx new file mode 100644 index 000000000..daf3f9f20 --- /dev/null +++ b/frontend/src/renderer/components/IntakeFields.tsx @@ -0,0 +1,175 @@ +import { Info } from "lucide-react"; +import type { components } from "../../api/schema"; +import { Label } from "./ui/label"; +import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "./ui/tooltip"; + +type TrackerIntakeConfig = components["schemas"]["TrackerIntakeConfig"]; + +// IntakeForm is the flat, string-backed shape both the create sheet and the +// project settings form edit. repo has no input today (it's derived from the +// git origin server-side) but is plumbed so a value set via the CLI +// (--tracker-repo) survives a UI save instead of being wiped. +export type IntakeForm = { + enabled: boolean; + repo: string; + assignee: string; +}; + +// Only "github" is a valid TrackerIntakeConfig["provider"] today (see the +// backend's openapi enum). Adding Linear/Jira later means: the backend enum +// grows, IntakeFields gains a provider onChange({ enabled: e.target.checked })} + /> + Enable issue intake + + {compact && ( + + + + + + Auto-spawns a worker session for each matching GitHub issue. + + + )} +
+ {form.enabled && ( + <> + {repoPreview && ( + + {repoPreview.value ? ( + + {repoPreview.value} + + ) : ( + + Could not detect a GitHub repo from this project's git origin. + + )} + + )} + + onChange({ assignee: e.target.value })} + placeholder="type username or * for any" + /> + + {!compact && needsRule && ( +

Enabling intake requires an assignee.

+ )} + + )} + + ); +} + +function IntakeField({ label, htmlFor, children }: { label: string; htmlFor?: string; children: React.ReactNode }) { + return ( +
+ + {children} +
+ ); +} diff --git a/frontend/src/renderer/components/ProjectSettingsForm.test.tsx b/frontend/src/renderer/components/ProjectSettingsForm.test.tsx index 41bb524ac..914790320 100644 --- a/frontend/src/renderer/components/ProjectSettingsForm.test.tsx +++ b/frontend/src/renderer/components/ProjectSettingsForm.test.tsx @@ -239,4 +239,76 @@ describe("ProjectSettingsForm", () => { }); expect(await screen.findByText("Saved.")).toBeInTheDocument(); }); + + it("saves GitHub tracker intake settings, deriving the repo from the project's git origin", async () => { + getMock.mockResolvedValue({ + data: { + status: "ok", + project: { + id: "proj-1", + name: "Project One", + kind: "single_repo", + path: "/repo/project-one", + repo: "git@github.com:acme/project-one.git", + defaultBranch: "main", + config: { + worker: { agent: "codex" }, + orchestrator: { agent: "claude-code" }, + }, + }, + }, + error: undefined, + }); + + renderSettings(); + + await userEvent.click(await screen.findByLabelText("Enable issue intake")); + + // Repository is display-only, derived from the project's own git origin — no input to + // fill. Assignee is the only eligibility rule in v1. + expect(screen.getByRole("link", { name: "acme/project-one" })).toHaveAttribute( + "href", + "https://github.com/acme/project-one", + ); + await userEvent.type(screen.getByLabelText("Assignee"), "octocat"); + + await userEvent.click(screen.getByRole("button", { name: "Save changes" })); + + await waitFor(() => expect(putMock).toHaveBeenCalledTimes(1)); + const body = putMock.mock.calls[0]?.[1]?.body; + expect(body.config.trackerIntake).toEqual({ + enabled: true, + provider: "github", + assignee: "octocat", + }); + }); + + it("blocks save when intake is enabled with no assignee", async () => { + getMock.mockResolvedValue({ + data: { + status: "ok", + project: { + id: "proj-1", + name: "Project One", + kind: "single_repo", + path: "/repo/project-one", + repo: "git@github.com:acme/project-one.git", + defaultBranch: "main", + config: { + worker: { agent: "codex" }, + orchestrator: { agent: "claude-code" }, + }, + }, + }, + error: undefined, + }); + + renderSettings(); + + await userEvent.click(await screen.findByLabelText("Enable issue intake")); + await userEvent.click(screen.getByRole("button", { name: "Save changes" })); + + expect(await screen.findAllByText("Enabling intake requires an assignee.")).toHaveLength(2); + expect(putMock).not.toHaveBeenCalled(); + }); }); diff --git a/frontend/src/renderer/components/ProjectSettingsForm.tsx b/frontend/src/renderer/components/ProjectSettingsForm.tsx index c6be21a19..59f30fc63 100644 --- a/frontend/src/renderer/components/ProjectSettingsForm.tsx +++ b/frontend/src/renderer/components/ProjectSettingsForm.tsx @@ -6,6 +6,7 @@ import { DEFAULT_PROJECT_AGENT } from "../lib/agent-options"; import { workspaceQueryKey } from "../hooks/useWorkspaceQuery"; import { RequiredAgentField } from "./CreateProjectAgentSheet"; import { DashboardSubhead } from "./DashboardSubhead"; +import { buildIntake, deriveGitHubRepo, IntakeFields, type IntakeForm, intakeNeedsRule } from "./IntakeFields"; import { Button } from "./ui/button"; import { Card, CardContent, CardHeader, CardTitle } from "./ui/card"; import { Label } from "./ui/label"; @@ -13,6 +14,7 @@ import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from ". type Project = components["schemas"]["Project"]; type ProjectConfig = components["schemas"]["ProjectConfig"]; +type TrackerIntakeConfig = components["schemas"]["TrackerIntakeConfig"]; const PERMISSION_MODE_OPTIONS = [ { value: "default", label: "Default" }, @@ -67,6 +69,7 @@ export function ProjectSettingsForm({ projectId }: { projectId: string }) { function SettingsBody({ project, projectId, onSaved }: { project: Project; projectId: string; onSaved: () => void }) { const queryClient = useQueryClient(); const config = project.config ?? {}; + const intake: TrackerIntakeConfig = config.trackerIntake ?? {}; const [form, setForm] = useState({ defaultBranch: config.defaultBranch ?? project.defaultBranch ?? "", sessionPrefix: config.sessionPrefix ?? "", @@ -75,8 +78,32 @@ function SettingsBody({ project, projectId, onSaved }: { project: Project; proje model: config.agentConfig?.model ?? "", permissions: config.agentConfig?.permissions ?? "", reviewerHarness: config.reviewers?.[0]?.harness ?? "", + intakeEnabled: intake.enabled ?? false, + intakeRepo: intake.repo ?? "", + intakeAssignee: intake.assignee ?? "", }); const [savedAt, setSavedAt] = useState(null); + const [validationError, setValidationError] = useState(null); + const missingRequiredAgent = form.workerAgent === "" || form.orchestratorAgent === ""; + + // The Electron app only registers git projects today, so the daemon always has a usable + // git origin to derive owner/repo from (trackerRepo() in observer.go) when + // trackerIntake.repo is unset — there's no manual override input here. This mirrors that + // same derivation client-side purely for display (a link to the repo being polled). + const intakeForm: IntakeForm = { + enabled: form.intakeEnabled, + repo: form.intakeRepo, + assignee: form.intakeAssignee, + }; + const patchIntake = (patch: Partial) => + setForm((f) => ({ + ...f, + intakeEnabled: patch.enabled ?? f.intakeEnabled, + intakeRepo: patch.repo ?? f.intakeRepo, + intakeAssignee: patch.assignee ?? f.intakeAssignee, + })); + const effectiveIntakeRepo = form.intakeRepo.trim() || deriveGitHubRepo(project.repo); + const intakeIncomplete = intakeNeedsRule(intakeForm); const mutation = useMutation({ mutationFn: async () => { @@ -94,6 +121,7 @@ function SettingsBody({ project, projectId, onSaved }: { project: Project; proje permissions: form.permissions || undefined, }), reviewers: form.reviewerHarness ? [{ harness: form.reviewerHarness }] : undefined, + trackerIntake: buildIntake(intakeForm), }; const { error } = await apiClient.PUT("/api/v1/projects/{id}/config", { params: { path: { id: projectId } }, @@ -114,6 +142,15 @@ function SettingsBody({ project, projectId, onSaved }: { project: Project; proje onSubmit={(event) => { event.preventDefault(); setSavedAt(null); + if (missingRequiredAgent) { + setValidationError("Worker and orchestrator agents are required."); + return; + } + if (intakeIncomplete) { + setValidationError("Enabling intake requires an assignee."); + return; + } + setValidationError(null); mutation.mutate(); }} > @@ -207,10 +244,20 @@ function SettingsBody({ project, projectId, onSaved }: { project: Project; proje + + + Tracker intake + + + + + +
+ {validationError && {validationError}} {mutation.isError && ( {mutation.error instanceof Error ? mutation.error.message : "Save failed"} diff --git a/frontend/src/renderer/components/SessionInspector.test.tsx b/frontend/src/renderer/components/SessionInspector.test.tsx index 4ae34cea9..78a32c608 100644 --- a/frontend/src/renderer/components/SessionInspector.test.tsx +++ b/frontend/src/renderer/components/SessionInspector.test.tsx @@ -345,6 +345,13 @@ describe("SessionInspector tabs", () => { const tabs = screen.getAllByRole("tab").map((el) => el.textContent?.trim()); expect(tabs).toEqual(["Summary", "Reviews", "Browser"]); }); + + it("shows the intake issue id in the summary overview when present", () => { + renderWithQuery(); + + expect(screen.getByText("Issue")).toBeInTheDocument(); + expect(screen.getByText("github:acme/project-one#42")).toBeInTheDocument(); + }); }); describe("SessionInspector reviews tab", () => { diff --git a/frontend/src/renderer/components/SessionInspector.tsx b/frontend/src/renderer/components/SessionInspector.tsx index 80d8c5918..9be031a8e 100644 --- a/frontend/src/renderer/components/SessionInspector.tsx +++ b/frontend/src/renderer/components/SessionInspector.tsx @@ -8,7 +8,7 @@ import { formatTimeCompact } from "../lib/format-time"; import { useSessionScmSummary, type SessionPRSummary } from "../hooks/useSessionScmSummary"; import { prBrowserUrl, prStatusRows, sessionPRDisplaySummaries, type PRDisplayTone } from "../lib/pr-display"; import type { SessionActivityState, WorkspaceSession } from "../types/workspace"; -import { sortedPRs } from "../types/workspace"; +import { canonicalTrackerIssueId, sortedPRs } from "../types/workspace"; import { BrowserPanelView } from "./BrowserPanel"; import type { BrowserViewModel } from "../hooks/useBrowserView"; import { Badge } from "./ui/badge"; @@ -169,6 +169,7 @@ function SummaryView({ session }: { session: WorkspaceSession }) { const prSummaries = sessionPRDisplaySummaries(session, query.data); const prSectionTitle = prSummaries.length > 1 ? `Pull requests (${prSummaries.length})` : "Pull request"; const branchLabel = session.branch || `session/${session.id}`; + const issueId = canonicalTrackerIssueId(session.issueId); return (
@@ -191,6 +192,7 @@ function SummaryView({ session }: { session: WorkspaceSession }) {
+ {issueId && } diff --git a/frontend/src/renderer/components/SessionsBoard.tsx b/frontend/src/renderer/components/SessionsBoard.tsx index 7da1b8a30..261ca406c 100644 --- a/frontend/src/renderer/components/SessionsBoard.tsx +++ b/frontend/src/renderer/components/SessionsBoard.tsx @@ -2,7 +2,13 @@ import { useState, type KeyboardEvent } from "react"; import { useQueryClient } from "@tanstack/react-query"; import { useNavigate } from "@tanstack/react-router"; import { Plus } from "lucide-react"; -import { type AttentionZone, type WorkspaceSession, attentionZone, workerSessions } from "../types/workspace"; +import { + type AttentionZone, + type WorkspaceSession, + attentionZone, + canonicalTrackerIssueId, + workerSessions, +} from "../types/workspace"; import { useSessionScmSummary, type SessionPRSummary } from "../hooks/useSessionScmSummary"; import { useWorkspaceQuery, workspaceQueryKey } from "../hooks/useWorkspaceQuery"; import { DashboardSubhead } from "./DashboardSubhead"; @@ -260,6 +266,7 @@ function ZoneColumn({ function SessionCard({ session, onOpen }: { session: WorkspaceSession; onOpen: () => void }) { const badge = sessionBadge(session); + const issueId = canonicalTrackerIssueId(session.issueId); const branch = session.branch || ""; const showBranch = branch !== "" && !sameLabel(branch, session.title) && !sameLabel(branch, session.id); const prSummaries = sessionPRDisplaySummaries(session, useSessionScmSummary(session.id).data); @@ -285,6 +292,14 @@ function SessionCard({ session, onOpen }: { session: WorkspaceSession; onOpen: ( {badge.label} + {issueId && ( + + {issueId} + + )} {agentLabel(session.provider)} diff --git a/frontend/src/renderer/components/Sidebar.tsx b/frontend/src/renderer/components/Sidebar.tsx index 81f8a9bc3..e1200349f 100644 --- a/frontend/src/renderer/components/Sidebar.tsx +++ b/frontend/src/renderer/components/Sidebar.tsx @@ -81,7 +81,7 @@ type SidebarProps = { underTopbar?: boolean; workspaceError?: string; workspaces: WorkspaceSummary[]; - onCreateProject: (input: { path: string; workerAgent: string; orchestratorAgent: string }) => Promise; + onCreateProject: (input: { path: string } & CreateProjectAgentSelection) => Promise; onRemoveProject: (projectId: string) => Promise; }; diff --git a/frontend/src/renderer/hooks/useWorkspaceQuery.test.tsx b/frontend/src/renderer/hooks/useWorkspaceQuery.test.tsx index 69daa089e..21b402872 100644 --- a/frontend/src/renderer/hooks/useWorkspaceQuery.test.tsx +++ b/frontend/src/renderer/hooks/useWorkspaceQuery.test.tsx @@ -62,6 +62,7 @@ describe("useWorkspaceQuery", () => { projectId: "proj-1", terminalHandleId: "term-1", displayName: "fix-bug", + issueId: "github:acme/project-one#42", harness: "claude-code", branch: "qa/modal-worker", status: "mergeable", @@ -97,6 +98,7 @@ describe("useWorkspaceQuery", () => { id: "sess-1", terminalHandleId: "term-1", title: "fix-bug", + issueId: "github:acme/project-one#42", provider: "claude-code", branch: "qa/modal-worker", status: "mergeable", diff --git a/frontend/src/renderer/hooks/useWorkspaceQuery.ts b/frontend/src/renderer/hooks/useWorkspaceQuery.ts index c07efa195..b46373aff 100644 --- a/frontend/src/renderer/hooks/useWorkspaceQuery.ts +++ b/frontend/src/renderer/hooks/useWorkspaceQuery.ts @@ -52,6 +52,7 @@ async function fetchWorkspaces(): Promise { workspaceId: project.id, workspaceName: project.name, title: session.displayName ?? session.issueId ?? session.id, + issueId: session.issueId, provider: toAgentProvider(session.harness), kind: session.kind === "orchestrator" ? "orchestrator" : session.kind === "worker" ? "worker" : undefined, branch: session.branch ?? `session/${session.id}`, diff --git a/frontend/src/renderer/routes/_shell.tsx b/frontend/src/renderer/routes/_shell.tsx index 72c12a11e..6dcbb2c42 100644 --- a/frontend/src/renderer/routes/_shell.tsx +++ b/frontend/src/renderer/routes/_shell.tsx @@ -14,6 +14,7 @@ import { ShellProvider } from "../lib/shell-context"; import { spawnOrchestrator } from "../lib/spawn-orchestrator"; import { readStoredTheme, type Theme, useUiStore } from "../stores/ui-store"; import type { WorkspaceSummary } from "../types/workspace"; +import type { components } from "../../api/schema"; export const Route = createFileRoute("/_shell")({ // Prefetch the workspace list for the whole shell (parent loaders run before @@ -54,7 +55,12 @@ function ShellLayout() { ); const createProject = useCallback( - async (input: { path: string; workerAgent: string; orchestratorAgent: string }) => { + async (input: { + path: string; + workerAgent: string; + orchestratorAgent: string; + trackerIntake?: components["schemas"]["TrackerIntakeConfig"]; + }) => { void addRendererExceptionStep("Project add requested", { source: "project-add", operation: "project_add", @@ -67,6 +73,7 @@ function ShellLayout() { config: { worker: { agent: input.workerAgent }, orchestrator: { agent: input.orchestratorAgent }, + trackerIntake: input.trackerIntake, }, }, }); diff --git a/frontend/src/renderer/types/workspace.test.ts b/frontend/src/renderer/types/workspace.test.ts index c0ffab127..0125bd75a 100644 --- a/frontend/src/renderer/types/workspace.test.ts +++ b/frontend/src/renderer/types/workspace.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { attentionZone, + canonicalTrackerIssueId, findProjectOrchestrator, sessionIsActive, sessionNeedsAttention, @@ -20,6 +21,14 @@ import { type WorkspaceSummary, } from "./workspace"; +describe("canonicalTrackerIssueId", () => { + it("keeps provider-prefixed intake ids and rejects manual task titles", () => { + expect(canonicalTrackerIssueId("github:acme/project#42")).toBe("github:acme/project#42"); + expect(canonicalTrackerIssueId("Fix fallback renderer")).toBeUndefined(); + expect(canonicalTrackerIssueId(undefined)).toBeUndefined(); + }); +}); + function sessionWith(overrides: Partial): WorkspaceSession { return { id: "sess-1", diff --git a/frontend/src/renderer/types/workspace.ts b/frontend/src/renderer/types/workspace.ts index 49a98f427..e8bb86f7e 100644 --- a/frontend/src/renderer/types/workspace.ts +++ b/frontend/src/renderer/types/workspace.ts @@ -118,6 +118,8 @@ export type WorkspaceSession = { workspaceId: string; workspaceName: string; title: string; + /** Raw issue/task identifier from the daemon. Intake ids are provider-prefixed. */ + issueId?: string; provider: AgentProvider; kind?: SessionKind; branch: string; @@ -156,6 +158,22 @@ export type WorkspaceSession = { displayStatus?: WorkerDisplayStatus; }; +// Tracker providers whose ids the intake daemon stamps sessions with, in +// ":" form. Adding a provider (Linear, Jira, ...) later is +// just another prefix in this list — no caller of canonicalTrackerIssueId +// needs to change. +const TRACKER_PROVIDER_PREFIXES = ["github:"] as const; + +/** + * The provider-prefixed issue id if `issueId` came from tracker intake, or + * undefined for manually created sessions (whose issueId, if any, is a plain + * task title with no provider prefix). + */ +export function canonicalTrackerIssueId(issueId?: string): string | undefined { + if (!issueId) return undefined; + return TRACKER_PROVIDER_PREFIXES.some((prefix) => issueId.startsWith(prefix)) ? issueId : undefined; +} + /** Glanceable worker status. Maps 1:1 to the accent colors in DESIGN.md. */ export type WorkerDisplayStatus = "working" | "needs_you" | "mergeable" | "ci_failed" | "no_signal" | "done" | "unknown";