From c02de4b59250354b621824d5241b443406c61b92 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 29 Jul 2022 13:57:29 +0200
Subject: [PATCH 01/54] Initial implementation of merge trains (#77)

This passes all current tests but some edge cases may be untested.

Here's what it works for:

* build success events
* build failure events
* PR closing events

Here's what it may not work for:

* new commits in earlier wagons of a train
* rebase failures

These will be sorted out in further commits.
---
 src/Logic.hs           | 150 ++++++++++++++++++++++++---------
 src/Project.hs         |  45 +++++++++-
 tests/EventLoopSpec.hs |  18 ++--
 tests/Spec.hs          | 186 ++++++++++++++++++++++++++---------------
 4 files changed, 281 insertions(+), 118 deletions(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index fadf077a..1f11c927 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -36,14 +36,14 @@ where
 import Control.Concurrent.STM.TBQueue (TBQueue, newTBQueue, readTBQueue, writeTBQueue)
 import Control.Concurrent.STM.TMVar (TMVar, newTMVarIO, readTMVar, swapTMVar)
 import Control.Exception (assert)
-import Control.Monad (foldM, unless, void, when)
+import Control.Monad (foldM, unless, void, when, (>=>))
 import Control.Monad.Free (Free (..), foldFree, hoistFree, liftF)
 import Control.Monad.STM (atomically)
 import Data.Bifunctor (first)
 import Data.Either.Extra (maybeToEither)
 import Data.Functor.Sum (Sum (InL, InR))
 import Data.IntSet (IntSet)
-import Data.Maybe (fromJust, isJust, listToMaybe)
+import Data.Maybe (fromJust, isJust, listToMaybe, fromMaybe)
 import Data.Text (Text)
 import Data.Text.Lazy (toStrict)
 import GHC.Natural (Natural)
@@ -79,6 +79,7 @@ data ActionFree a
     -- This is a record type, but the names are currently only used for documentation.
     { _mergeCommitMessage   :: Text
     , _integrationCandidate :: (PullRequestId, Branch, Sha)
+    , _train                :: [PullRequestId]
     , _alwaysAddMergeCommit :: Bool
     , _cont                 :: Either IntegrationFailure Sha -> a
     }
@@ -97,6 +98,7 @@ data ActionFree a
 data PRCloseCause =
       User            -- ^ The user closed the PR.
     | StopIntegration -- ^ We close and reopen the PR internally to stop its integration if it is approved.
+  deriving Show
 
 type Action = Free ActionFree
 
@@ -118,8 +120,8 @@ doGit = hoistFree (InR . InL)
 doGithub :: GithubOperation a -> Operation a
 doGithub = hoistFree (InR . InR)
 
-tryIntegrate :: Text -> (PullRequestId, Branch, Sha) -> Bool -> Action (Either IntegrationFailure Sha)
-tryIntegrate mergeMessage candidate alwaysAddMergeCommit = liftF $ TryIntegrate mergeMessage candidate alwaysAddMergeCommit id
+tryIntegrate :: Text -> (PullRequestId, Branch, Sha) -> [PullRequestId] -> Bool -> Action (Either IntegrationFailure Sha)
+tryIntegrate mergeMessage candidate train alwaysAddMergeCommit = liftF $ TryIntegrate mergeMessage candidate train alwaysAddMergeCommit id
 
 -- Try to fast-forward the remote target branch (usually master) to the new sha.
 -- Before doing so, force-push that SHA to the pull request branch, and after
@@ -161,7 +163,7 @@ getDateTime = liftF $ GetDateTime id
 -- Interpreter that translates high-level actions into more low-level ones.
 runAction :: ProjectConfiguration -> Action a -> Operation a
 runAction config = foldFree $ \case
-  TryIntegrate message (pr, ref, sha) alwaysAddMergeCommit cont -> do
+  TryIntegrate message (pr, ref, sha) train alwaysAddMergeCommit cont -> do
     doGit $ ensureCloned config
 
     -- Needed for backwards compatibility with existing repositories
@@ -173,7 +175,7 @@ runAction config = foldFree $ \case
       message
       ref
       sha
-      (Git.RemoteBranch $ Config.branch config)
+      (fromMaybe (Git.RemoteBranch $ Config.branch config) (trainBranch train))
       (testBranch config pr)
       alwaysAddMergeCommit
 
@@ -223,7 +225,8 @@ runAction config = foldFree $ \case
     openPrIds <- doGithub GithubApi.getOpenPullRequests
     pure $ cont openPrIds
 
-  GetLatestVersion sha cont -> doGit $
+  GetLatestVersion sha cont -> doGit $ do
+    Git.fetchBranchWithTags $ Branch (Config.branch config)
     cont . maybe (Right 0) (\t -> maybeToEither t $ parseVersion t) <$> Git.lastTag sha
 
   GetChangelog prevTag curHead cont -> doGit $
@@ -231,6 +234,11 @@ runAction config = foldFree $ \case
 
   GetDateTime cont -> doTime $ cont <$> Time.getDateTime
 
+  where
+  trainBranch :: [PullRequestId] -> Maybe Git.RemoteBranch
+  trainBranch [] = Nothing
+  trainBranch train = Just $ last [testRemoteBranch config pr | pr <- train]
+
 ensureCloned :: ProjectConfiguration -> GitOperation ()
 ensureCloned config =
   let
@@ -370,9 +378,14 @@ handlePullRequestClosedByUser = handlePullRequestClosed User
 
 handlePullRequestClosed :: PRCloseCause -> PullRequestId -> ProjectState -> Action ProjectState
 handlePullRequestClosed closingReason pr state = do
-  when (pr `elem` Pr.integratedPullRequests state) $
+  when (pr `elem` Pr.unfailingIntegratedPullRequests state) $
     leaveComment pr $ prClosingMessage closingReason
-  pure $ Pr.deletePullRequest pr state
+  -- actually delete the pull request
+  pure . Pr.deletePullRequest pr
+       $ case Pr.lookupPullRequest pr state of
+         Just (Pr.PullRequest{Pr.integrationStatus = Promoted}) -> state
+         -- we unintegrate PRs after it if it has not been promoted to master
+         _ -> unintegrateAfter pr $ state
 
 handlePullRequestEdited :: PullRequestId -> Text -> BaseBranch -> ProjectState -> Action ProjectState
 handlePullRequestEdited prId newTitle newBaseBranch state =
@@ -542,20 +555,36 @@ handleMergeRequested projectConfig prId author state pr approvalType = do
     then pure $ Pr.setIntegrationStatus prId IncorrectBaseBranch state''
     else pure state''
 
+-- | Given a pull request id, mark all pull requests that follow from it
+--   in the merge train as NotIntegrated
+unintegrateAfter :: PullRequestId -> ProjectState -> ProjectState
+unintegrateAfter pid state =
+  compose [ Pr.updatePullRequest pid' unintegrate
+          | pid' <- Pr.integratedPullRequestsAfter pid state] state
+  where
+  unintegrate pr = pr{Pr.integrationStatus = NotIntegrated}
+
+-- | If there is an integration candidate, and its integration sha matches that of the build,
+--   then update the build status for that pull request. Otherwise do nothing.
 handleBuildStatusChanged :: Sha -> BuildStatus -> ProjectState -> Action ProjectState
-handleBuildStatusChanged buildSha newStatus = pure . Pr.updatePullRequests setBuildStatus
+handleBuildStatusChanged buildSha newStatus state = pure $
+  compose [ Pr.updatePullRequest pid setBuildStatus
+          . case newStatus of
+            BuildFailed _ -> unintegrateAfter pid
+            _             -> id
+          | pid <- Pr.filterPullRequestsBy shouldUpdate state
+          ] state
   where
-  setBuildStatus pr = case Pr.integrationStatus pr of
-    -- If there is an integration candidate, and its integration sha matches that of the build,
-    -- then update the build status for that pull request. Otherwise do nothing.
-    Integrated candidateSha oldStatus | candidateSha == buildSha && newStatus `supersedes` oldStatus ->
-      pr { Pr.integrationStatus = Integrated buildSha newStatus
-         , Pr.needsFeedback = case newStatus of
-                              BuildStarted _ -> True
-                              BuildFailed _  -> True
-                              _              -> Pr.needsFeedback pr -- unchanged
-         }
-    _ -> pr
+  shouldUpdate pr = case Pr.integrationStatus pr of
+    Integrated candidateSha oldStatus -> candidateSha == buildSha && newStatus `supersedes` oldStatus
+    _                                 -> False
+  setBuildStatus pr = pr
+                    { Pr.integrationStatus = Integrated buildSha newStatus
+                    , Pr.needsFeedback = case newStatus of
+                                         BuildStarted _ -> True
+                                         BuildFailed _  -> True
+                                         _              -> Pr.needsFeedback pr -- unchanged
+                    }
 
 -- | Does the first build status supersedes the second?
 --
@@ -612,15 +641,19 @@ synchronizeState stateInitial =
 -- should find a new candidate. Or after the pull request for which a build is
 -- in progress is closed, we should find a new candidate.
 proceed :: ProjectState -> Action ProjectState
-proceed state = do
-  state' <- provideFeedback state
-  case (Pr.integratedPullRequests state', Pr.candidatePullRequests state') of
-                           -- Proceed with an already integrated candidate
-    (candidate:_, _)    -> proceedCandidate candidate state'
-                           -- Found a new candidate, try to integrate it.
-    (_,           pr:_) -> tryIntegratePullRequest pr state'
-                           -- No pull requests eligible, do nothing.
-    (_,           _)    -> return state'
+proceed = provideFeedback
+      >=> proceedSomeCandidate
+      >=> tryIntegrateSomePullRequest
+
+proceedSomeCandidate :: ProjectState -> Action ProjectState
+proceedSomeCandidate state = case Pr.unfailingIntegratedPullRequests state of
+  (candidate:_) -> proceedCandidate candidate state
+  _ -> pure state
+
+tryIntegrateSomePullRequest :: ProjectState -> Action ProjectState
+tryIntegrateSomePullRequest state = case Pr.candidatePullRequests state of
+  (pr:_) -> tryIntegratePullRequest pr state
+  _ -> pure state
 
 -- | Pushes the given integrated PR to be the new master if the build succeeded
 proceedCandidate :: PullRequestId -> ProjectState -> Action ProjectState
@@ -637,6 +670,19 @@ proceedCandidate pullRequestId state =
 getPullRequestRef :: PullRequestId -> Branch
 getPullRequestRef (PullRequestId n) = Branch $ format "refs/pull/{}/head" [n]
 
+-- TODO: get rid of the getTrain function in favour of just *integratedPullRequests?
+getTrain :: ProjectState -> [PullRequestId]
+getTrain state =
+  [ pid
+  | pid <- Pr.unfailingIntegratedPullRequests state
+  , Just pr <- [Pr.lookupPullRequest pid state]
+  , Integrated _ buildStatus <- [Pr.integrationStatus pr]
+  , case buildStatus of
+    BuildPending   -> True
+    BuildStarted _ -> True
+    _              -> False
+  ]
+
 -- Integrates proposed changes from the pull request into the target branch.
 -- The pull request must exist in the project.
 tryIntegratePullRequest :: PullRequestId -> ProjectState -> Action ProjectState
@@ -657,7 +703,7 @@ tryIntegratePullRequest pr state =
       ]
     mergeMessage = Text.unlines mergeMessageLines
   in do
-    result <- tryIntegrate mergeMessage candidate $ Pr.alwaysAddMergeCommit approvalType
+    result <- tryIntegrate mergeMessage candidate (getTrain state) $ Pr.alwaysAddMergeCommit approvalType
     case result of
       Left (IntegrationFailure targetBranch reason) ->
         -- If integrating failed, perform no further actions but do set the
@@ -749,8 +795,14 @@ describeStatus prId pr state = case Pr.classifyPullRequest pr of
       0 -> format "Pull request approved for {} by @{}, rebasing now." [approvalCommand, approvedBy]
       1 -> format "Pull request approved for {} by @{}, waiting for rebase behind one pull request." [approvalCommand, approvedBy]
       n -> format "Pull request approved for {} by @{}, waiting for rebase behind {} pull requests." (approvalCommand, approvedBy, n)
-  PrStatusBuildPending -> let Sha sha = fromJust $ getIntegrationSha pr
-                          in  Text.concat ["Rebased as ", sha, ", waiting for CI …"]
+  PrStatusBuildPending -> let Sha sha = fromJust $ Pr.integrationSha pr
+                              train = takeWhile (/= prId) $ getTrain state
+                          in case train of
+                             []    -> Text.concat ["Rebased as ", sha, ", waiting for CI …"]
+                             (_:_) -> Text.concat [ "Speculatively rebased as ", sha
+                                                  , " behind ", prettyPullRequestIds train
+                                                  , ", waiting for CI …"
+                                                  ]
   PrStatusBuildStarted url -> Text.concat ["[CI job](", url, ") started."]
   PrStatusIntegrated -> "The build succeeded."
   PrStatusIncorrectBaseBranch -> "Merge rejected: the target branch must be the integration branch."
@@ -773,12 +825,6 @@ describeStatus prId pr state = case Pr.classifyPullRequest pr of
                               Just url' -> format "The build failed: {}\nIf this is the result of a flaky test, close and reopen the PR, then tag me again.\nOtherwise, push a new commit and tag me again." [url']
                               -- This should probably never happen
                               Nothing   -> "The build failed, but GitHub did not provide an URL to the build failure."
-  where
-    getIntegrationSha :: PullRequest -> Maybe Sha
-    getIntegrationSha pullRequest =
-      case Pr.integrationStatus pullRequest of
-        Integrated sha _ -> Just sha
-        _                -> Nothing
 
 -- Leave a comment with the feedback from 'describeStatus' and set the
 -- 'needsFeedback' flag to 'False'.
@@ -829,3 +875,29 @@ pullRequestIdToText (PullRequestId prid) = Text.pack $ show prid
 
 testBranch :: ProjectConfiguration -> PullRequestId -> Git.Branch
 testBranch config pullRequestId = Git.Branch $ Config.testBranch config <> "/" <> pullRequestIdToText pullRequestId
+
+testRemoteBranch :: ProjectConfiguration -> PullRequestId -> Git.RemoteBranch
+testRemoteBranch config pullRequestId = Git.RemoteBranch $ Config.testBranch config <> "/" <> pullRequestIdToText pullRequestId
+
+-- | Textual rendering of a list of 'PullRequestId's
+--
+-- >>> prettyPullRequestIds [PullRequestId 12, PullRequestId 60, PullRequestId 1337]
+-- "#12, #60 and #1337"
+prettyPullRequestIds :: [PullRequestId] -> Text
+prettyPullRequestIds = commaAnd . map prettyPullRequestId
+  where
+  prettyPullRequestId (PullRequestId n) = "#" <> Text.pack (show n)
+
+-- | Pretty printing of a list of Text with comma and and.
+--
+-- >>> commaAnd ["a", "b", "c" :: Text]
+-- "a, b and c"
+commaAnd :: [Text] -> Text
+commaAnd [] = "none"
+commaAnd ss = case init ss of
+              [] -> last ss
+              is -> Text.intercalate ", " is <> " and " <> last ss
+
+-- | Fold a list of unary functions by composition
+compose :: [a -> a] -> a -> a
+compose = foldr (.) id
diff --git a/src/Project.hs b/src/Project.hs
index 6e0c4120..5f36154e 100644
--- a/src/Project.hs
+++ b/src/Project.hs
@@ -22,6 +22,8 @@ module Project
   Owner,
   approvedPullRequests,
   integratedPullRequests,
+  integratedPullRequestsAfter,
+  unfailingIntegratedPullRequests,
   candidatePullRequests,
   classifyPullRequest,
   classifyPullRequests,
@@ -30,6 +32,8 @@ module Project
   existsPullRequest,
   getQueuePosition,
   insertPullRequest,
+  integrationSha,
+  lookupIntegrationSha,
   loadProjectState,
   lookupPullRequest,
   saveProjectState,
@@ -44,11 +48,14 @@ module Project
   updatePullRequest,
   updatePullRequestM,
   updatePullRequests,
+  updatePullRequestsWithId,
   getOwners,
   wasIntegrationAttemptFor,
+  filterPullRequestsBy,
   MergeWindow(..))
 where
 
+import Control.Monad ((<=<))
 import Data.Aeson (FromJSON, ToJSON)
 import Data.ByteString (readFile)
 import Data.ByteString.Lazy (writeFile)
@@ -259,6 +266,11 @@ updatePullRequests f state = state {
   pullRequests = IntMap.map f $ pullRequests state
 }
 
+updatePullRequestsWithId :: (PullRequestId -> PullRequest -> PullRequest) -> ProjectState -> ProjectState
+updatePullRequestsWithId f state = state {
+  pullRequests = IntMap.mapWithKey (f . PullRequestId) $ pullRequests state
+}
+
 -- Marks the pull request as approved by somebody or nobody.
 setApproval :: PullRequestId -> Maybe Approval -> ProjectState -> ProjectState
 setApproval pr newApproval = updatePullRequest pr changeApproval
@@ -383,10 +395,28 @@ wasIntegrationAttemptFor commit pr = case integrationStatus pr of
 integratedPullRequests :: ProjectState -> [PullRequestId]
 integratedPullRequests = filterPullRequestsBy $ isIntegrated . integrationStatus
   where
-  isIntegrated (Integrated _ BuildPending)     = True
-  isIntegrated (Integrated _ (BuildStarted _)) = True
-  isIntegrated (Integrated _ BuildSucceeded)   = True
-  isIntegrated _                               = False
+  isIntegrated (Integrated _ _) = True
+  isIntegrated _                = False
+
+-- | Lists the pull requests that are integrated on top of the given id.
+integratedPullRequestsAfter :: PullRequestId -> ProjectState -> [PullRequestId]
+integratedPullRequestsAfter pid state =
+  case approvalOrder <$> (approval =<< lookupPullRequest pid state) of
+  Nothing    -> []
+  Just order -> filterPullRequestsBy (isIntegratedAfter order) state
+  where
+  isIntegratedAfter order pr = isIntegrated (integrationStatus pr)
+                            && (approvalOrder <$> approval pr) > Just order
+  isIntegrated (Integrated _ _) = True
+  isIntegrated _                = False
+
+unfailingIntegratedPullRequests :: ProjectState -> [PullRequestId]
+unfailingIntegratedPullRequests = filterPullRequestsBy $ isUnfailingIntegrated . integrationStatus
+  where
+  isUnfailingIntegrated (Integrated _ BuildPending)     = True
+  isUnfailingIntegrated (Integrated _ (BuildStarted _)) = True
+  isUnfailingIntegrated (Integrated _ BuildSucceeded)   = True
+  isUnfailingIntegrated _                               = False
 
 -- Returns the pull requests that have not been integrated yet, in order of
 -- ascending id.
@@ -425,3 +455,10 @@ needsTag :: ApprovedFor -> Bool
 needsTag Merge          = False
 needsTag MergeAndDeploy = True
 needsTag MergeAndTag    = True
+
+integrationSha :: PullRequest -> Maybe Sha
+integrationSha PullRequest{integrationStatus = Integrated s _} = Just s
+integrationSha _                                               = Nothing
+
+lookupIntegrationSha :: PullRequestId -> ProjectState -> Maybe Sha
+lookupIntegrationSha pid = integrationSha <=< lookupPullRequest pid
diff --git a/tests/EventLoopSpec.hs b/tests/EventLoopSpec.hs
index b1f1bfce..ab7cf243 100644
--- a/tests/EventLoopSpec.hs
+++ b/tests/EventLoopSpec.hs
@@ -378,7 +378,7 @@ withTestEnv' body = do
 -- | lists the integration Shas from the state for all PRs which are Integrated
 integrationShas :: ProjectState -> [Sha]
 integrationShas state = [ sha
-                        | prId <- Project.integratedPullRequests state
+                        | prId <- Project.unfailingIntegratedPullRequests state
                         , Just pr <- [Project.lookupPullRequest prId state]
                         , Integrated sha _ <- [Project.integrationStatus pr]
                         ]
@@ -726,7 +726,7 @@ eventLoopSpec = parallel $ do
           ]
 
         -- Extract the sha of the rebased commit from the project state.
-        let [rebasedSha] = integrationShas state
+        let [rebasedSha,_] = integrationShas state
 
         -- The rebased commit should have been pushed to the remote repository
         -- 'integration' branch. Tell that building it succeeded.
@@ -767,7 +767,7 @@ eventLoopSpec = parallel $ do
             Logic.CommentAdded pr4 "rachael" "@bot merge and tag"
           ]
 
-        let [rebasedSha] = integrationShas state
+        let [rebasedSha,_] = integrationShas state
 
         state' <- runLoop state [Logic.BuildStatusChanged rebasedSha BuildSucceeded]
 
@@ -853,7 +853,7 @@ eventLoopSpec = parallel $ do
 
         -- The second pull request should still be pending, awaiting the build
         -- result.
-        Project.integratedPullRequests state `shouldBe` [pr4]
+        Project.unfailingIntegratedPullRequests state `shouldBe` [pr4]
         let Just pullRequest4 = Project.lookupPullRequest pr4 state
             Integrated _ buildStatus = Project.integrationStatus pullRequest4
         -- Expect no CI url
@@ -898,7 +898,7 @@ eventLoopSpec = parallel $ do
 
         -- The push should have failed, hence there should still be an
         -- integration candidate.
-        Project.integratedPullRequests state' `shouldSatisfy` (not . null)
+        Project.unfailingIntegratedPullRequests state' `shouldSatisfy` (not . null)
 
         -- Again notify build success, now for the new commit.
         let [rebasedSha'] = integrationShas state'
@@ -906,7 +906,7 @@ eventLoopSpec = parallel $ do
 
         -- After the second build success, the pull request should have been
         -- integrated properly, so there should not be a new candidate.
-        Project.integratedPullRequests state'' `shouldBe` []
+        Project.unfailingIntegratedPullRequests state'' `shouldBe` []
 
       history `shouldBe`
         [ "*   Merge #6"
@@ -1181,7 +1181,7 @@ eventLoopSpec = parallel $ do
         --The pull request should not be integrated. Moreover, the presence of
         --orphan fixups should make the PR ineligible for being a candidate for integration.
         --That is, we expect no candidates for integration.
-        Project.integratedPullRequests state' `shouldBe` []
+        Project.unfailingIntegratedPullRequests state' `shouldBe` []
 
       -- Here we expect that the fixup commit is not present.
       history `shouldBe`
@@ -1210,7 +1210,7 @@ eventLoopSpec = parallel $ do
             Logic.CommentAdded pr8 "rachael" "@bot merge"
           ]
 
-        Project.integratedPullRequests state `shouldBe` [pr8]
+        Project.unfailingIntegratedPullRequests state `shouldBe` [pr8]
 
         let [rebasedSha] = integrationShas state
 
@@ -1222,7 +1222,7 @@ eventLoopSpec = parallel $ do
             Logic.CommentAdded pr6 "rachael" "@bot merge"
           ]
 
