[frankbria]

Closes #1523.

What changes

• Generators leave the web root. site/make-og-image.sh and site/make-banner.py move to tools/site/. Everything under site/ is now servable, so a deploy copies the directory as-is — no exclude list to remember, which is the thing the first draft of Automate codeframe.sh site deploy (pull-based, no tailnet credential in CI) #1523 got wrong. make-og-image.sh now cds into ../../site, so its inputs and outputs are unchanged.
• Pull-based deploy. tools/site/deploy.sh runs on the server: pulls the public repo (no credential), keeps one rolling backup, mirrors site/ into the web root with rsync --delete, writes the deployed commit to .deployed-sha.
• Trigger is manual: tailscale ssh root@frank-prod-server deploy-codeframe-site. Setup and usage in tools/site/README.md.

Against #1523's acceptance criteria

Criterion
Shallow clone at /opt/codeframe-site	created by the script on first run
Deploy script in this repo, installed to the box	tools/site/deploy.sh → /usr/local/bin/deploy-codeframe-site
Web root byte-identical to site/ at origin/main	rsync -a --delete; nothing to exclude any more
Deployed commit reportable	/var/www/codeframe.sh/.deployed-sha
Removed/renamed asset disappears	--delete
make-og-image.sh not reachable over HTTP	not in site/ at all
Trigger decided and documented	manual; tools/site/README.md
No long-lived credential in repo secrets	none added; public clone

.well-known/ in the web root is excluded from --delete in case certificate renewal ever writes there.

Verification

• bash -n passes on deploy.sh and make-og-image.sh
• py_compile passes on make-banner.py
• site/ contains no .sh or .py
• ran tools/site/make-og-image.sh from its new location: exit 0, wrote site/og-image.png at 1200x630; regenerated binaries discarded, not committed
• uv run pytest tests/ci/ -q → 81 passed in 1.76s

🤖 Generated with Claude Code

https://claude.ai/code/session_019cEymaSpmZ1arjngxx1VNM

[github-actions]

Claude finished @frankbria's task in 5m 58s —— View job

---

Third-party bug-hunt review

Method: read the full diff against origin/main and every changed file in full (tools/site/deploy.sh, tools/site/README.md, tools/site/make-og-image.sh, tools/site/make-banner.py, site/index.html, .claude/.codebase-info/directory-structure.md), checked existing PR comments to avoid repeats, and verified referenced paths (site/ inputs for make-og-image.sh, tests/ci/test_site_1465.py invariants, repo-wide references to the moved scripts).

The earlier review already covered the unguarded rsync --delete, missing deploy locking, stale CHANGELOG.md references, and chown -R touching .well-known/ — not repeated here. One new finding survived verification:

Severity	Location	Finding
minor	tools/site/deploy.sh:22-23	Rolling backup is taken unconditionally — a no-op re-run right after a bad deploy archives the broken docroot and destroys the last-good rollback copy. Inline suggestion adds an already-deployed guard.

Everything else checked out clean: the make-og-image.sh cd ../../site resolves correctly from every documented invocation, its ffmpeg inputs (codeframe-lockup-reverse.png, proof9-loop.gif) exist in site/, make-banner.py takes all paths via argv so the move breaks nothing, site/ now contains only servable files, and the rsync excludes are correctly anchored. No critical or major defects found.

Model: k3[1m]
· Branch: site-deploy

[claude]

Review

Moving the generators out of site/ and deploying by pull is a sound approach, and the README is clear. I read the diff and grepped for stale references to the moved scripts. I didn't run deploy.sh. Findings, most important first:

1. rsync --delete has no guard against an empty or missing source (should fix).
If $SRC/site/ is ever absent or empty, rsync -a --delete wipes the web root. That could happen after a bad merge that renames site/, or a partial clone. set -e doesn't help, because rsync exits 0 on an empty source. A cheap guard before the rsync:

[ -f "$SRC/site/index.html" ] || { echo "refusing: $SRC/site/index.html missing" >&2; exit 1; }

