Keep the problems from a partly-applied sync

Found by actually syncing a real repository rather than a stub: the outcome said
"3 created, 1 problem(s)" and the problem itself was nowhere to be seen.

Problems lived only in the response to a manual sync. A page reload lost them, and
a webhook-triggered sync never showed them at all — so on the trigger that matters
most, the operator learned that something was skipped but never what. Reporting a
count without the detail is worse than not reporting it, because it looks like
information.

Now stored on the configuration and read back from there, so the manual and webhook
paths cannot disagree. Cleared by the next clean run.

Author: Mario Ferreira

backend/src/main/java/com/devforge/sync/application/SyncRunner.java

@@ -245,7 +245,8 @@ private SyncOutcome record(SyncConfiguration configuration, SyncOutcome outcome,
                     outcome.created(),
                     outcome.updated(),
                     outcome.archived(),
-                    outcome.unchanged());
+                    outcome.unchanged(),
+                    outcome.problems());
         }
 
         auditTrail.record(actorId, AuditEntry

backend/src/main/java/com/devforge/sync/application/SyncService.java

@@ -57,7 +57,7 @@ public SyncSettingsResponse describe(UUID workspaceId, UUID userId) {
         workspaceAccess.requireAccess(workspaceId, userId, WorkspaceRole.ADMIN);
 
         return repository.findByWorkspaceId(workspaceId)
-                .map(configuration -> SyncSettingsResponse.from(configuration, List.of()))
+                .map(configuration -> SyncSettingsResponse.from(configuration))
                 .orElseGet(SyncSettingsResponse::notConfigured);
     }
 
@@ -105,7 +105,7 @@ public SyncSettingsResponse configure(
                 .with("enabled", saved.isEnabled())
                 .with("firstTime", isNew));
 
-        return SyncSettingsResponse.from(saved, List.of());
+        return SyncSettingsResponse.from(saved);
     }
 
     /**
@@ -147,7 +147,7 @@ public SyncSettingsResponse rotateWebhookUrl(UUID workspaceId, UUID userId) {
                 .inWorkspace(workspaceId)
                 .with("webhookUrlRotated", true));
 
-        return SyncSettingsResponse.from(configuration, List.of());
+        return SyncSettingsResponse.from(configuration);
     }
 
     @Transactional
@@ -171,8 +171,10 @@ public SyncSettingsResponse syncNow(UUID workspaceId, UUID userId) {
         workspaceAccess.requireAccess(workspaceId, userId, WorkspaceRole.ADMIN);
         SyncConfiguration configuration = require(workspaceId);
 
-        SyncOutcome outcome = runner.run(configuration, userId);
-        return SyncSettingsResponse.from(configuration, outcome.problems());
+        runner.run(configuration, userId);
+        // Read back from the configuration rather than from the outcome: the two
+        // must agree, and a webhook-triggered sync only ever populates the former.
+        return SyncSettingsResponse.from(configuration);
     }
 
     /**

backend/src/main/java/com/devforge/sync/application/SyncSettingsResponse.java

@@ -46,7 +46,7 @@ public record SyncSettingsResponse(
         List<String> problems
 ) {
 
-    public static SyncSettingsResponse from(SyncConfiguration configuration, List<String> problems) {
+    public static SyncSettingsResponse from(SyncConfiguration configuration) {
         return new SyncSettingsResponse(
                 configuration.isConfigured(),
                 configuration.getRepositoryUrl(),
@@ -68,7 +68,7 @@ public static SyncSettingsResponse from(SyncConfiguration configuration, List<St
                 configuration.getLastUpdated(),
                 configuration.getLastArchived(),
                 configuration.getLastUnchanged(),
-                problems == null ? List.of() : problems);
+                configuration.getLastProblems());
     }
 
     /** Before anything has been set up. */

backend/src/main/java/com/devforge/sync/domain/SyncConfiguration.java

@@ -9,6 +9,7 @@
 import jakarta.persistence.Table;
 
 import java.time.Instant;
+import java.util.List;
 import java.time.temporal.ChronoUnit;
 import java.util.UUID;
 
