Brainstorm: no repeat-comment, prefer novelty — Implementation Plan¶

// commentsBySlugNum is keyed "slug#number" -> comments returned for that issue.
// Add this field to the goalCapturingReader struct:
//   commentsBySlugNum map[string][]scm.IssueComment
func (g *goalCapturingReader) ListIssueComments(_ context.Context, owner, repo string, number int) ([]scm.IssueComment, error) {
    key := fmt.Sprintf("%s/%s#%d", owner, repo, number)
    return g.commentsBySlugNum[key], nil
}

// TestBrainstorm_BotEngagedIssueFlagged verifies that an issue the bot already
// commented on is marked [bot-engaged] in the goal, while an untouched issue is not.
func TestBrainstorm_BotEngagedIssueFlagged(t *testing.T) {
    proj, repos := seedBrainstormProject(t, "bs-botengaged", []string{"o/eng1", "o/eng2"}, 5)

    reader := &goalCapturingReader{
        issuesByRepo: map[string][]scm.IssueRef{
            "o/eng1": {
                {Repo: "o/eng1", Number: 7, Title: "improve caching layer", UpdatedAt: time.Now()},
            },
            "o/eng2": {
                {Repo: "o/eng2", Number: 2, Title: "fix login redirect", UpdatedAt: time.Now()},
            },
        },
        commentsBySlugNum: map[string][]scm.IssueComment{
            // Bot already commented on o/eng1#7 (off-limits); o/eng2#2 untouched.
            "o/eng1#7": {{Author: "tatara-bot", Body: "looking into this"}},
        },
    }

    r := newScanReconciler(reader)
    r.Metrics = obs.NewOperatorMetrics(prometheus.NewRegistry())

    act := tatarav1alpha1.BrainstormActivity{Enabled: true, MaxOpenProposals: 5}
    budget := 99
    r.brainstorm(context.Background(), proj, reader, repos, nil, act, &budget)

    tasks := listBrainstormTasks(t, "bs-botengaged")
    if len(tasks) != 1 {
        t.Fatalf("want 1 brainstorm task, got %d", len(tasks))
    }
    goal := tasks[0].Spec.Goal

    if !strings.Contains(goal, "o/eng1#7 [] improve caching layer [bot-engaged]") {
        t.Fatalf("bot-engaged issue not flagged:\n%s", goal)
    }
    if strings.Contains(goal, "fix login redirect [bot-engaged]") {
        t.Fatalf("untouched issue wrongly flagged:\n%s", goal)
    }
}

func (r *ProjectReconciler) buildIssuesContext(ctx context.Context, proj *tatarav1alpha1.Project, reader scm.SCMReader, issuesBySlug map[string][]scm.IssueRef, repos []tatarav1alpha1.Repository) string {
    l := log.FromContext(ctx)
    botLogin := ""
    if proj.Spec.Scm != nil {
        botLogin = proj.Spec.Scm.BotLogin
    }
    var lines []string
    total := 0
    for i := range repos {
        owner, name, err := scm.OwnerRepo(repos[i].Spec.URL)
        if err != nil {
            continue
        }
        slug := owner + "/" + name
        issues := issuesBySlug[slug]
        for _, iss := range issues {
            if iss.IsPR {
                continue
            }
            if len(lines) >= maxIssuesContext {
                total++
                continue
            }
            total++
            labels := strings.Join(iss.Labels, ",")
            title := iss.Title
            // Collapse newlines in title for a single-line entry.
            title = strings.ReplaceAll(title, "\n", " ")
            title = strings.ReplaceAll(title, "\r", "")
            line := fmt.Sprintf("%s#%d [%s] %s", slug, iss.Number, labels, title)
            if botCommentedOnIssue(ctx, reader, owner, name, iss.Number, botLogin) {
                line += " [bot-engaged]"
            }
            lines = append(lines, line)
        }
    }
    if len(lines) == 0 {
        return ""
    }
    omitted := total - len(lines)
    result := strings.Join(lines, "\n")
    if omitted > 0 {
        result += fmt.Sprintf("\n(+%d more omitted)", omitted)
        l.Info("brainstorm: buildIssuesContext: capped issues context",
            "shown", len(lines), "omitted", omitted)
    }
    return result
}

// botCommentedOnIssue reports whether botLogin already authored a comment on the
// issue. Empty botLogin or any SCM read error -> false (best-effort flag; the
// commentOnIssue egress gate is the authoritative backstop).
func botCommentedOnIssue(ctx context.Context, reader scm.SCMReader, owner, name string, number int, botLogin string) bool {
    if botLogin == "" {
        return false
    }
    comments, err := reader.ListIssueComments(ctx, owner, name, number)
    if err != nil {
        return false
    }
    for _, c := range comments {
        if c.Author == botLogin {
            return true
        }
    }
    return false
}

