Address Momus branch-review findings: harden ship, advance, and locking

- ship: no interactive override of the untested-commit guard; the tag targets the pinned commit the artifacts were built from (never the branch head); a tagged-but-incomplete train is found and resumed instead of vanishing from open-train detection; a fully shipped train reports as such - start: a failed edge→rc advance fails the command loudly (both start and the advance are idempotent) instead of opening a train against stale rc - rc trigger: bootstraps the server's rc worktree on first use, so a host set up before the rc branch existed can run its first RC build - advance-channel: fast-ring packages are excluded from edge→rc (the stable build replicated by parity is authoritative for rc — same filename, other bytes); differing destination bytes abort instead of warn; a package whose signature copy was interrupted gets its .sig restored on resume - the release lock now also covers direct promote/update/clean/remove/sync invocations, not just release/advance/upload-prebuilt
2026-08-27 01:10:33 -04:00
parent da92095f75
commit 5fae475743
7 changed files with 132 additions and 27 deletions
@@ -201,6 +201,11 @@ package_eligible() {

  if [[ "$FAST_RING_ONLY" == true ]]; then
    package_is_fast_ring "$pkgdir" || return 1
+  elif [[ "$FROM" == "edge" && "$TO" == "rc" ]]; then
+    # Fast-ring packages reach rc by replication of the STABLE build (same
+    # bytes stable users get). Carrying the independently built edge artifact
+    # forward would race it under the same filename.
+    package_is_fast_ring "$pkgdir" && return 1
  fi

  package_moves_to_channel "$pkgdir" "$TO"
@@ -237,6 +242,17 @@ while IFS=$'\t' read -r name base filename; do

  if [[ -f "$dest" ]]; then
    if cmp -s "$src" "$dest"; then
+      # A crash between the package and signature copies leaves an unsigned
+      # package behind; the bytes are identical, so the source signature is
+      # valid for it. Package + signature resume as one unit.
+      if [[ ! -f "$dest.sig" ]]; then
+        if [[ "$DRY_RUN" == true ]]; then
+          echo "  would restore missing signature: $filename.sig"
+        else
+          cp -p "$sig" "$dest.sig"
+          echo "  restored missing signature: $filename.sig"
+        fi
+      fi
      PRESENT=$((PRESENT + 1))
    else
      DIFFERING+=("$filename")
@@ -274,10 +290,13 @@ if [[ ${#MISSING_SIGS[@]} -gt 0 ]]; then
 fi

 if [[ ${#DIFFERING[@]} -gt 0 ]]; then
-  print_warning "${#DIFFERING[@]} file(s) already published in $TO with different bytes (kept as-is):"
+  print_error "${#DIFFERING[@]} file(s) already published in $TO with DIFFERENT bytes:"
  printf '  %s\n' "${DIFFERING[@]}"
-  echo "Published filenames are never rewritten. The destination copy stays"
-  echo "authoritative; a genuinely new build needs a new pkgver/pkgrel."
+  echo "Published filenames are never rewritten, and two artifacts fighting over"
+  echo "one name means something built twice from different inputs. Resolve it"
+  echo "deliberately: bump pkgrel and rebuild so the new artifact gets a new"
+  echo "filename, or remove the source copy if the destination is correct."
+  exit 1
 fi

 if [[ "$DRY_RUN" == true ]]; then
@@ -8,6 +8,7 @@ set -e
 BUILD_ROOT=$(realpath "${BASH_SOURCE[0]%/*}/..")
 source "$BUILD_ROOT/helpers/message-helpers.sh"
 source "$BUILD_ROOT/helpers/paths.sh"
+source "$BUILD_ROOT/helpers/lock-helpers.sh"

 # Repository configuration
 KEEP_VERSIONS=2
@@ -223,6 +224,10 @@ main() {
    print_warning "DRY RUN MODE - No changes will be made"
  fi

+  if [[ "$DRY_RUN" != true && "$SHOW_USAGE" != true ]]; then
+    acquire_release_lock || exit 1
+  fi
+
  # Execute based on options
  if [[ "$SHOW_USAGE" == true ]]; then
    show_disk_usage
@@ -111,13 +111,19 @@ resolve_tag_commit() {
  echo "${peeled:-$sha}"
 }

-# Newest v*-*-* release branch whose final tag does not exist yet. Shipping
-# creates the tag, so "tag exists" is what closes a train.
-open_train_branch() {
+# Newest v*-*-* release branch, optionally only untagged ones. Shipping tags
+# the version, so "tag exists" is what closes a train — but ship itself also
+# needs to find a tagged train whose remaining steps (release, ISO, website)
+# didn't finish, so it can resume.
+newest_release_branch() { # newest_release_branch [--untagged]
+  local untagged_only=false
+  [[ "${1:-}" == "--untagged" ]] && untagged_only=true
  local branch best_ver="" best_branch="" ver
  while IFS= read -r branch; do
    ver=$(branch_to_version "$branch") || continue
-    tag_exists "v$ver" && continue
+    if [[ "$untagged_only" == true ]] && tag_exists "v$ver"; then
+      continue
+    fi
    if [[ -z "$best_ver" ]] || [[ $(vercmp "$ver" "$best_ver") -gt 0 ]]; then
      best_ver="$ver" best_branch="$branch"
    fi
@@ -125,6 +131,8 @@ open_train_branch() {
  [[ -n "$best_branch" ]] && echo "$best_branch"
 }

+open_train_branch() { newest_release_branch --untagged; }
+
 ensure_mirror_clone() {
  if [[ -d "$MIRROR_CLONE" ]]; then
    git -C "$MIRROR_CLONE" fetch --quiet origin
@@ -201,12 +209,27 @@ host_or_print() { # prints the host, or prints manual instructions and fails
  return 1
 }

+# Creates the rc worktree on first use (a host set up before the rc branch
+# existed has none), then syncs it and kicks the service.
+RC_TRIGGER_SCRIPT='
+set -e
+git -C /root/omarchy-pkgs fetch origin rc
+if [ ! -d /root/omarchy-pkgs-rc ]; then
+  git -C /root/omarchy-pkgs worktree add /root/omarchy-pkgs-rc rc 2>/dev/null ||
+    git -C /root/omarchy-pkgs worktree add --track -b rc /root/omarchy-pkgs-rc origin/rc
+fi
+git -C /root/omarchy-pkgs-rc fetch origin rc
+git -C /root/omarchy-pkgs-rc reset --hard origin/rc
+mkdir -p /root/.state
+touch /root/.state/.sync-needed-rc
+systemctl start --no-block omarchy-auto-release-rc.service
+'
+
 trigger_rc_build() {
  local host
-  host=$(host_or_print "build the rc channel" \
-    "git -C /root/omarchy-pkgs-rc fetch origin rc && git -C /root/omarchy-pkgs-rc reset --hard origin/rc && touch /root/.state/.sync-needed-rc && systemctl start --no-block omarchy-auto-release-rc.service") || return 1
+  host=$(host_or_print "build the rc channel" "$RC_TRIGGER_SCRIPT") || return 1
  print_info "Triggering rc build on $host..."
-  ssh "$host" 'git -C /root/omarchy-pkgs-rc fetch origin rc && git -C /root/omarchy-pkgs-rc reset --hard origin/rc && mkdir -p /root/.state && touch /root/.state/.sync-needed-rc && systemctl start --no-block omarchy-auto-release-rc.service'
+  ssh "$host" "$RC_TRIGGER_SCRIPT"
 }

 host_advance() { # host_advance <from> <to> [extra args...]
@@ -408,9 +431,15 @@ cmd_start() {
  fi

  # Minor/major trains ship new edge packages: carry edge forward into rc so
-  # RC testing runs against the set stable users will get.
+  # RC testing runs against the set stable users will get. A failed advance
+  # must not pass silently — RCs would test against the previous rc set.
  if [[ "$kind" != "patch" ]]; then
-    host_advance edge rc || true
+    if ! host_advance edge rc; then
+      print_error "edge → rc advance did not complete — the train is NOT fully open"
+      echo "Fix the advance (it is idempotent), then re-run: omarchy-release start $version"
+      echo "start is idempotent too — the branch and PR above are kept."
+      exit 1
+    fi
  else
    print_info "Patch train: rc already mirrors stable — nothing to advance"
  fi
@@ -577,10 +606,25 @@ cmd_ship() {
  local branch version
  branch=$(open_train_branch) || true
  if [[ -z "$branch" ]]; then
-    print_error "No open release train — nothing to ship"
+    # A tagged train whose later steps (release, ISO, website) failed is
+    # invisible to open_train_branch — find it so a re-run can resume.
+    branch=$(newest_release_branch) || true
+    if [[ -z "$branch" ]]; then
+      print_error "No release train found — nothing to ship"
      exit 1
    fi
    version=$(branch_to_version "$branch")
+    local stable_now
+    stable_now=$(published_version stable 2>/dev/null) || stable_now=""
+    if [[ "${stable_now%-*}" == "$version" ]] &&
+      gh release view "v$version" --repo "$UPSTREAM_REPO" >/dev/null 2>&1; then
+      print_success "Nothing to ship — $version is tagged, released, and live on stable"
+      exit 0
+    fi
+    print_info "Resuming tagged-but-incomplete train $version"
+  else
+    version=$(branch_to_version "$branch")
+  fi
  print_header "Ship $version"

  local head pin pin_ver pin_commit
@@ -589,19 +633,30 @@ cmd_ship() {
  pin_ver="${pin%% *}"
  pin_commit="${pin##* }"

-  # The ship guard: only a commit an RC was actually cut from may ship.
-  if [[ "$pin_ver" != "$version" ]]; then
-    if [[ -z "$pin" || ! "$pin_ver" =~ ^${version//./\\.}rc[0-9]+$ ]]; then
+  # The ship guard: only the exact commit an RC was cut from may ship. There
+  # is no override — a moved branch means an untested tree; cut another rc.
+  if [[ -z "$pin" ]]; then
    print_error "No RC has been cut for $version — run: omarchy-release rc"
    exit 1
  fi
+  if [[ "$pin_ver" == "$version" ]]; then
+    # Final already pinned (resume path). The artifacts come from pin_commit;
+    # a branch that moved afterwards changes nothing already built or tagged.
+    if [[ -n "$head" && "$head" != "$pin_commit" ]]; then
+      print_warning "$branch moved after the final was pinned — shipping the pinned ${pin_commit:0:12}; newer commits need the next patch train"
+    fi
+  else
+    if [[ ! "$pin_ver" =~ ^${version//./\\.}rc[0-9]+$ ]]; then
+      print_error "No RC has been cut for $version (rc branch pins $pin_ver) — run: omarchy-release rc"
+      exit 1
+    fi
    if [[ "$pin_commit" != "$head" ]]; then
      local behind
      behind=$(git -C "$WORK_CLONE" rev-list --count "$pin_commit..origin/$branch" 2>/dev/null || echo "?")
      print_error "$branch has moved since $pin_ver was cut ($behind commit(s) untested)"
-      echo "Cut another candidate first: omarchy-release rc"
-      [[ "$ASSUME_YES" == true ]] && exit 1
-      confirm "Ship anyway from ${head:0:12} WITHOUT an RC of it? (not recommended)" || exit 1
+      echo "Only a commit an RC was cut from can ship. Cut another candidate:"
+      echo "  omarchy-release rc"
+      exit 1
    fi
    local pub
    pub=$(published_version rc 2>/dev/null) || pub=""
@@ -614,7 +669,7 @@ cmd_ship() {

  echo ""
  print_info "This will, in order (steps already done are skipped):"
-  echo "  1. Pin the final $version from $branch@${head:0:12} and publish it to rc"
+  echo "  1. Pin the final $version from $branch@${pin_commit:0:12} and publish it to rc"
  echo "  2. Promote the rc channel to stable (packages + signatures + db)"
  echo "  3. Tag v$version on $UPSTREAM_REPO"
  echo "  4. Merge the final pins to master (edge overlap + record)"
@@ -632,7 +687,7 @@ cmd_ship() {
  else
    if [[ "$pin_ver" != "$version" ]]; then
      print_info "1/7 Pinning final $version..."
-      cut_pins "v$version" --commit "$head"
+      cut_pins "v$version" --commit "$pin_commit"
    else
      print_info "1/7 Final $version pinned — re-triggering build"
    fi
@@ -654,13 +709,14 @@ cmd_ship() {
    print_success "2/7 Promoted to stable: omarchy $stable_pub"
  fi

-  # 3. Tag
+  # 3. Tag — at the pinned commit the artifacts were built from, never the
+  # branch head (they can differ on a resumed ship).
  if tag_exists "v$version"; then
    print_success "3/7 Tag v$version already exists"
  else
    ensure_mirror_clone
-    git -C "$MIRROR_CLONE" push --quiet origin "$head:refs/tags/v$version"
-    print_success "3/7 Tagged v$version at ${head:0:12}"
+    git -C "$MIRROR_CLONE" push --quiet origin "$pin_commit:refs/tags/v$version"
+    print_success "3/7 Tagged v$version at ${pin_commit:0:12}"
  fi

  # 4. Final pins onto master (keeps edge overlap publishing and the repo record)
@@ -735,6 +791,17 @@ gather_status() {

 next_step() { # prints "<command>|<description>"
  if [[ -z "$TRAIN" ]]; then
+    # A tagged train may still have unfinished ship steps (release/ISO/site).
+    local last last_ver
+    last=$(newest_release_branch 2>/dev/null) || last=""
+    if [[ -n "$last" && "$STABLE_VER" != "<unreachable>" ]]; then
+      last_ver=$(branch_to_version "$last")
+      if [[ "${STABLE_VER%-*}" != "$last_ver" ]] ||
+        { command -v gh >/dev/null && ! gh release view "v$last_ver" --repo "$UPSTREAM_REPO" >/dev/null 2>&1; }; then
+        echo "ship|$last_ver is tagged but not fully shipped — resume ship"
+        return
+      fi
+    fi
    echo "start|No open train — start the next release"
    return
  fi
@@ -6,6 +6,7 @@ set -e
 BUILD_ROOT=$(realpath "${BASH_SOURCE[0]%/*}/..")
 source "$BUILD_ROOT/helpers/message-helpers.sh"
 source "$BUILD_ROOT/helpers/paths.sh"
+source "$BUILD_ROOT/helpers/lock-helpers.sh"

 print_header "Promote Build to Production"

@@ -47,6 +48,10 @@ while [[ $# -gt 0 ]]; do
  esac
 done

+if [[ "$DRY_RUN" != true ]]; then
+  acquire_release_lock || exit 1
+fi
+
 print_info "Mirror: $MIRROR"
 print_info "Build output: $BUILD_OUTPUT_DIR"
 print_info "Final output: $REPO_DIR"
@@ -5,6 +5,7 @@ BUILD_ROOT=$(realpath "${BASH_SOURCE[0]%/*}/..")
 source "$BUILD_ROOT/helpers/message-helpers.sh"
 source "$BUILD_ROOT/helpers/docker-helpers.sh"
 source "$BUILD_ROOT/helpers/paths.sh"
+source "$BUILD_ROOT/helpers/lock-helpers.sh"

 print_header "Remove Package"

@@ -90,6 +91,8 @@ fi
 # Build/update the Docker image (always use x86_64 for removal - it's architecture independent)
 build_docker_image "$BUILD_DIR" "x86_64" "$MIRROR"

+acquire_release_lock || exit 1
+
 print_info "Removing package..."

 # Ensure directory is writable by container user
@@ -4,6 +4,7 @@
 BUILD_ROOT=$(realpath "${BASH_SOURCE[0]%/*}/..")
 source "$BUILD_ROOT/helpers/message-helpers.sh"
 source "$BUILD_ROOT/helpers/paths.sh"
+source "$BUILD_ROOT/helpers/lock-helpers.sh"

 # Default remote (production)
 DEFAULT_REMOTE="pkgs.omarchy.org:omarchy-pkgs"
@@ -100,6 +101,8 @@ if [[ "$REMOTE" == "$DEFAULT_REMOTE" ]] && [[ "$SKIP_PROD_CHECK" != true ]]; the
  fi
 fi

+acquire_release_lock || exit 1
+
 print_info "Syncing to: $REMOTE/$DESTINATION_DIRECTORY"

 # The database is what users actually resolve against, and repo-add builds it
@@ -9,6 +9,7 @@ BUILD_ROOT=$(realpath "${BASH_SOURCE[0]%/*}/..")
 source "$BUILD_ROOT/helpers/message-helpers.sh"
 source "$BUILD_ROOT/helpers/docker-helpers.sh"
 source "$BUILD_ROOT/helpers/paths.sh"
+source "$BUILD_ROOT/helpers/lock-helpers.sh"

 # Function to update repository database using Docker
 update_database() {
@@ -77,6 +78,8 @@ main() {
  print_info "Mirror: $MIRROR"
  print_info "Output directory: $REPO_DIR"

+  acquire_release_lock || exit 1
+
  update_database

  echo ""