@@ -86,6 +87,15 @@ public class SyncConfiguration extends BaseEntity {
     @Column(name = "last_unchanged", nullable = false)
     private int lastUnchanged;
 
+    /**
+     * What the last attempt could not use, newline separated.
+     *
+     * <p>Persisted rather than returned once, so a reload still shows them and a
+     * webhook-triggered sync shows them at all.
+     */
+    @Column(name = "last_problems", columnDefinition = "text")
+    private String lastProblems;
+
     protected SyncConfiguration() {
         // for JPA
     }
@@ -158,8 +168,12 @@ public void recordSuccess(
             int created,
             int updated,
             int archived,
-            int unchanged
+            int unchanged,
+            List<String> problems
     ) {
+        this.lastProblems = problems == null || problems.isEmpty()
+                ? null
+                : String.join("\n", problems);
         this.lastStatus = status;
         this.lastRef = ref;
         this.lastMessage = message;
@@ -179,6 +193,7 @@ public void recordSuccess(
     public void recordFailure(String message) {
         this.lastStatus = SyncStatus.FAILED;
         this.lastMessage = message;
+        this.lastProblems = null;
         this.lastCreated = 0;
         this.lastUpdated = 0;
         this.lastArchived = 0;
@@ -268,4 +283,11 @@ public int getLastArchived() {
     public int getLastUnchanged() {
         return lastUnchanged;
     }
+
+    /** Empty when the last attempt used everything it found. */
+    public List<String> getLastProblems() {
+        return lastProblems == null || lastProblems.isBlank()
+                ? List.of()
+                : List.of(lastProblems.split("\n"));
+    }
 }

backend/src/main/resources/db/migration/V8__sync_problems.sql

@@ -0,0 +1,11 @@
+-- Keep the detail of a partly-applied sync, not just its count.
+--
+-- The outcome already recorded "3 created, 1 problem(s)", but the problems
+-- themselves lived only in the response to a manual sync. So a reload lost them,
+-- and a webhook-triggered sync never showed them at all — which makes reporting a
+-- problem pointless, since the operator cannot act on a number.
+--
+-- Newline separated rather than a table: they belong to the last attempt only, and
+-- are replaced wholesale by the next one.
+ALTER TABLE sync_configurations
+    ADD COLUMN last_problems TEXT;

backend/src/test/java/com/devforge/sync/api/GitSyncIntegrationTest.java

@@ -357,6 +357,49 @@ void appliesWhatItUnderstandsAndReportsWhatItDoesNot() throws Exception {
                         org.hamcrest.Matchers.containsString("NONSENSE")));
     }
 
+    /**
+     * The regression this guards: problems used to live only in the response to a
+     * manual sync, so a reload lost them and a webhook-triggered sync never showed
+     * them at all. Reporting "1 problem(s)" without saying which is useless.
+     */
+    @Test
+    void keepsTheProblemsAfterTheResponseThatReportedThem() throws Exception {
+        TestUser owner = registerUser("owner@acme.test", "Owner");
+        UUID ws = createWorkspace(owner, "Platform", "platform");
+        configure(owner, ws, "ARCHIVE");
+
+        source.files = List.of(
+                new SourceFile("docs/good.md", "# Good"),
+                new SourceFile("docs/odd.md", "---\ntype: NONSENSE\n---\nbody"));
+
+        mockMvc.perform(authed(post("/api/workspaces/{w}/sync/run", ws), owner))
+                .andExpect(jsonPath("$.problems[0]").value(
+                        org.hamcrest.Matchers.containsString("NONSENSE")));
+
+        // Read fresh, as a reload would.
+        mockMvc.perform(authed(get("/api/workspaces/{w}/sync", ws), owner))
+                .andExpect(jsonPath("$.lastStatus").value("PARTIAL"))
+                .andExpect(jsonPath("$.problems[0]").value(
+                        org.hamcrest.Matchers.containsString("NONSENSE")));
+    }
+
+    /** A clean run must not leave the previous run's problems lying around. */
+    @Test
+    void clearsTheProblemsWhenTheNextSyncIsClean() throws Exception {
+        TestUser owner = registerUser("owner@acme.test", "Owner");
+        UUID ws = createWorkspace(owner, "Platform", "platform");
+        configure(owner, ws, "ARCHIVE");
+
+        source.files = List.of(new SourceFile("docs/odd.md", "---\ntype: NONSENSE\n---\nbody"));
+        mockMvc.perform(authed(post("/api/workspaces/{w}/sync/run", ws), owner))
+                .andExpect(jsonPath("$.problems.length()").value(1));
+
+        source.files = List.of(new SourceFile("docs/odd.md", "---\ntype: RUNBOOK\n---\nbody"));
+        mockMvc.perform(authed(post("/api/workspaces/{w}/sync/run", ws), owner))
+                .andExpect(jsonPath("$.lastStatus").value("OK"))
+                .andExpect(jsonPath("$.problems.length()").value(0));
+    }
+
     @Test
     void recordsEverySyncInTheWorkspaceActivityLog() throws Exception {
         TestUser owner = registerUser("owner@acme.test", "Owner");