issuesCtx := r.buildIssuesContext(ctx, proj, reader, issuesBySlug, sortedRepos)

issuesCtx := r.buildIssuesContext(ctx, proj, reader, issuesBySlug, sortedRepos)

// TestGoalProjects_NoReCommentInstruction verifies both project goals tell the
// agent not to re-comment a [bot-engaged] issue and to prefer new improvements.
func TestGoalProjects_NoReCommentInstruction(t *testing.T) {
    slugs := []string{"o/alpha"}
    ctx := "o/alpha#1 [] Fix login bug [bot-engaged]"
    for _, g := range []string{
        brainstormGoalProject(slugs, ctx),
        healthCheckGoalProject(slugs, ctx),
    } {
        if !strings.Contains(g, "[bot-engaged]") {
            t.Fatalf("goal does not reference the bot-engaged marker:\n%s", g)
        }
        // Must instruct: do not comment again on a bot-engaged issue.
        if !strings.Contains(g, "do NOT comment again") {
            t.Fatalf("goal missing no-re-comment instruction:\n%s", g)
        }
        // Must still embed the context and keep the three action verbs.
        for _, kw := range []string{"comment_on_issue", "propose_issue", ctx} {
            if !strings.Contains(g, kw) {
                t.Fatalf("goal missing %q:\n%s", kw, g)
            }
        }
    }
}

        "2. If the best idea is a sub-aspect or connecting improvement TO an existing issue: " +
        "call comment_on_issue(repo, number, body) on that issue. Do NOT call propose_issue.\n" +

        "2. If the best idea is a sub-aspect or connecting improvement TO an existing issue " +
        "that is NOT marked [bot-engaged]: call comment_on_issue(repo, number, body) on that issue. " +
        "Do NOT call propose_issue.\n" +
        "   An issue marked [bot-engaged] already has your comment - do NOT comment again on it. " +
        "Prefer a NEW improvement instead: a genuinely novel standalone idea (path 3, in ANY repo or " +
        "project-wide), or a comment on a DIFFERENT issue that is not [bot-engaged]. Never comment " +
        "twice on the same issue.\n" +

        "2. If the best finding is a sub-aspect or connecting improvement TO an existing issue " +
        "that is NOT marked [bot-engaged]: call comment_on_issue(repo, number, body) on that issue. " +
        "Do NOT call propose_issue.\n" +
        "   An issue marked [bot-engaged] already has your comment - do NOT comment again on it. " +
        "Prefer a NEW finding instead: a genuinely novel standalone issue (path 3, in ANY repo or " +
        "project-wide), or a comment on a DIFFERENT issue that is not [bot-engaged]. Never comment " +
        "twice on the same issue.\n" +

// fakeReader returns canned issue comments for the gate test.
type fakeReader struct {
    comments []scm.IssueComment
}

func (f *fakeReader) ListOpenPRs(_ context.Context, _, _ string) ([]scm.PRRef, error) {
    return nil, nil
}
func (f *fakeReader) ListOpenIssues(_ context.Context, _, _ string) ([]scm.IssueRef, error) {
    return nil, nil
}
func (f *fakeReader) ListBoardItems(_ context.Context, _ scm.BoardRef) ([]scm.BoardItem, error) {
    return nil, nil
}
func (f *fakeReader) GetCommitCIStatus(_ context.Context, _, _, _ string) (string, error) {
    return "", nil
}
func (f *fakeReader) ListIssueComments(_ context.Context, _, _ string, _ int) ([]scm.IssueComment, error) {
    return f.comments, nil
}
func (f *fakeReader) GetIssue(_ context.Context, _, _ string, _ int) (scm.IssueContent, error) {
    return scm.IssueContent{}, nil
}

// buildRouterWithReader injects both an SCMFor writer and a ReaderFor reader.
func buildRouterWithReader(t *testing.T, writer scm.SCMWriter, reader scm.SCMReader, objs ...client.Object) *chi.Mux {
    t.Helper()
    scheme := runtime.NewScheme()
    require.NoError(t, tatarav1alpha1.AddToScheme(scheme))
    require.NoError(t, corev1.AddToScheme(scheme))
    fc := fake.NewClientBuilder().
        WithScheme(scheme).
        WithObjects(objs...).
        WithStatusSubresource(&tatarav1alpha1.Project{}, &tatarav1alpha1.Repository{},
            &tatarav1alpha1.Task{}, &tatarav1alpha1.Subtask{}).
        Build()
    s := restapi.NewServer(restapi.Config{
        Client:    fc,
        Namespace: "tatara",
        SCMFor: func(_ string) (scm.SCMWriter, error) {
            return writer, nil
        },
        ReaderFor: func(_, _ string) (scm.SCMReader, error) {
            return reader, nil
        },
    })
    r := chi.NewRouter()
    s.Mount(r, nil)
    return r
}

