From ec8bb0cc63794db16ea4757b3f91395ca7e432d6 Mon Sep 17 00:00:00 2001 From: Michael Hohn Date: Fri, 25 Jul 2025 16:31:34 -0700 Subject: [PATCH] fix repo id handling and PostgreSQL Query Ordering Patch 1: PostgreSQL Query Ordering Fixed GetJobList() to return jobs ordered by job_repo_id Added JOIN with job_repo_map table to ensure proper ordering This ensures slice indices match the stored repository IDs Patch 2: Updated Comments Removed the TODO comment about hacky job IDing Added explanation that ordering is now consistent Patch 3: Added Validation Added runtime validation to catch ID mismatches Logs warnings/errors if slice index doesn't match expected job_repo_id Helps debug issues in different state implementations --- pkg/server/server.go | 18 ++++++++++++++++-- pkg/state/state_postgres.go | 7 +++++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/pkg/server/server.go b/pkg/server/server.go index 13e91e1..a6a24f4 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -176,9 +176,23 @@ func (c *CommanderSingle) submitStatusResponse(w http.ResponseWriter, js common. } // Loop through all jobs under the same session id - // TODO: as a high priority, fix this hacky job IDing by index - // this may break with other state implementations + // The jobRepoId from the range should now match the stored job_repo_id + // due to the ORDER BY job_repo_id in GetJobList() for jobRepoId, job := range jobs { + // Validate that the slice index matches the expected repo ID + // This helps catch ordering issues between different state implementations + if expectedJobSpec, err := c.v.State.GetJobSpecByRepoId(js.SessionID, jobRepoId); err != nil { + slog.Warn("Job repo ID validation failed", + "sessionId", js.SessionID, + "jobRepoId", jobRepoId, + "error", err) + } else if expectedJobSpec.Owner != job.Spec.Owner || expectedJobSpec.Repo != job.Spec.Repo { + slog.Error("Job repo ID mismatch detected", + "sessionId", js.SessionID, + "jobRepoId", jobRepoId, + "expected", expectedJobSpec.NameWithOwner, + "actual", job.Spec.NameWithOwner) + } // Get the job status status, err := c.v.State.GetStatus(job.Spec) if err != nil { diff --git a/pkg/state/state_postgres.go b/pkg/state/state_postgres.go index 9deadf7..b9b7749 100644 --- a/pkg/state/state_postgres.go +++ b/pkg/state/state_postgres.go @@ -385,8 +385,11 @@ func (s *PGState) GetJobList(sessionId int) ([]queue.AnalyzeJob, error) { ctx := context.Background() rows, err := s.pool.Query(ctx, ` - SELECT payload FROM analyze_jobs - WHERE session_id = $1 + SELECT aj.payload FROM analyze_jobs aj + JOIN job_repo_map jrm ON aj.session_id = jrm.session_id + AND aj.owner = jrm.owner AND aj.repo = jrm.repo + WHERE aj.session_id = $1 + ORDER BY jrm.job_repo_id `, sessionId) if err != nil { return nil, err