-        Project.integratedPullRequests state' `shouldBe` []
+        Project.unfailingIntegratedPullRequests state' `shouldBe` []
 
         let Just pullRequest' = Project.lookupPullRequest pr6 state'
         Project.integrationStatus pullRequest' `shouldBe`
diff --git a/tests/Spec.hs b/tests/Spec.hs
index 66e668d4..6027b40f 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -89,6 +89,7 @@ data ActionFlat
   = ATryIntegrate
     { mergeMessage         :: Text
     , integrationCandidate :: (PullRequestId, Branch, Sha)
+    , mergeTrain           :: [PullRequestId]
     , alwaysAddMergeCommit :: Bool
     }
   | ATryPromote Branch Sha
@@ -202,8 +203,8 @@ runActionRws =
     isReviewer username = elem username ["deckard", "bot"]
   in
     foldFree $ \case
-      TryIntegrate msg candidate alwaysAddMergeCommit' cont -> do
-        Rws.tell [ATryIntegrate msg candidate alwaysAddMergeCommit']
+      TryIntegrate msg candidate train alwaysAddMergeCommit' cont -> do
+        Rws.tell [ATryIntegrate msg candidate train alwaysAddMergeCommit']
         cont <$> takeResultIntegrate
       TryPromote prBranch headSha cont -> do
         Rws.tell [ATryPromote prBranch headSha]
@@ -249,11 +250,11 @@ handleEventTest = Logic.handleEvent testTriggerConfig testProjectConfig testmerg
 handleEventsTest :: [Event] -> ProjectState -> Action ProjectState
 handleEventsTest events state = foldlM (flip $ Logic.handleEvent testTriggerConfig testProjectConfig testmergeWindowExemptionConfig) state events
 
--- Same as 'integratedPullRequests' but paired with the underlying objects.
+-- Same as 'unfailingIntegratedPullRequests' but paired with the underlying objects.
 getIntegrationCandidates :: ProjectState -> [(PullRequestId, PullRequest)]
 getIntegrationCandidates state =
   [ (pullRequestId, candidate)
-  | pullRequestId <- Project.integratedPullRequests state
+  | pullRequestId <- Project.unfailingIntegratedPullRequests state
   , Just candidate <- [Project.lookupPullRequest pullRequestId state]
   ]
 
@@ -291,13 +292,13 @@ main = hspec $ do
       let event  = PullRequestClosed (PullRequestId 1)
           state  = candidateState (PullRequestId 1) (Branch "p") masterBranch (Sha "ea0") "frank" "deckard" (Sha "cf4")
           state' = fst $ runAction $ handleEventTest event state
-      Project.integratedPullRequests state' `shouldBe` []
+      Project.unfailingIntegratedPullRequests state' `shouldBe` []
 
     it "does not modify the integration candidate if a different PR was closed" $ do
       let event  = PullRequestClosed (PullRequestId 1)
           state  = candidateState (PullRequestId 2) (Branch "p") masterBranch (Sha "a38") "franz" "deckard" (Sha "ed0")
           state' = fst $ runAction $ handleEventTest event state
-      Project.integratedPullRequests state' `shouldBe` [PullRequestId 2]
+      Project.unfailingIntegratedPullRequests state' `shouldBe` [PullRequestId 2]
 
     it "loses approval after the PR commit has changed" $ do
       let event  = PullRequestCommitChanged (PullRequestId 1) (Sha "def")
@@ -455,7 +456,7 @@ main = hspec $ do
         [ AIsReviewer "deckard"
         , ALeaveComment (PullRequestId 1) "Pull request approved for merge by @deckard, rebasing now."
         , ATryIntegrate "Merge #1: Untitled\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "a38") False
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "a38") [] False
         , ALeaveComment (PullRequestId 1)
             "Failed to rebase, please rebase manually using\n\n\
             \    git rebase --interactive --autosquash origin/master p"
@@ -475,19 +476,34 @@ main = hspec $ do
           , CommentAdded (PullRequestId 3) "deckard" "@bot merge and deploy"
           ]
         -- For this test, we assume all integrations and pushes succeed.
-        results = defaultResults { resultIntegrate = [Right (Sha "b71")] }
+        results = defaultResults { resultIntegrate = [ Right (Sha "b71")
+                                                     , Right (Sha "c82")
+                                                     , Right (Sha "d93")
+                                                     ] }
         run = runActionCustom results
         actions = snd $ run $ handleEventsTest events state
       actions `shouldBe`
         [ AIsReviewer "deckard"
         , ALeaveComment (PullRequestId 1) "Pull request approved for merge by @deckard, rebasing now."
         , ATryIntegrate "Merge #1: Add Nexus 7 experiment\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "a38") False
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "a38") [] False
         , ALeaveComment (PullRequestId 1) "Rebased as b71, waiting for CI …"
+
         , AIsReviewer "deckard"
         , ALeaveComment (PullRequestId 2) "Pull request approved for merge by @deckard, waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #2: Some PR\n\nApproved-by: deckard\nAuto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "dec")
+                        [PullRequestId 1]
+                        False
+        , ALeaveComment (PullRequestId 2) "Speculatively rebased as c82 behind #1, waiting for CI …"
+
         , AIsReviewer "deckard"
         , ALeaveComment (PullRequestId 3) "Pull request approved for merge and deploy by @deckard, waiting for rebase behind 2 pull requests."
+        , ATryIntegrate "Merge #3: Another PR\n\nApproved-by: deckard\nAuto-deploy: true\n"
+                        (PullRequestId 3, Branch "refs/pull/3/head", Sha "f16")
+                        [PullRequestId 1, PullRequestId 2]
+                        True
+        , ALeaveComment (PullRequestId 3) "Speculatively rebased as d93 behind #1 and #2, waiting for CI …"
         ]
     it "keeps the order position for the comments" $ do
       let
@@ -503,19 +519,25 @@ main = hspec $ do
           , CommentAdded (PullRequestId 2) "deckard" "@bot merge"
           ]
         -- For this test, we assume all integrations and pushes succeed.
-        results = defaultResults { resultIntegrate = [Right (Sha "b71")] }
+        results = defaultResults { resultIntegrate = [Right (Sha "b71"), Right (Sha "b72"), Right (Sha "b73")] }
         run = runActionCustom results
         (state', actions) = run $ handleEventsTest events state
       actions `shouldBe`
         [ AIsReviewer "deckard"
         , ALeaveComment (PullRequestId 1) "Pull request approved for merge by @deckard, rebasing now."
         , ATryIntegrate "Merge #1: Add Nexus 7 experiment\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "a38") False
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "a38") [] False
         , ALeaveComment (PullRequestId 1) "Rebased as b71, waiting for CI …"
         , AIsReviewer "deckard"
         , ALeaveComment (PullRequestId 3) "Pull request approved for merge by @deckard, waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #3: Another PR\n\nApproved-by: deckard\nAuto-deploy: false\n"
+                        (PullRequestId 3, Branch "refs/pull/3/head", Sha "f16") [PullRequestId 1] False
+        , ALeaveComment (PullRequestId 3) "Speculatively rebased as b72 behind #1, waiting for CI …"
         , AIsReviewer "deckard"
         , ALeaveComment (PullRequestId 2) "Pull request approved for merge by @deckard, waiting for rebase behind 2 pull requests."
+        , ATryIntegrate "Merge #2: Some PR\n\nApproved-by: deckard\nAuto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "dec") [PullRequestId 1, PullRequestId 3] False
+        , ALeaveComment (PullRequestId 2) "Speculatively rebased as b73 behind #1 and #3, waiting for CI …"
         ]
       Project.pullRequestApprovalIndex state' `shouldBe` 3
       Project.pullRequests state' `shouldBe`
@@ -538,7 +560,7 @@ main = hspec $ do
             title = "Some PR",
             author = Username "rachael",
             approval = Just (Approval (Username "deckard") Project.Merge 2),
-            integrationStatus = Project.NotIntegrated,
+            integrationStatus = Project.Integrated (Sha "b73") Project.BuildPending,
             integrationAttempts = [],
             needsFeedback = False
             })
@@ -549,7 +571,7 @@ main = hspec $ do
             title = "Another PR",
             author = Username "rachael",
             approval = Just (Approval (Username "deckard") Project.Merge 1),
-            integrationStatus = Project.NotIntegrated,
+            integrationStatus = Project.Integrated (Sha "b72") Project.BuildPending,
             integrationAttempts = [],
             needsFeedback = False
             })
@@ -568,12 +590,20 @@ main = hspec $ do
         [ AIsReviewer "deckard"
         , ALeaveComment (PullRequestId 2) "Pull request approved for merge by @deckard, rebasing now."
         , ATryIntegrate "Merge #2: Some PR\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "dec") False
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "dec") [] False
         , ALeaveComment (PullRequestId 2) "Rebased as b71, waiting for CI …"
+
         , AIsReviewer "deckard"
         , ALeaveComment (PullRequestId 1) "Pull request approved for merge by @deckard, waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #1: Add Nexus 7 experiment\n\nApproved-by: deckard\nAuto-deploy: false\n"
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "a38") [PullRequestId 2] False
+        , ALeaveComment (PullRequestId 1) "Speculatively rebased as b72 behind #2, waiting for CI …"
+
         , AIsReviewer "deckard"
         , ALeaveComment (PullRequestId 3) "Pull request approved for merge by @deckard, waiting for rebase behind 2 pull requests."
+        , ATryIntegrate "Merge #3: Another PR\n\nApproved-by: deckard\nAuto-deploy: false\n"
+                        (PullRequestId 3, Branch "refs/pull/3/head", Sha "f16") [PullRequestId 2, PullRequestId 1] False
+        , ALeaveComment (PullRequestId 3) "Speculatively rebased as b73 behind #2 and #1, waiting for CI …"
         ]
 
     it "abandons integration when a pull request is closed" $ do
@@ -590,27 +620,29 @@ main = hspec $ do
           ]
         -- For this test, we assume all integrations and pushes succeed.
         results = defaultResults
-          { resultIntegrate = [Right (Sha "b71"), Right (Sha "b72")]
-          }
+          { resultIntegrate = [Right (Sha "b71"), Right (Sha "b72"), Right (Sha "b73")] }
         run = runActionCustom results
         (state', actions) = run $ handleEventsTest events state
 
       -- The first pull request should be dropped, and a comment should be
       -- left indicating why. Then the second pull request should be at the
       -- front of the queue.
-      Project.integratedPullRequests state' `shouldBe` [PullRequestId 2]
+      Project.unfailingIntegratedPullRequests state' `shouldBe` [PullRequestId 2]
       actions `shouldBe`
         [ AIsReviewer "deckard"
         , ALeaveComment (PullRequestId 1) "Pull request approved for merge by @deckard, rebasing now."
         , ATryIntegrate "Merge #1: Add Nexus 7 experiment\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "a38") False
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "a38") [] False
         , ALeaveComment (PullRequestId 1) "Rebased as b71, waiting for CI …"
         , AIsReviewer "deckard"
         , ALeaveComment (PullRequestId 2) "Pull request approved for merge by @deckard, waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #2: Some PR\n\nApproved-by: deckard\nAuto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "dec") [PullRequestId 1] False
+        , ALeaveComment (PullRequestId 2) "Speculatively rebased as b72 behind #1, waiting for CI …"
         , ALeaveComment (PullRequestId 1) "Abandoning this pull request because it was closed."
         , ATryIntegrate "Merge #2: Some PR\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "dec") False
-        , ALeaveComment (PullRequestId 2) "Rebased as b72, waiting for CI …"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "dec") [] False
+        , ALeaveComment (PullRequestId 2) "Rebased as b73, waiting for CI …"
         ]
 
     it "ignores comments on unknown pull requests" $ do
@@ -717,7 +749,7 @@ main = hspec $ do
         [ AIsReviewer "deckard"
         , ALeaveComment prId "Pull request approved for merge and deploy by @deckard, rebasing now."
         , ATryIntegrate "Merge #1: Untitled\n\nApproved-by: deckard\nAuto-deploy: true\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") True
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") [] True
         , ALeaveComment prId "Rebased as def2345, waiting for CI \x2026"
         ]
 
@@ -738,7 +770,7 @@ main = hspec $ do
         [ AIsReviewer "deckard"
         , ALeaveComment prId "Pull request approved for merge and deploy by @deckard, rebasing now."
         , ATryIntegrate "Merge #1: Untitled\n\nApproved-by: deckard\nAuto-deploy: true\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") True
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") [] True
         , ALeaveComment prId "Rebased as def2345, waiting for CI \x2026"
         ]
 
@@ -759,7 +791,7 @@ main = hspec $ do
         [ AIsReviewer "deckard"
         , ALeaveComment prId "Pull request approved for merge and tag by @deckard, rebasing now."
         , ATryIntegrate "Merge #1: Untitled\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") False
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") [] False
         , ALeaveComment prId "Rebased as def2345, waiting for CI \x2026"
         ]
 
@@ -780,7 +812,7 @@ main = hspec $ do
         [ AIsReviewer "deckard"
         , ALeaveComment prId "Pull request approved for merge and tag by @deckard, rebasing now."
         , ATryIntegrate "Merge #1: Untitled\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") False
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") [] False
         , ALeaveComment prId "Rebased as def2345, waiting for CI \x2026"
         ]
 
@@ -801,7 +833,7 @@ main = hspec $ do
         [ AIsReviewer "deckard"
         , ALeaveComment prId "Pull request approved for merge and tag by @deckard, rebasing now."
         , ATryIntegrate "Merge #1: Untitled\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") False
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") [] False
         , ALeaveComment prId "Rebased as def2345, waiting for CI \x2026"
         ]
 
@@ -822,7 +854,7 @@ main = hspec $ do
         [ AIsReviewer "deckard"
         , ALeaveComment prId "Pull request approved for merge and tag by @deckard, rebasing now."
         , ATryIntegrate "Merge #1: Untitled\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") False
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") [] False
         , ALeaveComment prId "Rebased as def2345, waiting for CI \x2026"
         ]
 
@@ -843,7 +875,7 @@ main = hspec $ do
         [ AIsReviewer "deckard"
         , ALeaveComment prId "Pull request approved for merge by @deckard, rebasing now."
         , ATryIntegrate "Merge #1: Untitled\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") False
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") [] False
         , ALeaveComment prId "Rebased as def2345, waiting for CI \x2026"
         ]
 
@@ -864,7 +896,7 @@ main = hspec $ do
         [ AIsReviewer "deckard"
         , ALeaveComment prId "Pull request approved for merge by @deckard, rebasing now."
         , ATryIntegrate "Merge #1: Untitled\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") False
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") [] False
         , ALeaveComment prId "Rebased as def2345, waiting for CI \x2026"
         ]
 
@@ -898,7 +930,7 @@ main = hspec $ do
         [ AIsReviewer "bot"
         , ALeaveComment prId "Pull request approved for merge by @bot, rebasing now."
         , ATryIntegrate "Merge #1: Untitled\n\nApproved-by: bot\nAuto-deploy: false\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") False
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") [] False
         , ALeaveComment prId "Rebased as def2345, waiting for CI \x2026"
         ]
 
@@ -965,6 +997,7 @@ main = hspec $ do
         , ATryIntegrate
             { mergeMessage = "Merge #1: Untitled\n\nApproved-by: deckard\nAuto-deploy: false\n"
             , integrationCandidate = (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234")
+            , mergeTrain = []
             , alwaysAddMergeCommit = False
             }
         , ALeaveComment (PullRequestId 1)
@@ -1007,7 +1040,7 @@ main = hspec $ do
         , AIsReviewer (Username "deckard")
         , ALeaveComment (PullRequestId 1) "Pull request approved for merge by @deckard, rebasing now."
         , ATryIntegrate "Merge #1: Untitled\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") False
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") [] False
         , ALeaveComment (PullRequestId 1) "Rebased as def2345, waiting for CI \8230"
         ]
 
@@ -1032,13 +1065,13 @@ main = hspec $ do
         [ AIsReviewer (Username "deckard")
         , ALeaveComment (PullRequestId 1) "Pull request approved for merge by @deckard, rebasing now."
         , ATryIntegrate "Merge #1: Untitled\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") False
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") [] False
         , ALeaveComment (PullRequestId 1) "Rebased as def2345, waiting for CI \8230"
         , ALeaveComment (PullRequestId 1) "Stopping integration because the PR changed after approval."
         ]
 
       Project.approval pr `shouldBe` Nothing
-      Project.integratedPullRequests state' `shouldBe` []
+      Project.unfailingIntegratedPullRequests state' `shouldBe` []
 
     it "shows an appropriate message when the commit is changed on an approved PR" $ do
       let
@@ -1058,13 +1091,13 @@ main = hspec $ do
         [ AIsReviewer (Username "deckard")
         , ALeaveComment (PullRequestId 1) "Pull request approved for merge by @deckard, rebasing now."
         , ATryIntegrate "Merge #1: Untitled\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") False
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "abc1234") [] False
         , ALeaveComment (PullRequestId 1) "Rebased as def2345, waiting for CI \8230"
         , ALeaveComment (PullRequestId 1) "Stopping integration because the PR changed after approval."
         ]
 
       Project.approval pr `shouldBe` Nothing
-      Project.integratedPullRequests state' `shouldBe` []
+      Project.unfailingIntegratedPullRequests state' `shouldBe` []
 
 
   describe "Logic.proceedUntilFixedPoint" $ do
@@ -1084,7 +1117,7 @@ main = hspec $ do
       prId    `shouldBe` PullRequestId 1
       actions `shouldBe`
         [ ATryIntegrate "Merge #1: Untitled\n\nApproved-by: fred\nAuto-deploy: false\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "f34") False
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "f34") [] False
         , ALeaveComment (PullRequestId 1) "Rebased as 38c, waiting for CI \x2026"
         ]
     it "finds a new candidate with multiple PRs" $ do
@@ -1097,17 +1130,20 @@ main = hspec $ do
             , PullRequestOpened (PullRequestId 2) (Branch "s") masterBranch (Sha "g35") "Another untitled" "rachael"
             ] Project.emptyProjectState
         results = defaultResults
-          { resultIntegrate = [Right (Sha "38c")]
+          { resultIntegrate = [Right (Sha "38c"), Right (Sha "49d")]
           , resultPush = [PushRejected "test"]
           }
         (state', actions) = runActionCustom results $ Logic.proceedUntilFixedPoint state
-        [(prId, pullRequest)] = getIntegrationCandidates state'
+        (prId, pullRequest):_ = getIntegrationCandidates state'
       Project.integrationStatus pullRequest `shouldBe` Project.Integrated (Sha "38c") Project.BuildPending
       prId    `shouldBe` PullRequestId 2
       actions `shouldBe`
         [ ATryIntegrate "Merge #2: Another untitled\n\nApproved-by: fred\nAuto-deploy: false\n"
-                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "g35") False
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "g35") [] False
         , ALeaveComment (PullRequestId 2) "Rebased as 38c, waiting for CI \x2026"
+        , ATryIntegrate "Merge #1: Untitled\n\nApproved-by: fred\nAuto-deploy: false\n"
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "f34") [PullRequestId 2] False
+        , ALeaveComment (PullRequestId 1) "Speculatively rebased as 49d behind #2, waiting for CI \x2026"
         ]
 
     it "pushes after a successful build" $ do
@@ -1264,7 +1300,7 @@ main = hspec $ do
       actions `shouldBe`
         [ ATryPromote (Branch "results/rachael") (Sha "38d")
         , ATryIntegrate "Merge #1: Add my test results\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "f35") False
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "f35") [] False
         , ALeaveComment (PullRequestId 1) "Rebased as 38e, waiting for CI \x2026"
         ]
 
@@ -1302,7 +1338,7 @@ main = hspec $ do
       actions `shouldBe`
         [ ATryPromoteWithTag (Branch "results/rachael") (Sha "38d") (TagName "v2") (TagMessage "v2\n\nchangelog")
         , ATryIntegrate "Merge #1: Add my test results\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "f35") False
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "f35") [] False
         , ALeaveComment (PullRequestId 1) "Rebased as 38e, waiting for CI \x2026"
         ]
 
@@ -1343,14 +1379,14 @@ main = hspec $ do
         [ AIsReviewer "deckard"
         , ALeaveComment (PullRequestId 1) "Pull request approved for merge by @deckard, rebasing now."
         , ATryIntegrate "Merge #1: Add Nexus 7 experiment\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "a39") False
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "a39") [] False
           -- The first rebase succeeds.
         , ALeaveComment (PullRequestId 1) "Rebased as b71, waiting for CI \x2026"
           -- The first promotion attempt fails
         , ATryPromote (Branch "n7") (Sha "b71")
           -- The second rebase fails.
         , ATryIntegrate "Merge #1: Add Nexus 7 experiment\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "a39") False
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "a39") [] False
         , ALeaveComment (PullRequestId 1)
             "Failed to rebase, please rebase manually using\n\n\
             \    git rebase --interactive --autosquash origin/master n7"
@@ -1398,7 +1434,7 @@ main = hspec $ do
         [ ATryPromote (Branch "results/leon") (Sha "38d")
         , ACleanupTestBranch (PullRequestId 1)
         , ATryIntegrate "Merge #2: Add my test results\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "f37") False
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "f37") [] False
         , ALeaveComment (PullRequestId 2) "Rebased as 38e, waiting for CI \x2026"
         ]
 
@@ -1431,7 +1467,7 @@ main = hspec $ do
         [ AIsReviewer "deckard"
         , ALeaveComment (PullRequestId 1) "Pull request approved for merge by @deckard, rebasing now."
         , ATryIntegrate "Merge #1: Add Nexus 7 experiment\n\nApproved-by: deckard\nAuto-deploy: false\n"
-                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "a39") False
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "a39") [] False
         , ALeaveComment (PullRequestId 1) "Rebased as b71, waiting for CI \x2026"
         , ALeaveComment (PullRequestId 1) "[CI job](https://status.example.com/b71) started."
         , ALeaveComment (PullRequestId 1) "The build failed: https://example.com/build-status\nIf this is the result of a flaky test, close and reopen the PR, then tag me again.\nOtherwise, push a new commit and tag me again."
