Notify users about duplicate jobs

app-e2e/src/Test/E2E/GitHubIssue.purs

@@ -40,6 +40,19 @@ spec = do
       assertNoComment "Job completed successfully" requests
       assertOpen requests
 
+    Spec.it "links to existing logs without polling a duplicate job" do
+      let event = mkPublishEvent Fixtures.effectPublishData
+      _ <- runWorkflow event
+      WireMock.clearGithubRequests
+
+      requests <- runWorkflow event
+      assertComment "Duplicate job" requests
+      assertComment "Logs:" requests
+      assertNoComment "Job started" requests
+      assertNoComment "Job completed successfully" requests
+      assertNoComment "Job failed" requests
+      assertOpen requests
+
     Spec.it "calls Teams API to verify trustee membership for authenticated operation" do
       requests <- runWorkflow $ mkAuthenticatedEvent packagingTeamUser Fixtures.trusteeAuthenticatedData
       assertComment "Job started" requests

app/src/App/GitHubIssue.purs

@@ -114,22 +114,39 @@ runGitHubIssue env = do
     submitResult <- Run.liftAff $ submitJob (registryApiUrl <> endpoint) jsonBody
     case submitResult of
       Left err -> Except.throw $ "Failed to submit job: " <> err
-      Right { jobId } -> do
+      Right (DuplicateJob { jobId }) -> do
         let jobIdStr = unwrap jobId
+        let logsUrl = registryApiUrl <> "/v1/jobs/" <> jobIdStr
+        Log.debug $ "Duplicate job: " <> jobIdStr
+        Run.liftAff $ void $ Octokit.request env.octokit $ Octokit.createCommentRequest
+          { address: Constants.registry
+          , issue: env.issue
+          , body: "Duplicate job: `" <> jobIdStr <> "`\nLogs: " <> logsUrl
+          }
+        pure false
+      Right (CreatedJob { jobId }) -> do
+        let jobIdStr = unwrap jobId
+        let logsUrl = registryApiUrl <> "/v1/jobs/" <> jobIdStr
         Log.debug $ "Job created: " <> jobIdStr
 
         -- Post initial comment with job ID
         Run.liftAff $ void $ Octokit.request env.octokit $ Octokit.createCommentRequest
           { address: Constants.registry
           , issue: env.issue
-          , body: "Job started: `" <> jobIdStr <> "`\nLogs: " <> registryApiUrl <> "/v1/jobs/" <> jobIdStr
+          , body: "Job started: `" <> jobIdStr <> "`\nLogs: " <> logsUrl
           }
 
         -- Poll for completion, posting logs as comments
         pollAndReport env.octokit env.issue env.pollConfig registryApiUrl jobId
 
+-- | The V1 response body is unchanged; the HTTP status distinguishes whether
+-- | the server created a job (201) or returned an existing duplicate (200).
+data JobSubmissionResult
+  = CreatedJob V1.JobCreatedResponse
+  | DuplicateJob V1.JobCreatedResponse
+
 -- | Submit a job to the registry API
