syz-cluster: make report reply tracking more flexible

Replace an UpdateReport() call with a RecordReply(). This will
eventually allow us to support the email sender implementations for
which we do not immediately know the MessageID of the reported message.

13 files changed, 39 insertions, 90 deletions

diff --git a/syz-cluster/email-reporter/handler.go b/syz-cluster/email-reporter/handler.go
index 78eb8846e..8b0ff6a6c 100644
--- a/syz-cluster/email-reporter/handler.go
+++ b/syz-cluster/email-reporter/handler.go
@@ -93,11 +93,13 @@ func (h *Handler) report(ctx context.Context, rep *api.SessionReport) error {
 	if err != nil {
 		return fmt.Errorf("failed to send: %w", err)
 	}
-
-	// Now that the report is sent, update the link to the email discussion.
-	err = h.apiClient.UpdateReport(ctx, rep.ID, &api.UpdateReportReq{
+	// Record MessageID so that we could later trace user replies back to it.
+	_, err = h.apiClient.RecordReply(ctx, &api.RecordReplyReq{
 		// TODO: for Lore emails, set Link = lore.Link(msgID).
 		MessageID: msgID,
+		Time:      time.Now(),
+		ReportID:  rep.ID,
+		Reporter:  h.reporter,
 	})
 	if err != nil {
 		return fmt.Errorf("failed to update: %w", err)
diff --git a/syz-cluster/email-reporter/stream_test.go b/syz-cluster/email-reporter/stream_test.go
index 522d78486..0d79b158f 100644
--- a/syz-cluster/email-reporter/stream_test.go
+++ b/syz-cluster/email-reporter/stream_test.go
@@ -29,8 +29,10 @@ func TestEmailStream(t *testing.T) {
 	err = reporterClient.ConfirmReport(ctx, report.ID)
 	assert.NoError(t, err)
 	const messageID = "<message-id>"
-	err = reporterClient.UpdateReport(ctx, report.ID, &api.UpdateReportReq{
+	_, err = reporterClient.RecordReply(ctx, &api.RecordReplyReq{
 		MessageID: messageID,
+		ReportID:  report.ID,
+		Reporter:  api.LKMLReporter,
 	})
 	assert.NoError(t, err)
 
diff --git a/syz-cluster/pkg/api/reporter.go b/syz-cluster/pkg/api/reporter.go
index e73bdb668..ea83dfbe6 100644
--- a/syz-cluster/pkg/api/reporter.go
+++ b/syz-cluster/pkg/api/reporter.go
@@ -30,16 +30,6 @@ func (client ReporterClient) GetNextReport(ctx context.Context, reporter string)
 	return postJSON[any, NextReportResp](ctx, client.baseURL+"/reports?"+v.Encode(), nil)
 }
 
-type UpdateReportReq struct {
-	MessageID string `json:"message_id"`
-}
-
-// UpdateReport may be used to remember the message ID and the link to the discussion.
-func (client ReporterClient) UpdateReport(ctx context.Context, id string, req *UpdateReportReq) error {
-	_, err := postJSON[UpdateReportReq, any](ctx, client.baseURL+"/reports/"+id+"/update", req)
-	return err
-}
-
 // ConfirmReport should be called to mark a report as sent.
 func (client ReporterClient) ConfirmReport(ctx context.Context, id string) error {
 	_, err := postJSON[any, any](ctx, client.baseURL+"/reports/"+id+"/confirm", nil)
@@ -56,7 +46,9 @@ func (client ReporterClient) UpstreamReport(ctx context.Context, id string, req
 }
 
 type RecordReplyReq struct {
-	MessageID string    `json:"message_id"`
+	MessageID string `json:"message_id"`
+	ReportID  string `json:"report_id"`
+	// If ReportID is not set, InReplyTo will help identify the original report.
 	InReplyTo string    `json:"in_reply_to"`
 	Reporter  string    `json:"reporter"`
 	Time      time.Time `json:"time"`
diff --git a/syz-cluster/pkg/db/entities.go b/syz-cluster/pkg/db/entities.go
index 13c440926..c914d7361 100644
--- a/syz-cluster/pkg/db/entities.go
+++ b/syz-cluster/pkg/db/entities.go
@@ -137,7 +137,6 @@ type SessionReport struct {
 	SessionID  string           `spanner:"SessionID"`
 	ReportedAt spanner.NullTime `spanner:"ReportedAt"`
 	Moderation bool             `spanner:"Moderation"`
-	MessageID  string           `spanner:"MessageID"`
 	Reporter   string           `spanner:"Reporter"`
 }
 
diff --git a/syz-cluster/pkg/db/migrations/1_initialize.down.sql b/syz-cluster/pkg/db/migrations/1_initialize.down.sql
index 04bdb97e2..64c1fbb35 100644
--- a/syz-cluster/pkg/db/migrations/1_initialize.down.sql
+++ b/syz-cluster/pkg/db/migrations/1_initialize.down.sql
@@ -20,7 +20,6 @@ DROP INDEX SessionsByFinishedAt;
 DROP INDEX NoDupFindings;
 DROP INDEX NoDupSessionReports;
 DROP INDEX SessionReportsByStatus;
-DROP INDEX SessionReportsByMessageID;
 
 DROP TABLE ReportReplies;
 DROP TABLE Findings;
diff --git a/syz-cluster/pkg/db/migrations/1_initialize.up.sql b/syz-cluster/pkg/db/migrations/1_initialize.up.sql
index baec817cb..139a9c21f 100644
--- a/syz-cluster/pkg/db/migrations/1_initialize.up.sql
+++ b/syz-cluster/pkg/db/migrations/1_initialize.up.sql
@@ -107,14 +107,12 @@ CREATE TABLE SessionReports (
     SessionID STRING(36) NOT NULL, -- UUID
     ReportedAt TIMESTAMP,
     Moderation BOOL,
-    MessageID STRING(512),
     Reporter STRING(256),
     CONSTRAINT FK_SessionReports FOREIGN KEY (SessionID) REFERENCES Sessions (ID),
 ) PRIMARY KEY(ID);
 
 CREATE UNIQUE INDEX NoDupSessionReports ON SessionReports(SessionID, Moderation);
 CREATE INDEX SessionReportsByStatus ON SessionReports (Reporter, ReportedAt);
-CREATE INDEX SessionReportsByMessageID ON SessionReports(Reporter, MessageID);
 
 -- Replies on a session report.
 CREATE TABLE ReportReplies (
diff --git a/syz-cluster/pkg/db/report_repo.go b/syz-cluster/pkg/db/report_repo.go
index 37a09a746..4e039c4a7 100644
--- a/syz-cluster/pkg/db/report_repo.go
+++ b/syz-cluster/pkg/db/report_repo.go
@@ -44,14 +44,3 @@ func (repo *ReportRepository) ListNotReported(ctx context.Context, reporter stri
 	addLimit(&stmt, limit)
 	return repo.readEntities(ctx, stmt)
 }
-
-func (repo *ReportRepository) FindByMessageID(ctx context.Context, reporter, messageID string) (*SessionReport, error) {
-	stmt := spanner.Statement{
-		SQL: "SELECT * FROM `SessionReports` WHERE `Reporter` = @reporter AND `MessageID` = @messageID",
-		Params: map[string]interface{}{
-			"reporter":  reporter,
-			"messageID": messageID,
-		},
-	}
-	return repo.readEntity(ctx, stmt)
-}
diff --git a/syz-cluster/pkg/db/report_repo_test.go b/syz-cluster/pkg/db/report_repo_test.go
index a0cee2a11..a9e23fd75 100644
--- a/syz-cluster/pkg/db/report_repo_test.go
+++ b/syz-cluster/pkg/db/report_repo_test.go
@@ -40,30 +40,17 @@ func TestReportRepository(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Len(t, list, 3)
 
-	const messageID = "message-id"
 	err = reportRepo.Update(ctx, keys[0], func(rep *SessionReport) error {
 		rep.SetReportedAt(time.Now())
-		rep.MessageID = messageID
 		return nil
 	})
 	assert.NoError(t, err)
-
 	t.Run("not-reported-count", func(t *testing.T) {
 		// Now one less.
 		list, err := reportRepo.ListNotReported(ctx, dummyReporter, 10)
 		assert.NoError(t, err)
 		assert.Len(t, list, 2)
 	})
-	t.Run("find-by-id-found", func(t *testing.T) {
-		report, err := reportRepo.FindByMessageID(ctx, dummyReporter, messageID)
-		assert.NoError(t, err)
-		assert.NotNil(t, report)
-	})
-	t.Run("find-by-id-empty", func(t *testing.T) {
-		report, err := reportRepo.FindByMessageID(ctx, dummyReporter, "non-existing-id")
-		assert.NoError(t, err)
-		assert.Nil(t, report)
-	})
 }
 
 func TestSessionsWithoutReports(t *testing.T) {
diff --git a/syz-cluster/pkg/db/spanner.go b/syz-cluster/pkg/db/spanner.go
index 9bdeee53b..1aee715f8 100644
--- a/syz-cluster/pkg/db/spanner.go
+++ b/syz-cluster/pkg/db/spanner.go
@@ -343,10 +343,3 @@ func (g *genericEntityOps[EntityType, KeyType]) readEntities(ctx context.Context
 	defer iter.Stop()
 	return readEntities[EntityType](iter)
 }
-
-func (g *genericEntityOps[EntityType, KeyType]) readEntity(ctx context.Context,
-	stmt spanner.Statement) (*EntityType, error) {
-	iter := g.client.Single().Query(ctx, stmt)
-	defer iter.Stop()
-	return readOne[EntityType](iter)
-}
diff --git a/syz-cluster/pkg/reporter/api.go b/syz-cluster/pkg/reporter/api.go
index e06e1312c..89bd21eb1 100644
--- a/syz-cluster/pkg/reporter/api.go
+++ b/syz-cluster/pkg/reporter/api.go
@@ -29,7 +29,6 @@ func NewAPIServer(env *app.AppEnvironment) *APIServer {
 
 func (s *APIServer) Mux() *http.ServeMux {
 	mux := http.NewServeMux()
-	mux.HandleFunc("/reports/{report_id}/update", s.updateReport)
 	mux.HandleFunc("/reports/{report_id}/upstream", s.upstreamReport)
 	mux.HandleFunc("/reports/{report_id}/confirm", s.confirmReport)
 	mux.HandleFunc("/reports/record_reply", s.recordReply)
@@ -39,16 +38,6 @@ func (s *APIServer) Mux() *http.ServeMux {
 }
 
 // nolint: dupl
-func (s *APIServer) updateReport(w http.ResponseWriter, r *http.Request) {
-	req := api.ParseJSON[api.UpdateReportReq](w, r)
-	if req == nil {
-		return // TODO: return StatusBadRequest here and below.
-	}
-	err := s.reportService.Update(r.Context(), r.PathValue("report_id"), req)
-	reply[interface{}](w, nil, err)
-}
-
-// nolint: dupl
 func (s *APIServer) upstreamReport(w http.ResponseWriter, r *http.Request) {
 	req := api.ParseJSON[api.UpstreamReportReq](w, r)
 	if req == nil {
diff --git a/syz-cluster/pkg/reporter/api_test.go b/syz-cluster/pkg/reporter/api_test.go
index 9f94ce5b6..14c61f5bc 100644
--- a/syz-cluster/pkg/reporter/api_test.go
+++ b/syz-cluster/pkg/reporter/api_test.go
@@ -108,15 +108,18 @@ func TestReplyReporting(t *testing.T) {
 	err = reportClient.ConfirmReport(ctx, reportID)
 	assert.NoError(t, err)
 
-	err = reportClient.UpdateReport(ctx, reportID, &api.UpdateReportReq{
-		MessageID: "message-id",
+	const reportMessageID = "message-id"
+	_, err = reportClient.RecordReply(ctx, &api.RecordReplyReq{
+		MessageID: reportMessageID,
+		ReportID:  reportID,
+		Reporter:  api.LKMLReporter,
 	})
 	assert.NoError(t, err)
 
 	// Direct reply to the report.
 	resp, err := reportClient.RecordReply(ctx, &api.RecordReplyReq{
 		MessageID: "direct-reply-id",
-		InReplyTo: "message-id",
+		InReplyTo: reportMessageID,
 		Reporter:  api.LKMLReporter,
 		Time:      time.Now(),
 	})
diff --git a/syz-cluster/pkg/service/discussion.go b/syz-cluster/pkg/service/discussion.go
index f4be21fbd..8c17f1623 100644
--- a/syz-cluster/pkg/service/discussion.go
+++ b/syz-cluster/pkg/service/discussion.go
@@ -31,22 +31,10 @@ func NewDiscussionService(env *app.AppEnvironment) *DiscussionService {
 }
 
 func (d *DiscussionService) RecordReply(ctx context.Context, req *api.RecordReplyReq) (*api.RecordReplyResp, error) {
-	// First check if the message was a directl reply to the report.
-	report, err := d.reportRepo.FindByMessageID(ctx, req.Reporter, req.InReplyTo)
+	reportID, err := d.identifyReport(ctx, req)
 	if err != nil {
-		return nil, fmt.Errorf("failed to search among the reports: %w", err)
-	}
-	var reportID string
-	if report != nil {
-		reportID = report.ID
-	} else {
-		// Now try to find a matching reply.
-		reportID, err = d.reportReplyRepo.FindParentReportID(ctx, req.Reporter, req.InReplyTo)
-		if err != nil {
-			return nil, fmt.Errorf("failed to search among the replies: %w", err)
-		}
-	}
-	if reportID == "" {
+		return nil, err
+	} else if reportID == "" {
 		// We could not find the related report.
 		return &api.RecordReplyResp{}, nil
 	}
@@ -78,3 +66,22 @@ func (d *DiscussionService) LastReply(ctx context.Context, reporter string) (*ap
 	}
 	return &api.LastReplyResp{}, nil
 }
+
+func (d *DiscussionService) identifyReport(ctx context.Context, req *api.RecordReplyReq) (string, error) {
+	// If the report ID was passed explicitly, just verify it.
+	if req.ReportID != "" {
+		report, err := d.reportRepo.GetByID(ctx, req.ReportID)
+		if err != nil {
+			return "", fmt.Errorf("failed to query the report: %w", err)
+		} else if report != nil {
+			return report.ID, nil
+		}
+		return "", nil
+	}
+	// Now try to find a matching reply.
+	reportID, err := d.reportReplyRepo.FindParentReportID(ctx, req.Reporter, req.InReplyTo)
+	if err != nil {
+		return "", fmt.Errorf("search among the replies failed: %w", err)
+	}
+	return reportID, nil
+}
diff --git a/syz-cluster/pkg/service/report.go b/syz-cluster/pkg/service/report.go
index 5e6b6756a..4faef5b1e 100644
--- a/syz-cluster/pkg/service/report.go
+++ b/syz-cluster/pkg/service/report.go
@@ -30,17 +30,6 @@ func NewReportService(env *app.AppEnvironment) *ReportService {
 
 var ErrReportNotFound = errors.New("report is not found")
 
-func (rs *ReportService) Update(ctx context.Context, id string, req *api.UpdateReportReq) error {
-	err := rs.reportRepo.Update(ctx, id, func(rep *db.SessionReport) error {
-		rep.MessageID = req.MessageID
-		return nil
-	})
-	if errors.Is(err, db.ErrEntityNotFound) {
-		return ErrReportNotFound
-	}
-	return err
-}
-
 func (rs *ReportService) Confirm(ctx context.Context, id string) error {
 	err := rs.reportRepo.Update(ctx, id, func(rep *db.SessionReport) error {
 		if rep.ReportedAt.IsNull() {