@@ -1759,6 +1795,7 @@ main = hspec $ do
                         \Approved-by: deckard\n\
                         \Auto-deploy: false\n"
                         (PullRequestId 12,Branch "refs/pull/12/head",Sha "12a")
+                        []
                         False
         , ALeaveComment (PullRequestId 12) "Rebased as 1b2, waiting for CI …"
         , ALeaveComment (PullRequestId 12) "[CI job](example.com/1b2) started."
@@ -1798,6 +1835,7 @@ main = hspec $ do
                         \Approved-by: deckard\n\
                         \Auto-deploy: false\n"
                         (PullRequestId 12,Branch "refs/pull/12/head",Sha "12a")
+                        []
                         False
         , ALeaveComment (PullRequestId 12) "Rebased as 1b2, waiting for CI …"
         , ALeaveComment (PullRequestId 12) "[CI job](example.com/1b2) started."
@@ -1862,34 +1900,37 @@ main = hspec $ do
                         \Approved-by: deckard\n\
                         \Auto-deploy: false\n"
                         (PullRequestId 1, Branch "refs/pull/1/head", Sha "ab1")
+                        []
                         False
         , ALeaveComment (PullRequestId 1) "Rebased as 1ab, waiting for CI …"
         , AIsReviewer "deckard"
         , ALeaveComment (PullRequestId 2)
                        "Pull request approved for merge by @deckard, \
                        \waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        [PullRequestId 1]
+                        False
+        , ALeaveComment (PullRequestId 2) "Speculatively rebased as 2bc behind #1, waiting for CI …"
         , ALeaveComment (PullRequestId 1) "[CI job](example.com/1ab) started."
         , AIsReviewer "deckard"
         , ALeaveComment (PullRequestId 3)
                         "Pull request approved for merge by @deckard, \
                         \waiting for rebase behind 2 pull requests."
-        , ATryPromote (Branch "fst") (Sha "1ab")
-        , ACleanupTestBranch (PullRequestId 1)
-        , ATryIntegrate "Merge #2: Second PR\n\n\
+        , ATryIntegrate "Merge #3: Third PR\n\n\
                         \Approved-by: deckard\n\
                         \Auto-deploy: false\n"
-                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        (PullRequestId 3, Branch "refs/pull/3/head", Sha "ef3")
+                        [PullRequestId 1, PullRequestId 2]
                         False
-        , ALeaveComment (PullRequestId 2) "Rebased as 2bc, waiting for CI …"
+        , ALeaveComment (PullRequestId 3) "Speculatively rebased as 3cd behind #1 and #2, waiting for CI …"
+        , ATryPromote (Branch "fst") (Sha "1ab")
+        , ACleanupTestBranch (PullRequestId 1)
         , ALeaveComment (PullRequestId 2) "[CI job](example.com/2bc) started."
         , ATryPromote (Branch "snd") (Sha "2bc")
         , ACleanupTestBranch (PullRequestId 2)
-        , ATryIntegrate "Merge #3: Third PR\n\n\
-                        \Approved-by: deckard\n\
-                        \Auto-deploy: false\n"
-                        (PullRequestId 3, Branch "refs/pull/3/head", Sha "ef3")
-                        False
-        , ALeaveComment (PullRequestId 3) "Rebased as 3cd, waiting for CI …"
         , ALeaveComment (PullRequestId 3) "[CI job](example.com/3cd) started."
         , ATryPromote (Branch "trd") (Sha "3cd")
         , ACleanupTestBranch (PullRequestId 3)
@@ -1929,19 +1970,21 @@ main = hspec $ do
           , CommentAdded (PullRequestId 8) "bot" "Rebased as 2bc, waiting for CI …"
           , BuildStatusChanged (Sha "2bc") (Project.BuildStarted "example.com/2bc")
           , CommentAdded (PullRequestId 8) "bot" "[CI job](example.com/2bc) started."
+          , BuildStatusChanged (Sha "3cd") (Project.BuildSucceeded) -- testing build passed on PR#7
           , BuildStatusChanged (Sha "36a") (Project.BuildSucceeded) -- arbitrary sha, ignored
           , BuildStatusChanged (Sha "2bc") (Project.BuildFailed (Just "example.com/2bc")) -- PR#8
           , BuildStatusChanged (Sha "2bc") (Project.BuildFailed (Just "example.com/2bc")) -- dup!
           , CommentAdded (PullRequestId 8) "bot" "The build failed: example.com/2bc"
           , CommentAdded (PullRequestId 7) "bot" "Rebased as 3cd, waiting for CI …"
-          , BuildStatusChanged (Sha "3cd") (Project.BuildStarted "example.com/3cd")
-          , BuildStatusChanged (Sha "3cd") (Project.BuildSucceeded) -- testing build passed on PR#7
+          , BuildStatusChanged (Sha "3ef") (Project.BuildStarted "example.com/3ef")
+          , BuildStatusChanged (Sha "3ef") (Project.BuildSucceeded) -- testing build passed on PR#7
           , PullRequestClosed (PullRequestId 7)
           ]
         -- For this test, we assume all integrations and pushes succeed.
         results = defaultResults { resultIntegrate = [ Right (Sha "1ab")
                                                      , Right (Sha "2bc")
-                                                     , Right (Sha "3cd") ] }
+                                                     , Right (Sha "3cd")
+                                                     , Right (Sha "3ef") ] }
         run = runActionCustom results
         actions = snd $ run $ handleEventsTest events state
       actions `shouldBe`
@@ -1952,38 +1995,49 @@ main = hspec $ do
                         \Approved-by: deckard\n\
                         \Auto-deploy: false\n"
                         (PullRequestId 9, Branch "refs/pull/9/head", Sha "ab9")
+                        []
                         False
         , ALeaveComment (PullRequestId 9) "Rebased as 1ab, waiting for CI …"
         , AIsReviewer "deckard"
         , ALeaveComment (PullRequestId 8)
                        "Pull request approved for merge by @deckard, \
                        \waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #8: Eighth PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 8, Branch "refs/pull/8/head", Sha "cd8")
+                        [PullRequestId 9]
+                        False
+        , ALeaveComment (PullRequestId 8) "Speculatively rebased as 2bc behind #9, waiting for CI …"
         , ALeaveComment (PullRequestId 9) "[CI job](example.com/1ab) started."
         , AIsReviewer "deckard"
         , ALeaveComment (PullRequestId 7)
                         "Pull request approved for merge by @deckard, \
                         \waiting for rebase behind 2 pull requests."
-        , ATryPromote (Branch "nth") (Sha "1ab")
-        , ACleanupTestBranch (PullRequestId 9)
-        , ATryIntegrate "Merge #8: Eighth PR\n\n\
+        , ATryIntegrate "Merge #7: Seventh PR\n\n\
                         \Approved-by: deckard\n\
                         \Auto-deploy: false\n"
-                        (PullRequestId 8, Branch "refs/pull/8/head", Sha "cd8")
+                        (PullRequestId 7, Branch "refs/pull/7/head", Sha "ef7")
+                        [PullRequestId 9, PullRequestId 8]
                         False
-        , ALeaveComment (PullRequestId 8) "Rebased as 2bc, waiting for CI …"
+        , ALeaveComment (PullRequestId 7) "Speculatively rebased as 3cd behind #9 and #8, waiting for CI …"
+        , ATryPromote (Branch "nth") (Sha "1ab")
+        , ACleanupTestBranch (PullRequestId 9)
         , ALeaveComment (PullRequestId 8) "[CI job](example.com/2bc) started."
         , ALeaveComment (PullRequestId 8)
                         "The build failed: example.com/2bc\n\
                         \If this is the result of a flaky test, \
                         \close and reopen the PR, then tag me again.\n\
                         \Otherwise, push a new commit and tag me again."
+        -- Since #8 failed, #7 becomes the head of a new train and is rebased again
         , ATryIntegrate "Merge #7: Seventh PR\n\n\
                         \Approved-by: deckard\n\
                         \Auto-deploy: false\n"
                         (PullRequestId 7, Branch "refs/pull/7/head", Sha "ef7")
+                        []
                         False
-        , ALeaveComment (PullRequestId 7) "Rebased as 3cd, waiting for CI …"
-        , ALeaveComment (PullRequestId 7) "[CI job](example.com/3cd) started."
-        , ATryPromote (Branch "sth") (Sha "3cd")
+        , ALeaveComment (PullRequestId 7) "Rebased as 3ef, waiting for CI …"
+        , ALeaveComment (PullRequestId 7) "[CI job](example.com/3ef) started."
+        , ATryPromote (Branch "sth") (Sha "3ef")
         , ACleanupTestBranch (PullRequestId 7)
         ]

From 41f05ac71ec0628a504bb70690947c4ef669b3ce Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Thu, 11 Aug 2022 16:48:50 +0200
Subject: [PATCH 02/54] Replace use of getTrain by unfailingIntegratedPRs

---
 src/Logic.hs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index 1f11c927..f482d79f 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -796,7 +796,7 @@ describeStatus prId pr state = case Pr.classifyPullRequest pr of
       1 -> format "Pull request approved for {} by @{}, waiting for rebase behind one pull request." [approvalCommand, approvedBy]
       n -> format "Pull request approved for {} by @{}, waiting for rebase behind {} pull requests." (approvalCommand, approvedBy, n)
   PrStatusBuildPending -> let Sha sha = fromJust $ Pr.integrationSha pr
-                              train = takeWhile (/= prId) $ getTrain state
+                              train = takeWhile (/= prId) $ Pr.unfailingIntegratedPullRequests state
                           in case train of
                              []    -> Text.concat ["Rebased as ", sha, ", waiting for CI …"]
                              (_:_) -> Text.concat [ "Speculatively rebased as ", sha

From d9cd7b46064a5a4464be23a86fb2b826e0d85e09 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Thu, 11 Aug 2022 17:38:25 +0200
Subject: [PATCH 03/54] Logic.tryIntegratePullRequest: use explicit var

---
 src/Logic.hs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index f482d79f..14698710 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -702,8 +702,9 @@ tryIntegratePullRequest pr state =
       , format "Auto-deploy: {}" [if approvalType == MergeAndDeploy then "true" else "false" :: Text]
       ]
     mergeMessage = Text.unlines mergeMessageLines
+    train = getTrain state
   in do
-    result <- tryIntegrate mergeMessage candidate (getTrain state) $ Pr.alwaysAddMergeCommit approvalType
+    result <- tryIntegrate mergeMessage candidate train $ Pr.alwaysAddMergeCommit approvalType
     case result of
       Left (IntegrationFailure targetBranch reason) ->
         -- If integrating failed, perform no further actions but do set the

From a753484bedbfe2f47ec0f975440afc769eb7c201 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Thu, 11 Aug 2022 17:50:25 +0200
Subject: [PATCH 04/54] Avoid the need for getTrain (and remove it)

---
 src/Logic.hs | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index 14698710..67860be3 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -670,19 +670,6 @@ proceedCandidate pullRequestId state =
 getPullRequestRef :: PullRequestId -> Branch
 getPullRequestRef (PullRequestId n) = Branch $ format "refs/pull/{}/head" [n]
 
--- TODO: get rid of the getTrain function in favour of just *integratedPullRequests?
-getTrain :: ProjectState -> [PullRequestId]
-getTrain state =
-  [ pid
-  | pid <- Pr.unfailingIntegratedPullRequests state
-  , Just pr <- [Pr.lookupPullRequest pid state]
-  , Integrated _ buildStatus <- [Pr.integrationStatus pr]
-  , case buildStatus of
-    BuildPending   -> True
-    BuildStarted _ -> True
-    _              -> False
-  ]
-
 -- Integrates proposed changes from the pull request into the target branch.
 -- The pull request must exist in the project.
 tryIntegratePullRequest :: PullRequestId -> ProjectState -> Action ProjectState
@@ -702,7 +689,8 @@ tryIntegratePullRequest pr state =
       , format "Auto-deploy: {}" [if approvalType == MergeAndDeploy then "true" else "false" :: Text]
       ]
     mergeMessage = Text.unlines mergeMessageLines
-    train = getTrain state
+    -- the takeWhile here is needed in case of reintegrations after failing pushes
+    train = takeWhile (/= pr) $ Pr.unfailingIntegratedPullRequests state
   in do
     result <- tryIntegrate mergeMessage candidate train $ Pr.alwaysAddMergeCommit approvalType
     case result of

From 68756b39562a29409eec5b07f86c618f4d28bbe0 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 10:41:02 +0200
Subject: [PATCH 05/54] Spec: add automated test for new commit

... in the head of the train
---
 tests/Spec.hs | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/tests/Spec.hs b/tests/Spec.hs
index 6027b40f..5fbda52c 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -1843,6 +1843,78 @@ main = hspec $ do
         , ACleanupTestBranch (PullRequestId 12)
         ]
 
+    it "after the PR commit has changed, resets the integration of PRs in the train (FOCUS)" $ do
+      let
+        state
+          = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")
+          $ Project.insertPullRequest (PullRequestId 2) (Branch "snd") masterBranch (Sha "cd2") "Second PR" (Username "rachael")
+          $ Project.insertPullRequest (PullRequestId 3) (Branch "trd") masterBranch (Sha "ef3") "Third PR"  (Username "rachael")
+          $ Project.emptyProjectState
+        events =
+          [ CommentAdded (PullRequestId 1) "deckard" "@bot merge"
+          , CommentAdded (PullRequestId 2) "deckard" "@bot merge"
+          , CommentAdded (PullRequestId 3) "deckard" "@bot merge"
+          , PullRequestCommitChanged (PullRequestId 1) (Sha "4ba")
+          ]
+        -- For this test, we assume all integrations and pushes succeed.
+        results = defaultResults { resultIntegrate = [ Right (Sha "1ab")
+                                                     , Right (Sha "2bc")
+                                                     , Right (Sha "3cd")
+                                                     , Right (Sha "5bc")
+                                                     , Right (Sha "6cd") ] }
+        run = runActionCustom results
+        actions = snd $ run $ handleEventsTest events state
+      actions `shouldBe`
+        [ AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 1)
+                        "Pull request approved for merge by @deckard, rebasing now."
+        , ATryIntegrate "Merge #1: First PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "ab1")
+                        []
+                        False
+        , ALeaveComment (PullRequestId 1) "Rebased as 1ab, waiting for CI …"
+        , AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 2)
+                       "Pull request approved for merge by @deckard, \
+                       \waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        [PullRequestId 1]
+                        False
+        , ALeaveComment (PullRequestId 2) "Speculatively rebased as 2bc behind #1, waiting for CI …"
+        , AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 3)
+                        "Pull request approved for merge by @deckard, \
+                        \waiting for rebase behind 2 pull requests."
+        , ATryIntegrate "Merge #3: Third PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 3, Branch "refs/pull/3/head", Sha "ef3")
+                        [PullRequestId 1, PullRequestId 2]
+                        False
+        , ALeaveComment (PullRequestId 3) "Speculatively rebased as 3cd behind #1 and #2, waiting for CI …"
+        , ALeaveComment (PullRequestId 1) "Stopping integration because the PR changed after approval."
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        []
+                        False
+        , ALeaveComment (PullRequestId 2) "Rebased as 5bc, waiting for CI …"
+        , ATryIntegrate "Merge #3: Third PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 3, Branch "refs/pull/3/head", Sha "ef3")
+                        [PullRequestId 2]
+                        False
+        , ALeaveComment (PullRequestId 3) "Speculatively rebased as 6cd behind #2, waiting for CI …"
+        ]
+
+
     it "handles a sequence of merges: success, success, success" $ do
       -- An afternoon of work on PRs:
       -- * three PRs are merged and approved in order

From 506312ffee9ba3f109c81b8a13f8ea5ff9a4c856 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 11:12:44 +0200
Subject: [PATCH 06/54] Fix test title

---
 tests/Spec.hs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Spec.hs b/tests/Spec.hs
index 5fbda52c..879cfac6 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -1843,7 +1843,7 @@ main = hspec $ do
         , ACleanupTestBranch (PullRequestId 12)
         ]
 
-    it "after the PR commit has changed, resets the integration of PRs in the train (FOCUS)" $ do
+    it "after the PR commit has changed, resets the integration of PRs in the train" $ do
       let
         state
           = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")

From e33e56a0bbcefd7837185d7bbfe1282dee4bc9b7 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 11:34:49 +0200
Subject: [PATCH 07/54] Add pending test of rebase failures

---
 tests/Spec.hs | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/tests/Spec.hs b/tests/Spec.hs
index 879cfac6..a75ee082 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -1914,6 +1914,69 @@ main = hspec $ do
         , ALeaveComment (PullRequestId 3) "Speculatively rebased as 6cd behind #2, waiting for CI …"
         ]
 
+    it "only notifies rebase failures on top of the master branch (success, rebasefailure, success)" $ do
+      pendingWith "TODO: fix code so this test passes"
+      let
+        state
+          = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")
+          $ Project.insertPullRequest (PullRequestId 2) (Branch "snd") masterBranch (Sha "cd2") "Second PR" (Username "rachael")
+          $ Project.insertPullRequest (PullRequestId 3) (Branch "trd") masterBranch (Sha "ef3") "Third PR"  (Username "rachael")
+          $ Project.emptyProjectState
+        events =
+          [ CommentAdded (PullRequestId 1) "deckard" "@bot merge"
+          , CommentAdded (PullRequestId 2) "deckard" "@bot merge"
+          , CommentAdded (PullRequestId 3) "deckard" "@bot merge"
+          , BuildStatusChanged (Sha "1ab") (Project.BuildSucceeded)
+          ]
+        -- For this test, we assume all integrations and pushes succeed.
+        results = defaultResults { resultIntegrate = [ Right (Sha "1ab")
+                                                     , Left (IntegrationFailure (BaseBranch "master") RebaseFailed)
+                                                     , Right (Sha "3cd")
+                                                     , Right (Sha "5bc")
+                                                     , Right (Sha "6cd") ] }
+        run = runActionCustom results
+        actions = snd $ run $ handleEventsTest events state
+      actions `shouldBe`
+        [ AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 1)
+                        "Pull request approved for merge by @deckard, rebasing now."
+        , ATryIntegrate "Merge #1: First PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "ab1")
+                        []
+                        False
+        , ALeaveComment (PullRequestId 1) "Rebased as 1ab, waiting for CI …"
+        , AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 2)
+                       "Pull request approved for merge by @deckard, \
+                       \waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        [PullRequestId 1]
+                        False
+        -- We could post a comment like this, but it would be confusing...
+        -- , ALeaveComment (PullRequestId 2) "Failed speculative rebase.  Waiting in the queue for a rebase on master."
+        , AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 3)
+                        "Pull request approved for merge by @deckard, \
+                        \waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #3: Third PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 3, Branch "refs/pull/3/head", Sha "ef3")
+                        [PullRequestId 1]
+                        False
+        , ALeaveComment (PullRequestId 3) "Speculatively rebased as 3cd behind #1, waiting for CI …"
+        , ATryPromote (Branch "fst") (Sha "1ab")
+        , ACleanupTestBranch (PullRequestId 1)
+        -- PR#2 is only notified after PR#1 passes or fails
+        , ALeaveComment (PullRequestId 2)
+                        "Failed to rebase, please rebase manually using\n\n\
+                        \    git rebase --interactive --autosquash origin/master snd"
+        ]
 
     it "handles a sequence of merges: success, success, success" $ do
       -- An afternoon of work on PRs:

From 2f4bd860a0118354a34e84794a53fefee08ad4f6 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 11:48:43 +0200
Subject: [PATCH 08/54] Register correct baseBranch for rebase failures

---
 src/Git.hs   | 4 ++++
 src/Logic.hs | 6 ++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/Git.hs b/src/Git.hs
index b26ea45f..4ab04c4c 100644
--- a/src/Git.hs
+++ b/src/Git.hs
@@ -46,6 +46,7 @@ module Git
   push,
   pushAtomic,
   rebase,
+  remoteToBaseBranch,
   runGit,
   runGitReadOnly,
   tag,
@@ -90,6 +91,9 @@ newtype RemoteBranch = RemoteBranch Text deriving newtype (Show, Eq)
 localBranch :: RemoteBranch -> Branch
 localBranch (RemoteBranch name) = Branch name
 
+remoteToBaseBranch :: RemoteBranch -> BaseBranch
+remoteToBaseBranch (RemoteBranch b) = BaseBranch b
+
 -- | A commit hash is stored as its hexadecimal representation.
 newtype Sha = Sha Text deriving newtype (Show, Eq)
 
diff --git a/src/Logic.hs b/src/Logic.hs
index 67860be3..415f4d5e 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -171,16 +171,18 @@ runAction config = foldFree $ \case
     -- When no repositories have a testing branch, this can safely be removed.
     _ <- doGit $ Git.deleteRemoteBranch $ Git.Branch $ Config.testBranch config
 
+    let targetBranch = fromMaybe (Git.RemoteBranch $ Config.branch config) (trainBranch train)
+
     shaOrFailed <- doGit $ Git.tryIntegrate
       message
       ref
       sha
-      (fromMaybe (Git.RemoteBranch $ Config.branch config) (trainBranch train))
+      targetBranch
       (testBranch config pr)
       alwaysAddMergeCommit
 
     case shaOrFailed of
-      Left failure -> pure $ cont $ Left $ IntegrationFailure (BaseBranch $ Config.branch config) failure
+      Left failure -> pure $ cont $ Left $ IntegrationFailure (Git.remoteToBaseBranch targetBranch) failure
       Right integratedSha -> pure $ cont $ Right integratedSha
 
   TryPromote prBranch sha cont -> do

From 6207205a3a35eefff4fc78185c061e179e77ba7e Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 12:08:29 +0200
Subject: [PATCH 09/54] Do not give feedback when speculative rebase fails

... though we have to give feedback at some point,
this will be updated in a further change.
---
 src/Logic.hs   | 3 ++-
 src/Project.hs | 2 ++
 tests/Spec.hs  | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index 415f4d5e..ce4a9293 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -699,8 +699,9 @@ tryIntegratePullRequest pr state =
       Left (IntegrationFailure targetBranch reason) ->
         -- If integrating failed, perform no further actions but do set the
         -- state to conflicted.
+        -- If this is a speculative rebase, we wait before giving feedback.
         pure $ Pr.setIntegrationStatus pr (Conflicted targetBranch reason) $
