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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CVbaKucEYBjMqVeUgJUsc1
This commit is contained in:
+43
-39
@@ -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("/healthz", func(w http.ResponseWriter, r *http.Request) { w.Write([]byte("ok\n")) })
|
||||||
mux.HandleFunc("/readyz", adminServer.Readiness)
|
mux.HandleFunc("/readyz", adminServer.Readiness)
|
||||||
mux.HandleFunc("/v1/providers/health", func(w http.ResponseWriter, r *http.Request) {
|
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
|
// Reconciliation runs immediately before every lease, which is where
|
||||||
// ownership of a task begins. A configured source that cannot be read
|
// ownership of a task begins. A configured source that cannot be read
|
||||||
// refuses the lease rather than letting a successor resume from an older
|
// refuses the lease rather than letting a successor resume from an older
|
||||||
|
|||||||
Reference in New Issue
Block a user