// projectWithBot is projectWithSCM plus a BotLogin (gate is a no-op without it).
func projectWithBot(name, secretName, bot string) *tatarav1alpha1.Project {
    p := projectWithSCM(name, secretName)
    p.Spec.Scm.BotLogin = bot
    return p
}

func TestCommentOnIssue_BlockedWhenBotAlreadyCommented(t *testing.T) {
    writer := &fakeWriter{}
    reader := &fakeReader{comments: []scm.IssueComment{
        {Author: "someone", Body: "first"},
        {Author: "tatara-bot", Body: "already weighed in"},
    }}
    proj := projectWithBot("projblk", "projblk-scm", "tatara-bot")
    secret := scmSecret("projblk-scm", "tok")
    repo := repoForProject("projblk-repo", "projblk", "https://github.com/o/r.git")

    r := buildRouterWithReader(t, writer, reader, proj, secret, repo)

    body := strings.NewReader(`{"repo":"o/r","number":9,"body":"again"}`)
    req := httptest.NewRequest(http.MethodPost, "/projects/projblk/issue-comment", body)
    req.Header.Set("Content-Type", "application/json")
    w := httptest.NewRecorder()
    r.ServeHTTP(w, req)

    require.Equal(t, http.StatusConflict, w.Code, "body: %s", w.Body.String())
    writer.mu.Lock()
    defer writer.mu.Unlock()
    require.Len(t, writer.comments, 0, "must not post when already commented")
}

func TestCommentOnIssue_PostsWhenBotNotYetCommented(t *testing.T) {
    writer := &fakeWriter{}
    reader := &fakeReader{comments: []scm.IssueComment{
        {Author: "someone", Body: "first"},
    }}
    proj := projectWithBot("projok", "projok-scm", "tatara-bot")
    secret := scmSecret("projok-scm", "tok")
    repo := repoForProject("projok-repo", "projok", "https://github.com/o/r.git")

    r := buildRouterWithReader(t, writer, reader, proj, secret, repo)

    body := strings.NewReader(`{"repo":"o/r","number":9,"body":"my take"}`)
    req := httptest.NewRequest(http.MethodPost, "/projects/projok/issue-comment", body)
    req.Header.Set("Content-Type", "application/json")
    w := httptest.NewRecorder()
    r.ServeHTTP(w, req)

    require.Equal(t, http.StatusOK, w.Code, "body: %s", w.Body.String())
    writer.mu.Lock()
    defer writer.mu.Unlock()
    require.Len(t, writer.comments, 1)
}

    // ReaderFor returns a token-bound SCMReader for the given provider name.
    // Used by the issue-comment gate to detect an existing bot comment.
    // When nil, the gate is skipped (post proceeds).
    ReaderFor func(provider, token string) (scm.SCMReader, error)

    readerFor func(provider, token string) (scm.SCMReader, error)

    return &Server{c: cfg.Client, ns: cfg.Namespace, scmFor: cfg.SCMFor, readerFor: cfg.ReaderFor, log: l, metrics: cfg.Metrics}

    // Hard-gate (cap 1): refuse a second bot comment on the same issue. Best-effort -
    // empty BotLogin, no reader factory, or an SCM read error all fall open (post proceeds);
    // the brainstorm prompt is the first line of defence, this is the authoritative backstop.
    botLogin := ""
    if proj.Spec.Scm != nil {
        botLogin = proj.Spec.Scm.BotLogin
    }
    if botLogin != "" && s.readerFor != nil {
        if reader, rerr := s.readerFor(provider, token); rerr == nil {
            if owner, name, oerr := scm.OwnerRepo(matchedRepoURL); oerr == nil {
                if comments, cerr := reader.ListIssueComments(r.Context(), owner, name, req.Number); cerr == nil {
                    for _, cm := range comments {
                        if cm.Author == botLogin {
                            if s.metrics != nil {
                                s.metrics.SCMWrite(provider, "comment", "blocked")
                            }
                            s.log.InfoContext(r.Context(), "restapi: commentOnIssue blocked",
                                append(reqLogFields(r),
                                    "action", "scm_issue_comment_blocked",
                                    "reason", "already_commented",
                                    "project", projName,
                                    "repo", req.Repo,
                                    "number", req.Number)...)
                            writeError(w, http.StatusConflict, "bot already commented on this issue; pick another action")
                            return
                        }
                    }
                }
            }
        }
    }

        ReaderFor: func(provider, token string) (scm.SCMReader, error) {
            return scm.ReaderByProvider(provider, token)
        },