fix(ownership): make publisher RESULT contract hold on every path

Addresses review on #29:
- drop errtrace: a failure inside $(...) printed two RESULT lines
- explicit failures go through fail(), which always prints
  RESULT: FAILED stage=...; idx check gets its own `preflight` stage;
  the never-failing published-manifest read is no longer a stage
- validate the built latest_as_of_date (`// empty` + date regex) so a
  corrupt manifest cannot compare as "newer"
- freshness: validate --now (exit 2) and latest_as_of_date (exit 3)
  before calling date
- flake: package an explicit script list instead of scripts/*.sh
- docs: GH_TOKEN prerequisite for the packaged publisher

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017a5Cv5Z1NzdYKgKnTiAF4E

docs/OWNERSHIP_SELF_HOSTED.md

@@ -33,6 +33,11 @@ nix build github:0xrsydn/idx-cli#ownership-publisher
 ./result/bin/idx-ownership-freshness --max-age-days 40
 ```
 
+The publisher needs `GH_TOKEN` in its environment: a token that can create
+and upload assets on the target repo's releases (the examples read it from
+`/etc/idx-ownership-snapshot.env`; clan-private uses a Clan var). The
+freshness check needs no token.
+
 `idx-ownership-publish` wraps `scripts/publish-ownership-snapshot.sh` with a
 pinned `idx` (itself wrapped with curl-impersonate and mupdf) plus jq, gh,
 sqlite and curl. On NixOS, add this repo as a flake input and run
@@ -47,7 +52,7 @@ The publisher is idempotent and prints exactly one final line:
 | --- | --- | --- |
 | `RESULT: published <as-of>` | 0 | a newer snapshot was uploaded |
 | `RESULT: up-to-date <as-of>` | 0 | the published snapshot already has the latest report; nothing uploaded |
-| `RESULT: FAILED stage=<stage> (exit N)` | N | failed in `check-published`, `discover`, `build-snapshot`, `stage-output`, or `upload` |
+| `RESULT: FAILED stage=<stage> (exit N)` | N | failed in `arguments`, `build`, `preflight`, `discover`, `build-snapshot`, `stage-output`, or `upload` |
 
 It checks the published manifest first and, for XLSX sources, compares as-of
 dates before downloading anything, so it is cheap to run **daily**. `--force`

flake.nix

@@ -62,24 +62,26 @@
           jq
           sqlite
         ];
-        ownershipPublisher = pkgs.stdenvNoCC.mkDerivation {
-          pname = "idx-ownership-publisher";
-          version = cargoManifest.package.version;
-          src = lib.fileset.toSource {
-            root = ./.;
-            fileset = lib.fileset.unions [
+        publisherScripts = [
           ./scripts/build-ownership-snapshot.sh
           ./scripts/build-latest-ownership-snapshot.sh
           ./scripts/publish-ownership-snapshot.sh
           ./scripts/check-ownership-snapshot-freshness.sh
         ];
+        ownershipPublisher = pkgs.stdenvNoCC.mkDerivation {
+          pname = "idx-ownership-publisher";
+          version = cargoManifest.package.version;
+          src = lib.fileset.toSource {
+            root = ./.;
+            fileset = lib.fileset.unions publisherScripts;
           };
           nativeBuildInputs = [ pkgs.makeWrapper ];
           dontBuild = true;
           installPhase = ''
             runHook preInstall
             mkdir -p "$out/libexec/idx-ownership" "$out/bin"
-            cp scripts/*.sh "$out/libexec/idx-ownership/"
+            cp ${lib.concatMapStringsSep " " (script: "scripts/${baseNameOf script}") publisherScripts} \
+              "$out/libexec/idx-ownership/"
             chmod +x "$out"/libexec/idx-ownership/*.sh
             patchShebangs "$out/libexec/idx-ownership"
 

scripts/check-ownership-snapshot-freshness.sh

@@ -38,6 +38,10 @@ if ! [[ "$MAX_AGE_DAYS" =~ ^[0-9]+$ ]]; then
     echo "--max-age-days must be a non-negative integer" >&2
     exit 2
 fi
+if ! [[ "$NOW" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}$ ]] || ! date -u -d "$NOW" +%s >/dev/null 2>&1; then
+    echo "--now must be a YYYY-MM-DD date" >&2
+    exit 2
+fi
 
 if [[ "$MANIFEST_URL" =~ ^https?:// ]]; then
     manifest="$(curl --fail --silent --show-error --location --retry 3 "$MANIFEST_URL")" || {
@@ -49,8 +53,8 @@ else
 fi
 
 as_of="$(jq -r '.snapshot.latest_as_of_date // empty' <<< "$manifest" 2>/dev/null || true)"
-if [[ -z "$as_of" ]]; then
-    echo "FRESHNESS: manifest has no snapshot.latest_as_of_date" >&2
+if ! [[ "$as_of" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}$ ]] || ! date -u -d "$as_of" +%s >/dev/null 2>&1; then
+    echo "FRESHNESS: manifest has no valid snapshot.latest_as_of_date ('$as_of')" >&2
     exit 3
 fi
 

scripts/publish-ownership-snapshot.sh

@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 
-set -Eeuo pipefail
+set -euo pipefail
 
 usage() {
     cat <<'EOF'
@@ -43,6 +43,17 @@ FORCE="0"
 PUBLISH_WORKDIR=""
 STAGE="arguments"
 
+# Explicit failures: print the reason, then the RESULT line, then exit.
+fail() {
+    local status="$1"
+    shift
+    printf '%s\n' "$*" >&2
+    printf 'RESULT: FAILED stage=%s (exit %s)\n' "$STAGE" "$status" >&2
+    exit "$status"
+}
+
+# Unexpected command failures (set -e). errtrace is deliberately off: with
+# it, a failure inside $(...) would run this in the subshell and the parent.
 on_error() {
     local status=$?
     printf 'RESULT: FAILED stage=%s (exit %s)\n' "$STAGE" "$status" >&2
@@ -89,30 +100,26 @@ while [[ $# -gt 0 ]]; do
             exit 0
             ;;
         *)
-            echo "unknown argument: $1" >&2
             usage >&2
-            exit 2
+            fail 2 "unknown argument: $1"
             ;;
     esac
 done
 
 if [[ -z "$OUTPUT_DIR" ]]; then
-    echo "--output-dir is required" >&2
     usage >&2
-    exit 2
+    fail 2 "--output-dir is required"
 fi
 
 if ! command -v gh >/dev/null 2>&1; then
-    echo "gh is required for GitHub release upload" >&2
-    exit 1
+    fail 1 "gh is required for GitHub release upload"
 fi
 
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 BUILDER="$SCRIPT_DIR/build-latest-ownership-snapshot.sh"
 
 if [[ ! -x "$BUILDER" ]]; then
-    echo "required helper script is missing or not executable: $BUILDER" >&2
-    exit 1
+    fail 1 "required helper script is missing or not executable: $BUILDER"
 fi
 
 mkdir -p "$OUTPUT_DIR"
@@ -146,13 +153,12 @@ if [[ "$BUILD_FIRST" == "1" ]]; then
     cargo build
 fi
 
+STAGE="preflight"
 if ! "$IDX_BIN" version >/dev/null 2>&1; then
-    echo "failed to run idx binary: $IDX_BIN" >&2
-    echo "build the CLI first or pass --idx-bin <path>" >&2
-    exit 1
+    fail 1 "failed to run idx binary: $IDX_BIN; build the CLI first or pass --idx-bin <path>"
 fi
 
-STAGE="check-published"
+# Best effort: a missing or unreadable published manifest means "publish".
 PUBLISHED_MANIFEST="$(published_manifest)"
 PUBLISHED_AS_OF="$(jq -r '.snapshot.latest_as_of_date // empty' <<< "$PUBLISHED_MANIFEST" 2>/dev/null || true)"
 
@@ -190,8 +196,7 @@ printf 'Preparing latest ownership snapshot artifacts...\n'
 
 STAGED_MANIFEST_PATH="$PUBLISH_WORKDIR/ownership-snapshot-manifest.json"
 if [[ ! -f "$STAGED_MANIFEST_PATH" ]]; then
-    echo "manifest was not generated: $STAGED_MANIFEST_PATH" >&2
-    exit 1
+    fail 1 "manifest was not generated: $STAGED_MANIFEST_PATH"
 fi
 
 shopt -s nullglob
@@ -200,12 +205,14 @@ existing_snapshot_paths=("$OUTPUT_DIR"/ownership-snapshot-*.sqlite)
 shopt -u nullglob
 
 if [[ "${#sqlite_matches[@]}" -ne 1 ]]; then
-    echo "expected exactly one SQLite artifact in $PUBLISH_WORKDIR" >&2
-    exit 1
+    fail 1 "expected exactly one SQLite artifact in $PUBLISH_WORKDIR"
 fi
 
 STAGED_SQLITE_PATH="${sqlite_matches[0]}"
-BUILT_AS_OF="$(jq -r '.snapshot.latest_as_of_date' "$STAGED_MANIFEST_PATH")"
+BUILT_AS_OF="$(jq -r '.snapshot.latest_as_of_date // empty' "$STAGED_MANIFEST_PATH")"
+if ! [[ "$BUILT_AS_OF" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}$ ]]; then
+    fail 1 "built manifest has no valid snapshot.latest_as_of_date: '$BUILT_AS_OF'"
+fi
 
 # Post-build check covers legacy PDF sources, whose as-of date is only known
 # after import.