-          Pr.setNeedsFeedback pr True state
+          Pr.setNeedsFeedback pr (null train) state
 
       Right (Sha sha) -> do
         -- If it succeeded, set the build to pending,
diff --git a/src/Project.hs b/src/Project.hs
index 5f36154e..4194e8b4 100644
--- a/src/Project.hs
+++ b/src/Project.hs
@@ -304,6 +304,8 @@ classifyPullRequest pr = case approval pr of
   Just _  -> case integrationStatus pr of
     NotIntegrated -> PrStatusApproved
     IncorrectBaseBranch -> PrStatusIncorrectBaseBranch
+    -- checks if this is a speculative rebase, if it is, we have to wait for the train status
+    Conflicted baseBranch' _ | baseBranch' /= baseBranch pr -> PrStatusBuildPending -- TODO: proper status?
     Conflicted _ WrongFixups -> PrStatusWrongFixups
     Conflicted _ EmptyRebase -> PrStatusEmptyRebase
     Conflicted _ _  -> PrStatusFailedConflict
diff --git a/tests/Spec.hs b/tests/Spec.hs
index a75ee082..9d57bf41 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -1930,7 +1930,7 @@ main = hspec $ do
           ]
         -- For this test, we assume all integrations and pushes succeed.
         results = defaultResults { resultIntegrate = [ Right (Sha "1ab")
-                                                     , Left (IntegrationFailure (BaseBranch "master") RebaseFailed)
+                                                     , Left (IntegrationFailure (BaseBranch "testing/1") RebaseFailed)
                                                      , Right (Sha "3cd")
                                                      , Right (Sha "5bc")
                                                      , Right (Sha "6cd") ] }

From b697ef16daa8f849997d5ad2affc31484ea7c4d2 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 12:32:02 +0200
Subject: [PATCH 10/54] Refactor integratedPullRequestsAfter

---
 src/Project.hs | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/Project.hs b/src/Project.hs
index 4194e8b4..cb13db63 100644
--- a/src/Project.hs
+++ b/src/Project.hs
@@ -400,15 +400,20 @@ integratedPullRequests = filterPullRequestsBy $ isIntegrated . integrationStatus
   isIntegrated (Integrated _ _) = True
   isIntegrated _                = False
 
--- | Lists the pull requests that are integrated on top of the given id.
-integratedPullRequestsAfter :: PullRequestId -> ProjectState -> [PullRequestId]
-integratedPullRequestsAfter pid state =
+-- | Lists the pull requests that were approved after a given PR
+--   matching a given property
+pullRequestsAfterThat :: (PullRequest -> Bool) -> PullRequestId -> ProjectState -> [PullRequestId]
+pullRequestsAfterThat p pid state =
   case approvalOrder <$> (approval =<< lookupPullRequest pid state) of
   Nothing    -> []
-  Just order -> filterPullRequestsBy (isIntegratedAfter order) state
+  Just order -> filterPullRequestsBy (isAfter order) state
+  where
+  isAfter order pr = p pr && (approvalOrder <$> approval pr) > Just order
+
+-- | Lists the pull requests that are integrated on top of the given id.
+integratedPullRequestsAfter :: PullRequestId -> ProjectState -> [PullRequestId]
+integratedPullRequestsAfter = pullRequestsAfterThat (isIntegrated . integrationStatus)
   where
-  isIntegratedAfter order pr = isIntegrated (integrationStatus pr)
-                            && (approvalOrder <$> approval pr) > Just order
   isIntegrated (Integrated _ _) = True
   isIntegrated _                = False
 

From 30183cc2a15da6a68d6df2a42c3661e02af28958 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 13:00:15 +0200
Subject: [PATCH 11/54] Report rebase failures after base promotion

---
 src/Logic.hs   | 16 +++++++++++++++-
 src/Project.hs | 13 +++++++++++++
 tests/Spec.hs  |  1 -
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index ce4a9293..748afd52 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -759,12 +759,26 @@ pushCandidate (pullRequestId, pullRequest) newHead state =
       -- the integration candidate, so we proceed with the next pull request.
       PushOk -> do
         cleanupTestBranch pullRequestId
-        pure $ Pr.setIntegrationStatus pullRequestId Promoted state
+        pure $ Pr.updatePullRequests (unspeculateConflictsAfter pullRequest)
+             $ Pr.setIntegrationStatus pullRequestId Promoted state
       -- If something was pushed to the target branch while the candidate was
       -- being tested, try to integrate again and hope that next time the push
       -- succeeds.
       PushRejected _why -> tryIntegratePullRequest pullRequestId state
 
+unspeculateConflictsAfter :: PullRequest -> PullRequest -> PullRequest
+unspeculateConflictsAfter promotedPullRequest pr
+  | Pr.PullRequest{ Pr.integrationStatus = Conflicted specBase reason
+                  , Pr.baseBranch        = realBase
+                  } <- pr
+  , specBase /= realBase && pr `Pr.approvedAfter` promotedPullRequest
+  = pr { Pr.integrationStatus = Conflicted realBase reason
+       , Pr.needsFeedback = True
+       }
+  | otherwise
+  = pr
+
+
 -- Keep doing a proceed step until the state doesn't change any more. For this
 -- to work properly, it is essential that "proceed" does not have any side
 -- effects if it does not change the state.
diff --git a/src/Project.hs b/src/Project.hs
index cb13db63..d4c4bac9 100644
--- a/src/Project.hs
+++ b/src/Project.hs
@@ -52,6 +52,7 @@ module Project
   getOwners,
   wasIntegrationAttemptFor,
   filterPullRequestsBy,
+  approvedAfter,
   MergeWindow(..))
 where
 
@@ -469,3 +470,15 @@ integrationSha _                                               = Nothing
 
 lookupIntegrationSha :: PullRequestId -> ProjectState -> Maybe Sha
 lookupIntegrationSha pid = integrationSha <=< lookupPullRequest pid
+
+-- | Returns whether the first pull request was approved after the second.
+-- To be used in infix notation:
+--
+-- > pr1 `approvedAfter` pr2
+approvedAfter :: PullRequest -> PullRequest -> Bool
+pr1 `approvedAfter` pr2 = case (mo1, mo2) of
+                          (Just order1, Just order2) -> order1 > order2
+                          _                          -> False
+  where
+  mo1 = approvalOrder <$> approval pr1
+  mo2 = approvalOrder <$> approval pr2
diff --git a/tests/Spec.hs b/tests/Spec.hs
index 9d57bf41..2dd907ba 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -1915,7 +1915,6 @@ main = hspec $ do
         ]
 
     it "only notifies rebase failures on top of the master branch (success, rebasefailure, success)" $ do
-      pendingWith "TODO: fix code so this test passes"
       let
         state
           = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")

From 294676ff941a225fcd26bf0bdeb0eb81df441e04 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 14:05:05 +0200
Subject: [PATCH 12/54] Add pending (failing) test involving rebase fails

... in merge trains.
---
 tests/Spec.hs | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/tests/Spec.hs b/tests/Spec.hs
index 2dd907ba..32047bee 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -1933,6 +1933,7 @@ main = hspec $ do
                                                      , Right (Sha "3cd")
                                                      , Right (Sha "5bc")
                                                      , Right (Sha "6cd") ] }
+        -- TODO: cleanup the above resultIntegrate to only what is needed.
         run = runActionCustom results
         actions = snd $ run $ handleEventsTest events state
       actions `shouldBe`
@@ -1977,6 +1978,80 @@ main = hspec $ do
                         \    git rebase --interactive --autosquash origin/master snd"
         ]
 
+    it "recovers from speculative rebase failures by starting a new train (failure, rebasefailure, success)" $ do
+      pendingWith "TODO: make sure this test passes"
+      let
+        state
+          = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")
+          $ Project.insertPullRequest (PullRequestId 2) (Branch "snd") masterBranch (Sha "cd2") "Second PR" (Username "rachael")
+          $ Project.insertPullRequest (PullRequestId 3) (Branch "trd") masterBranch (Sha "ef3") "Third PR"  (Username "rachael")
+          $ Project.emptyProjectState
+        events =
+          [ CommentAdded (PullRequestId 1) "deckard" "@bot merge"
+          , CommentAdded (PullRequestId 2) "deckard" "@bot merge"
+          , CommentAdded (PullRequestId 3) "deckard" "@bot merge"
+          , BuildStatusChanged (Sha "1ab") (Project.BuildFailed (Just "ci.example.com/1ab"))
+          ]
+        -- For this test, we assume all integrations and pushes succeed.
+        results = defaultResults { resultIntegrate = [ Right (Sha "1ab")
+                                                     , Left (IntegrationFailure (BaseBranch "testing/1") RebaseFailed)
+                                                     , Right (Sha "3cd")
+                                                     , Right (Sha "5bc")
+                                                     , Right (Sha "6cd") ] }
+        run = runActionCustom results
+        actions = snd $ run $ handleEventsTest events state
+      actions `shouldBe`
+        [ AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 1)
+                        "Pull request approved for merge by @deckard, rebasing now."
+        , ATryIntegrate "Merge #1: First PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "ab1")
+                        []
+                        False
+        , ALeaveComment (PullRequestId 1) "Rebased as 1ab, waiting for CI …"
+        , AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 2)
+                       "Pull request approved for merge by @deckard, \
+                       \waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        [PullRequestId 1]
+                        False
+        -- We could post a comment like this, but it would be confusing...
+        -- , ALeaveComment (PullRequestId 2) "Failed speculative rebase.  Waiting in the queue for a rebase on master."
+        , AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 3)
+                        "Pull request approved for merge by @deckard, \
+                        \waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #3: Third PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 3, Branch "refs/pull/3/head", Sha "ef3")
+                        [PullRequestId 1]
+                        False
+        , ALeaveComment (PullRequestId 3) "Speculatively rebased as 3cd behind #1, waiting for CI …"
+        , ALeaveComment (PullRequestId 1) "The build failed: ci.example.com/1ab\n\
+                                          \If this is the result of a flaky test, \
+                                          \close and reopen the PR, then tag me again.\n\
+                                          \Otherwise, push a new commit and tag me again."
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 3, Branch "refs/pull/2/head", Sha "cd2")
+                        [PullRequestId 2]
+                        False
+        , ATryIntegrate "Merge #3: Third PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 3, Branch "refs/pull/3/head", Sha "ef3")
+                        [PullRequestId 2]
+                        False
+        ]
+
     it "handles a sequence of merges: success, success, success" $ do
       -- An afternoon of work on PRs:
       -- * three PRs are merged and approved in order

From d1e23fbc88a8d3ef9e89763fb108265eed1b70e4 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 14:56:57 +0200
Subject: [PATCH 13/54] Refactor pullRequestsAfterThat

---
 src/Project.hs | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/Project.hs b/src/Project.hs
index d4c4bac9..c2d129bd 100644
--- a/src/Project.hs
+++ b/src/Project.hs
@@ -405,11 +405,9 @@ integratedPullRequests = filterPullRequestsBy $ isIntegrated . integrationStatus
 --   matching a given property
 pullRequestsAfterThat :: (PullRequest -> Bool) -> PullRequestId -> ProjectState -> [PullRequestId]
 pullRequestsAfterThat p pid state =
-  case approvalOrder <$> (approval =<< lookupPullRequest pid state) of
-  Nothing    -> []
-  Just order -> filterPullRequestsBy (isAfter order) state
-  where
-  isAfter order pr = p pr && (approvalOrder <$> approval pr) > Just order
+  case lookupPullRequest pid state of
+  Nothing -> []
+  Just pr0 -> filterPullRequestsBy (\pr -> p pr && pr `approvedAfter` pr0) state
 
 -- | Lists the pull requests that are integrated on top of the given id.
 integratedPullRequestsAfter :: PullRequestId -> ProjectState -> [PullRequestId]

From 095a636754894df152f805a6dfdc4fd3a8de3acc Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 15:12:39 +0200
Subject: [PATCH 14/54] Handle rebase failures on merge trains

---
 src/Logic.hs   | 4 +++-
 src/Project.hs | 8 ++++++++
 tests/Spec.hs  | 7 ++++---
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index 748afd52..0def26f1 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -562,9 +562,11 @@ handleMergeRequested projectConfig prId author state pr approvalType = do
 unintegrateAfter :: PullRequestId -> ProjectState -> ProjectState
 unintegrateAfter pid state =
   compose [ Pr.updatePullRequest pid' unintegrate
-          | pid' <- Pr.integratedPullRequestsAfter pid state] state
+          | pid' <- Pr.integratedPullRequestsAfter pid state
+                 ++ Pr.speculativelyConflictedPullRequestsAfter pid state] state
   where
   unintegrate pr = pr{Pr.integrationStatus = NotIntegrated}
+-- TODO: refactor unintegrateAfter
 
 -- | If there is an integration candidate, and its integration sha matches that of the build,
 --   then update the build status for that pull request. Otherwise do nothing.
diff --git a/src/Project.hs b/src/Project.hs
index c2d129bd..b42c46d8 100644
--- a/src/Project.hs
+++ b/src/Project.hs
@@ -24,6 +24,7 @@ module Project
   integratedPullRequests,
   integratedPullRequestsAfter,
   unfailingIntegratedPullRequests,
+  speculativelyConflictedPullRequestsAfter,
   candidatePullRequests,
   classifyPullRequest,
   classifyPullRequests,
@@ -416,6 +417,13 @@ integratedPullRequestsAfter = pullRequestsAfterThat (isIntegrated . integrationS
   isIntegrated (Integrated _ _) = True
   isIntegrated _                = False
 
+speculativelyConflictedPullRequestsAfter :: PullRequestId -> ProjectState -> [PullRequestId]
+speculativelyConflictedPullRequestsAfter = pullRequestsAfterThat isSpeculativelyConflicted
+  where
+  isSpeculativelyConflicted pr = case integrationStatus pr of
+                                 Conflicted base _ | base /= baseBranch pr -> True
+                                 _ -> False
+
 unfailingIntegratedPullRequests :: ProjectState -> [PullRequestId]
 unfailingIntegratedPullRequests = filterPullRequestsBy $ isUnfailingIntegrated . integrationStatus
   where
diff --git a/tests/Spec.hs b/tests/Spec.hs
index 32047bee..a92f44de 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -1979,7 +1979,6 @@ main = hspec $ do
         ]
 
     it "recovers from speculative rebase failures by starting a new train (failure, rebasefailure, success)" $ do
-      pendingWith "TODO: make sure this test passes"
       let
         state
           = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")
@@ -2041,15 +2040,17 @@ main = hspec $ do
         , ATryIntegrate "Merge #2: Second PR\n\n\
                         \Approved-by: deckard\n\
                         \Auto-deploy: false\n"
-                        (PullRequestId 3, Branch "refs/pull/2/head", Sha "cd2")
-                        [PullRequestId 2]
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        []
                         False
+        , ALeaveComment (PullRequestId 2) "Rebased as 5bc, waiting for CI …"
         , ATryIntegrate "Merge #3: Third PR\n\n\
                         \Approved-by: deckard\n\
                         \Auto-deploy: false\n"
                         (PullRequestId 3, Branch "refs/pull/3/head", Sha "ef3")
                         [PullRequestId 2]
                         False
+        , ALeaveComment (PullRequestId 3) "Speculatively rebased as 6cd behind #2, waiting for CI …"
         ]
 
     it "handles a sequence of merges: success, success, success" $ do

From 8d6ae93db37d1149244d80c2151e922670c97930 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 15:21:23 +0200
Subject: [PATCH 15/54] Spec: add a few stub tests

---
 tests/Spec.hs | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tests/Spec.hs b/tests/Spec.hs
index a92f44de..83225583 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -2037,6 +2037,7 @@ main = hspec $ do
                                           \If this is the result of a flaky test, \
                                           \close and reopen the PR, then tag me again.\n\
                                           \Otherwise, push a new commit and tag me again."
+        -- Since #1 failed, #2 takes over as the head of the new merge train
         , ATryIntegrate "Merge #2: Second PR\n\n\
                         \Approved-by: deckard\n\
                         \Auto-deploy: false\n"
@@ -2053,6 +2054,18 @@ main = hspec $ do
         , ALeaveComment (PullRequestId 3) "Speculatively rebased as 6cd behind #2, waiting for CI …"
         ]
 
+    it "handles a 2-wagon merge train with build successes coming in the right order: success (1), success (2)" $ do
+      pendingWith "TODO: implement me"
+
+    it "handles a 2-wagon merge train with build successes coming in the reverse order: success (2), success (1)" $ do
+      pendingWith "TODO: implement me"
+
+    it "handles a 2-wagon merge train with build failures coming in the right order: failure (1), failure (2)" $ do
+      pendingWith "TODO: implement me"
+
+    it "handles a 2-wagon merge train with build failures coming in the reverse order: failure (2), failure (1)" $ do
+      pendingWith "TODO: implement me"
+
     it "handles a sequence of merges: success, success, success" $ do
       -- An afternoon of work on PRs:
       -- * three PRs are merged and approved in order

From 4e2eefa87b27519a19b5eaddcc0020fa95a9f66f Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 15:25:14 +0200
Subject: [PATCH 16/54] Spec: test success (1), success (2)

---
 tests/Spec.hs | 43 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/tests/Spec.hs b/tests/Spec.hs
index 83225583..1b91b5d6 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -2055,7 +2055,48 @@ main = hspec $ do
         ]
 
     it "handles a 2-wagon merge train with build successes coming in the right order: success (1), success (2)" $ do
-      pendingWith "TODO: implement me"
+      let
+        state
+          = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")
+          $ Project.insertPullRequest (PullRequestId 2) (Branch "snd") masterBranch (Sha "cd2") "Second PR" (Username "rachael")
+          $ Project.emptyProjectState
+        events =
+          [ CommentAdded (PullRequestId 1) "deckard" "@bot merge"
+          , CommentAdded (PullRequestId 2) "deckard" "@bot merge"
+          , BuildStatusChanged (Sha "1ab") (Project.BuildSucceeded)
+          , BuildStatusChanged (Sha "2cd") (Project.BuildSucceeded)
+          ]
+        results = defaultResults { resultIntegrate = [ Right (Sha "1ab")
+                                                     , Right (Sha "2cd") ] }
+        run = runActionCustom results
+        actions = snd $ run $ handleEventsTest events state
+      actions `shouldBe`
+        [ AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 1)
+                        "Pull request approved for merge by @deckard, rebasing now."
+        , ATryIntegrate "Merge #1: First PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "ab1")
+                        []
+                        False
+        , ALeaveComment (PullRequestId 1) "Rebased as 1ab, waiting for CI …"
+        , AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 2)
+                       "Pull request approved for merge by @deckard, \
+                       \waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        [PullRequestId 1]
+                        False
+        , ALeaveComment (PullRequestId 2) "Speculatively rebased as 2cd behind #1, waiting for CI …"
+        , ATryPromote (Branch "fst") (Sha "1ab")
+        , ACleanupTestBranch (PullRequestId 1)
+        , ATryPromote (Branch "snd") (Sha "2cd")
+        , ACleanupTestBranch (PullRequestId 2)
+        ]
 
     it "handles a 2-wagon merge train with build successes coming in the reverse order: success (2), success (1)" $ do
       pendingWith "TODO: implement me"

From a57d0eef10298a705ea2cf1efdc723bc409cf2eb Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 15:26:40 +0200
Subject: [PATCH 17/54] Spec: test success (2), success (1)

---
 tests/Spec.hs | 44 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/tests/Spec.hs b/tests/Spec.hs
index 1b91b5d6..38b4172a 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -2099,7 +2099,49 @@ main = hspec $ do
         ]
 
     it "handles a 2-wagon merge train with build successes coming in the reverse order: success (2), success (1)" $ do
-      pendingWith "TODO: implement me"
+      let
+        state
+          = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")
+          $ Project.insertPullRequest (PullRequestId 2) (Branch "snd") masterBranch (Sha "cd2") "Second PR" (Username "rachael")
+          $ Project.emptyProjectState
+        events =
+          [ CommentAdded (PullRequestId 1) "deckard" "@bot merge"
+          , CommentAdded (PullRequestId 2) "deckard" "@bot merge"
+          -- Build of #2 finishes before build of #1
+          , BuildStatusChanged (Sha "2cd") (Project.BuildSucceeded)
+          , BuildStatusChanged (Sha "1ab") (Project.BuildSucceeded)
+          ]
+        results = defaultResults { resultIntegrate = [ Right (Sha "1ab")
+                                                     , Right (Sha "2cd") ] }
+        run = runActionCustom results
+        actions = snd $ run $ handleEventsTest events state
+      actions `shouldBe`
+        [ AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 1)
+                        "Pull request approved for merge by @deckard, rebasing now."
+        , ATryIntegrate "Merge #1: First PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "ab1")
+                        []
+                        False
+        , ALeaveComment (PullRequestId 1) "Rebased as 1ab, waiting for CI …"
+        , AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 2)
+                       "Pull request approved for merge by @deckard, \
+                       \waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        [PullRequestId 1]
+                        False
+        , ALeaveComment (PullRequestId 2) "Speculatively rebased as 2cd behind #1, waiting for CI …"
+        , ATryPromote (Branch "fst") (Sha "1ab")
+        , ACleanupTestBranch (PullRequestId 1)
+        , ATryPromote (Branch "snd") (Sha "2cd")
+        , ACleanupTestBranch (PullRequestId 2)
+        ]
 
     it "handles a 2-wagon merge train with build failures coming in the right order: failure (1), failure (2)" $ do
       pendingWith "TODO: implement me"

From 85504b1a0f01ef6f48ac6bea760dfd7857c78425 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 15:30:40 +0200
Subject: [PATCH 18/54] Spec: test failure (1), failure (2)

---
 tests/Spec.hs | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/tests/Spec.hs b/tests/Spec.hs
index 38b4172a..4f7b3983 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -2144,7 +2144,57 @@ main = hspec $ do
         ]
 
     it "handles a 2-wagon merge train with build failures coming in the right order: failure (1), failure (2)" $ do