2. No locking and a non-atomic publish (minor).
Two concurrent runs would race on the git reset, the tar backup and the rsync. Visitors can also see a half-synced tree during a deploy. For a small static page, flock -n /var/lock/deploy-codeframe-site at the top is enough. A real atomic swap isn't worth it here.

3. Stale doc references (minor).

• CHANGELOG.md:39 still says site/make-banner.py.

• CHANGELOG.md:31 mentions the bare make-og-image.sh.

  The first is a historical entry, so leaving it is arguably correct. A one-line note under the next Unreleased heading would still help anyone searching for the script.

• .claude-pr/.claude/.codebase-info/directory-structure.md is already updated. It's a stray copy of the tracked file, though. Check that it isn't committed by accident.

4. Deploy script details (nits).

• chown -R over the whole docroot also touches .well-known/, which you intentionally excluded from --delete. Using rsync --chown=www-data:www-data would limit ownership changes to the synced files.
• The backup tarball has no $SRC or .deployed-sha handling. That's fine, but the README says "one rolling backup" without saying how to restore. A one-line restore command would help during an incident.
• git reset --hard origin/main always deploys the tip of main, not the commit that was reviewed. That's consistent with a manual trigger, but you could document it.

Tests: there's no automated coverage, which is reasonable for a shell script on the server. tests/ci/ could cheaply pin that site/ contains no *.sh or *.py files, so a generator can't drift back into the web root. This is the invariant #1523 cares about.

None of this is blocking, but I'd add the guard in #1 before merging, since --delete against a bad source is the one destructive failure mode.

[github-actions]

Claude finished @frankbria's task in 3m 8s —— View job

---

Third-party bug-hunt review (synchronize)

