From d7e0eaf7643ca63f227c01082936a6efc6aa3677 Mon Sep 17 00:00:00 2001
From: Dotta <bippadotta@protonmail.com>
Date: Fri, 26 Jun 2026 18:23:15 +0000
Subject: [PATCH] Address paperclip page publish review

Co-Authored-By: Paperclip <noreply@paperclip.ing>
---
 skills/paperclip-page/scripts/publish.sh      | 12 +--
 .../paperclip-page/scripts/publish.test.mjs   | 89 ++++++++++++++++++-
 2 files changed, 94 insertions(+), 7 deletions(-)

diff --git a/skills/paperclip-page/scripts/publish.sh b/skills/paperclip-page/scripts/publish.sh
index 8e295bbcee0..2aa3a356add 100755
--- a/skills/paperclip-page/scripts/publish.sh
+++ b/skills/paperclip-page/scripts/publish.sh
@@ -318,6 +318,10 @@ fi
 prefix="$(join_prefix "$default_prefix" "$slug")"
 target_exists=0
 
+if [[ "$update" == "1" ]]; then
+  assert_update_ownership "$source_dir" "$bucket" "$prefix"
+fi
+
 if [[ "$dry_run" == "0" ]]; then
   if object_exists "$bucket" "$prefix"; then
     target_exists=1
@@ -343,10 +347,6 @@ if [[ "$dry_run" == "0" ]]; then
 
     [[ "$target_exists" == "0" ]] || die "could not find an unused generated slug after 5 attempts"
   fi
-
-  if [[ "$target_exists" == "1" && "$update" == "1" ]]; then
-    assert_update_ownership "$source_dir" "$bucket" "$prefix"
-  fi
 fi
 
 url="${base_url}/${prefix}"
@@ -373,11 +373,11 @@ aws_cli s3 sync "$source_dir/" "s3://$bucket/$prefix" \
   --cache-control 'public,max-age=60' \
   --only-show-errors
 
-curl -fsSIL --max-time 20 "$url" >/dev/null
-
 source_hash="$(compute_source_hash "$source_dir")"
 write_state "$source_dir" "$bucket" "$prefix" "$slug" "$url" "$base_url" "$source_hash"
 
+curl -fsSIL --max-time 20 "$url" >/dev/null
+
 cat <<EOF
 paperclip-page published
 mode: $mode
diff --git a/skills/paperclip-page/scripts/publish.test.mjs b/skills/paperclip-page/scripts/publish.test.mjs
index cea51b0dfaa..8ef4b47bd2a 100644
--- a/skills/paperclip-page/scripts/publish.test.mjs
+++ b/skills/paperclip-page/scripts/publish.test.mjs
@@ -1,24 +1,46 @@
 import assert from "node:assert/strict";
 import { execFileSync } from "node:child_process";
 import {
+  chmodSync,
+  existsSync,
   mkdirSync,
   mkdtempSync,
+  readFileSync,
+  rmSync,
   statSync,
   writeFileSync,
 } from "node:fs";
 import { tmpdir } from "node:os";
 import { dirname, join } from "node:path";
 import { fileURLToPath } from "node:url";
-import test from "node:test";
+import { after, test } from "node:test";
 
 const scriptPath = join(dirname(fileURLToPath(import.meta.url)), "publish.sh");
+const tempDirs = new Set();
+
+after(() => {
+  for (const dir of tempDirs) {
+    rmSync(dir, { force: true, recursive: true });
+  }
+});
 
 function createSite(name = "paperclip-page-test") {
   const siteDir = mkdtempSync(join(tmpdir(), `${name}-`));
+  tempDirs.add(siteDir);
   writeFileSync(join(siteDir, "index.html"), "<!doctype html><title>Paperclip</title>\n");
   return siteDir;
 }
 
+function writeExecutable(path, body) {
+  writeFileSync(path, body, { mode: 0o755 });
+  chmodSync(path, 0o755);
+}
+
+function writeState(siteDir, state) {
+  mkdirSync(join(siteDir, ".paperclip-page"), { recursive: true });
+  writeFileSync(join(siteDir, ".paperclip-page", "state.json"), `${JSON.stringify(state)}\n`);
+}
+
 function runPublish(args, env = {}) {
   try {
     return {
@@ -73,6 +95,24 @@ test("dry run normalizes a safe default prefix", () => {
   assert.match(result.output, /^url: https:\/\/pages\.example\.test\/reports\/launches\/demo-page\/$/m);
 });
 
+test("dry run update requires matching local ownership state", () => {
+  const siteDir = createSite();
+  const missingState = runPublish([siteDir, "--slug", "demo-page", "--update", "--dry-run"]);
+
+  assert.notEqual(missingState.status, 0);
+  assert.match(missingState.output, /requires ownership state/);
+
+  writeState(siteDir, {
+    bucket: "paperclip-pages-test",
+    prefix: "demo-page/",
+  });
+
+  const result = runPublish([siteDir, "--slug", "demo-page", "--update", "--dry-run"]);
+
+  assert.equal(result.status, 0);
+  assert.match(result.output, /^mode: update$/m);
+});
+
 test("rejects nested slugs", () => {
   const result = runPublish([createSite(), "--slug", "nested/path", "--dry-run"]);
 
@@ -90,3 +130,50 @@ test("rejects hidden files in the source tree", () => {
   assert.notEqual(result.status, 0);
   assert.match(result.output, /hidden files and dot paths are not allowed/);
 });
+
+test("live publish writes state before URL verification", () => {
+  const siteDir = createSite();
+  const binDir = mkdtempSync(join(tmpdir(), "paperclip-page-bin-"));
+  tempDirs.add(binDir);
+
+  writeExecutable(
+    join(binDir, "aws"),
+    `#!/usr/bin/env bash
+set -euo pipefail
+while [[ "$1" == "--region" || "$1" == "--profile" ]]; do
+  shift 2
+done
+if [[ "$1" == "s3api" ]]; then
+  echo "None"
+  exit 0
+fi
+if [[ "$1" == "s3" && "$2" == "sync" ]]; then
+  exit 0
+fi
+echo "unexpected aws call: $*" >&2
+exit 1
+`,
+  );
+  writeExecutable(
+    join(binDir, "curl"),
+    `#!/usr/bin/env bash
+set -euo pipefail
+echo "simulated CloudFront propagation miss" >&2
+exit 22
+`,
+  );
+
+  const result = runPublish([siteDir, "--slug", "demo-page"], {
+    AWS_REGION: "us-east-1",
+    PATH: `${binDir}:${process.env.PATH}`,
+  });
+
+  assert.notEqual(result.status, 0);
+  assert.match(result.output, /simulated CloudFront propagation miss/);
+  assert.equal(existsSync(join(siteDir, ".paperclip-page", "state.json")), true);
+
+  const state = JSON.parse(readFileSync(join(siteDir, ".paperclip-page", "state.json"), "utf8"));
+  assert.equal(state.bucket, "paperclip-pages-test");
+  assert.equal(state.prefix, "demo-page/");
+  assert.equal(state.url, "https://pages.example.test/demo-page/");
+});