-      pendingWith "TODO: implement me"
+      let
+        state
+          = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")
+          $ Project.insertPullRequest (PullRequestId 2) (Branch "snd") masterBranch (Sha "cd2") "Second PR" (Username "rachael")
+          $ Project.emptyProjectState
+        events =
+          [ CommentAdded (PullRequestId 1) "deckard" "@bot merge"
+          , CommentAdded (PullRequestId 2) "deckard" "@bot merge"
+          -- Build of #2 finishes before build of #1
+          , BuildStatusChanged (Sha "1ab") (Project.BuildFailed Nothing)
+          , BuildStatusChanged (Sha "2cd") (Project.BuildFailed Nothing)
+          , BuildStatusChanged (Sha "22e") (Project.BuildFailed Nothing)
+          ]
+        results = defaultResults { resultIntegrate = [ Right (Sha "1ab")
+                                                     , Right (Sha "2cd")
+                                                     , Right (Sha "22e") ] }
+        run = runActionCustom results
+        actions = snd $ run $ handleEventsTest events state
+      actions `shouldBe`
+        [ AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 1)
+                        "Pull request approved for merge by @deckard, rebasing now."
+        , ATryIntegrate "Merge #1: First PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "ab1")
+                        []
+                        False
+        , ALeaveComment (PullRequestId 1) "Rebased as 1ab, waiting for CI …"
+        , AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 2)
+                       "Pull request approved for merge by @deckard, \
+                       \waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        [PullRequestId 1]
+                        False
+        , ALeaveComment (PullRequestId 2) "Speculatively rebased as 2cd behind #1, waiting for CI …"
+        , ALeaveComment (PullRequestId 1) "The build failed, but GitHub did not provide an URL to the build failure."
+        -- #2 is integrated again as its speculative base failed
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        []
+                        False
+        , ALeaveComment (PullRequestId 2) "Rebased as 22e, waiting for CI …"
+        , ALeaveComment (PullRequestId 2) "The build failed, but GitHub did not provide an URL to the build failure."
+        ]
 
     it "handles a 2-wagon merge train with build failures coming in the reverse order: failure (2), failure (1)" $ do
       pendingWith "TODO: implement me"

From 06dc4d08e31a02606c63bc641268bf91794abd6d Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 15:33:30 +0200
Subject: [PATCH 19/54] Spec: pending test failure (2), failure (1)

---
 tests/Spec.hs | 55 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 2 deletions(-)

diff --git a/tests/Spec.hs b/tests/Spec.hs
index 4f7b3983..1df5ab5a 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -2152,7 +2152,6 @@ main = hspec $ do
         events =
           [ CommentAdded (PullRequestId 1) "deckard" "@bot merge"
           , CommentAdded (PullRequestId 2) "deckard" "@bot merge"
-          -- Build of #2 finishes before build of #1
           , BuildStatusChanged (Sha "1ab") (Project.BuildFailed Nothing)
           , BuildStatusChanged (Sha "2cd") (Project.BuildFailed Nothing)
           , BuildStatusChanged (Sha "22e") (Project.BuildFailed Nothing)
@@ -2197,7 +2196,59 @@ main = hspec $ do
         ]
 
     it "handles a 2-wagon merge train with build failures coming in the reverse order: failure (2), failure (1)" $ do
-      pendingWith "TODO: implement me"
+      pendingWith "TODO: fix this failing test"
+      let
+        state
+          = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")
+          $ Project.insertPullRequest (PullRequestId 2) (Branch "snd") masterBranch (Sha "cd2") "Second PR" (Username "rachael")
+          $ Project.emptyProjectState
+        events =
+          [ CommentAdded (PullRequestId 1) "deckard" "@bot merge"
+          , CommentAdded (PullRequestId 2) "deckard" "@bot merge"
+          -- Build of #2 finishes before build of #1
+          , BuildStatusChanged (Sha "2cd") (Project.BuildFailed Nothing)
+          , BuildStatusChanged (Sha "1ab") (Project.BuildFailed Nothing)
+          , BuildStatusChanged (Sha "22e") (Project.BuildFailed Nothing)
+          ]
+        results = defaultResults { resultIntegrate = [ Right (Sha "1ab")
+                                                     , Right (Sha "2cd")
+                                                     , Right (Sha "22e") ] }
+        run = runActionCustom results
+        actions = snd $ run $ handleEventsTest events state
+      actions `shouldBe`
+        [ AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 1)
+                        "Pull request approved for merge by @deckard, rebasing now."
+        , ATryIntegrate "Merge #1: First PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "ab1")
+                        []
+                        False
+        , ALeaveComment (PullRequestId 1) "Rebased as 1ab, waiting for CI …"
+        , AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 2)
+                       "Pull request approved for merge by @deckard, \
+                       \waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        [PullRequestId 1]
+                        False
+        , ALeaveComment (PullRequestId 2) "Speculatively rebased as 2cd behind #1, waiting for CI …"
+        -- Build of #2 fails here, but since it is speculative, we delay reporting.
+        , ALeaveComment (PullRequestId 1) "The build failed, but GitHub did not provide an URL to the build failure."
+        -- #2 is integrated again as its speculative base failed
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        []
+                        False
+        , ALeaveComment (PullRequestId 2) "Rebased as 22e, waiting for CI …"
+        , ALeaveComment (PullRequestId 2) "The build failed, but GitHub did not provide an URL to the build failure."
+        ]
 
     it "handles a sequence of merges: success, success, success" $ do
       -- An afternoon of work on PRs:

From 142a53ebde11bab483ee5e47b2082b97603a9cea Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 16:11:48 +0200
Subject: [PATCH 20/54] Fix behaviour when failure arrive in reverse order

---
 src/Logic.hs  | 17 +++++++++++++----
 tests/Spec.hs |  3 +--
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index 0def26f1..e56d6178 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -829,10 +829,19 @@ describeStatus prId pr state = case Pr.classifyPullRequest pr of
       , " "
       , prBranchName
       ]
-  PrStatusFailedBuild url -> case url of
-                              Just url' -> format "The build failed: {}\nIf this is the result of a flaky test, close and reopen the PR, then tag me again.\nOtherwise, push a new commit and tag me again." [url']
-                              -- This should probably never happen
-                              Nothing   -> "The build failed, but GitHub did not provide an URL to the build failure."
+  PrStatusFailedBuild url
+    -- TODO: specify what the base is here...
+    | speculativeIntegration pr state -> "Speculative build failed. \
+                                         \ I will automatically retry after base build results."
+    | otherwise -> case url of
+                   Just url' -> format "The build failed: {}\nIf this is the result of a flaky test, close and reopen the PR, then tag me again.\nOtherwise, push a new commit and tag me again." [url']
+                   -- This should probably never happen
+                   Nothing   -> "The build failed, but GitHub did not provide an URL to the build failure."
+
+speculativeIntegration :: PullRequest -> ProjectState -> Bool
+speculativeIntegration pr state = case Pr.unfailingIntegratedPullRequests state of
+  [] -> False
+  (trainHead:_) -> maybe False (pr `Pr.approvedAfter`) (Pr.lookupPullRequest trainHead state)
 
 -- Leave a comment with the feedback from 'describeStatus' and set the
 -- 'needsFeedback' flag to 'False'.
diff --git a/tests/Spec.hs b/tests/Spec.hs
index 1df5ab5a..4e43976a 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -2196,7 +2196,6 @@ main = hspec $ do
         ]
 
     it "handles a 2-wagon merge train with build failures coming in the reverse order: failure (2), failure (1)" $ do
-      pendingWith "TODO: fix this failing test"
       let
         state
           = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")
@@ -2237,7 +2236,7 @@ main = hspec $ do
                         [PullRequestId 1]
                         False
         , ALeaveComment (PullRequestId 2) "Speculatively rebased as 2cd behind #1, waiting for CI …"
-        -- Build of #2 fails here, but since it is speculative, we delay reporting.
+        , ALeaveComment (PullRequestId 2) "Speculative build failed.  I will automatically retry after base build results."
         , ALeaveComment (PullRequestId 1) "The build failed, but GitHub did not provide an URL to the build failure."
         -- #2 is integrated again as its speculative base failed
         , ATryIntegrate "Merge #2: Second PR\n\n\

From 90e6facb77451a98ca227100cc25ed14fd7fda91 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 16:23:09 +0200
Subject: [PATCH 21/54] Add another test: success (1), failure (2)

---
 tests/Spec.hs | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/tests/Spec.hs b/tests/Spec.hs
index 4e43976a..26900783 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -2249,6 +2249,49 @@ main = hspec $ do
         , ALeaveComment (PullRequestId 2) "The build failed, but GitHub did not provide an URL to the build failure."
         ]
 
+    it "handles a 2-wagon merge train with success and failure coming in the right order: success (1), failure (2)" $ do
+      let
+        state
+          = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")
+          $ Project.insertPullRequest (PullRequestId 2) (Branch "snd") masterBranch (Sha "cd2") "Second PR" (Username "rachael")
+          $ Project.emptyProjectState
+        events =
+          [ CommentAdded (PullRequestId 1) "deckard" "@bot merge"
+          , CommentAdded (PullRequestId 2) "deckard" "@bot merge"
+          , BuildStatusChanged (Sha "1ab") Project.BuildSucceeded
+          , BuildStatusChanged (Sha "2cd") (Project.BuildFailed Nothing)
+          ]
+        results = defaultResults { resultIntegrate = [ Right (Sha "1ab")
+                                                     , Right (Sha "2cd") ] }
+        run = runActionCustom results
+        actions = snd $ run $ handleEventsTest events state
+      actions `shouldBe`
+        [ AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 1)
+                        "Pull request approved for merge by @deckard, rebasing now."
+        , ATryIntegrate "Merge #1: First PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "ab1")
+                        []
+                        False
+        , ALeaveComment (PullRequestId 1) "Rebased as 1ab, waiting for CI …"
+        , AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 2)
+                       "Pull request approved for merge by @deckard, \
+                       \waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        [PullRequestId 1]
+                        False
+        , ALeaveComment (PullRequestId 2) "Speculatively rebased as 2cd behind #1, waiting for CI …"
+        , ATryPromote (Branch "fst") (Sha "1ab")
+        , ACleanupTestBranch (PullRequestId 1)
+        , ALeaveComment (PullRequestId 2) "The build failed, but GitHub did not provide an URL to the build failure."
+        ]
+
     it "handles a sequence of merges: success, success, success" $ do
       -- An afternoon of work on PRs:
       -- * three PRs are merged and approved in order

From 2e531f301e1d076268da45a223f24e90c7cdfd8c Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 16:25:51 +0200
Subject: [PATCH 22/54] Spec: add a (pending) failing test.

---
 tests/Spec.hs | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/tests/Spec.hs b/tests/Spec.hs
index 26900783..469b9e75 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -2292,6 +2292,51 @@ main = hspec $ do
         , ALeaveComment (PullRequestId 2) "The build failed, but GitHub did not provide an URL to the build failure."
         ]
 
+    it "handles a 2-wagon merge train with success and failure coming in the right order: success (2), failure (1)" $ do
+      pendingWith "TODO: fix behaviour for this test"
+      let
+        state
+          = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")
+          $ Project.insertPullRequest (PullRequestId 2) (Branch "snd") masterBranch (Sha "cd2") "Second PR" (Username "rachael")
+          $ Project.emptyProjectState
+        events =
+          [ CommentAdded (PullRequestId 1) "deckard" "@bot merge"
+          , CommentAdded (PullRequestId 2) "deckard" "@bot merge"
+          , BuildStatusChanged (Sha "2cd") (Project.BuildFailed Nothing)
+          , BuildStatusChanged (Sha "1ab") Project.BuildSucceeded
+          ]
+        results = defaultResults { resultIntegrate = [ Right (Sha "1ab")
+                                                     , Right (Sha "2cd") ] }
+        run = runActionCustom results
+        actions = snd $ run $ handleEventsTest events state
+      actions `shouldBe`
+        [ AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 1)
+                        "Pull request approved for merge by @deckard, rebasing now."
+        , ATryIntegrate "Merge #1: First PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "ab1")
+                        []
+                        False
+        , ALeaveComment (PullRequestId 1) "Rebased as 1ab, waiting for CI …"
+        , AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 2)
+                       "Pull request approved for merge by @deckard, \
+                       \waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        [PullRequestId 1]
+                        False
+        , ALeaveComment (PullRequestId 2) "Speculatively rebased as 2cd behind #1, waiting for CI …"
+        , ALeaveComment (PullRequestId 2) "Speculative build failed.  I will automatically retry after base build results."
+        , ATryPromote (Branch "fst") (Sha "1ab")
+        , ACleanupTestBranch (PullRequestId 1)
+        , ALeaveComment (PullRequestId 2) "The build failed, but GitHub did not provide an URL to the build failure."
+        ]
+
     it "handles a sequence of merges: success, success, success" $ do
       -- An afternoon of work on PRs:
       -- * three PRs are merged and approved in order

From 07f2a7df2b00cc91d3ce387a5dee78ea52eaa8d9 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 16:29:15 +0200
Subject: [PATCH 23/54] fix test name

---
 tests/Spec.hs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Spec.hs b/tests/Spec.hs
index 469b9e75..09e7e5c6 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -2292,7 +2292,7 @@ main = hspec $ do
         , ALeaveComment (PullRequestId 2) "The build failed, but GitHub did not provide an URL to the build failure."
         ]
 
-    it "handles a 2-wagon merge train with success and failure coming in the right order: success (2), failure (1)" $ do
+    it "handles a 2-wagon merge train with success and failure coming in the reverse order: success (2), failure (1)" $ do
       pendingWith "TODO: fix behaviour for this test"
       let
         state

From 1b5072ace7cec2df0379a526b4dd323e6803c38b Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 16:51:18 +0200
Subject: [PATCH 24/54] Fix one buildfailure behaviour

---
 src/Logic.hs  | 8 ++++++++
 tests/Spec.hs | 1 -
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index e56d6178..8e1886a0 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -762,6 +762,7 @@ pushCandidate (pullRequestId, pullRequest) newHead state =
       PushOk -> do
         cleanupTestBranch pullRequestId
         pure $ Pr.updatePullRequests (unspeculateConflictsAfter pullRequest)
+             $ Pr.updatePullRequests (unspeculateFailuresAfter pullRequest)
              $ Pr.setIntegrationStatus pullRequestId Promoted state
       -- If something was pushed to the target branch while the candidate was
       -- being tested, try to integrate again and hope that next time the push
@@ -780,6 +781,13 @@ unspeculateConflictsAfter promotedPullRequest pr
   | otherwise
   = pr
 
+unspeculateFailuresAfter :: PullRequest -> PullRequest -> PullRequest
+unspeculateFailuresAfter promotedPullRequest pr
+  | Pr.PullRequest{Pr.integrationStatus = Integrated _ (BuildFailed _)} <- pr
+  , pr `Pr.approvedAfter` promotedPullRequest
+  = pr{Pr.needsFeedback = True}
+  | otherwise
+  = pr
 
 -- Keep doing a proceed step until the state doesn't change any more. For this
 -- to work properly, it is essential that "proceed" does not have any side
diff --git a/tests/Spec.hs b/tests/Spec.hs
index 09e7e5c6..2655930b 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -2293,7 +2293,6 @@ main = hspec $ do
         ]
 
     it "handles a 2-wagon merge train with success and failure coming in the reverse order: success (2), failure (1)" $ do
-      pendingWith "TODO: fix behaviour for this test"
       let
         state
           = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")

From b3ad8a0ab473ec948fdb7d99bfe91d1d791e22cc Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 16:51:40 +0200
Subject: [PATCH 25/54] Spec: failure (1), success (2)

---
 tests/Spec.hs | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/tests/Spec.hs b/tests/Spec.hs
index 2655930b..a6f4606c 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -2336,6 +2336,58 @@ main = hspec $ do
         , ALeaveComment (PullRequestId 2) "The build failed, but GitHub did not provide an URL to the build failure."
         ]
 
+    it "handles a 2-wagon merge train with success and failure coming in the right order: failure (1), success (2)" $ do
+      let
+        state
+          = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")
+          $ Project.insertPullRequest (PullRequestId 2) (Branch "snd") masterBranch (Sha "cd2") "Second PR" (Username "rachael")
+          $ Project.emptyProjectState
+        events =
+          [ CommentAdded (PullRequestId 1) "deckard" "@bot merge"
+          , CommentAdded (PullRequestId 2) "deckard" "@bot merge"
+          , BuildStatusChanged (Sha "1ab") (Project.BuildFailed Nothing)
+          , BuildStatusChanged (Sha "2cd") (Project.BuildFailed Nothing)
+          , BuildStatusChanged (Sha "22e") Project.BuildSucceeded
+          ]
+        results = defaultResults { resultIntegrate = [ Right (Sha "1ab")
+                                                     , Right (Sha "2cd")
+                                                     , Right (Sha "22e") ] }
+        run = runActionCustom results
+        actions = snd $ run $ handleEventsTest events state
+      actions `shouldBe`
+        [ AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 1)
+                        "Pull request approved for merge by @deckard, rebasing now."
+        , ATryIntegrate "Merge #1: First PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "ab1")
+                        []
+                        False
+        , ALeaveComment (PullRequestId 1) "Rebased as 1ab, waiting for CI …"
+        , AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 2)
+                       "Pull request approved for merge by @deckard, \
+                       \waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        [PullRequestId 1]
+                        False
+        , ALeaveComment (PullRequestId 2) "Speculatively rebased as 2cd behind #1, waiting for CI …"
+        , ALeaveComment (PullRequestId 1) "The build failed, but GitHub did not provide an URL to the build failure."
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        []
+                        False
+        , ALeaveComment (PullRequestId 2) "Rebased as 22e, waiting for CI …"
+        , ATryPromote (Branch "snd") (Sha "22e")
+        , ACleanupTestBranch (PullRequestId 2)
+        ]
+
     it "handles a sequence of merges: success, success, success" $ do
       -- An afternoon of work on PRs:
       -- * three PRs are merged and approved in order

From 6e077c240077a315d97b313c10d611505ee0b6eb Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 16:54:16 +0200
Subject: [PATCH 26/54] Spec: failure (2), success (1)

---
 tests/Spec.hs | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/tests/Spec.hs b/tests/Spec.hs
index a6f4606c..c7cf0999 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -2388,6 +2388,58 @@ main = hspec $ do
         , ACleanupTestBranch (PullRequestId 2)
         ]
 
+    it "handles a 2-wagon merge train with success and failure coming in the right order: failure (2), success (1)" $ do
+      let
+        state
+          = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")
+          $ Project.insertPullRequest (PullRequestId 2) (Branch "snd") masterBranch (Sha "cd2") "Second PR" (Username "rachael")
+          $ Project.emptyProjectState
+        events =
+          [ CommentAdded (PullRequestId 1) "deckard" "@bot merge"
+          , CommentAdded (PullRequestId 2) "deckard" "@bot merge"
+          , BuildStatusChanged (Sha "2cd") Project.BuildSucceeded
+          , BuildStatusChanged (Sha "1ab") (Project.BuildFailed Nothing)
+          , BuildStatusChanged (Sha "22e") Project.BuildSucceeded
+          ]
+        results = defaultResults { resultIntegrate = [ Right (Sha "1ab")
+                                                     , Right (Sha "2cd")
+                                                     , Right (Sha "22e") ] }
+        run = runActionCustom results
+        actions = snd $ run $ handleEventsTest events state
+      actions `shouldBe`
+        [ AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 1)
+                        "Pull request approved for merge by @deckard, rebasing now."
+        , ATryIntegrate "Merge #1: First PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "ab1")
+                        []
+                        False
+        , ALeaveComment (PullRequestId 1) "Rebased as 1ab, waiting for CI …"
+        , AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 2)
+                       "Pull request approved for merge by @deckard, \
+                       \waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        [PullRequestId 1]
+                        False
+        , ALeaveComment (PullRequestId 2) "Speculatively rebased as 2cd behind #1, waiting for CI …"
+        , ALeaveComment (PullRequestId 1) "The build failed, but GitHub did not provide an URL to the build failure."
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        []
+                        False
+        , ALeaveComment (PullRequestId 2) "Rebased as 22e, waiting for CI …"
+        , ATryPromote (Branch "snd") (Sha "22e")
+        , ACleanupTestBranch (PullRequestId 2)
+        ]
+
     it "handles a sequence of merges: success, success, success" $ do
       -- An afternoon of work on PRs:
       -- * three PRs are merged and approved in order

From ab811ac2093863c3ff60fbcd8d58f790f52f3fea Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 16:59:10 +0200
Subject: [PATCH 27/54] Test closing PRs early on the train

---
 tests/Spec.hs | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/tests/Spec.hs b/tests/Spec.hs
index c7cf0999..81d8d9ac 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -2388,7 +2388,7 @@ main = hspec $ do
         , ACleanupTestBranch (PullRequestId 2)
         ]
 
-    it "handles a 2-wagon merge train with success and failure coming in the right order: failure (2), success (1)" $ do
+    it "handles a 2-wagon merge train with success and failure coming in the reverse order: failure (2), success (1)" $ do
       let
         state
           = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")
@@ -2440,6 +2440,58 @@ main = hspec $ do
         , ACleanupTestBranch (PullRequestId 2)
         ]
 
+    it "handles a 2-wagon merge train with closing and success coming in the reverse order: closing (2), success (1)" $ do
+      let
+        state
+          = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")
+          $ Project.insertPullRequest (PullRequestId 2) (Branch "snd") masterBranch (Sha "cd2") "Second PR" (Username "rachael")
+          $ Project.emptyProjectState
+        events =
+          [ CommentAdded (PullRequestId 1) "deckard" "@bot merge"
+          , CommentAdded (PullRequestId 2) "deckard" "@bot merge"
+          , BuildStatusChanged (Sha "2cd") Project.BuildSucceeded
+          , PullRequestClosed (PullRequestId 1)
+          , BuildStatusChanged (Sha "22e") Project.BuildSucceeded
+          ]
+        results = defaultResults { resultIntegrate = [ Right (Sha "1ab")
+                                                     , Right (Sha "2cd")
+                                                     , Right (Sha "22e") ] }
+        run = runActionCustom results
+        actions = snd $ run $ handleEventsTest events state
+      actions `shouldBe`
+        [ AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 1)
+                        "Pull request approved for merge by @deckard, rebasing now."
+        , ATryIntegrate "Merge #1: First PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "ab1")
+                        []
+                        False
+        , ALeaveComment (PullRequestId 1) "Rebased as 1ab, waiting for CI …"
+        , AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 2)
+                       "Pull request approved for merge by @deckard, \
+                       \waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        [PullRequestId 1]
+                        False
+        , ALeaveComment (PullRequestId 2) "Speculatively rebased as 2cd behind #1, waiting for CI …"
+        , ALeaveComment (PullRequestId 1) "Abandoning this pull request because it was closed."
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        []
+                        False
+        , ALeaveComment (PullRequestId 2) "Rebased as 22e, waiting for CI …"
+        , ATryPromote (Branch "snd") (Sha "22e")
+        , ACleanupTestBranch (PullRequestId 2)
+        ]
+
     it "handles a sequence of merges: success, success, success" $ do
       -- An afternoon of work on PRs:
       -- * three PRs are merged and approved in order

