feat: Skip dependants of skipped changes

  A skipped change left its dependants unprotected: on the next cycle
  they would be attempted and either fail spuriously (e.g.
  `MISSING_PARENT`) or, worse, succeed partially and create an
  orphan. The multi-error loop makes this more visible since
  dependants are no longer shielded by the first-failure break.

  When a change is skipped (automatically via `handleSyncError`
  returning `'skipped'`, or by the user), the `syncBatch` loop now
  adds it to `blockedIds`. Dependents detected via
  `directPrerequisites` whose prereq has `doc.skipped === true` are
  themselves skipped (`skipChange` persists the flag) and emit a
  synthetic `SKIPPED_DEPENDENCY` alert. `blockedIds` propagates
  the skip transitively across the topologically ordered batch.

  The synthetic `SyncError` carries `prereqPath` as a dedicated
  field, transported by `makeAlert` to the GUI. `viewByCode`
  interpolates it into a localized message with the path wrapped
  in backticks. The restored `Util.DecorationParser` parses those
  backticks in `alertContent`, and `decoratedName` renders the path
  as a clickable chip (blue background, `title` with full path,
  `ShowInParent` on click).

Author: taratatach

core/metadata.js

@@ -166,7 +166,7 @@ export type Metadata = {
 
   trashed?: true,
   errors?: number,
-  skipped?: boolean,
+  skipped?: string,
   overwrite?: SavedMetadata,
   childMove?: boolean,
   incompatibilities?: *,

core/sync/errors.js

@@ -25,8 +25,10 @@ const EXCLUDED_DIR_CODE = 'ExcludedDir'
 const INCOMPATIBLE_DOC_CODE = 'IncompatibleDoc'
 const MISSING_PERMISSIONS_CODE = 'MissingPermissions'
 const NO_DISK_SPACE_CODE = 'NoDiskSpace'
+const SKIPPED_DEPENDENCY_CODE = 'SkippedDependency'
 const UNSYNCED_PARENT_MOVE_CODE = 'UnsyncedParentMove'
 const UNKNOWN_SYNC_ERROR_CODE = 'UnknownSyncError'
+const USER_SKIPPED_CODE = 'UserSkipped'
 
 class UnsyncedParentMoveError extends Error {
   /*::
@@ -52,7 +54,7 @@ class SyncError extends Error {
 
   code: string
   message: string
-  sideName: SideName
+  sideName: ?SideName
   originalErr: Error
   doc: SavedMetadata
   */
@@ -63,7 +65,7 @@ class SyncError extends Error {
       sideName,
       err,
       doc
-    } /*: { code?: string, sideName: SideName, err: Error, doc: SavedMetadata } */
+    } /*: { code?: string, sideName: ?SideName, err: Error, doc: SavedMetadata } */
   ) {
     super(err.message)
 
@@ -157,6 +159,27 @@ const minRetryDelay = (
   return Math.min(...causes.map(c => retryDelay(c.err)))
 }
 
+// Synthetic error emitted when a change is skipped because one of its
+// prerequisite changes was skipped. `prereqPath` is carried to the GUI so
+// `viewByCode` can build a localized message with the path wrapped in
+// backticks, rendered as a clickable chip by `Util.DecorationParser`.
+const skippedDependencyErr = (
+  change /*: Change */,
+  prereqPath /*: string */
+) /*: SyncError */ => {
+  const err = new Error(
+    `Change skipped: a prerequisite change on ${prereqPath} was skipped`
+  )
+  // $FlowFixMe: Error has no prereqPath by default, we add it on purpose.
+  err.prereqPath = prereqPath
+  return new SyncError({
+    code: SKIPPED_DEPENDENCY_CODE,
+    sideName: null,
+    err,
+    doc: change.doc
+  })
+}
+
 const retryAll = async (
   causes /*: Array<{| err: RemoteError |} | {| err: SyncError, change: Change |}> */,
   sync /*: Sync */
@@ -219,7 +242,16 @@ const skip = async (
   clearInterval(sync.retryInterval)
 
   if (cause.change) {
-    await sync.skipChange(cause.change, cause.err)
+    // Wrap the original error so the skip is recorded as user-initiated
+    // (fatal), regardless of the original error's code (which may be
+    // non-fatal like MISSING_DOCUMENT).
+    const wrappedErr = new SyncError({
+      code: USER_SKIPPED_CODE,
+      sideName: cause.err.sideName,
+      err: cause.err,
+      doc: cause.change.doc
+    })
+    await sync.skipChange(cause.change, wrappedErr)
   }
 
   // Fire-and-forget: awaiting watcher.start() would deadlock with the
@@ -353,12 +385,15 @@ module.exports = {
   INCOMPATIBLE_DOC_CODE,
   MISSING_PERMISSIONS_CODE,
   NO_DISK_SPACE_CODE,
+  SKIPPED_DEPENDENCY_CODE,
   UNKNOWN_SYNC_ERROR_CODE,
   UNSYNCED_PARENT_MOVE_CODE,
+  USER_SKIPPED_CODE,
   UnsyncedParentMoveError,
   SyncError,
   retryDelay,
   minRetryDelay,
+  skippedDependencyErr,
   retry,
   retryAll,
   skip,

core/sync/index.js

@@ -75,6 +75,19 @@ const wasSkipped = (change /*: PouchDBFeedData */) => {
   return change.doc.skipped
 }
 
+// Skip codes whose dependants should be cascaded (fatal skips).
+// Non-fatal skips (e.g. CONFLICTING_NAME, INVALID_METADATA,
+// MISSING_DOCUMENT when marked for deletion) are auto-resolved by a
+// future merge/re-scan: cascading them would leave dependants stuck.
+const FATAL_SKIP_CODES = new Set([
+  remoteErrors.MISSING_PARENT_CODE,
+  syncErrors.SKIPPED_DEPENDENCY_CODE,
+  syncErrors.USER_SKIPPED_CODE
+])
+
+const isFatalSkip = (skipped /*: ?string */) =>
+  typeof skipped === 'string' && FATAL_SKIP_CODES.has(skipped)
+
 // Returns the given side metadata of the given PouchDB record.
 // It is meant to get the outdated side metadata of the record to compare it
 // against the new metadata and decide which actions to take.
@@ -548,12 +561,29 @@ class Sync {
           if (blockedIds.size === 0) {
             await this.pouch.setLocalSeq(change.seq)
           }
+          blockedIds.add(change.id)
           this.resolveBlockingCause(change.id)
           continue
         }
 
-        if (graph.directPrerequisites(change).some(d => blockedIds.has(d.id))) {
+        const prereqs = graph.directPrerequisites(change)
+        if (prereqs.some(c => blockedIds.has(c.id))) {
           blockedIds.add(change.id)
+
+          const skippedPrereq = prereqs.find(c => isFatalSkip(c.doc.skipped))
+          if (skippedPrereq) {
+            const err = syncErrors.skippedDependencyErr(
+              change,
+              skippedPrereq.doc.path
+            )
+            await this.skipChange(change, err)
+            this.events.emit(
+              'user-alert',
+              err,
+              change.seq,
+              change.operation.side != null && change.operation.side
+            )
+          }
           continue
         }
 
@@ -569,7 +599,8 @@ class Sync {
 
           const result = await this.handleSyncError(err, change)
           if (result === 'fatal') return
-          if (result === 'blocked') blockedIds.add(change.id)
+          if (result === 'blocked' || result === 'skipped')
+            blockedIds.add(change.id)
           if (
             (result === 'skipped' || result === 'recovered') &&
             blockedIds.size === 0
@@ -594,8 +625,8 @@ class Sync {
     err /*: SyncError */,
     change /*: Change */
   ) /*: Promise<'blocked' | 'skipped' | 'recovered' | 'fatal'> */ {
+    const sideName = err.sideName || 'local'
     const {
-      sideName,
       doc: { path }
     } = err
 
@@ -1337,7 +1368,7 @@ class Sync {
       doc.errors = (doc.errors || 0) + 1
 
       // Make sure isUpToDate(sourceSideName, doc) is still true
-      const sourceSideName = otherSide(err.sideName)
+      const sourceSideName = err.sideName ? otherSide(err.sideName) : 'local'
       metadata.markSide(sourceSideName, doc, doc)
 
       change.doc = await this.pouch.put(doc, { checkInvariants: false })
@@ -1377,7 +1408,7 @@ class Sync {
 
     this._blockedCauses.delete(change.id)
 
-    doc.skipped = true
+    doc.skipped = err.code
     await this.pouch.put(doc, { checkInvariants: false })
   }
 

core/syncstate.js

@@ -75,6 +75,7 @@ const makeAlert = (
     side: side || null,
     doc: doc ? { id: doc._id, docType: doc.docType, path: doc.path } : null,
     links: links || null,
+    prereqPath: err.prereqPath || null,
     lastSeenAt: Date.now()
   }
 }

gui/elm/Data/UserAlert.elm

@@ -20,6 +20,7 @@ import I18n exposing (Helpers)
 import Icons
 import Time
 import Util.Conditional exposing (ShowInWeb, inWeb, onOS)
+import Util.DecorationParser exposing (DecorationResult(..), findDecorations)
 
 
 type alias UserAlertCode =
@@ -44,7 +45,7 @@ type Side
 
 
 type alias SynchronizationErrorInfo =
-    { status : UserActionStatus, seq : Int, id : String, docType : String, path : String, side : Maybe Side, lastSeenAt : Time.Posix }
+    { status : UserActionStatus, seq : Int, id : String, docType : String, path : String, side : Maybe Side, prereqPath : Maybe String, lastSeenAt : Time.Posix }
 
 
 type alias RemoteWarningInfo =
@@ -119,6 +120,7 @@ type alias EncodedUserAlert =
         Maybe
             { self : String
             }
+    , prereqPath : Maybe String
     , lastSeenAt : Maybe Int
     }
 
@@ -128,7 +130,7 @@ type alias EncodedCommand =
 
 
 decode : EncodedUserAlert -> Maybe UserAlert
-decode { seq, status, code, side, doc, links, lastSeenAt } =
+decode { seq, status, code, side, doc, links, prereqPath, lastSeenAt } =
     let
         decodedStatus =
             decodeUserActionStatus status
@@ -149,6 +151,7 @@ decode { seq, status, code, side, doc, links, lastSeenAt } =
                     , docType = docType
                     , path = path
                     , side = decodedSide side
+                    , prereqPath = prereqPath
                     , lastSeenAt = decodedLastSeenAt
                     }
                 )
@@ -167,6 +170,7 @@ encode alert =
             , side = encodedSide a.side
             , doc = Just { id = a.id, docType = a.docType, path = a.path }
             , links = Nothing
+            , prereqPath = a.prereqPath
             , lastSeenAt = Just (Time.posixToMillis a.lastSeenAt)
             }
 
@@ -177,6 +181,7 @@ encode alert =
             , side = Nothing
             , doc = Nothing
             , links = Nothing
+            , prereqPath = Nothing
             , lastSeenAt = Nothing
             }
 
@@ -187,6 +192,7 @@ encode alert =
             , side = Nothing
             , links = Just { self = a.link }
             , doc = Nothing
+            , prereqPath = Nothing
             , lastSeenAt = Nothing
             }
 
@@ -350,7 +356,7 @@ viewSyncError helpers platform now alert info =
                 [ text (Path.toString dirPath) ]
             ]
         , span [ class "file-line-content u-spacenormal u-errorColorDark u-mt-half" ]
-            (alertContent helpers content)
+            (alertContent helpers platform content)
         , div [ class "u-flex u-mt-half u-pb-1" ] buttons
         ]
 