-submitJob :: String -> String -> Aff (Either String V1.JobCreatedResponse)
+submitJob :: String -> String -> Aff (Either String JobSubmissionResult)
 submitJob url body = do
   result <- Aff.attempt $ Fetch.fetch url
     { method: POST
@@ -140,10 +157,12 @@ submitJob url body = do
     Left err -> pure $ Left $ "Network error: " <> Aff.message err
     Right response -> do
       responseBody <- response.text
-      if response.status >= 200 && response.status < 300 then
+      if response.status == 200 || response.status == 201 then
         case JSON.parse responseBody >>= \json -> lmap CJ.DecodeError.print (CJ.decode V1.jobCreatedResponseCodec json) of
           Left err -> pure $ Left $ "Failed to parse response: " <> err
-          Right r -> pure $ Right r
+          Right jobResponse
+            | response.status == 200 -> pure $ Right $ DuplicateJob jobResponse
+            | otherwise -> pure $ Right $ CreatedJob jobResponse
       else
         pure $ Left $ "HTTP " <> show response.status <> ": " <> responseBody
 

app/src/App/Server/Env.purs

@@ -8,6 +8,7 @@ import Data.String as String
 import Effect.Aff as Aff
 import HTTPurple (JsonDecoder(..), JsonEncoder(..), Request, Response)
 import HTTPurple as HTTPurple
+import HTTPurple.Status as Status
 import Node.Path as Path
 import Registry.API.V1 (JobId, Route)
 import Registry.App.API (COMPILER_CACHE, PURS_GRAPH_CACHE, _compilerCache, _pursGraphCache)
@@ -134,6 +135,9 @@ jsonEncoder codec = JsonEncoder (stringifyJson codec)
 jsonOk :: forall m a. MonadAff m => CJ.Codec a -> a -> m Response
 jsonOk codec datum = HTTPurple.ok' HTTPurple.jsonHeaders $ HTTPurple.toJson (jsonEncoder codec) datum
 
+jsonCreated :: forall m a. MonadAff m => CJ.Codec a -> a -> m Response
+jsonCreated codec datum = HTTPurple.response' Status.created HTTPurple.jsonHeaders $ HTTPurple.toJson (jsonEncoder codec) datum
+
 runEffects :: forall a. ServerEnv -> Run ServerEffects a -> Aff (Either Aff.Error a)
 runEffects env operation = Aff.attempt do
   today <- nowUTC

app/src/App/Server/Router.purs

@@ -19,7 +19,7 @@ import Registry.App.Auth as Auth
 import Registry.App.Effect.Db as Db
 import Registry.App.Effect.Env as Env
 import Registry.App.Effect.Log as Log
-import Registry.App.Server.Env (ServerEffects, ServerEnv, jsonDecoder, jsonOk, runEffects)
+import Registry.App.Server.Env (ServerEffects, ServerEnv, jsonCreated, jsonDecoder, jsonOk, runEffects)
 import Registry.Operation (PackageSetOperation(..))
 import Registry.Operation as Operation
 import Run (Run)
@@ -80,33 +80,31 @@ router { route, method, body } = HTTPurple.usingCont case route, method of
     publish <- HTTPurple.fromJson (jsonDecoder Operation.publishCodec) body
     lift $ Log.info $ "Received Publish request: " <> printJson Operation.publishCodec publish
 
-    jobId <- lift (Db.selectPublishJob publish.name publish.version) >>= case _ of
+    lift (Db.selectPublishJob publish.name publish.version) >>= case _ of
       Just job -> do
         lift $ Log.warn $ "Duplicate publish job insertion, returning existing one: " <> unwrap job.jobId
-        pure job.jobId
+        jsonOk V1.jobCreatedResponseCodec { jobId: job.jobId }
       Nothing -> do
-        lift $ Db.insertPublishJob { payload: publish }
-
-    jsonOk V1.jobCreatedResponseCodec { jobId }
+        jobId <- lift $ Db.insertPublishJob { payload: publish }
+        jsonCreated V1.jobCreatedResponseCodec { jobId }
 
   Unpublish, Post -> do
     auth <- HTTPurple.fromJson (jsonDecoder Operation.authenticatedCodec) body
     case auth.payload of
       Operation.Unpublish payload -> do
         lift $ Log.info $ "Received Unpublish request: " <> printJson Operation.unpublishCodec payload
 
-        jobId <- lift (Db.selectUnpublishJob payload.name payload.version) >>= case _ of
+        lift (Db.selectUnpublishJob payload.name payload.version) >>= case _ of
           Just job -> do
             lift $ Log.warn $ "Duplicate unpublish job insertion, returning existing one: " <> unwrap job.jobId
-            pure job.jobId
+            jsonOk V1.jobCreatedResponseCodec { jobId: job.jobId }
           Nothing -> do
-            lift $ Db.insertUnpublishJob
+            jobId <- lift $ Db.insertUnpublishJob
               { payload: payload
               , rawPayload: auth.rawPayload
               , signature: auth.signature
               }
-
-        jsonOk V1.jobCreatedResponseCodec { jobId }
+            jsonCreated V1.jobCreatedResponseCodec { jobId }
       _ ->
         HTTPurple.badRequest "Expected unpublish operation."
 
@@ -116,18 +114,17 @@ router { route, method, body } = HTTPurple.usingCont case route, method of
       Operation.Transfer payload -> do
         lift $ Log.info $ "Received Transfer request: " <> printJson Operation.transferCodec payload
 
-        jobId <- lift (Db.selectTransferJob payload.name) >>= case _ of
+        lift (Db.selectTransferJob payload.name) >>= case _ of
           Just job -> do
             lift $ Log.warn $ "Duplicate transfer job insertion, returning existing one: " <> unwrap job.jobId
-            pure job.jobId
+            jsonOk V1.jobCreatedResponseCodec { jobId: job.jobId }
           Nothing -> do
-            lift $ Db.insertTransferJob
+            jobId <- lift $ Db.insertTransferJob
               { payload: payload
               , rawPayload: auth.rawPayload
               , signature: auth.signature
               }
-
-        jsonOk V1.jobCreatedResponseCodec { jobId }
+            jsonCreated V1.jobCreatedResponseCodec { jobId }
       _ ->
         HTTPurple.badRequest "Expected transfer operation."
 
@@ -190,18 +187,17 @@ router { route, method, body } = HTTPurple.usingCont case route, method of
           lift $ Log.info "Package set authentication successful."
 
         -- Check for duplicate pending job with the same payload
-        jobId <- lift (Db.selectPackageSetJobByPayload request.payload) >>= case _ of
+        lift (Db.selectPackageSetJobByPayload request.payload) >>= case _ of
           Just job -> do
             lift $ Log.warn $ "Duplicate package set job insertion, returning existing one: " <> unwrap job.jobId
-            pure job.jobId
+            jsonOk V1.jobCreatedResponseCodec { jobId: job.jobId }
           Nothing -> do
-            lift $ Db.insertPackageSetJob
+            jobId <- lift $ Db.insertPackageSetJob
               { payload: request.payload
               , rawPayload: request.rawPayload
               , signature: request.signature
               }
-
-        jsonOk V1.jobCreatedResponseCodec { jobId }
+            jsonCreated V1.jobCreatedResponseCodec { jobId }
 
   Status, Get ->
     HTTPurple.emptyResponse Status.ok