From 6f1da5c3aa832278b04b6c056b771a3a97a0e26d Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 17:07:26 +0200
Subject: [PATCH 28/54] Add a proper PrStatusSpeculativeConflict

---
 src/Logic.hs   | 2 ++
 src/Project.hs | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index 8e1886a0..d30a90e5 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -837,6 +837,8 @@ describeStatus prId pr state = case Pr.classifyPullRequest pr of
       , " "
       , prBranchName
       ]
+  PrStatusSpeculativeConflict -> "Failed to speculatively rebase. \
+                                 \ I will retry rebasing automatically when the queue clears."
   PrStatusFailedBuild url
     -- TODO: specify what the base is here...
     | speculativeIntegration pr state -> "Speculative build failed. \
diff --git a/src/Project.hs b/src/Project.hs
index b42c46d8..ea5bb053 100644
--- a/src/Project.hs
+++ b/src/Project.hs
@@ -114,6 +114,7 @@ data PullRequestStatus
   | PrStatusWrongFixups               -- Failed to integrate due to the presence of orphan fixup commits.
   | PrStatusEmptyRebase               -- Rebase was empty (changes already in the target branch?)
   | PrStatusFailedConflict            -- Failed to integrate due to merge conflict.
+  | PrStatusSpeculativeConflict       -- Failed to integrate but this was a speculative build
   | PrStatusFailedBuild (Maybe Text)  -- Integrated, but the build failed. Field should contain the URL to a page explaining the build failure.
   deriving (Eq)
 
@@ -306,8 +307,7 @@ classifyPullRequest pr = case approval pr of
   Just _  -> case integrationStatus pr of
     NotIntegrated -> PrStatusApproved
     IncorrectBaseBranch -> PrStatusIncorrectBaseBranch
-    -- checks if this is a speculative rebase, if it is, we have to wait for the train status
-    Conflicted baseBranch' _ | baseBranch' /= baseBranch pr -> PrStatusBuildPending -- TODO: proper status?
+    Conflicted base _ | base /= baseBranch pr -> PrStatusSpeculativeConflict
     Conflicted _ WrongFixups -> PrStatusWrongFixups
     Conflicted _ EmptyRebase -> PrStatusEmptyRebase
     Conflicted _ _  -> PrStatusFailedConflict

From 596e122b1ed8494fc50c786d6e4744b368a8405b Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 17:19:57 +0200
Subject: [PATCH 29/54] Report waiting for which PRs on failed builds

---
 src/Logic.hs   | 22 +++++++++-------------
 src/Project.hs | 13 +++++++++++++
 tests/Spec.hs  |  4 ++--
 3 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index d30a90e5..93d47248 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -839,19 +839,15 @@ describeStatus prId pr state = case Pr.classifyPullRequest pr of
       ]
   PrStatusSpeculativeConflict -> "Failed to speculatively rebase. \
                                  \ I will retry rebasing automatically when the queue clears."
-  PrStatusFailedBuild url
-    -- TODO: specify what the base is here...
-    | speculativeIntegration pr state -> "Speculative build failed. \
-                                         \ I will automatically retry after base build results."
-    | otherwise -> case url of
-                   Just url' -> format "The build failed: {}\nIf this is the result of a flaky test, close and reopen the PR, then tag me again.\nOtherwise, push a new commit and tag me again." [url']
-                   -- This should probably never happen
-                   Nothing   -> "The build failed, but GitHub did not provide an URL to the build failure."
-
-speculativeIntegration :: PullRequest -> ProjectState -> Bool
-speculativeIntegration pr state = case Pr.unfailingIntegratedPullRequests state of
-  [] -> False
-  (trainHead:_) -> maybe False (pr `Pr.approvedAfter`) (Pr.lookupPullRequest trainHead state)
+  PrStatusFailedBuild url -> case Pr.unfailingIntegratedPullRequestsBefore pr state of
+    [] -> case url of
+          Just url' -> format "The build failed: {}\nIf this is the result of a flaky test, close and reopen the PR, then tag me again.\nOtherwise, push a new commit and tag me again." [url']
+          -- This should probably never happen
+          Nothing   -> "The build failed, but GitHub did not provide an URL to the build failure."
+    -- TODO: use build link here somehow
+    trainBefore -> format "Speculative build failed. \
+                          \ I will automatically retry after {} build results."
+                          [prettyPullRequestIds trainBefore]
 
 -- Leave a comment with the feedback from 'describeStatus' and set the
 -- 'needsFeedback' flag to 'False'.
diff --git a/src/Project.hs b/src/Project.hs
index ea5bb053..eee55682 100644
--- a/src/Project.hs
+++ b/src/Project.hs
@@ -24,6 +24,7 @@ module Project
   integratedPullRequests,
   integratedPullRequestsAfter,
   unfailingIntegratedPullRequests,
+  unfailingIntegratedPullRequestsBefore,
   speculativelyConflictedPullRequestsAfter,
   candidatePullRequests,
   classifyPullRequest,
@@ -416,6 +417,7 @@ integratedPullRequestsAfter = pullRequestsAfterThat (isIntegrated . integrationS
   where
   isIntegrated (Integrated _ _) = True
   isIntegrated _                = False
+  -- TODO: remove repeated code isIntegrated
 
 speculativelyConflictedPullRequestsAfter :: PullRequestId -> ProjectState -> [PullRequestId]
 speculativelyConflictedPullRequestsAfter = pullRequestsAfterThat isSpeculativelyConflicted
@@ -432,6 +434,17 @@ unfailingIntegratedPullRequests = filterPullRequestsBy $ isUnfailingIntegrated .
   isUnfailingIntegrated (Integrated _ BuildSucceeded)   = True
   isUnfailingIntegrated _                               = False
 
+unfailingIntegratedPullRequestsBefore :: PullRequest -> ProjectState -> [PullRequestId]
+unfailingIntegratedPullRequestsBefore referencePullRequest = filterPullRequestsBy $
+  \pr -> isUnfailingIntegrated (integrationStatus pr)
+      && referencePullRequest `approvedAfter` pr
+  where
+  isUnfailingIntegrated (Integrated _ BuildPending)     = True
+  isUnfailingIntegrated (Integrated _ (BuildStarted _)) = True
+  isUnfailingIntegrated (Integrated _ BuildSucceeded)   = True
+  isUnfailingIntegrated _                               = False
+  -- TODO: remove repeated code isUnfailingIntegrated
+
 -- Returns the pull requests that have not been integrated yet, in order of
 -- ascending id.
 unintegratedPullRequests :: ProjectState -> [PullRequestId]
diff --git a/tests/Spec.hs b/tests/Spec.hs
index 81d8d9ac..ce0cf300 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -2236,7 +2236,7 @@ main = hspec $ do
                         [PullRequestId 1]
                         False
         , ALeaveComment (PullRequestId 2) "Speculatively rebased as 2cd behind #1, waiting for CI …"
-        , ALeaveComment (PullRequestId 2) "Speculative build failed.  I will automatically retry after base build results."
+        , ALeaveComment (PullRequestId 2) "Speculative build failed.  I will automatically retry after #1 build results."
         , ALeaveComment (PullRequestId 1) "The build failed, but GitHub did not provide an URL to the build failure."
         -- #2 is integrated again as its speculative base failed
         , ATryIntegrate "Merge #2: Second PR\n\n\
@@ -2330,7 +2330,7 @@ main = hspec $ do
                         [PullRequestId 1]
                         False
         , ALeaveComment (PullRequestId 2) "Speculatively rebased as 2cd behind #1, waiting for CI …"
-        , ALeaveComment (PullRequestId 2) "Speculative build failed.  I will automatically retry after base build results."
+        , ALeaveComment (PullRequestId 2) "Speculative build failed.  I will automatically retry after #1 build results."
         , ATryPromote (Branch "fst") (Sha "1ab")
         , ACleanupTestBranch (PullRequestId 1)
         , ALeaveComment (PullRequestId 2) "The build failed, but GitHub did not provide an URL to the build failure."

From 53b011d21c632e1d8a7854d5f3c7aeb5470142cb Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 17:30:04 +0200
Subject: [PATCH 30/54] Remove TODO item.

I tried to solve it,
but it would make my current pull request too big with unrelated changes (#137).
I will leave this for later.
---
 src/Logic.hs | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index 93d47248..9f2ba770 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -844,7 +844,6 @@ describeStatus prId pr state = case Pr.classifyPullRequest pr of
           Just url' -> format "The build failed: {}\nIf this is the result of a flaky test, close and reopen the PR, then tag me again.\nOtherwise, push a new commit and tag me again." [url']
           -- This should probably never happen
           Nothing   -> "The build failed, but GitHub did not provide an URL to the build failure."
-    -- TODO: use build link here somehow
     trainBefore -> format "Speculative build failed. \
                           \ I will automatically retry after {} build results."
                           [prettyPullRequestIds trainBefore]

From 321334bd993141ce0f3218a0f701e521a4dd4455 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 17:34:34 +0200
Subject: [PATCH 31/54] Remove repeated code

---
 src/Project.hs | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/src/Project.hs b/src/Project.hs
index eee55682..83812ec1 100644
--- a/src/Project.hs
+++ b/src/Project.hs
@@ -399,9 +399,6 @@ wasIntegrationAttemptFor commit pr = case integrationStatus pr of
 
 integratedPullRequests :: ProjectState -> [PullRequestId]
 integratedPullRequests = filterPullRequestsBy $ isIntegrated . integrationStatus
-  where
-  isIntegrated (Integrated _ _) = True
-  isIntegrated _                = False
 
 -- | Lists the pull requests that were approved after a given PR
 --   matching a given property
@@ -414,10 +411,6 @@ pullRequestsAfterThat p pid state =
 -- | Lists the pull requests that are integrated on top of the given id.
 integratedPullRequestsAfter :: PullRequestId -> ProjectState -> [PullRequestId]
 integratedPullRequestsAfter = pullRequestsAfterThat (isIntegrated . integrationStatus)
-  where
-  isIntegrated (Integrated _ _) = True
-  isIntegrated _                = False
-  -- TODO: remove repeated code isIntegrated
 
 speculativelyConflictedPullRequestsAfter :: PullRequestId -> ProjectState -> [PullRequestId]
 speculativelyConflictedPullRequestsAfter = pullRequestsAfterThat isSpeculativelyConflicted
@@ -428,22 +421,11 @@ speculativelyConflictedPullRequestsAfter = pullRequestsAfterThat isSpeculatively
 
 unfailingIntegratedPullRequests :: ProjectState -> [PullRequestId]
 unfailingIntegratedPullRequests = filterPullRequestsBy $ isUnfailingIntegrated . integrationStatus
-  where
-  isUnfailingIntegrated (Integrated _ BuildPending)     = True
-  isUnfailingIntegrated (Integrated _ (BuildStarted _)) = True
-  isUnfailingIntegrated (Integrated _ BuildSucceeded)   = True
-  isUnfailingIntegrated _                               = False
 
 unfailingIntegratedPullRequestsBefore :: PullRequest -> ProjectState -> [PullRequestId]
 unfailingIntegratedPullRequestsBefore referencePullRequest = filterPullRequestsBy $
   \pr -> isUnfailingIntegrated (integrationStatus pr)
       && referencePullRequest `approvedAfter` pr
-  where
-  isUnfailingIntegrated (Integrated _ BuildPending)     = True
-  isUnfailingIntegrated (Integrated _ (BuildStarted _)) = True
-  isUnfailingIntegrated (Integrated _ BuildSucceeded)   = True
-  isUnfailingIntegrated _                               = False
-  -- TODO: remove repeated code isUnfailingIntegrated
 
 -- Returns the pull requests that have not been integrated yet, in order of
 -- ascending id.
@@ -501,3 +483,14 @@ pr1 `approvedAfter` pr2 = case (mo1, mo2) of
   where
   mo1 = approvalOrder <$> approval pr1
   mo2 = approvalOrder <$> approval pr2
+
+isIntegrated :: IntegrationStatus -> Bool
+isIntegrated (Integrated _ _) = True
+isIntegrated _                = False
+
+isUnfailingIntegrated :: IntegrationStatus -> Bool
+isUnfailingIntegrated (Integrated _ BuildPending)     = True
+isUnfailingIntegrated (Integrated _ (BuildStarted _)) = True
+isUnfailingIntegrated (Integrated _ BuildSucceeded)   = True
+isUnfailingIntegrated (Integrated _ (BuildFailed _))  = False
+isUnfailingIntegrated _                               = False

From 818270ef91702a8540d90b09ac2f8dbc7f55775f Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 17:35:49 +0200
Subject: [PATCH 32/54] Spec: remove uneeded resultIntegrate values

---
 tests/Spec.hs | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/tests/Spec.hs b/tests/Spec.hs
index ce0cf300..378beda7 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -1927,13 +1927,9 @@ main = hspec $ do
           , CommentAdded (PullRequestId 3) "deckard" "@bot merge"
           , BuildStatusChanged (Sha "1ab") (Project.BuildSucceeded)
           ]
-        -- For this test, we assume all integrations and pushes succeed.
         results = defaultResults { resultIntegrate = [ Right (Sha "1ab")
                                                      , Left (IntegrationFailure (BaseBranch "testing/1") RebaseFailed)
-                                                     , Right (Sha "3cd")
-                                                     , Right (Sha "5bc")
-                                                     , Right (Sha "6cd") ] }
-        -- TODO: cleanup the above resultIntegrate to only what is needed.
+                                                     , Right (Sha "3cd") ] }
         run = runActionCustom results
         actions = snd $ run $ handleEventsTest events state
       actions `shouldBe`

From 13bf8fe1c80ad8686fa8d6ea7784e6f3a4175abf Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 17:51:28 +0200
Subject: [PATCH 33/54] Refactor unintegrateAfter

---
 src/Logic.hs   | 16 ++++++++++------
 src/Project.hs |  8 ++++++++
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index 9f2ba770..bfedcdeb 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -560,13 +560,17 @@ handleMergeRequested projectConfig prId author state pr approvalType = do
 -- | Given a pull request id, mark all pull requests that follow from it
 --   in the merge train as NotIntegrated
 unintegrateAfter :: PullRequestId -> ProjectState -> ProjectState
-unintegrateAfter pid state =
-  compose [ Pr.updatePullRequest pid' unintegrate
-          | pid' <- Pr.integratedPullRequestsAfter pid state
-                 ++ Pr.speculativelyConflictedPullRequestsAfter pid state] state
+unintegrateAfter pid state = case Pr.lookupPullRequest pid state of
+  Nothing -> state -- should not happen
+  Just pr -> unintegrateAfter' pr state
   where
-  unintegrate pr = pr{Pr.integrationStatus = NotIntegrated}
--- TODO: refactor unintegrateAfter
+  unintegrateAfter' :: PullRequest -> ProjectState -> ProjectState
+  unintegrateAfter' pr0 = Pr.updatePullRequests unintegrate
+    where
+    unintegrate pr | pr `Pr.approvedAfter` pr0 && Pr.isIntegratedOrSpeculativelyConflicted pr
+                   = pr{Pr.integrationStatus = NotIntegrated}
+                   | otherwise
+                   = pr
 
 -- | If there is an integration candidate, and its integration sha matches that of the build,
 --   then update the build status for that pull request. Otherwise do nothing.
diff --git a/src/Project.hs b/src/Project.hs
index 83812ec1..f08662a2 100644
--- a/src/Project.hs
+++ b/src/Project.hs
@@ -41,6 +41,7 @@ module Project
   saveProjectState,
   alwaysAddMergeCommit,
   needsDeploy,
+  isIntegratedOrSpeculativelyConflicted,
   needsTag,
   displayApproval,
   setApproval,
@@ -494,3 +495,10 @@ isUnfailingIntegrated (Integrated _ (BuildStarted _)) = True
 isUnfailingIntegrated (Integrated _ BuildSucceeded)   = True
 isUnfailingIntegrated (Integrated _ (BuildFailed _))  = False
 isUnfailingIntegrated _                               = False
+
+isIntegratedOrSpeculativelyConflicted :: PullRequest -> Bool
+isIntegratedOrSpeculativelyConflicted pr =
+  case integrationStatus pr of
+  (Integrated _ _)                            -> True
+  (Conflicted base _) | base /= baseBranch pr -> True
+  _                                           -> False

From 9ca84da6aefa7a75323dabc2625b448daa84dcd2 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 17:52:50 +0200
Subject: [PATCH 34/54] Remove now-unused crufty code

---
 src/Project.hs | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/src/Project.hs b/src/Project.hs
index f08662a2..b80923f0 100644
--- a/src/Project.hs
+++ b/src/Project.hs
@@ -22,10 +22,8 @@ module Project
   Owner,
   approvedPullRequests,
   integratedPullRequests,
-  integratedPullRequestsAfter,
   unfailingIntegratedPullRequests,
   unfailingIntegratedPullRequestsBefore,
-  speculativelyConflictedPullRequestsAfter,
   candidatePullRequests,
   classifyPullRequest,
   classifyPullRequests,
@@ -401,25 +399,6 @@ wasIntegrationAttemptFor commit pr = case integrationStatus pr of
 integratedPullRequests :: ProjectState -> [PullRequestId]
 integratedPullRequests = filterPullRequestsBy $ isIntegrated . integrationStatus
 
--- | Lists the pull requests that were approved after a given PR
---   matching a given property
-pullRequestsAfterThat :: (PullRequest -> Bool) -> PullRequestId -> ProjectState -> [PullRequestId]
-pullRequestsAfterThat p pid state =
-  case lookupPullRequest pid state of
-  Nothing -> []
-  Just pr0 -> filterPullRequestsBy (\pr -> p pr && pr `approvedAfter` pr0) state
-
--- | Lists the pull requests that are integrated on top of the given id.
-integratedPullRequestsAfter :: PullRequestId -> ProjectState -> [PullRequestId]
-integratedPullRequestsAfter = pullRequestsAfterThat (isIntegrated . integrationStatus)
-
-speculativelyConflictedPullRequestsAfter :: PullRequestId -> ProjectState -> [PullRequestId]
-speculativelyConflictedPullRequestsAfter = pullRequestsAfterThat isSpeculativelyConflicted
-  where
-  isSpeculativelyConflicted pr = case integrationStatus pr of
-                                 Conflicted base _ | base /= baseBranch pr -> True
-                                 _ -> False
-
 unfailingIntegratedPullRequests :: ProjectState -> [PullRequestId]
 unfailingIntegratedPullRequests = filterPullRequestsBy $ isUnfailingIntegrated . integrationStatus
 

From bfa11f32ce272b762ff049d5930bd9b7ca2bfd7f Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 17:57:43 +0200
Subject: [PATCH 35/54] Break down long line

---
 src/Logic.hs | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index bfedcdeb..37abafcf 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -845,7 +845,10 @@ describeStatus prId pr state = case Pr.classifyPullRequest pr of
                                  \ I will retry rebasing automatically when the queue clears."
   PrStatusFailedBuild url -> case Pr.unfailingIntegratedPullRequestsBefore pr state of
     [] -> case url of
-          Just url' -> format "The build failed: {}\nIf this is the result of a flaky test, close and reopen the PR, then tag me again.\nOtherwise, push a new commit and tag me again." [url']
+          Just url' -> format "The build failed: {}\n\
+                              \If this is the result of a flaky test, \
+                              \close and reopen the PR, then tag me again.\n\
+                              \Otherwise, push a new commit and tag me again." [url']
           -- This should probably never happen
           Nothing   -> "The build failed, but GitHub did not provide an URL to the build failure."
     trainBefore -> format "Speculative build failed. \

From 885bc3321549ac0ee88cec72274ce6fe83ee3951 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 18:04:30 +0200
Subject: [PATCH 36/54] Refactor branch handling

---
 src/Git.hs   | 13 ++++++++-----
 src/Logic.hs | 13 +++++--------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/Git.hs b/src/Git.hs
index 4ab04c4c..95534bd8 100644
--- a/src/Git.hs
+++ b/src/Git.hs
@@ -1,5 +1,4 @@
--- Hoff -- A gatekeeper for your commits
--- Copyright 2016 Ruud van Asseldonk
+-- Hoff -- A gatekeeper for your commits -- Copyright 2016 Ruud van Asseldonk
 --
 -- Licensed under the Apache License, Version 2.0 (the "License");
 -- you may not use this file except in compliance with the License.
@@ -46,11 +45,12 @@ module Git
   push,
   pushAtomic,
   rebase,
-  remoteToBaseBranch,
   runGit,
   runGitReadOnly,
   tag,
   tag',
+  toBaseBranch,
+  toRemoteBranch,
   tryIntegrate,
 )
 where
@@ -91,8 +91,11 @@ newtype RemoteBranch = RemoteBranch Text deriving newtype (Show, Eq)
 localBranch :: RemoteBranch -> Branch
 localBranch (RemoteBranch name) = Branch name
 
-remoteToBaseBranch :: RemoteBranch -> BaseBranch
-remoteToBaseBranch (RemoteBranch b) = BaseBranch b
+toRemoteBranch :: Branch -> RemoteBranch
+toRemoteBranch (Branch name) = RemoteBranch name
+
+toBaseBranch :: Branch -> BaseBranch
+toBaseBranch (Branch name) = BaseBranch name
 
 -- | A commit hash is stored as its hexadecimal representation.
 newtype Sha = Sha Text deriving newtype (Show, Eq)
diff --git a/src/Logic.hs b/src/Logic.hs
index 37abafcf..3f74ad3f 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -171,18 +171,18 @@ runAction config = foldFree $ \case
     -- When no repositories have a testing branch, this can safely be removed.
     _ <- doGit $ Git.deleteRemoteBranch $ Git.Branch $ Config.testBranch config
 
-    let targetBranch = fromMaybe (Git.RemoteBranch $ Config.branch config) (trainBranch train)
+    let targetBranch = fromMaybe (Git.Branch $ Config.branch config) (trainBranch train)
 
     shaOrFailed <- doGit $ Git.tryIntegrate
       message
       ref
       sha
-      targetBranch
+      (Git.toRemoteBranch targetBranch)
       (testBranch config pr)
       alwaysAddMergeCommit
 
     case shaOrFailed of