@@ -373,7 +379,7 @@ viewRemoteError helpers platform now alert =
             [ span [ class "file-parent-folder" ] [ text (helpers.t "UserAlert System") ]
             ]
         , span [ class "file-line-content u-spacenormal u-errorColorDark u-mt-half" ]
-            (alertContent helpers content)
+            (alertContent helpers platform content)
         , div [ class "u-flex u-mt-half u-pb-1" ] buttons
         ]
 
@@ -588,6 +594,15 @@ viewByCode helpers alert =
                 [ actionButton helpers (SendCommand Retry alert) "UserAlert Retry" Primary ]
             }
 
+        SynchronizationError "SkippedDependency" { prereqPath } ->
+            { title = "Error Skipped dependency"
+            , content =
+                [ Maybe.withDefault "" prereqPath
+                    |> (\p -> helpers.interpolate [ p ] "Error Change skipped: a prerequisite change on `{0}` was skipped.")
+                ]
+            , buttons = []
+            }
+
         SynchronizationError "UnknownRemoteError" { docType } ->
             let
                 localDocType =
@@ -617,14 +632,43 @@ localDocTypeLabel docType =
         "Helpers file"
 
 
-alertContent : Helpers -> List String -> List (Html Msg)
-alertContent helpers details =
+alertContent : Helpers -> Platform -> List String -> List (Html Msg)
+alertContent helpers platform details =
     details
         |> List.map helpers.capitalize
-        |> List.map text
+        |> List.map (viewActionContentLine platform)
         |> List.intersperse (br [] [])
 
 
+viewActionContentLine : Platform -> String -> Html Msg
+viewActionContentLine platform line =
+    let
+        toHTML =
+            \decoration ->
+                case decoration of
+                    Decorated path ->
+                        Path.fromString platform path
+                            |> decoratedName
+
+                    Normal str ->
+                        text str
+    in
+    span []
+        (findDecorations line
+            |> List.map toHTML
+        )
+
+
+decoratedName : Path -> Html Msg
+decoratedName path =
+    span
+        [ class "u-bg-frenchPass u-bdrs-4 u-ph-half u-pv-0 u-c-pointer"
+        , title (Path.toString path)
+        , onClick (ShowInParent path onOS)
+        ]
+        [ text (Path.name path) ]
+
+
 classList : List String -> List ( Maybe Bool, String ) -> String
 classList baseList optionalClasses =
     let