Make publish job handling retry-safe

SPEC.md

@@ -118,10 +118,17 @@ The registry responds immediately with a job ID that can be polled for status:
 
 ```json
 {
-  "jobId": "01234567-89ab-cdef-0123-456789abcdef"
+  "jobId": "01234567-89ab-cdef-0123-456789abcdef",
+  "disposition": "created"
 }
 ```
 
+The publish response disposition is `created` for a new job,
+`duplicate-active` when an unfinished job already owns the package version, or
+`already-published` when an equivalent successful publish job exists. New jobs
+return HTTP 201, while `duplicate-active` and `already-published` responses
+return HTTP 200.
+
 #### Step 2: Source Fetched and Validated
 
 The registry fetches the package source from the specified location at the given ref. It expects to find a supported manifest format—currently `purs.json` or `spago.yaml`, though additional formats may be supported in the future. For example, a `purs.json` manifest:
@@ -620,10 +627,12 @@ Next, the registry will use the provided [`Location`](#location) to fetch the pa
 
 1. The manifest MUST be usable to produce a well-formed [`Manifest`](#33-manifest).
 2. The manifest package name and JSON payload package name MUST match.
-3. The manifest location and JSON payload location (if provided) MUST match.
-4. The manifest version MUST NOT have been published or unpublished before, according to the metadata file.
-5. Either a `resolutions` key was provided in the JSON payload, in which case the resolutions MUST include every dependency listed in the manifest dependencies at a version within the specified version bounds for each dependency, OR the manifest dependencies MUST be solvable by the registry solver. ALL package versions indicated in the resolutions MUST already be registered.
-6. If the package source code includes a LICENSE file and/or a license listed in a bower.json or package.json manifest, then all licenses MUST match with the license specified in the PureScript [`Manifest`](#33-manifest). For example, if the package.json file specifies the "MIT" license and a LICENSE file specifies "BSD-3-Clause", then the PureScript license should admit at least "MIT AND BSD-3-Clause".
+3. The manifest version and JSON payload version MUST match.
+4. The manifest ref and JSON payload ref MUST match.
+5. The manifest location MUST match the authoritative registry metadata location (and therefore the JSON payload location, when provided).
+6. The manifest version MUST NOT have been unpublished before, according to the metadata file. A version already present in metadata, the manifest index, storage, and Pursuit is treated as an idempotent success; incomplete prior publications are reconciled.
+7. Either a `resolutions` key was provided in the JSON payload, in which case the resolutions MUST include every dependency listed in the manifest dependencies at a version within the specified version bounds for each dependency, OR the manifest dependencies MUST be solvable by the registry solver. ALL package versions indicated in the resolutions MUST already be registered.
+8. If the package source code includes a LICENSE file and/or a license listed in a bower.json or package.json manifest, then all licenses MUST match with the license specified in the PureScript [`Manifest`](#33-manifest). For example, if the package.json file specifies the "MIT" license and a LICENSE file specifies "BSD-3-Clause", then the PureScript license should admit at least "MIT AND BSD-3-Clause".
 
 Next, the registry will verify the package source code.
 
@@ -988,12 +997,22 @@ POST to `/api/v1/publish` with a JSON body matching the [`PublishData`](#51-publ
 }
 ```
 
-The response contains a job ID:
+The response contains a job ID and a publish submission disposition:
 
 ```json
-{ "jobId": "01234567-89ab-cdef-0123-456789abcdef" }
+{
+  "jobId": "01234567-89ab-cdef-0123-456789abcdef",
+  "disposition": "created"
+}
 ```
 
+`disposition` is one of `created`, `duplicate-active`, or
+`already-published`. New jobs return HTTP 201; `duplicate-active` and
+`already-published` responses return HTTP 200. An active job owns its package
+name and version even if its payload differs, because registry package versions
+are immutable; clients can inspect the returned job's `payload` when handling
+`duplicate-active`.
+
 #### Polling Job Status
 
 GET `/api/v1/jobs/{jobId}` with optional query parameters:
@@ -1011,6 +1030,7 @@ The response contains job status and logs:
   "startedAt": "2024-01-15T10:30:01.0Z",
   "finishedAt": "2024-01-15T10:31:00.0Z",
   "success": true,
+  "disposition": "published",
   "logs": [
     {
       "level": "INFO",
@@ -1025,7 +1045,14 @@ The response contains job status and logs:
 }
 ```
 
-Poll until `finishedAt` is present, then check `success` to determine the outcome. The `since` parameter can be used for pagination, and to stream logs to the user.
+Poll until `finishedAt` is present, then check `success` to determine the
+outcome. Publish jobs additionally include a terminal `disposition` of
+`published` or `already-published` on success. On failure they instead include
+an `error` object such as
+`{ "code": "job-failed", "message": "..." }` (or code `job-timeout`), so
+clients do not need to parse logs. Neither field is present while the job is
+unfinished. The `since` parameter can be used for pagination, and to stream
+logs to the user.
 
 #### Authenticated Operations
 

app-e2e/src/Test/E2E/Endpoint/Publish.purs

@@ -2,11 +2,13 @@ module Test.E2E.Endpoint.Publish (spec) where
 
 import Registry.App.Prelude
 
+import Control.Monad.Reader (ask, runReaderT)
 import Data.Array as Array
 import Data.Array.NonEmpty as NEA
 import Data.Map as Map
 import Data.Set as Set
 import Data.String as String
+import Effect.Aff as Aff
 import Registry.API.V1 (Job(..))
 import Registry.API.V1 as V1
 import Registry.Manifest (Manifest(..))
@@ -25,9 +27,15 @@ spec :: E2ESpec
 spec = do
   Spec.describe "Publish workflow" do
     Spec.it "can publish effect@4.0.0 and verify all state changes" do
-      { jobId } <- Client.publish Fixtures.effectPublishData
+      { jobId, disposition: submissionDisposition } <- Client.publish Fixtures.effectPublishData
+      Assert.shouldEqual submissionDisposition (Just V1.Created)
       job <- Env.pollJobOrFail jobId
       Assert.shouldSatisfy (V1.jobInfo job).finishedAt isJust
+      case job of
+        PublishJob details -> do
+          Assert.shouldEqual details.disposition (Just V1.Published)
+          Assert.shouldEqual details.error Nothing
+        _ -> Assert.fail "Expected a publish job."
 
       uploadOccurred <- Env.hasStorageUpload Fixtures.effect
       unless uploadOccurred do
@@ -91,8 +99,38 @@ spec = do
         Assert.fail $ "Expected version " <> Version.print Fixtures.slug.version <> " in manifest index"
 
   Spec.describe "Publish state machine" do
-    Spec.it "returns same jobId for duplicate publish requests" do
-      { jobId: id1 } <- Client.publish Fixtures.effectPublishData
-      _ <- Env.pollJobOrFail id1
-      { jobId: id2 } <- Client.publish Fixtures.effectPublishData
-      Assert.shouldEqual id1 id2
+    Spec.it "atomically deduplicates concurrent active and equivalent successful requests" do
+      testEnv <- ask
+      responses <- liftAff do
+        fibers <- for (Array.range 1 5) \_ ->
+          Aff.forkAff $ runReaderT (Client.publish Fixtures.effectPublishData) testEnv
+        traverse Aff.joinFiber fibers
+
+      case Array.uncons responses of
+        Nothing -> Assert.fail "Expected publish responses."
+        Just { head, tail } -> do
+          unless (Array.all (_.jobId >>> (_ == head.jobId)) tail) do
+            Assert.fail "Expected concurrent publish requests to return one job ID."
+          let created = Array.filter (_.disposition >>> (_ == Just V1.Created)) responses
+          let duplicateActive = Array.filter (_.disposition >>> (_ == Just V1.DuplicateActive)) responses
+          Assert.shouldEqual (Array.length created) 1
+          Assert.shouldEqual (Array.length duplicateActive) 4
+
+          _ <- Env.pollJobOrFail head.jobId
+          completedDuplicate <- Client.publish Fixtures.effectPublishData
+          Assert.shouldEqual completedDuplicate.jobId head.jobId
+          Assert.shouldEqual completedDuplicate.disposition (Just V1.AlreadyPublishedSubmission)
+
+    Spec.it "reports an already-published package as idempotent success" do
+      created <- Client.publish Fixtures.effectAlreadyPublishedData
+      Assert.shouldEqual created.disposition (Just V1.Created)
+      job <- Env.pollJobOrFail created.jobId
+      case job of
+        PublishJob details -> do
+          Assert.shouldEqual details.disposition (Just V1.AlreadyPublished)
+          Assert.shouldEqual details.error Nothing
+        _ -> Assert.fail "Expected a publish job."
+
+      duplicate <- Client.publish Fixtures.effectAlreadyPublishedData
+      Assert.shouldEqual duplicate.jobId created.jobId
+      Assert.shouldEqual duplicate.disposition (Just V1.AlreadyPublishedSubmission)

app-e2e/src/Test/E2E/Endpoint/Unpublish.purs

@@ -52,6 +52,18 @@ spec = do
       when existsAfter do
         Assert.fail "Expected version to be removed from manifest index after unpublish"
 
+      republish <- Client.publish Fixtures.effectPublishData
+      Assert.shouldEqual republish.disposition (Just V1.Created)
+      when (republish.jobId == publishJobId) do
+        Assert.fail "A later unpublish must invalidate successful publish-job deduplication."
+      republishJob <- Env.pollJobExpectFailure republish.jobId
+      case republishJob of
+        V1.PublishJob { error: Just error } ->
+          unless (String.contains (String.Pattern "has been unpublished") error.message) do
+            Assert.fail $ "Expected an unpublished-version error, got: " <> error.message
+        V1.PublishJob { error: Nothing } -> Assert.fail "Expected a structured republish error."
+        _ -> Assert.fail "Expected a publish job."
+
     -- Test race condition: submit unpublish while publish is still running.
     -- Job priority (Unpublish > Matrix) ensures unpublish runs before matrix jobs.
     Spec.it "unpublishing before matrix jobs complete causes them to fail gracefully" do

app-e2e/src/Test/E2E/Support/Client.purs

@@ -157,10 +157,10 @@ getStatus = do
       throw $ HttpError { status: response.status, body }
 
 -- | Publish a package
-publish :: PublishData -> E2E V1.JobCreatedResponse
+publish :: PublishData -> E2E V1.PublishJobResponse
 publish reqBody = do
   { clientConfig } <- ask
-  liftAff $ post Operation.publishCodec V1.jobCreatedResponseCodec clientConfig.baseUrl (printRoute Publish) reqBody
+  liftAff $ post Operation.publishCodec V1.publishJobResponseCodec clientConfig.baseUrl (printRoute Publish) reqBody
 
 -- | Unpublish a package (requires authentication)
 unpublish :: AuthenticatedData -> E2E V1.JobCreatedResponse

app-e2e/src/Test/E2E/Support/Fixtures.purs

@@ -7,6 +7,7 @@ module Test.E2E.Support.Fixtures
   , prelude
   , slug
   , unsafeCoerce
+  , effectAlreadyPublishedData
   , effectPublishData
   , effectPublishDataDifferentLocation
   , consolePublishData
@@ -75,6 +76,14 @@ effectPublishData =
   , version: effect.version
   }
 
+-- | Publish data for effect@4.0.0, which is already present in the registry,
+-- | manifest index, storage, and Pursuit fixtures.
+effectAlreadyPublishedData :: Operation.PublishData
+effectAlreadyPublishedData = effectPublishData
+  { ref = "v4.0.0"
+  , version = Utils.unsafeVersion "4.0.0"
+  }
+
 -- | Publish data for effect@99.0.0 with a DIFFERENT location.
 -- | Uses a non-existent version to avoid duplicate job detection,
 -- | but still targets an existing package to test location conflicts.

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

@@ -50,10 +50,26 @@ spec = do
       unless hasDependencyError do
         Assert.fail $ "Expected dependency resolution error, got:\n" <> String.joinWith "\n" logMessages
 
+      case consoleJob of
+        V1.PublishJob { disposition, error: Just error } -> do
+          Assert.shouldEqual disposition Nothing
+          Assert.shouldEqual error.code V1.JobFailed
+          unless (String.contains (String.Pattern "Could not produce valid dependencies") error.message) do
+            Assert.fail $ "Expected structured dependency error, got: " <> error.message
+        V1.PublishJob { error: Nothing } -> Assert.fail "Expected a structured terminal publish error."
+        _ -> Assert.fail "Expected a publish job."
+
       consoleUploadOccurred <- Env.hasStorageUpload Fixtures.console
       when consoleUploadOccurred do
         Assert.fail "Expected no tarball upload for console@6.1.0 after failed publish"
 
+      retry <- Client.publish Fixtures.consolePublishData
+      Assert.shouldEqual retry.disposition (Just V1.Created)
+      when (retry.jobId == consoleJobId) do
+        Assert.fail "Expected a new publish job after terminal failure."
+      _ <- Env.pollJobExpectFailure retry.jobId
+      pure unit
+
     Spec.it "unpublishing a package fails when dependents exist in manifest index" do
       { jobId: effectJobId } <- Client.publish Fixtures.effectPublishData
       _ <- Env.pollJobOrFail effectJobId

app/fixtures/registry-index/ef/fe/effect

@@ -1 +1 @@
-{"name":"effect","version":"4.0.0","license":"BSD-3-Clause","location":{"githubOwner":"purescript","githubRepo":"purescript-effect"},"ref":"v4.0.0","description":"Native side effects","dependencies":{"prelude":">=6.0.0 <7.0.0"}}
+{"name":"effect","version":"4.0.0","license":"BSD-3-Clause","location":{"githubOwner":"purescript","githubRepo":"purescript-effect"},"ref":"v4.0.0","dependencies":{"prelude":">=6.0.0 <7.0.0"}}