-      Left failure -> pure $ cont $ Left $ IntegrationFailure (Git.remoteToBaseBranch targetBranch) failure
+      Left failure -> pure $ cont $ Left $ IntegrationFailure (Git.toBaseBranch targetBranch) failure
       Right integratedSha -> pure $ cont $ Right integratedSha
 
   TryPromote prBranch sha cont -> do
@@ -237,9 +237,9 @@ runAction config = foldFree $ \case
   GetDateTime cont -> doTime $ cont <$> Time.getDateTime
 
   where
-  trainBranch :: [PullRequestId] -> Maybe Git.RemoteBranch
+  trainBranch :: [PullRequestId] -> Maybe Git.Branch
   trainBranch [] = Nothing
-  trainBranch train = Just $ last [testRemoteBranch config pr | pr <- train]
+  trainBranch train = Just $ last [testBranch config pr | pr <- train]
 
 ensureCloned :: ProjectConfiguration -> GitOperation ()
 ensureCloned config =
@@ -905,9 +905,6 @@ pullRequestIdToText (PullRequestId prid) = Text.pack $ show prid
 testBranch :: ProjectConfiguration -> PullRequestId -> Git.Branch
 testBranch config pullRequestId = Git.Branch $ Config.testBranch config <> "/" <> pullRequestIdToText pullRequestId
 
-testRemoteBranch :: ProjectConfiguration -> PullRequestId -> Git.RemoteBranch
-testRemoteBranch config pullRequestId = Git.RemoteBranch $ Config.testBranch config <> "/" <> pullRequestIdToText pullRequestId
-
 -- | Textual rendering of a list of 'PullRequestId's
 --
 -- >>> prettyPullRequestIds [PullRequestId 12, PullRequestId 60, PullRequestId 1337]

From 0582816d178b2c2ad3b5ec244561a59f7150ebfc Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 18:05:38 +0200
Subject: [PATCH 37/54] Fix comment line break

---
 src/Git.hs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/Git.hs b/src/Git.hs
index 95534bd8..0b334127 100644
--- a/src/Git.hs
+++ b/src/Git.hs
@@ -1,4 +1,5 @@
--- Hoff -- A gatekeeper for your commits -- Copyright 2016 Ruud van Asseldonk
+-- Hoff -- A gatekeeper for your commits
+-- Copyright 2016 Ruud van Asseldonk
 --
 -- Licensed under the Apache License, Version 2.0 (the "License");
 -- you may not use this file except in compliance with the License.

From c0c900b62362cbdd72b17e44930a31513ee1326b Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 18:10:08 +0200
Subject: [PATCH 38/54] document the compose auxiliary function

---
 src/Logic.hs | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/Logic.hs b/src/Logic.hs
index 3f74ad3f..971b3055 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -925,5 +925,13 @@ commaAnd ss = case init ss of
               is -> Text.intercalate ", " is <> " and " <> last ss
 
 -- | Fold a list of unary functions by composition
+--
+-- Writing
+--
+-- > compose [f,g,h]
+--
+-- translates to
+--
+-- > f . g . h
 compose :: [a -> a] -> a -> a
 compose = foldr (.) id

From 7804f5f0cb5fd3e7dc0e4b1406606a8e4e081ac7 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 18:11:32 +0200
Subject: [PATCH 39/54] Remove unused function

---
 src/Logic.hs   | 2 ++
 src/Project.hs | 6 ------
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index 971b3055..03c7b282 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -773,6 +773,7 @@ pushCandidate (pullRequestId, pullRequest) newHead state =
       -- succeeds.
       PushRejected _why -> tryIntegratePullRequest pullRequestId state
 
+-- TODO: describe unspeculateConflictsAfter
 unspeculateConflictsAfter :: PullRequest -> PullRequest -> PullRequest
 unspeculateConflictsAfter promotedPullRequest pr
   | Pr.PullRequest{ Pr.integrationStatus = Conflicted specBase reason
@@ -785,6 +786,7 @@ unspeculateConflictsAfter promotedPullRequest pr
   | otherwise
   = pr
 
+-- TODO: describe unspeculateFailuresAfter
 unspeculateFailuresAfter :: PullRequest -> PullRequest -> PullRequest
 unspeculateFailuresAfter promotedPullRequest pr
   | Pr.PullRequest{Pr.integrationStatus = Integrated _ (BuildFailed _)} <- pr
diff --git a/src/Project.hs b/src/Project.hs
index b80923f0..a2a894bf 100644
--- a/src/Project.hs
+++ b/src/Project.hs
@@ -49,7 +49,6 @@ module Project
   updatePullRequest,
   updatePullRequestM,
   updatePullRequests,
-  updatePullRequestsWithId,
   getOwners,
   wasIntegrationAttemptFor,
   filterPullRequestsBy,
@@ -269,11 +268,6 @@ updatePullRequests f state = state {
   pullRequests = IntMap.map f $ pullRequests state
 }
 
-updatePullRequestsWithId :: (PullRequestId -> PullRequest -> PullRequest) -> ProjectState -> ProjectState
-updatePullRequestsWithId f state = state {
-  pullRequests = IntMap.mapWithKey (f . PullRequestId) $ pullRequests state
-}
-
 -- Marks the pull request as approved by somebody or nobody.
 setApproval :: PullRequestId -> Maybe Approval -> ProjectState -> ProjectState
 setApproval pr newApproval = updatePullRequest pr changeApproval

From 59cfe3e3173e78e4bb5ab899395e69e2e992f765 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Fri, 12 Aug 2022 18:16:46 +0200
Subject: [PATCH 40/54] Add comments describing two new functions

---
 src/Logic.hs | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index 03c7b282..dfd3fd66 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -773,7 +773,12 @@ pushCandidate (pullRequestId, pullRequest) newHead state =
       -- succeeds.
       PushRejected _why -> tryIntegratePullRequest pullRequestId state
 
--- TODO: describe unspeculateConflictsAfter
+-- | When a pull request has been promoted to master this means that any
+-- conflicts (failed rebases) build on top of it are not speculative anymore:
+-- they are real conflicts on top of the (new) master.
+--
+-- This function updates the conflicted bases for all pull requests that come
+-- after the given PR and sets them to need feedback.
 unspeculateConflictsAfter :: PullRequest -> PullRequest -> PullRequest
 unspeculateConflictsAfter promotedPullRequest pr
   | Pr.PullRequest{ Pr.integrationStatus = Conflicted specBase reason
@@ -786,7 +791,12 @@ unspeculateConflictsAfter promotedPullRequest pr
   | otherwise
   = pr
 
--- TODO: describe unspeculateFailuresAfter
+-- | When a pull request has been promoted to master this means that any build
+-- failures build on top of it are not speculative anymore: they are real build
+-- failures on top of the (new) master.
+--
+-- This function simply sets them to be sent feedback again this time the build
+-- failure will be reported as a real definitive failure.
 unspeculateFailuresAfter :: PullRequest -> PullRequest -> PullRequest
 unspeculateFailuresAfter promotedPullRequest pr
   | Pr.PullRequest{Pr.integrationStatus = Integrated _ (BuildFailed _)} <- pr

From 6b0d8a932cef63643a58620ec483c2bb6c205d81 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Tue, 16 Aug 2022 11:55:08 +0200
Subject: [PATCH 41/54] Display build successes properly on the web iface

---
 src/WebInterface.hs | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/WebInterface.hs b/src/WebInterface.hs
index ff263dee..7582de75 100644
--- a/src/WebInterface.hs
+++ b/src/WebInterface.hs
@@ -167,13 +167,6 @@ viewProjectQueues info state = do
     -- TODO: Also render failure reason: conflicted or build failed.
     viewList viewPullRequestWithApproval info failed
 
-  -- TODO: Keep a list of the last n integrated pull requests, so they stay
-  -- around for a bit after they have been closed.
-  let integrated = filterPrs (== Project.PrStatusIntegrated)
-  unless (null integrated) $ do
-    h2 "Recently integrated"
-    viewList viewPullRequestWithApproval info integrated
-
   let awaitingApproval = reverse $ filterPrs (== Project.PrStatusAwaitingApproval)
   unless (null awaitingApproval) $ do
     h2 "Awaiting approval"
@@ -239,6 +232,7 @@ viewPullRequest info pullRequestId pullRequest = do
       case buildStatus of
         (BuildStarted ciUrl)       -> ciLink ciUrl "🟡"
         (BuildFailed (Just ciUrl)) -> ciLink ciUrl "❌"
+        BuildSucceeded             -> ciLink (commitUrl info sha) "✅"
         _                          -> pure ()
       a ! href (toValue $ commitUrl info sha) $ toHtml $ prettySha sha
       case buildStatus of
@@ -307,4 +301,10 @@ prFailed _                               = False
 prPending :: Project.PullRequestStatus -> Bool
 prPending Project.PrStatusBuildPending     = True
 prPending (Project.PrStatusBuildStarted _) = True
+-- PrStatusIntegrated here means that the PR successfully built
+-- but it has not been promoted to master yet for either of two reasons:
+-- 1. this is the split of a second between receiving the status and promoting;
+-- 2. this PR is not at the head of the merge train,
+--    we are waiting for the build status of the previous PR.
+prPending Project.PrStatusIntegrated       = True
 prPending _                                = False

From 4b21410abc5403f87ee0cda343ecd91dcf822104 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Wed, 17 Aug 2022 13:49:19 +0200
Subject: [PATCH 42/54] Add test with 4 PRs

https://github.com/channable/hoff/pull/137#discussion_r947671400
---
 tests/Spec.hs | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 114 insertions(+)

diff --git a/tests/Spec.hs b/tests/Spec.hs
index 378beda7..1ba57e89 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -2686,3 +2686,117 @@ main = hspec $ do
         , ATryPromote (Branch "sth") (Sha "3ef")
         , ACleanupTestBranch (PullRequestId 7)
         ]
+
+    it "handles a sequence of four successful merges: success, success, success, success" $ do
+      -- An afternoon of work on PRs:
+      -- * four PRs are merged and approved in order
+      -- * build always succeeds
+      -- * Hoff is notified of its own comments and of GH closing merged PRs
+      -- Hoff should process albeit ignore its own comments.
+      --
+      -- This serves to test and document a complete workflow.
+      let
+        state
+          = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")
+          $ Project.insertPullRequest (PullRequestId 2) (Branch "snd") masterBranch (Sha "cd2") "Second PR" (Username "rachael")
+          $ Project.insertPullRequest (PullRequestId 3) (Branch "trd") masterBranch (Sha "ef3") "Third PR"  (Username "rachael")
+          $ Project.insertPullRequest (PullRequestId 4) (Branch "fth") masterBranch (Sha "fe4") "Fourth PR" (Username "rachael")
+          $ Project.emptyProjectState
+        events =
+          [ BuildStatusChanged (Sha "ab1") (Project.BuildSucceeded) -- PR#1 sha, ignored
+          , CommentAdded (PullRequestId 1) "deckard" "@someone Thanks for your review."
+          , CommentAdded (PullRequestId 1) "deckard" "@bot merge"
+          , CommentAdded (PullRequestId 1) "bot" "Pull request approved for merge, rebasing now."
+          , CommentAdded (PullRequestId 1) "bot" "Rebased as 1ab, waiting for CI …"
+          , CommentAdded (PullRequestId 2) "deckard" "@bot merge"
+          , CommentAdded (PullRequestId 2) "bot" "Pull request approved for merge behind 1 PR."
+          , BuildStatusChanged (Sha "ef3") (Project.BuildSucceeded) -- PR#3 sha, ignored
+          , BuildStatusChanged (Sha "1ab") (Project.BuildPending) -- same status, ignored
+          , BuildStatusChanged (Sha "1ab") (Project.BuildStarted "example.com/1ab")
+          , BuildStatusChanged (Sha "1ab") (Project.BuildStarted "example.com/1ab") -- dup!
+          , CommentAdded (PullRequestId 1) "bot" "[CI job](example.com/1ab) started."
+          , CommentAdded (PullRequestId 3) "deckard" "@bot merge"
+          , CommentAdded (PullRequestId 3) "bot" "Pull request approved for merge behind 2 PRs."
+          , CommentAdded (PullRequestId 4) "deckard" "@bot merge"
+          , CommentAdded (PullRequestId 4) "bot" "Pull request approved for merge behind 3 PRs."
+          , BuildStatusChanged (Sha "cd2") (Project.BuildSucceeded) -- PR#2 sha, ignored
+          , BuildStatusChanged (Sha "1ab") (Project.BuildSucceeded) -- PR#1
+          , PullRequestClosed (PullRequestId 1)
+          , CommentAdded (PullRequestId 2) "bot" "Rebased as 2bc, waiting for CI …"
+          , BuildStatusChanged (Sha "2bc") (Project.BuildStarted "example.com/2bc")
+          , CommentAdded (PullRequestId 2) "bot" "[CI job](example.com/2bc) started."
+          , BuildStatusChanged (Sha "36a") (Project.BuildSucceeded) -- arbitrary sha, ignored
+          , BuildStatusChanged (Sha "2bc") (Project.BuildSucceeded) -- PR#2
+          , PullRequestClosed (PullRequestId 2)
+          , CommentAdded (PullRequestId 3) "bot" "Rebased as 3cd, waiting for CI …"
+          , BuildStatusChanged (Sha "3cd") (Project.BuildStarted "example.com/3cd")
+          , BuildStatusChanged (Sha "3cd") (Project.BuildSucceeded) -- PR#3
+          , PullRequestClosed (PullRequestId 3)
+          , BuildStatusChanged (Sha "4de") (Project.BuildStarted "example.com/4de")
+          , BuildStatusChanged (Sha "4de") (Project.BuildSucceeded) -- PR#4
+          , PullRequestClosed (PullRequestId 4)
+          ]
+        -- For this test, we assume all integrations and pushes succeed.
+        results = defaultResults { resultIntegrate = [ Right (Sha "1ab")
+                                                     , Right (Sha "2bc")
+                                                     , Right (Sha "3cd")
+                                                     , Right (Sha "4de") ] }
+        run = runActionCustom results
+        actions = snd $ run $ handleEventsTest events state
+      actions `shouldBe`
+        [ AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 1)
+                        "Pull request approved for merge by @deckard, rebasing now."
+        , ATryIntegrate "Merge #1: First PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "ab1")
+                        []
+                        False
+        , ALeaveComment (PullRequestId 1) "Rebased as 1ab, waiting for CI …"
+        , AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 2)
+                       "Pull request approved for merge by @deckard, \
+                       \waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        [PullRequestId 1]
+                        False
+        , ALeaveComment (PullRequestId 2) "Speculatively rebased as 2bc behind #1, waiting for CI …"
+        , ALeaveComment (PullRequestId 1) "[CI job](example.com/1ab) started."
+        , AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 3)
+                        "Pull request approved for merge by @deckard, \
+                        \waiting for rebase behind 2 pull requests."
+        , ATryIntegrate "Merge #3: Third PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 3, Branch "refs/pull/3/head", Sha "ef3")
+                        [PullRequestId 1, PullRequestId 2]
+                        False
+        , ALeaveComment (PullRequestId 3) "Speculatively rebased as 3cd behind #1 and #2, waiting for CI …"
+        , AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 4)
+                        "Pull request approved for merge by @deckard, \
+                        \waiting for rebase behind 3 pull requests."
+        , ATryIntegrate "Merge #4: Fourth PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 4, Branch "refs/pull/4/head", Sha "fe4")
+                        [PullRequestId 1, PullRequestId 2, PullRequestId 3]
+                        False
+        , ALeaveComment (PullRequestId 4) "Speculatively rebased as 4de behind #1, #2 and #3, waiting for CI …"
+        , ATryPromote (Branch "fst") (Sha "1ab")
+        , ACleanupTestBranch (PullRequestId 1)
+        , ALeaveComment (PullRequestId 2) "[CI job](example.com/2bc) started."
+        , ATryPromote (Branch "snd") (Sha "2bc")
+        , ACleanupTestBranch (PullRequestId 2)
+        , ALeaveComment (PullRequestId 3) "[CI job](example.com/3cd) started."
+        , ATryPromote (Branch "trd") (Sha "3cd")
+        , ACleanupTestBranch (PullRequestId 3)
+        , ALeaveComment (PullRequestId 4) "[CI job](example.com/4de) started."
+        , ATryPromote (Branch "fth") (Sha "4de")
+        , ACleanupTestBranch (PullRequestId 4)
+        ]

From 44e03ddacaa7a25759e5becf99f78425559f3717 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Wed, 17 Aug 2022 13:50:53 +0200
Subject: [PATCH 43/54] Remove uneeded comment

---
 tests/Spec.hs | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/Spec.hs b/tests/Spec.hs
index 1ba57e89..1af88c7d 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -1987,7 +1987,6 @@ main = hspec $ do
           , CommentAdded (PullRequestId 3) "deckard" "@bot merge"
           , BuildStatusChanged (Sha "1ab") (Project.BuildFailed (Just "ci.example.com/1ab"))
           ]
-        -- For this test, we assume all integrations and pushes succeed.
         results = defaultResults { resultIntegrate = [ Right (Sha "1ab")
                                                      , Left (IntegrationFailure (BaseBranch "testing/1") RebaseFailed)
                                                      , Right (Sha "3cd")

From a699fd28d687f1558c970724e4b92067a6304667 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Wed, 17 Aug 2022 13:55:40 +0200
Subject: [PATCH 44/54] refactor isUnfailingIntegrated

https://github.com/channable/hoff/pull/137#discussion_r947572784
---
 src/Project.hs | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/Project.hs b/src/Project.hs
index a2a894bf..3be8c182 100644
--- a/src/Project.hs
+++ b/src/Project.hs
@@ -463,11 +463,12 @@ isIntegrated (Integrated _ _) = True
 isIntegrated _                = False
 
 isUnfailingIntegrated :: IntegrationStatus -> Bool
-isUnfailingIntegrated (Integrated _ BuildPending)     = True
-isUnfailingIntegrated (Integrated _ (BuildStarted _)) = True
-isUnfailingIntegrated (Integrated _ BuildSucceeded)   = True
-isUnfailingIntegrated (Integrated _ (BuildFailed _))  = False
-isUnfailingIntegrated _                               = False
+isUnfailingIntegrated (Integrated _ buildStatus) = case buildStatus of
+                                                   BuildPending     -> True
+                                                   (BuildStarted _) -> True
+                                                   BuildSucceeded   -> True
+                                                   (BuildFailed _)  -> False
+isUnfailingIntegrated _ = False
 
 isIntegratedOrSpeculativelyConflicted :: PullRequest -> Bool
 isIntegratedOrSpeculativelyConflicted pr =

From 75d91765ae5e8fb20e2bad5637f0d357d5f61e26 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Wed, 17 Aug 2022 14:03:55 +0200
Subject: [PATCH 45/54] unintegrateAfter: document noop

---
 src/Logic.hs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index dfd3fd66..48845c89 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -561,7 +561,7 @@ handleMergeRequested projectConfig prId author state pr approvalType = do
 --   in the merge train as NotIntegrated
 unintegrateAfter :: PullRequestId -> ProjectState -> ProjectState
 unintegrateAfter pid state = case Pr.lookupPullRequest pid state of
-  Nothing -> state -- should not happen
+  Nothing -> state -- PR not found.  Keep the state as it is.
   Just pr -> unintegrateAfter' pr state
   where
   unintegrateAfter' :: PullRequest -> ProjectState -> ProjectState

From 6b60bd731829da061d9fce1bd120c5dc38bf7a69 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Wed, 17 Aug 2022 14:13:11 +0200
Subject: [PATCH 46/54] Report WrongFixups right away.

---
 src/Logic.hs   |  3 ++-
 src/Project.hs |  3 ++-
 tests/Spec.hs  | 42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index 48845c89..282a7b30 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -706,8 +706,9 @@ tryIntegratePullRequest pr state =
         -- If integrating failed, perform no further actions but do set the
         -- state to conflicted.
         -- If this is a speculative rebase, we wait before giving feedback.
+        -- For WrongFixups, we can report issues right away.
         pure $ Pr.setIntegrationStatus pr (Conflicted targetBranch reason) $
-          Pr.setNeedsFeedback pr (null train) state
+          Pr.setNeedsFeedback pr (null train || reason == WrongFixups) state
 
       Right (Sha sha) -> do
         -- If it succeeded, set the build to pending,
diff --git a/src/Project.hs b/src/Project.hs
index 3be8c182..1a75c6bf 100644
--- a/src/Project.hs
+++ b/src/Project.hs
@@ -301,8 +301,9 @@ classifyPullRequest pr = case approval pr of
   Just _  -> case integrationStatus pr of
     NotIntegrated -> PrStatusApproved
     IncorrectBaseBranch -> PrStatusIncorrectBaseBranch
-    Conflicted base _ | base /= baseBranch pr -> PrStatusSpeculativeConflict
+    -- Fixups can be reported regardless of whether we are doing an speculative rebase
     Conflicted _ WrongFixups -> PrStatusWrongFixups
+    Conflicted base _ | base /= baseBranch pr -> PrStatusSpeculativeConflict
     Conflicted _ EmptyRebase -> PrStatusEmptyRebase
     Conflicted _ _  -> PrStatusFailedConflict
     Integrated _ buildStatus -> case buildStatus of
diff --git a/tests/Spec.hs b/tests/Spec.hs
index 1af88c7d..5fb02f58 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -2049,6 +2049,48 @@ main = hspec $ do
         , ALeaveComment (PullRequestId 3) "Speculatively rebased as 6cd behind #2, waiting for CI …"
         ]
 
+    it "reports wrongfixups regardless of position in train (success, wrongfixups)" $ do
+      let
+        state
+          = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")
+          $ Project.insertPullRequest (PullRequestId 2) (Branch "snd") masterBranch (Sha "cd2") "Second PR" (Username "rachael")
+          $ Project.emptyProjectState
+        events =
+          [ CommentAdded (PullRequestId 1) "deckard" "@bot merge"
+          , CommentAdded (PullRequestId 2) "deckard" "@bot merge"
+          ]
+        results = defaultResults { resultIntegrate = [ Right (Sha "1ab")
+                                                     , Left (IntegrationFailure (BaseBranch "testing/1") WrongFixups)
+                                                     , Right (Sha "3cd") ] }
+        run = runActionCustom results
+        actions = snd $ run $ handleEventsTest events state
+      actions `shouldBe`
+        [ AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 1)
+                        "Pull request approved for merge by @deckard, rebasing now."
+        , ATryIntegrate "Merge #1: First PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 1, Branch "refs/pull/1/head", Sha "ab1")
+                        []
+                        False
+        , ALeaveComment (PullRequestId 1) "Rebased as 1ab, waiting for CI …"
+        , AIsReviewer "deckard"
+        , ALeaveComment (PullRequestId 2)
+                       "Pull request approved for merge by @deckard, \
+                       \waiting for rebase behind one pull request."
+        , ATryIntegrate "Merge #2: Second PR\n\n\
+                        \Approved-by: deckard\n\
+                        \Auto-deploy: false\n"
+                        (PullRequestId 2, Branch "refs/pull/2/head", Sha "cd2")
+                        [PullRequestId 1]
+                        False
+        , ALeaveComment (PullRequestId 2)
+                        "Pull request cannot be integrated\
+                        \ as it contains fixup commits that\
+                        \ do not belong to any other commits."
+        ]
+
     it "handles a 2-wagon merge train with build successes coming in the right order: success (1), success (2)" $ do
       let
         state

