From 4af9880b86b789549e3dafb0878a4fb21393a3d2 Mon Sep 17 00:00:00 2001 From: kami Date: Fri, 28 Aug 2026 10:53:47 +0400 Subject: [PATCH] Start the submission reflection loop after something can fill its map The loop that reads merged pull requests was guarded by len(pullRequests) > 0 at a point 600 lines before the Gitea wiring that writes to that map. The length was always zero, so the goroutine never started and a merged pull request could never complete its task. Live on the first submission this deployment made: PR #8 took a trusted comment and nothing moved. Moving the block below the wiring fixes the ordering and avoids the race that reading the map inside the tick would have introduced. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CVbaKucEYBjMqVeUgJUsc1 --- cmd/orchestra/main.go | 82 +++++++++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 39 deletions(-) diff --git a/cmd/orchestra/main.go b/cmd/orchestra/main.go index 55734d2..38e5dc1 100644 --- a/cmd/orchestra/main.go +++ b/cmd/orchestra/main.go @@ -1073,45 +1073,6 @@ func main() { } }() } - if len(pullRequests) > 0 { - // Submitted work is reconciled on its own loop, not behind - // Store.PreLease: an in-review task cannot be leased, so a pre-lease - // hook could never see the feedback that should make it leasable. - trust := human.Trust{ - Accepted: splitList(os.Getenv("ORCHESTRA_REVIEW_ACTORS")), - Ignored: splitList(os.Getenv("ORCHESTRA_REVIEW_IGNORE_ACTORS")), - } - go func() { - ticker := time.NewTicker(time.Minute) - defer ticker.Stop() - for range ticker.C { - for _, t := range s.Tasks() { - if t.Submission == nil || (t.State != domain.StateInReview && t.State != domain.StateQueued) { - continue - } - source, ok := pullRequests[t.Submission.PR.Provider] - if !ok { - continue - } - project, ok := rr.Project(t.Project) - if !ok { - continue - } - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - state, err := source.PullRequest(ctx, t) - cancel() - if err != nil { - // Observable, and the task stays exactly where it was. - log.Printf("reflect submission %s: %v", t.ID, err) - continue - } - if _, err := operations.ReflectSubmission(s, project, t.ID, state, trust); err != nil { - log.Printf("reflect submission %s: %v", t.ID, err) - } - } - } - }() - } mux.HandleFunc("/healthz", func(w http.ResponseWriter, r *http.Request) { w.Write([]byte("ok\n")) }) mux.HandleFunc("/readyz", adminServer.Readiness) mux.HandleFunc("/v1/providers/health", func(w http.ResponseWriter, r *http.Request) { @@ -1718,6 +1679,49 @@ func main() { } } } + // Started here, after the forge wiring above fills pullRequests. Guarding + // on the map before anything writes to it made this loop dead code: the + // length was always zero, so a merged pull request never completed its + // task, and moving the read into the tick would race the startup writes. + if len(pullRequests) > 0 { + // Submitted work is reconciled on its own loop, not behind + // Store.PreLease: an in-review task cannot be leased, so a pre-lease + // hook could never see the feedback that should make it leasable. + trust := human.Trust{ + Accepted: splitList(os.Getenv("ORCHESTRA_REVIEW_ACTORS")), + Ignored: splitList(os.Getenv("ORCHESTRA_REVIEW_IGNORE_ACTORS")), + } + go func() { + ticker := time.NewTicker(time.Minute) + defer ticker.Stop() + for range ticker.C { + for _, t := range s.Tasks() { + if t.Submission == nil || (t.State != domain.StateInReview && t.State != domain.StateQueued) { + continue + } + source, ok := pullRequests[t.Submission.PR.Provider] + if !ok { + continue + } + project, ok := rr.Project(t.Project) + if !ok { + continue + } + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + state, err := source.PullRequest(ctx, t) + cancel() + if err != nil { + // Observable, and the task stays exactly where it was. + log.Printf("reflect submission %s: %v", t.ID, err) + continue + } + if _, err := operations.ReflectSubmission(s, project, t.ID, state, trust); err != nil { + log.Printf("reflect submission %s: %v", t.ID, err) + } + } + } + }() + } // Reconciliation runs immediately before every lease, which is where // ownership of a task begins. A configured source that cannot be read // refuses the lease rather than letting a successor resume from an older