• Read the full diff against origin/main (merge commit 0b5ccf0 changed none of the PR's files — verified with git diff 13aaa64..HEAD)
• Checked existing PR comments to avoid repeats (unguarded rsync --delete, missing flock, stale CHANGELOG.md refs, chown -R on .well-known/, unconditional backup clobbering last-good — all already posted)
• Verified the moved scripts' path assumptions (cd ../../site in make-og-image.sh, argv-only paths in make-banner.py) and repo-wide references (docs/, workflows, tests/ci/test_site_1465.py invariants — all clean)
• Re-analyzed deploy.sh line by line for fetch/reset, rsync exclude, and set -e failure ordering

One new finding survived verification:

Severity	Location	Finding
minor	tools/site/deploy.sh:22-23	First deploy on a fresh server aborts at the tar backup because nothing ensures $DOCROOT exists yet — the script's own bootstrap path (clone-if-missing) fails one step later. Inline suggestion guards the backup.

Everything else checked out clean: the shallow fetch --depth 1 origin main + reset --hard origin/main pattern updates the remote-tracking ref correctly, the rsync excludes are anchored right and protect the destination .deployed-sha from --delete, a failed rsync aborts before the sha file is written (fail-closed), site/ contains only servable files, and no stale references to the moved scripts remain outside the already-flagged CHANGELOG.md. No critical or major defects found.

Model: k3[1m]
· Branch: site-deploy

[claude]

Follow-up review

The diff hasn't changed since the two earlier reviews. The findings below are still open, and I'm not adding new ones.

Still worth fixing before merge

1. Empty-source guard. rsync -a --delete from a missing or empty $SRC/site/ exits 0 and wipes the docroot. Add [ -f "$SRC/site/index.html" ] || { echo 'refusing: site/index.html missing' >&2; exit 1; } before the backup and rsync.
2. Rollback backup. The tarball is taken on every run. Re-running right after a bad deploy overwrites the last-good copy with the broken one. Skip the backup and rsync when .deployed-sha already equals origin/main. Add a one-line restore command to the README.
3. Locking. Wrap the script in exec 9>/var/lock/deploy-codeframe-site; flock -n 9 || exit 1.

Nits

• Use rsync --chown=www-data:www-data in place of chown -R, so .well-known/ isn't touched.
• Add a note about the moved scripts under the next Unreleased heading in CHANGELOG.md. The historical entries that mention them can stay as they are.

The rest looks fine. make-og-image.sh still resolves site/ correctly after the cd, make-banner.py takes its paths via argv, and the doc and comment references are updated. I didn't run deploy.sh. The PR's bash -n check doesn't exercise the empty-source or re-run cases.

🤖 Generated with Claude Code

[github-actions]

Claude finished @frankbria's task in 3m 8s —— View job

---

Third-party bug-hunt review (synchronize, 191baed)

• Reviewed the new commit 191baed in full (git diff 0b5ccf0..HEAD) — it reworks the backup condition, adds mkdir -p, env-overridable paths, and README updates
• Verified both prior inline findings are fixed: same-commit re-run now skips the backup while still syncing, and a first deploy no longer aborts at tar ([ -d "$DOCROOT" ] guard + mkdir -p)
• Re-walked the fixed logic for residual holes: set -e/set -u interaction with the $(cat … 2>/dev/null) test, sha written only after a successful rsync (fail-closed), rev-parse failure aborts before any state change
• Confirmed the skip logic preserves the last-good backup across the documented bad-deploy → re-run sequence, and that a sha-matching re-run still repairs a hand-edited root
• Rechecked unchanged parts against the new code: fetch/reset pattern, rsync excludes (anchored, protect .deployed-sha from --delete), site/ contains only servable files, no stale references to the moved scripts outside the already-flagged CHANGELOG.md

One new finding survived verification:

Severity	Location	Finding
minor	tools/site/deploy.sh:28	Re-run after an interrupted deploy (rsync died before .deployed-sha was rewritten) still archives the half-synced docroot over the last-good backup — the same-commit skip only covers completed deploys. Impact limited: the public repo is itself the rollback. No cheap local fix; a README caveat may suffice.

No critical or major defects. The earlier reviews' open items (empty-source rsync guard, flock locking, chown -R touching .well-known/) were not re-flagged per the no-repeat rule.

Model: k3[1m]
· Branch: site-deploy

[claude]

Follow-up review

The rolling-backup guard and the first-deploy case are now handled in deploy.sh. The README matches the script. Two items from earlier reviews are still open:

1. Empty-source guard (should fix). rsync -a --delete exits 0 when $SRC/site/ is missing or empty, so it would wipe the docroot. It would also write .deployed-sha as if the deploy had worked. Add this before the backup:

   [ -f "$SRC/site/index.html" ] || { echo "refusing: $SRC/site/index.html missing" >&2; exit 1; }

2. Locking (minor). Two concurrent runs would race on the reset, the tar and the rsync. Add exec 9>/var/lock/deploy-codeframe-site; flock -n 9 || exit 1 near the top.

Nit: rsync --chown=www-data:www-data would replace chown -R, which also touches .well-known/.

I read the diff and did not run deploy.sh. Nothing else new from me.

🤖 Generated with Claude Code

[github-actions]

Claude finished @frankbria's task in 2m 57s —— View job

---

Third-party bug-hunt review (synchronize, 2d89720)

• Reviewed the merge commit 2d89720 — only main's README.md and site/index.html doc updates; PR diff vs origin/main unchanged otherwise. Merge resolution verified clean.
• Checked existing PR comments to avoid repeats (empty-source rsync guard, flock, chown -R on .well-known/, interrupted-deploy backup clobber — all already posted)
• Re-read deploy.sh, README.md, make-og-image.sh, make-banner.py in full; verified the moved scripts' path assumptions (cd ../../site, argv-only paths) and repo-wide references

One new finding survived verification:

Severity	Location	Finding
minor	tools/site/deploy.sh:29	tar writes the backup in place — a mid-write failure (disk full, read error) aborts the deploy and truncates the last-good backup, destroying the rollback artifact even though nothing was deployed. Inline suggestion writes to a temp file and renames on success.

No critical or major defects. The backup skip logic, fail-closed sha ordering, and rsync excludes re-checked clean against the current code.

Model: k3[1m]
· Branch: site-deploy

[claude]

Follow-up review

The diff matches what the earlier reviews covered, so I'm not adding new findings. The backup-skip and first-deploy fixes in deploy.sh look right.

Still open:

1. Empty-source guard (should fix). rsync -a --delete exits 0 when $SRC/site/ is missing or empty, so it wipes the docroot. The script then writes .deployed-sha as if the deploy had worked. Add [ -f "$SRC/site/index.html" ] || { echo 'refusing: site/index.html missing' >&2; exit 1; } before the backup.
2. Locking (minor). Add exec 9>/var/lock/deploy-codeframe-site; flock -n 9 || exit 1 so two concurrent runs can't race.
3. Interrupted deploy (minor, from the last bot review). If rsync dies before .deployed-sha is rewritten, the old sha still matches. A re-run skips the backup, so the next deploy can archive a half-synced root over the last-good copy.
4. Nit. rsync --chown=www-data:www-data would avoid chown -R touching .well-known/. A CHANGELOG note about the moved scripts would also help.

I didn't run deploy.sh.

🤖 Generated with Claude Code

[github-actions]

Third-party bug-hunt review (synchronize, 2d89720)

• Reviewed the merge commit 2d89720 — it brought in only main's README.md and site/index.html doc updates; the PR's diff vs `origin/main` is unchanged except the one-line og-image comment. Merge resolution verified clean (no conflict artifacts).
• Re-read `deploy.sh`, `README.md`, `make-og-image.sh`, `make-banner.py` in full against the already-posted findings (empty-source rsync guard, flock, chown on .well-known/, interrupted-deploy backup clobber) — none re-flagged per the no-repeat rule.
• Verified the moved scripts: `make-banner.py` takes all paths via argv (lines 115, 191), `make-og-image.sh` resolves `cd ../../site` correctly and its ffmpeg inputs exist in `site/`. No repo references to the old paths outside the already-flagged CHANGELOG.md.

One new finding survived verification:

Severity	Location	Finding
minor	`tools/site/deploy.sh:29`	`tar` writes the backup in place — a mid-write failure (disk full, read error) aborts the deploy and truncates the last-good backup, destroying the rollback artifact even though nothing was deployed. Inline suggestion writes to a temp file and renames on success.

No critical or major defects. The backup skip logic, fail-closed sha ordering, and rsync excludes all re-checked clean against the current code.

Model: k3[1m]

[github-actions]

⚠️ Third Party Review did not complete

This run did not finish, so there is no verdict for this commit.
That is not the same as a clean bill of health — a review that ran
and found nothing says so explicitly ("✅ Third Party Review: no defects
found.").

Reason: the run was cancelled before the agent finished — usually a newer push superseding it, since this workflow cancels in-progress runs for the same pull request. Nothing is wrong with the review itself: the newest commit is reviewed by its own run, which posts its verdict as a separate comment. This notice is not replaced by that run and can be disregarded once it appears.

Model: k3[1m]

View job run — the agent transcript is attached there as a
glm-review-transcript-* artifact when the agent got far enough to
write one.

What this comment held when the run stopped

Third-party bug-hunt review (synchronize, 1288f72)

• Review the new commit 1288f72 (interrupted-deploy marker) in full
• Check existing PR comments to avoid repeats
• Verify findings against the checked-out code
• Post inline comments + summary

View job run

[claude]

Follow-up review

The .incomplete marker closes the interrupted-run hole from the last round. It's created before the rsync and removed only after chown, and a leftover marker suppresses the backup. The README describes this accurately.

Still open from my earlier reviews, with no change in deploy.sh:

1. Empty-source guard (should fix). rsync -a --delete from a missing or empty $SRC/site/ exits 0 and wipes the docroot. It then writes .deployed-sha as if the deploy had worked. Add this after rev-parse and before the backup:

   [ -f "$SRC/site/index.html" ] || { echo "refusing: $SRC/site/index.html missing" >&2; exit 1; }

2. Locking (minor). exec 9>/var/lock/deploy-codeframe-site; flock -n 9 || exit 1. This matters a little more now: two overlapping runs would share the .incomplete marker, and the first to finish would remove it while the second is still syncing.

Nit: rsync --chown=www-data:www-data instead of chown -R.

I read the diff and did not run deploy.sh. Nothing else new.

🤖 Generated with Claude Code

[claude]

Follow-up review

The temp-file-and-rename for the tar backup is in, and so is the .incomplete marker. Both look right. Nothing in deploy.sh has changed on the two items still open from my earlier reviews:

1. Empty-source guard (should fix). rsync -a --delete from a missing or empty $SRC/site/ exits 0 and wipes the docroot. The script then writes .deployed-sha as if the deploy had worked. Add this after rev-parse and before the backup:

   [ -f "$SRC/site/index.html" ] || { echo "refusing: $SRC/site/index.html missing" >&2; exit 1; }

2. Locking (minor). Add exec 9>/var/lock/deploy-codeframe-site; flock -n 9 || exit 1. Two overlapping runs share the .incomplete marker, so the first to finish removes it while the second is still syncing.

Nits: rsync --chown=www-data:www-data would avoid chown -R touching .well-known/. A CHANGELOG note about the moved scripts would also help.

I read the diff and did not run deploy.sh.

🤖 Generated with Claude Code

[github-actions]

Claude finished @frankbria's task in 2m 26s —— View job

---

Third-party bug-hunt review (synchronize, 72a168d)

• Reviewed the new commit 72a168d (tar temp-file + rename) — fix verified clean
• Checked existing PR comments to avoid repeats (empty-source guard, flock, chown on .well-known/, interrupted-deploy marker — none re-flagged)
• Re-walked the marker/backup/sha state machine for kill-at-every-line cases
• Posted inline finding + summary comment

One new finding:

Severity	Location	Finding
minor	tools/site/deploy.sh:44-46	.deployed-sha is written before the final fallible step (chown); a chown failure strands the .incomplete marker, silently suppressing every future rolling backup while the sha claims success.

Full summary posted as a separate review comment with the inline suggestion on the diff.

Model: k3[1m] · · Branch: site-deploy

[github-actions]

Third-party bug-hunt review (synchronize, 72a168d)

• Reviewed the new commit 72a168d (tar temp-file + rename) in full — fix verified: a failed tar now aborts with the previous backup untouched, and a stale `$BACKUP.tmp` is overwritten by the next run
• Checked existing PR comments to avoid repeats (empty-source rsync guard, flock, chown -R on .well-known/, interrupted-deploy marker — all already posted, none re-flagged)
• Re-walked the full marker/backup/sha state machine in deploy.sh for the kill-at-every-line cases: killed pre-marker re-takes an identical backup (no loss); killed mid-rsync, post-rsync-pre-sha, and post-sha-pre-rm all recover correctly with the last-good backup intact
• Re-verified the unchanged files: make-og-image.sh cd ../../site resolution and ffmpeg inputs, argv-only paths in make-banner.py, anchored rsync excludes, README matches script behavior

One new finding survived verification:

Severity	Location	Finding
minor	tools/site/deploy.sh:44-46	.deployed-sha is written before the final fallible step (chown); a chown failure (e.g. www-data missing on a fresh server) strands the .incomplete marker, which silently suppresses every future rolling backup while the recorded sha claims success. Inline suggestion moves chown ahead of the success markers.

No critical or major defects. The backup-skip logic, temp-file tar, and fail-closed sha ordering all re-checked clean against the current code.

Model: k3[1m]

[github-actions]

Claude finished @frankbria's task in 4m 26s —— View job

---

Third-party bug-hunt review (synchronize, 4d6b91b) — done

• Reviewed merge commit 4d6b91b — changed none of the PR's files; PR diff vs origin/main unchanged since 72a168d
• Checked existing comments to avoid repeats (empty-source rsync guard, flock, chown -R on .well-known/, sha-before-chown marker stranding — all still open, none re-flagged)
• Re-walked the marker/backup/sha state machine in deploy.sh for kill-at-every-line cases
• Re-verified moved scripts and repo-wide references

One new finding posted inline (minor): tools/site/deploy.sh:40-41 — mkdir -p "$DOCROOT" runs before the .incomplete marker is created; a run killed in that gap with a previously-absent docroot leaves an empty dir the next run's backup guard archives over the last-good backup. Suggestion swaps the two lines.

No critical or major defects. Full summary posted as a separate comment.

Model: k3[1m] · Branch: site-deploy ·

[claude]

Follow-up review

deploy.sh is unchanged on the points below. I read the diff and did not run the script.

Still open

1. Empty-source guard (should fix). If $SRC/site/ is missing or empty, rsync -a --delete exits 0 and wipes the docroot. The script then writes .deployed-sha as if the deploy had worked. Add this after rev-parse:

   [ -f "$SRC/site/index.html" ] || { echo "refusing: $SRC/site/index.html missing" >&2; exit 1; }

2. chown ordering (minor). .deployed-sha is written before chown, and .incomplete is removed after it. If chown fails (for example, www-data is missing on a fresh box), the marker stays and the sha already reports success. Every later run then skips the rolling backup. Run chown before the sha write, or use rsync --chown=www-data:www-data. That also stops chown -R from touching .well-known/.
3. Locking (minor). Add exec 9>/var/lock/deploy-codeframe-site; flock -n 9 || exit 1. Two overlapping runs share the .incomplete marker, so the first to finish clears it while the second is still syncing.

Nit: add a CHANGELOG note under Unreleased saying the generators moved to tools/site/.

The make-og-image.sh cd ../../site change and the make-banner.py docstring update are correct.

🤖 Generated with Claude Code

[github-actions]

Third-party bug-hunt review (synchronize, 4d6b91b)

• Reviewed the merge commit `4d6b91b` — it changed none of the PR's files (`git diff 72a168d..HEAD` on `tools/site/`, `site/`, `.claude/` is empty); merge resolution verified clean
• Checked existing PR comments to avoid repeats (empty-source rsync guard, `flock`, `chown -R` on `.well-known/`, sha-before-chown marker stranding — all still open, none re-flagged per the no-repeat rule)
• Re-walked the full marker/backup/sha state machine in `deploy.sh` for the kill-at-every-line cases, including the paths added since the marker landed
• Re-verified unchanged files: `make-og-image.sh` `cd ../../site` resolution and ffmpeg inputs exist in `site/`, argv-only paths in `make-banner.py`, anchored rsync excludes, no stale references to the moved scripts outside the already-flagged `CHANGELOG.md` (`.claude-pr/` is an untracked local artifact, not part of the PR)

One new finding survived verification:

Severity	Location	Finding
minor	`tools/site/deploy.sh:40-41`	`mkdir -p "$DOCROOT"` runs before the `.incomplete` marker is created — a run killed in that gap with a previously-absent docroot leaves an empty dir that the next run's backup guard reads as a complete deploy, archiving the empty root over the last-good backup. Inline suggestion swaps the two lines.

No critical or major defects. The tar temp-file fix, the interrupted-run marker, and the fail-closed sha ordering all re-checked clean against the current code.

Model: k3[1m]

[github-actions]

Claude finished @frankbria's task in 4m 35s —— View job

---

I'll analyze this and get back to you.

[claude]

Follow-up review

The chown-before-sha ordering is now fixed: chown runs before .deployed-sha is written, and the .incomplete marker is removed last. The moved scripts' paths still resolve (cd ../../site), and I found no stale references to the old locations. I read the diff and did not run the script.

Two items from earlier reviews are still open:

1. Empty-source guard (should fix). If $SRC/site/ is missing or empty, rsync -a --delete exits 0 and wipes the docroot. The script then records .deployed-sha as if the deploy had worked. Add this after rev-parse:

   [ -f "$SRC/site/index.html" ] || { echo "refusing: $SRC/site/index.html missing" >&2; exit 1; }

2. Locking (minor). Add exec 9>/var/lock/deploy-codeframe-site; flock -n 9 || exit 1. Two overlapping runs share the .incomplete marker, so the first to finish removes it while the second is still syncing.

Everything else looks good to merge once item 1 is addressed.

🤖 Generated with Claude Code

[github-actions]

Third-party bug-hunt review (synchronize, 48ec7f8)

• Reviewed the new commit `48ec7f8` in full (`git diff 4d6b91b..HEAD`) — the `www-data` pre-check and the chown-before-sha reorder
• Verified both prior inline findings are fixed: a `chown` failure now leaves the previous sha and the marker for the next run (reorder), and a missing `www-data` is an up-front refusal before any backup or sync
• Re-walked the full marker/backup/sha state machine for kill-at-every-line cases against the reordered code: kill post-rsync/pre-chown, post-chown/pre-sha, and post-sha/pre-`rm` all recover with the last-good backup intact; a stranded marker with an equal sha self-clears on the next run
• Checked existing PR comments to avoid repeats (empty-source rsync guard, `flock`, `chown -R` on `.well-known/`, mkdir-before-marker gap at lines 44–45 — all still open, none re-flagged)
• Re-verified the unchanged files: `site/` contains only servable files (no `.well-known/`, so the rsync exclude blocks nothing today), `make-og-image.sh` `cd ../../site` resolution and ffmpeg inputs, argv-only paths in `make-banner.py`

One new finding survived verification:

Severity	Location	Finding
minor	`tools/site/deploy.sh:20-22`	The clone-once bootstrap does not recover from a partial clone: a run killed mid-clone leaves a wedged `$SRC/.git`, and every later run skips the clone and aborts at `git fetch` until `/opt/codeframe-site` is removed by hand — the one kill-window in the script that doesn't self-heal. Inline suggestion re-clones when the checkout is unusable.

No critical or major defects. The `www-data` refusal, the success-recording reorder, and the README's new prerequisite list all re-checked clean against the current code.

Model: k3[1m]

[github-actions]

Claude finished @frankbria's task in 2m 34s —— View job

---

Third-party bug-hunt review (synchronize, 8d473e2)

• Reviewed the new commit 8d473e2 in full (git diff 4d6b91b..HEAD) — the touch/mkdir swap
• Verified the fix: the .incomplete marker now lands before the first change to the web root, closing the kill-window from the previous round
• Re-walked the marker/backup/sha state machine for kill-at-every-line cases — all recover with the last-good backup intact
• Checked existing PR comments to avoid repeats; re-verified the unchanged files

✅ No defects found in 8d473e2. One status note: the "Fixed in 8d473e2" reply landed on the partial-clone thread, but that suggestion (deploy.sh:20-22) was not applied — the commit implements the mkdir/marker fix.

Full summary posted as a separate comment.

Model: k3[1m] · Branch: site-deploy

[claude]

Follow-up review

I read the current deploy.sh and did not run it. The marker, backup and chown ordering all look right, and the moved scripts' paths still resolve (cd ../../site).

One item from earlier reviews is still open, and it is the one I'd fix before merge.

Empty-source guard. If $SRC/site/ is missing or empty, rsync -a --delete exits 0 and empties the docroot. The script then records .deployed-sha as a successful deploy. This can happen after a bad merge that renames site/. Add this after rev-parse:

[ -f "$SRC/site/index.html" ] || { echo "refusing: $SRC/site/index.html missing" >&2; exit 1; }

Locking (minor). Two overlapping runs share the .incomplete marker, so the first to finish removes it while the second is still syncing. exec 9>/var/lock/deploy-codeframe-site; flock -n 9 || exit 1 fixes that. The trigger is manual, so this is low risk.

🤖 Generated with Claude Code

[github-actions]

Third-party bug-hunt review (synchronize, 8d473e2)

• Reviewed the new commit 8d473e2 in full (git diff 4d6b91b..HEAD) — the touch/mkdir swap
• Verified the fix: the .incomplete marker now lands before the first change to the web root, so a kill in the former mkdir→touch gap leaves the marker set and the next run's backup guard skips — the last-good backup survives. Verified against the guard at lines 36-37.
• Re-walked the full marker/backup/sha state machine for kill-at-every-line cases against the reordered code: killed post-touch/pre-mkdir (root absent or present), mid-rsync, post-rsync/pre-chown, post-chown/pre-sha, and post-sha/pre-rm all recover with the last-good backup intact; a stranded marker self-clears on the next completed run
• Checked existing PR comments to avoid repeats (empty-source rsync guard, flock, chown -R on .well-known/, partial-clone recovery — all already posted, none re-flagged)
• Re-verified the unchanged files: README matches the script, make-og-image.sh cd ../../site resolution, argv-only paths in make-banner.py, anchored rsync excludes

✅ Third Party Review: no defects found in 8d473e2.

One status note, not a new finding: the reply "Fixed in 8d473e2 … the marker is now set before mkdir" was posted on the partial-clone recovery thread (deploy.sh:20-22), but that commit implements the mkdir/marker fix — the clone bootstrap is unchanged and has no rev-parse re-clone path. That inline suggestion is still unapplied; the reply appears to have landed on the wrong thread.

Model: k3[1m]

[github-actions]

Claude finished @frankbria's task in 2m 33s —— View job

---

Third-party bug-hunt review (synchronize, cc4e4f3)

• Reviewed the new commit cc4e4f3 in full (git diff 8d473e2..HEAD) — the partial-clone recovery guard at deploy.sh:20-25
• Verified the fix against git's clone ordering: refs are written only after all objects are fetched, so a clone killed mid-fetch leaves an unborn HEAD (rev-parse -q --verify HEAD fails → re-clone); a kill during checkout leaves refs valid, the guard correctly passes, and reset --hard origin/main repairs the working tree
• Verified the narrowing of the original suggestion: rm -rf fires only when $SRC/.git exists as a directory with no commit, so a non-checkout directory is never deleted (git clone then refuses the non-empty target and the run stops), and a healthy checkout whose fetch fails keeps its commit and is preserved
• Re-walked the kill-at-every-line cases for the new code: killed during rm -rf leaves either no .git (plain re-clone) or a still-commit-less .git (guard fires again) — both self-heal; killed during the re-clone is the same state the guard was written for
• Checked existing PR comments to avoid repeats (empty-source rsync guard, flock, chown -R on .well-known/ — all already posted by earlier reviews, none re-flagged)
• Confirmed the README still matches the script; the deploy.sh change needed no doc update

✅ Third Party Review: no defects found in cc4e4f3.

This closes the last kill-window in the script's state machine that didn't self-heal — the partial-clone thread from the previous round is now fixed as suggested (with a sound narrowing). Status note resolved: the fix reply that had landed on the wrong thread is now backed by this commit on the correct one.
· Branch: site-deploy

[claude]

Follow-up review

I read the current deploy.sh and did not run it. The chown-before-sha ordering, the .incomplete marker, the tar temp-and-rename and the www-data pre-check all look right. The moved scripts' paths still resolve (cd ../../site).

One item from my earlier reviews is still open, and I'd fix it before merge.

Empty-source guard (should fix). If $SRC/site/ is missing or empty, rsync -a --delete exits 0 and empties the docroot. The script then records .deployed-sha as a successful deploy. A bad merge that renames site/ would do it. Add this after rev-parse:

[ -f "$SRC/site/index.html" ] || { echo "refusing: $SRC/site/index.html missing" >&2; exit 1; }

Locking (minor). Two overlapping runs share the .incomplete marker, so the first to finish removes it while the second is still syncing. exec 9>/var/lock/deploy-codeframe-site; flock -n 9 || exit 1 near the top would fix that.

Tests: the PR reports tests/ci/ passing, but nothing exercises deploy.sh. A small CI test that runs it against temp SRC/DOCROOT (the env overrides exist for this) would pin the empty-source and backup-skip behaviour.

🤖 Generated with Claude Code

[github-actions]

Third-party bug-hunt review (synchronize, cc4e4f3)

• Reviewed the new commit cc4e4f3 in full — the partial-clone recovery guard at deploy.sh:20-25
• Verified the fix against git's clone ordering: a clone killed mid-fetch leaves an unborn HEAD (guard fires, re-clone); a kill during checkout leaves refs valid, the guard passes, and reset --hard origin/main repairs the working tree
• Verified the narrowing of the original suggestion: rm -rf fires only on a .git directory with no commit — a non-checkout directory is never deleted (clone refuses the non-empty target), a healthy checkout with a failed fetch is preserved
• Re-walked kill-at-every-line cases for the new code — all self-heal
• Checked existing PR comments to avoid repeats; README still matches the script

✅ Third Party Review: no defects found in cc4e4f3. The partial-clone thread from the previous round is fixed as suggested, closing the last non-self-healing kill-window in the script's state machine.

Model: k3[1m]