From 7886134ae056c4b68ac76be44de6daa63cdba3e6 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Wed, 17 Aug 2022 15:15:50 +0200
Subject: [PATCH 47/54] Fix grammar in message

---
 src/Logic.hs  | 2 +-
 tests/Spec.hs | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index 282a7b30..3b2fb7d8 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -865,7 +865,7 @@ describeStatus prId pr state = case Pr.classifyPullRequest pr of
           -- This should probably never happen
           Nothing   -> "The build failed, but GitHub did not provide an URL to the build failure."
     trainBefore -> format "Speculative build failed. \
-                          \ I will automatically retry after {} build results."
+                          \ I will automatically retry after getting build results for {}."
                           [prettyPullRequestIds trainBefore]
 
 -- Leave a comment with the feedback from 'describeStatus' and set the
diff --git a/tests/Spec.hs b/tests/Spec.hs
index 5fb02f58..fd2f924c 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -2273,7 +2273,7 @@ main = hspec $ do
                         [PullRequestId 1]
                         False
         , ALeaveComment (PullRequestId 2) "Speculatively rebased as 2cd behind #1, waiting for CI …"
-        , ALeaveComment (PullRequestId 2) "Speculative build failed.  I will automatically retry after #1 build results."
+        , ALeaveComment (PullRequestId 2) "Speculative build failed.  I will automatically retry after getting build results for #1."
         , ALeaveComment (PullRequestId 1) "The build failed, but GitHub did not provide an URL to the build failure."
         -- #2 is integrated again as its speculative base failed
         , ATryIntegrate "Merge #2: Second PR\n\n\
@@ -2367,7 +2367,7 @@ main = hspec $ do
                         [PullRequestId 1]
                         False
         , ALeaveComment (PullRequestId 2) "Speculatively rebased as 2cd behind #1, waiting for CI …"
-        , ALeaveComment (PullRequestId 2) "Speculative build failed.  I will automatically retry after #1 build results."
+        , ALeaveComment (PullRequestId 2) "Speculative build failed.  I will automatically retry after getting build results for #1."
         , ATryPromote (Branch "fst") (Sha "1ab")
         , ACleanupTestBranch (PullRequestId 1)
         , ALeaveComment (PullRequestId 2) "The build failed, but GitHub did not provide an URL to the build failure."

From 5751cb8e41f8ad719de9a76d1ddd0aec3a0c7980 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Wed, 17 Aug 2022 15:18:06 +0200
Subject: [PATCH 48/54] Add a comment about unused message

---
 src/Logic.hs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/Logic.hs b/src/Logic.hs
index 3b2fb7d8..523d0c0c 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -854,6 +854,9 @@ describeStatus prId pr state = case Pr.classifyPullRequest pr of
       , " "
       , prBranchName
       ]
+  -- The following is not actually shown to the user
+  -- as it is never set with needsFeedback=True,
+  -- but here in case we decide to show it.
   PrStatusSpeculativeConflict -> "Failed to speculatively rebase. \
                                  \ I will retry rebasing automatically when the queue clears."
   PrStatusFailedBuild url -> case Pr.unfailingIntegratedPullRequestsBefore pr state of

From 7c6391f93a68d8f478a7f7c97dc85798fdf866a6 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Wed, 17 Aug 2022 15:19:08 +0200
Subject: [PATCH 49/54] Fix grammar in comment

---
 src/Logic.hs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index 523d0c0c..d259e403 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -775,7 +775,7 @@ pushCandidate (pullRequestId, pullRequest) newHead state =
       PushRejected _why -> tryIntegratePullRequest pullRequestId state
 
 -- | When a pull request has been promoted to master this means that any
--- conflicts (failed rebases) build on top of it are not speculative anymore:
+-- conflicts (failed rebases) built on top of it are not speculative anymore:
 -- they are real conflicts on top of the (new) master.
 --
 -- This function updates the conflicted bases for all pull requests that come

From 55d236556857f0f8903e2ed03ed68a933f1feb2f Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Wed, 17 Aug 2022 15:22:57 +0200
Subject: [PATCH 50/54] test/Spec: update test title

---
 tests/Spec.hs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Spec.hs b/tests/Spec.hs
index fd2f924c..f106fa21 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -1843,7 +1843,7 @@ main = hspec $ do
         , ACleanupTestBranch (PullRequestId 12)
         ]
 
-    it "after the PR commit has changed, resets the integration of PRs in the train" $ do
+    it "resets the integration of PRs in the train after the PR commit has changed" $ do
       let
         state
           = Project.insertPullRequest (PullRequestId 1) (Branch "fst") masterBranch (Sha "ab1") "First PR"  (Username "tyrell")

From 70306a558daa218955004323f60755f57e6c6ab1 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Wed, 17 Aug 2022 15:28:23 +0200
Subject: [PATCH 51/54] rename unfailing to unfailed

... as this name is more accurate

https://github.com/channable/hoff/pull/137#discussion_r947895585
---
 src/Logic.hs           | 10 +++++-----
 src/Project.hs         | 12 ++++++------
 tests/EventLoopSpec.hs | 14 +++++++-------
 tests/Spec.hs          | 14 +++++++-------
 4 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index d259e403..1a3cf429 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -380,7 +380,7 @@ handlePullRequestClosedByUser = handlePullRequestClosed User
 
 handlePullRequestClosed :: PRCloseCause -> PullRequestId -> ProjectState -> Action ProjectState
 handlePullRequestClosed closingReason pr state = do
-  when (pr `elem` Pr.unfailingIntegratedPullRequests state) $
+  when (pr `elem` Pr.unfailedIntegratedPullRequests state) $
     leaveComment pr $ prClosingMessage closingReason
   -- actually delete the pull request
   pure . Pr.deletePullRequest pr
@@ -654,7 +654,7 @@ proceed = provideFeedback
       >=> tryIntegrateSomePullRequest
 
 proceedSomeCandidate :: ProjectState -> Action ProjectState
-proceedSomeCandidate state = case Pr.unfailingIntegratedPullRequests state of
+proceedSomeCandidate state = case Pr.unfailedIntegratedPullRequests state of
   (candidate:_) -> proceedCandidate candidate state
   _ -> pure state
 
@@ -698,7 +698,7 @@ tryIntegratePullRequest pr state =
       ]
     mergeMessage = Text.unlines mergeMessageLines
     -- the takeWhile here is needed in case of reintegrations after failing pushes
-    train = takeWhile (/= pr) $ Pr.unfailingIntegratedPullRequests state
+    train = takeWhile (/= pr) $ Pr.unfailedIntegratedPullRequests state
   in do
     result <- tryIntegrate mergeMessage candidate train $ Pr.alwaysAddMergeCommit approvalType
     case result of
@@ -829,7 +829,7 @@ describeStatus prId pr state = case Pr.classifyPullRequest pr of
       1 -> format "Pull request approved for {} by @{}, waiting for rebase behind one pull request." [approvalCommand, approvedBy]
       n -> format "Pull request approved for {} by @{}, waiting for rebase behind {} pull requests." (approvalCommand, approvedBy, n)
   PrStatusBuildPending -> let Sha sha = fromJust $ Pr.integrationSha pr
-                              train = takeWhile (/= prId) $ Pr.unfailingIntegratedPullRequests state
+                              train = takeWhile (/= prId) $ Pr.unfailedIntegratedPullRequests state
                           in case train of
                              []    -> Text.concat ["Rebased as ", sha, ", waiting for CI …"]
                              (_:_) -> Text.concat [ "Speculatively rebased as ", sha
@@ -859,7 +859,7 @@ describeStatus prId pr state = case Pr.classifyPullRequest pr of
   -- but here in case we decide to show it.
   PrStatusSpeculativeConflict -> "Failed to speculatively rebase. \
                                  \ I will retry rebasing automatically when the queue clears."
-  PrStatusFailedBuild url -> case Pr.unfailingIntegratedPullRequestsBefore pr state of
+  PrStatusFailedBuild url -> case Pr.unfailedIntegratedPullRequestsBefore pr state of
     [] -> case url of
           Just url' -> format "The build failed: {}\n\
                               \If this is the result of a flaky test, \
diff --git a/src/Project.hs b/src/Project.hs
index 1a75c6bf..b2559fdb 100644
--- a/src/Project.hs
+++ b/src/Project.hs
@@ -22,8 +22,8 @@ module Project
   Owner,
   approvedPullRequests,
   integratedPullRequests,
-  unfailingIntegratedPullRequests,
-  unfailingIntegratedPullRequestsBefore,
+  unfailedIntegratedPullRequests,
+  unfailedIntegratedPullRequestsBefore,
   candidatePullRequests,
   classifyPullRequest,
   classifyPullRequests,
@@ -394,11 +394,11 @@ wasIntegrationAttemptFor commit pr = case integrationStatus pr of
 integratedPullRequests :: ProjectState -> [PullRequestId]
 integratedPullRequests = filterPullRequestsBy $ isIntegrated . integrationStatus
 
-unfailingIntegratedPullRequests :: ProjectState -> [PullRequestId]
-unfailingIntegratedPullRequests = filterPullRequestsBy $ isUnfailingIntegrated . integrationStatus
+unfailedIntegratedPullRequests :: ProjectState -> [PullRequestId]
+unfailedIntegratedPullRequests = filterPullRequestsBy $ isUnfailingIntegrated . integrationStatus
 
-unfailingIntegratedPullRequestsBefore :: PullRequest -> ProjectState -> [PullRequestId]
-unfailingIntegratedPullRequestsBefore referencePullRequest = filterPullRequestsBy $
+unfailedIntegratedPullRequestsBefore :: PullRequest -> ProjectState -> [PullRequestId]
+unfailedIntegratedPullRequestsBefore referencePullRequest = filterPullRequestsBy $
   \pr -> isUnfailingIntegrated (integrationStatus pr)
       && referencePullRequest `approvedAfter` pr
 
diff --git a/tests/EventLoopSpec.hs b/tests/EventLoopSpec.hs
index ab7cf243..45b34796 100644
--- a/tests/EventLoopSpec.hs
+++ b/tests/EventLoopSpec.hs
@@ -378,7 +378,7 @@ withTestEnv' body = do
 -- | lists the integration Shas from the state for all PRs which are Integrated
 integrationShas :: ProjectState -> [Sha]
 integrationShas state = [ sha
-                        | prId <- Project.unfailingIntegratedPullRequests state
+                        | prId <- Project.unfailedIntegratedPullRequests state
                         , Just pr <- [Project.lookupPullRequest prId state]
                         , Integrated sha _ <- [Project.integrationStatus pr]
                         ]
@@ -853,7 +853,7 @@ eventLoopSpec = parallel $ do
 
         -- The second pull request should still be pending, awaiting the build
         -- result.
-        Project.unfailingIntegratedPullRequests state `shouldBe` [pr4]
+        Project.unfailedIntegratedPullRequests state `shouldBe` [pr4]
         let Just pullRequest4 = Project.lookupPullRequest pr4 state
             Integrated _ buildStatus = Project.integrationStatus pullRequest4
         -- Expect no CI url
@@ -898,7 +898,7 @@ eventLoopSpec = parallel $ do
 
         -- The push should have failed, hence there should still be an
         -- integration candidate.
-        Project.unfailingIntegratedPullRequests state' `shouldSatisfy` (not . null)
+        Project.unfailedIntegratedPullRequests state' `shouldSatisfy` (not . null)
 
         -- Again notify build success, now for the new commit.
         let [rebasedSha'] = integrationShas state'
@@ -906,7 +906,7 @@ eventLoopSpec = parallel $ do
 
         -- After the second build success, the pull request should have been
         -- integrated properly, so there should not be a new candidate.
-        Project.unfailingIntegratedPullRequests state'' `shouldBe` []
+        Project.unfailedIntegratedPullRequests state'' `shouldBe` []
 
       history `shouldBe`
         [ "*   Merge #6"
@@ -1181,7 +1181,7 @@ eventLoopSpec = parallel $ do
         --The pull request should not be integrated. Moreover, the presence of
         --orphan fixups should make the PR ineligible for being a candidate for integration.
         --That is, we expect no candidates for integration.
-        Project.unfailingIntegratedPullRequests state' `shouldBe` []
+        Project.unfailedIntegratedPullRequests state' `shouldBe` []
 
       -- Here we expect that the fixup commit is not present.
       history `shouldBe`
@@ -1210,7 +1210,7 @@ eventLoopSpec = parallel $ do
             Logic.CommentAdded pr8 "rachael" "@bot merge"
           ]
 
-        Project.unfailingIntegratedPullRequests state `shouldBe` [pr8]
+        Project.unfailedIntegratedPullRequests state `shouldBe` [pr8]
 
         let [rebasedSha] = integrationShas state
 
@@ -1222,7 +1222,7 @@ eventLoopSpec = parallel $ do
             Logic.CommentAdded pr6 "rachael" "@bot merge"
           ]
 
-        Project.unfailingIntegratedPullRequests state' `shouldBe` []
+        Project.unfailedIntegratedPullRequests state' `shouldBe` []
 
         let Just pullRequest' = Project.lookupPullRequest pr6 state'
         Project.integrationStatus pullRequest' `shouldBe`
diff --git a/tests/Spec.hs b/tests/Spec.hs
index f106fa21..bac6a4f9 100644
--- a/tests/Spec.hs
+++ b/tests/Spec.hs
@@ -250,11 +250,11 @@ handleEventTest = Logic.handleEvent testTriggerConfig testProjectConfig testmerg
 handleEventsTest :: [Event] -> ProjectState -> Action ProjectState
 handleEventsTest events state = foldlM (flip $ Logic.handleEvent testTriggerConfig testProjectConfig testmergeWindowExemptionConfig) state events
 
--- Same as 'unfailingIntegratedPullRequests' but paired with the underlying objects.
+-- Same as 'unfailedIntegratedPullRequests' but paired with the underlying objects.
 getIntegrationCandidates :: ProjectState -> [(PullRequestId, PullRequest)]
 getIntegrationCandidates state =
   [ (pullRequestId, candidate)
-  | pullRequestId <- Project.unfailingIntegratedPullRequests state
+  | pullRequestId <- Project.unfailedIntegratedPullRequests state
   , Just candidate <- [Project.lookupPullRequest pullRequestId state]
   ]
 
@@ -292,13 +292,13 @@ main = hspec $ do
       let event  = PullRequestClosed (PullRequestId 1)
           state  = candidateState (PullRequestId 1) (Branch "p") masterBranch (Sha "ea0") "frank" "deckard" (Sha "cf4")
           state' = fst $ runAction $ handleEventTest event state
-      Project.unfailingIntegratedPullRequests state' `shouldBe` []
+      Project.unfailedIntegratedPullRequests state' `shouldBe` []
 
     it "does not modify the integration candidate if a different PR was closed" $ do
       let event  = PullRequestClosed (PullRequestId 1)
           state  = candidateState (PullRequestId 2) (Branch "p") masterBranch (Sha "a38") "franz" "deckard" (Sha "ed0")
           state' = fst $ runAction $ handleEventTest event state
-      Project.unfailingIntegratedPullRequests state' `shouldBe` [PullRequestId 2]
+      Project.unfailedIntegratedPullRequests state' `shouldBe` [PullRequestId 2]
 
     it "loses approval after the PR commit has changed" $ do
       let event  = PullRequestCommitChanged (PullRequestId 1) (Sha "def")
@@ -627,7 +627,7 @@ main = hspec $ do
       -- The first pull request should be dropped, and a comment should be
       -- left indicating why. Then the second pull request should be at the
       -- front of the queue.
-      Project.unfailingIntegratedPullRequests state' `shouldBe` [PullRequestId 2]
+      Project.unfailedIntegratedPullRequests state' `shouldBe` [PullRequestId 2]
       actions `shouldBe`
         [ AIsReviewer "deckard"
         , ALeaveComment (PullRequestId 1) "Pull request approved for merge by @deckard, rebasing now."
@@ -1071,7 +1071,7 @@ main = hspec $ do
         ]
 
       Project.approval pr `shouldBe` Nothing
-      Project.unfailingIntegratedPullRequests state' `shouldBe` []
+      Project.unfailedIntegratedPullRequests state' `shouldBe` []
 
     it "shows an appropriate message when the commit is changed on an approved PR" $ do
       let
@@ -1097,7 +1097,7 @@ main = hspec $ do
         ]
 
       Project.approval pr `shouldBe` Nothing
-      Project.unfailingIntegratedPullRequests state' `shouldBe` []
+      Project.unfailedIntegratedPullRequests state' `shouldBe` []
 
 
   describe "Logic.proceedUntilFixedPoint" $ do

From 550c07d4d178cd405b7de4ec1d28e69c5ef70359 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Wed, 17 Aug 2022 15:33:17 +0200
Subject: [PATCH 52/54] use "First" instead of "Some"

---
 src/Logic.hs | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/Logic.hs b/src/Logic.hs
index 1a3cf429..94e1d5f9 100644
--- a/src/Logic.hs
+++ b/src/Logic.hs
@@ -650,16 +650,18 @@ synchronizeState stateInitial =
 -- in progress is closed, we should find a new candidate.
 proceed :: ProjectState -> Action ProjectState
 proceed = provideFeedback
-      >=> proceedSomeCandidate
-      >=> tryIntegrateSomePullRequest
+      >=> proceedFirstCandidate
+      >=> tryIntegrateFirstPullRequest
 
-proceedSomeCandidate :: ProjectState -> Action ProjectState
-proceedSomeCandidate state = case Pr.unfailedIntegratedPullRequests state of
+-- proceeds with the candidate that was approved first
+proceedFirstCandidate :: ProjectState -> Action ProjectState
+proceedFirstCandidate state = case Pr.unfailedIntegratedPullRequests state of
   (candidate:_) -> proceedCandidate candidate state
   _ -> pure state
 
-tryIntegrateSomePullRequest :: ProjectState -> Action ProjectState
-tryIntegrateSomePullRequest state = case Pr.candidatePullRequests state of
+-- try to integrate the pull request that was approved first
+tryIntegrateFirstPullRequest :: ProjectState -> Action ProjectState
+tryIntegrateFirstPullRequest state = case Pr.candidatePullRequests state of
   (pr:_) -> tryIntegratePullRequest pr state
   _ -> pure state
 

From fe85220bd5abf6d7d1a4cf13b3ced88eb590fee6 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Wed, 17 Aug 2022 16:19:05 +0200
Subject: [PATCH 53/54] isUnfailing -> isUnfailed

---
 src/Project.hs | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/Project.hs b/src/Project.hs
index b2559fdb..bdadb3ba 100644
--- a/src/Project.hs
+++ b/src/Project.hs
@@ -395,11 +395,11 @@ integratedPullRequests :: ProjectState -> [PullRequestId]
 integratedPullRequests = filterPullRequestsBy $ isIntegrated . integrationStatus
 
 unfailedIntegratedPullRequests :: ProjectState -> [PullRequestId]
-unfailedIntegratedPullRequests = filterPullRequestsBy $ isUnfailingIntegrated . integrationStatus
+unfailedIntegratedPullRequests = filterPullRequestsBy $ isUnfailedIntegrated . integrationStatus
 
 unfailedIntegratedPullRequestsBefore :: PullRequest -> ProjectState -> [PullRequestId]
 unfailedIntegratedPullRequestsBefore referencePullRequest = filterPullRequestsBy $
-  \pr -> isUnfailingIntegrated (integrationStatus pr)
+  \pr -> isUnfailedIntegrated (integrationStatus pr)
       && referencePullRequest `approvedAfter` pr
 
 -- Returns the pull requests that have not been integrated yet, in order of
@@ -463,13 +463,13 @@ isIntegrated :: IntegrationStatus -> Bool
 isIntegrated (Integrated _ _) = True
 isIntegrated _                = False
 
-isUnfailingIntegrated :: IntegrationStatus -> Bool
-isUnfailingIntegrated (Integrated _ buildStatus) = case buildStatus of
-                                                   BuildPending     -> True
-                                                   (BuildStarted _) -> True
-                                                   BuildSucceeded   -> True
-                                                   (BuildFailed _)  -> False
-isUnfailingIntegrated _ = False
+isUnfailedIntegrated :: IntegrationStatus -> Bool
+isUnfailedIntegrated (Integrated _ buildStatus) = case buildStatus of
+                                                  BuildPending     -> True
+                                                  (BuildStarted _) -> True
+                                                  BuildSucceeded   -> True
+                                                  (BuildFailed _)  -> False
+isUnfailedIntegrated _ = False
 
 isIntegratedOrSpeculativelyConflicted :: PullRequest -> Bool
 isIntegratedOrSpeculativelyConflicted pr =

From 6ce0638308580f56d5428a652edc221159c3c673 Mon Sep 17 00:00:00 2001
From: Rudy Matela <rudy.matela@channable.com>
Date: Wed, 17 Aug 2022 16:19:12 +0200
Subject: [PATCH 54/54] split of a second -> split-second

---
 src/WebInterface.hs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/WebInterface.hs b/src/WebInterface.hs
index 7582de75..5db6c3dd 100644
--- a/src/WebInterface.hs
+++ b/src/WebInterface.hs
@@ -303,7 +303,7 @@ prPending Project.PrStatusBuildPending     = True
 prPending (Project.PrStatusBuildStarted _) = True
 -- PrStatusIntegrated here means that the PR successfully built
 -- but it has not been promoted to master yet for either of two reasons:
--- 1. this is the split of a second between receiving the status and promoting;
+-- 1. this is the split-second between receiving the status and promoting;
 -- 2. this PR is not at the head of the merge train,
 --    we are waiting for the build status of the previous PR.
 prPending Project.PrStatusIntegrated       = True