Skip to content

Deploy to an environment and prove the release is the one answering - #16

Merged
ptr727 merged 11 commits into
developfrom
feature/two-environment-publishing
Aug 4, 2026
Merged

Deploy to an environment and prove the release is the one answering#16
ptr727 merged 11 commits into
developfrom
feature/two-environment-publishing

Conversation

@ptr727

@ptr727 ptr727 commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Why

The repo could address a single deploy root, which was enough for one mirror and is not enough for a staging site and a production site. make-release.sh also sourced its environment file with set -a, which overwrites a DEPLOY_ROOT the caller exported, so selecting an environment by exporting that variable silently deployed to the original root. On a host serving two sites that failure is not an error, it publishes to the other site.

What this adds

Per-environment configuration. ENV_FILE selects the file, the first argument still overrides the root, and a named file that does not exist is a hard failure rather than a fall-through to the ambient environment.

A deploy that proves itself. A release reaches a host by rsync with no restart, so nothing outside the verifier observes whether the rules answering are the rules shipped. Content follows the current symlink per request and is live immediately, while rules wait on an in-process config reload. The bundle now stamps its own version as X-Blog-Release, and check-live-urls.sh compares it against EXPECT_RELEASE before requesting any of the 1,245 URLs. It waits rather than sampling once, because the reload is asynchronous.

Environment identification. X-Blog-Env and X-Robots-Tag come from container variables, since every environment serves one bundle on one port and only the container distinguishes them. X-Robots-Tag defaults to the value that is harmless on production: an unset staging container is still behind its auth gate, where production inheriting noindex would deindex the site silently.

Auth-gate support. Staging keeps authentication on, so the checker presents a resource access token through a mode-600 curl config file rather than -H arguments, which keeps the credential out of ps across 1,245 requests and is the only form that survives the export -f the parallel checks run under.

The deploy workflows. deploy-site-task.yml is the repo-owned leaf that takes an environment input and names no host; deploy-site.yml validates through the same gate the pull request and a release use, asserts production deploys only from main, and calls the leaf.

Verification

Both local mirrors deploy with no restart and pass PASS - 1245 URLs honored. The transport was proven against the real remote host through its confined key: dry run, 601 MB upload, the site still serving the previous release between upload and flip, then the flip.

Every new gate was demonstrated failing before being trusted, per the rule in TODO.md:

  • half a credential pair exits 2
  • a failed preflight exits 1 and names whether a token was sent
  • a wrong environment refuses to check the contract
  • a container that never reloads fails after the timeout rather than passing against a stale config
  • a dangling symlink reports as a broken symlink rather than as a release mismatch

Notes

caddy run --watch stops watching permanently after one failed config load and logs nothing further, so anything that breaks current even briefly, including a test, ends that container's ability to pick up releases until it is restarted. The failure reports as healthy throughout, which is why the verifier names it as a likely cause.

trusted_proxies excludes the bridge gateway. Trusting the subnet trusts the host, verified by forging a client address from it.

🤖 Generated with Claude Code

ptr727 and others added 2 commits August 4, 2026 08:57
The release tooling could name a single deploy root, which was enough while
there was one mirror and will not be once the VPS grows a staging site and a
production site. Nothing here could address two at a time.

The blocker was not the obvious one. make-release.sh sourced secrets/.env
with `set -a`, which exports every assignment in the file and overwrites a
DEPLOY_ROOT the caller exported first, so sourcing a different environment
file silently deployed to the original root. On a host serving two sites that
failure is not an error, it publishes to the other site. ENV_FILE now selects
the file, the first argument still overrides the root because it is read
afterwards, and a named file that does not exist is a hard failure rather
than a fall-through to whatever the ambient environment holds.

check-live-urls.sh could not authenticate, so it could not check a site that
keeps its auth gate on. Staging keeps its gate on deliberately: it serves a
byte-identical copy of the public site, and an open one is a duplicate handed
to every crawler, which is the one thing a migration about preserving URLs
should not do. A Pangolin resource access token opens it instead.

The credential travels in a mode-600 curl config file rather than in -H
arguments. A command line is readable in ps for the life of the process and
this runs 1,245 of them, and the config file is also the only form that
survives the `export -f` the parallel checks run under, since bash cannot
export an array. It is sent to the base URL's own origin and nowhere else, so
a rule that one day redirects off-site cannot carry it away. A preflight
request runs first, because behind an auth gate a wrong token fails every URL
and the output then reads as a site that has vanished rather than as a bad
credential.

Two decisions the docs asserted the opposite of, both now recorded with their
reasoning rather than silently reversed. One deploy key covers both
environments instead of one per environment: the split pays off only where
the two keys never share a machine, and both sit on one workstation and in
one secret store. The cost is that the rrsync forced command can no longer
separate the environments, so the deploy roots move under a common parent and
one pinned command covers both. And staging takes the VPS wildcard at
blog.vps.insanegenius.net, which needs no new certificate and closes the open
FQDN decision.

Verified against both local mirrors, which are two containers in a sibling
repository's new blog stack: 1,245 URLs honored on each, each baking its own
base URL, neither disturbing the other's release history. Each new gate was
demonstrated failing before being trusted, per the rule in TODO.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A release reaches a host by rsync and no restart, so nothing outside the
verifier observes whether the rules that answer are the rules shipped.
Content follows the current symlink per request and is live immediately,
while rules wait on an in-process config reload. A container that never
reloads therefore serves new content under a previous release's redirects,
and the URL contract passes against a config that was never deployed.

The bundle stamps its own version, substituted at install time, and the
check compares it before requesting any of the 1,245 URLs. It waits rather
than sampling once, because the reload is asynchronous and a check run
straight after a deploy races it. The timeout still catches a container
that never converges.

Caddy also stops watching permanently after one failed config load, without
logging that it has given up, so anything that breaks the symlink even
briefly ends that container's ability to pick up releases until it is
restarted. The failure reports as healthy, which is why the verifier names
both causes.

Two more headers identify the environment, since every environment serves
one bundle on one port and only the container distinguishes them. Robots
defaults to the value that is harmless on production, because the failures
are asymmetric: an unset staging container is still behind its auth gate,
where production inheriting noindex would deindex the site silently.

trusted_proxies takes its ranges from the container, and excludes the
bridge gateway. Trusting the subnet trusts the host, which was verified by
forging a client address from it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 4, 2026 19:46

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the blog’s release tooling and deployment workflows to support distinct staging/production environments, and adds an end-to-end verification step that proves the deployed config (not just content) is what’s currently answering requests.

Changes:

  • Add per-environment configuration selection (ENV_FILE) and safety guards in deploy/make-release.sh.
  • Add deploy verification headers (X-Blog-Release, X-Blog-Env, X-Robots-Tag) and strengthen checks/check-live-urls.sh to preflight environment/release correctness (including async reload waiting and auth-gate support).
  • Introduce GitHub Actions deploy workflows (deploy-site.yml + reusable deploy-site-task.yml) that validate, deploy, and verify per environment.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
TODO.md Updates deployment planning notes/decisions reflecting multi-environment deploy.
spec/secrets.json Documents staging-only Pangolin access token secrets and updated environment secret rationale.
README.md Refreshes project links/badges and clarifies issue/reporting links.
OPERATIONS.md Documents the multi-environment model, reload/watch behavior, auth-gate verification, and ownership boundaries.
deploy/README.md Expands the deploy/serve contract (watch reload, env identification, trusted proxies, auth-gate variables).
deploy/make-release.sh Adds ENV_FILE selection, remote-root guardrails, and release stamping into shipped Caddyfile.
deploy/env.example Updates environment template for multi-env usage and adds Pangolin + verification variables.
deploy/Caddyfile Adds trusted proxy config and response headers for env/robots/release identification.
checks/check-live-urls.sh Adds auth-gate support, preflight diagnostics, environment assertion, and release/reload convergence check.
.github/workflows/deploy-site.yml Adds a dispatcher workflow to validate + deploy to a chosen environment with gating rules.
.github/workflows/deploy-site-task.yml Adds the reusable deploy job: build bundle, rsync upload/flip, then verify live contract.

Comment thread deploy/make-release.sh
Comment thread deploy/Caddyfile
Comment thread deploy/env.example Outdated
A relative ENV_FILE resolved against the caller's working directory while
the default resolved against the repo, so the same name meant different
files depending on where the script ran.

The container comment described a restart as the way a config change goes
live, which the in-process reload replaced.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 4, 2026 19:57

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

README.md:89

  • This section still says local configuration comes from secrets/.env, but the repo now supports one file per environment selected via ENV_FILE (e.g. secrets/staging.env). Updating this avoids steering operators toward a single-environment setup that no longer matches the deploy tooling/docs.
The deploy root and the base URL come from an untracked `secrets/.env`, copied from [deploy/env.example][env-example]. The whole `secrets/` directory is gitignored, so host-specific values stay out of the published history.

spec/secrets.json:30

  • This manifest lists PANGOLIN_ACCESS_TOKEN_{ID,} under environments.secrets, which implies they must exist for both staging and production, but secretsNote says they are staging-only and absent in production. As written, this is internally inconsistent and will mislead any secrets-name audit that treats environments.secrets as required per environment.
    "secrets": [
      "DEPLOY_SSH_PRIVATE_KEY",
      "PANGOLIN_ACCESS_TOKEN_ID",
      "PANGOLIN_ACCESS_TOKEN"
    ],

Comment thread .github/workflows/deploy-site-task.yml
The manifest listed the access token beside a credential every environment
carries, which reads as required in both. A name audit would then report it
missing from production, where it is deliberately absent.

The README described configuration as coming from one file, which the
per-environment selection replaced.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 4, 2026 20:33
@ptr727

ptr727 commented Aug 4, 2026

Copy link
Copy Markdown
Owner Author

Both suppressed comments were valid and are fixed in aff8581.

spec/secrets.json — the inconsistency was real, and the consequence you named is the one that matters: an audit treating environments.secrets as required per environment would report the token missing from production, where it is deliberately absent. secrets now lists only what every environment carries, and a separate environmentSecrets map names the per-environment difference explicitly.

README.md — corrected. It described configuration as coming from a single secrets/.env, which the per-environment selection replaced.

That second one is the more useful catch, because this repo has a rule requiring exactly the sweep I did not do: when a behavior changes, grep for prose asserting the old one, since no linter flags a claim that is merely untrue. I updated OPERATIONS.md and deploy/README.md and stopped there. I have now swept every remaining reference; the others correctly describe secrets/.env as the default, which it still is.

Noting for the record that these two arrived in the review body under Suppressed comments (2) rather than as line threads, so the reviewThreads API reported zero unresolved while both were outstanding. That is a gap in how I checked, not in how you reported, and it is now written into this repo's traps.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (2)

deploy/make-release.sh:154

  • The release-stamp substitution only fails when the placeholder remains after sed. If deploy/Caddyfile is edited in the future and the @@RELEASE@@ placeholder is removed entirely, this script will silently produce an unstamped bundle, and check-live-urls.sh will later fail with a misleading “watcher is dead / no X-Blog-Release header” message. Add an explicit pre-check that the placeholder exists before attempting substitution.
# Stamp the release into the config it ships with, so a response names the rules answering.
# A stale config otherwise passes the URL contract against rules that were never shipped.
sed -i "s/@@RELEASE@@/$VERSION/" "$STAGE/Caddyfile"
if grep -q "@@RELEASE@@" "$STAGE/Caddyfile"; then
	echo "release stamp was not substituted into the shipped Caddyfile" >&2
	exit 1
fi

.github/workflows/deploy-site-task.yml:33

  • inputs.environment is treated as an arbitrary string (used for the GitHub Environment name and interpolated into remote rsync paths). deploy-site.yml constrains the workflow_dispatch UI, but deploy-site-task.yml can also be invoked by other workflows; an unexpected value can lead to confusing new environments (no secrets) or path injection on the remote forced-command root. Add an explicit allowlist check at the start of the job steps.
    environment: ${{ inputs.environment }}

sed reports success when it matches nothing, so a Caddyfile that lost the
placeholder shipped unstamped and the live check then blamed a dead config
watcher for a bundle that never carried a release id.

The environment name selects a GitHub Environment and lands in a remote
path, and a workflow_call caller is not bound by the dispatch choice list.
It is asserted in its own job, because the environment binding resolves
before any step in the deploying job runs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 4, 2026 20:42
@ptr727

ptr727 commented Aug 4, 2026

Copy link
Copy Markdown
Owner Author

Both suppressed comments were valid and are fixed in 4ba6bb3.

make-release.sh release stamp — the sharper of the two, because the failure you describe is one this repo hit today. sed reports success when it matches nothing, so a Caddyfile that lost the placeholder shipped unstamped, and check-live-urls.sh then reported an absent X-Blog-Release as a dead config watcher. That is a correct bundle, a correct server, and a diagnostic pointing at neither.

The placeholder is now asserted in the source before substituting. Demonstrated both directions:

placeholder removed:  deploy/Caddyfile carries no @@RELEASE@@ placeholder to stamp   (exit 1)
placeholder present:  ==> done: probe live
                      X-Blog-Release "probe"

deploy-site-task.yml environment name — valid, and the reachability argument is the right one: the dispatch choice list constrains the entry point, not a workflow_call caller. The name selects a GitHub Environment and is interpolated into the remote path under the deploy key's confinement root.

Asserted against an allowlist, in its own job that the deploying job needs, because a job-level environment: binding resolves before any step in that job runs. A step inside the deploying job would have selected the unknown environment first and only then refused. The value reaches the shell through env: rather than direct interpolation.

Both findings again arrived as suppressed comments in the review body. I checked for those this round, and initially misread the result: the REST API reports the reviewer as copilot-pull-request-reviewer[bot] while GraphQL omits the suffix, so a filter written in the GraphQL form matched nothing and an empty result read as a clean review. Recorded alongside the earlier note.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (3)

checks/check-live-urls.sh:69

  • The redirect check accepts both 301 and 308, but the failure message says "expected 301". This can mislead debugging (especially if the contract is later updated to prefer 308s).
	case "$code" in
	301 | 308) ;;
	*)
		echo "redirect $url expected 301, got $code" >>"$FAILED"
		return

deploy/make-release.sh:24

  • The usage text still implies the deploy root only comes from the ambient environment or secrets/.env, but the script now supports per-environment files via ENV_FILE. Updating the message will prevent confusion when a user relies on ENV_FILE and sees usage output on failures.
# The deploy root and the base URL are the only host-specific values, and they pair per environment.
# ENV_FILE selects the environment, because `set -a` overwrites a value the caller exported.
# The first argument overrides the root, being read after this.
DEFAULT_ENV_FILE="$REPO/secrets/.env"
ENV_FILE="${ENV_FILE:-$DEFAULT_ENV_FILE}"

deploy/README.md:54

  • This section first instructs copying env.example to secrets/<environment>.env, but the default file the scripts read when ENV_FILE is unset is secrets/.env. A reader who follows only this first instruction for a single-environment setup could end up with secrets/production.env that is never read, leading to confusing failures.
The deploy root and the base URL are the only host-specific values, and they pair per
environment. Copy [`env.example`](./env.example) to `secrets/<environment>.env` and set both.
`secrets/` is gitignored as a whole directory, so a value naming one machine cannot reach a
public repo by being added to a file nobody remembered to ignore. CI passes them explicitly

The redirect failure named one status while the check accepted two, and the
usage text described a deploy root the environment file selection replaced.

The setup instruction named only the per-environment form, so a single
environment produced a file nothing reads.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 4, 2026 20:50
@ptr727

ptr727 commented Aug 4, 2026

Copy link
Copy Markdown
Owner Author

All three suppressed comments were valid and are fixed in 0f03139.

check-live-urls.sh — the message named one status while the check accepted two. Now reads expected 301 or 308.

make-release.sh usage — it described a deploy root coming from the environment or a fixed file, which the ENV_FILE selection replaced. It now names ENV_FILE and its default, and states that a relative path resolves against the repo:

usage: deploy/make-release.sh [deploy-root] [version]
       deploy-root defaults to DEPLOY_ROOT, from the environment or $ENV_FILE
       ENV_FILE defaults to secrets/.env, and a relative path resolves against the repo

deploy/README.md — the best of the three, because it is a trap rather than a wording slip. Following that instruction for a single environment produces secrets/production.env, which nothing reads, and the failure surfaces later as a deploy against the wrong root rather than as a missing file. The section now says to copy to secrets/.env, that this is the file read when ENV_FILE is unset, and that further environments add named files beside it.

These three are all the same defect as the two before them: I changed how configuration is selected and updated the places I was editing, without sweeping the prose that described the old behavior. This repo has a rule requiring that sweep precisely because no linter catches a claim that is merely untrue, and three consecutive rounds of your findings have been instances of it.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Comment thread checks/check-live-urls.sh
The base URL has its trailing slash stripped, so a prefix test also matched
a host that merely starts with it. A redirect to a lookalike registered as
an attacker's subdomain would have received the credential.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 4, 2026 20:57
@ptr727

ptr727 commented Aug 4, 2026

Copy link
Copy Markdown
Owner Author

Valid, and the best finding of this review. Fixed in 3cf9c34.

The base URL has its trailing slash stripped at the top of the script, so the prefix test accepted any host merely beginning with it. A redirect to a lookalike an attacker controls would have received the credential.

Demonstrated against the old and new comparisons:

old=SEND new=SEND  https://<base>/all/
old=SEND new=SEND  https://<base>
old=SEND new=skip  https://<base>.evil.example/steal
old=SEND new=skip  https://<base>-evil.example/steal
old=skip new=skip  https://elsewhere.example/x

The match now requires an origin boundary, accepting the base exactly or followed by /.

Worth naming why this one is sharper than a normal prefix bug: the guard existed specifically to keep the credential on its own origin, so it was the one line where a near-miss was worth nothing. It also reads as correct, which is why it survived review by me and by a reader who knew what it was for.

Verified the tightened check does not reject legitimate destinations, using the run that actually sends the token: against the live staging site behind the auth gate, all 1,245 URLs pass, and that path follows every one of the 917 redirect destinations rather than trusting their status codes.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (1)

deploy/make-release.sh:60

  • VERSION is treated as an arbitrary string but is used as (1) a directory name under "$ROOT/releases/$VERSION", (2) the current symlink target ("releases/$VERSION"), and now (3) a literal substitution into the shipped Caddyfile. A crafted value containing '/', '..', or other special characters can break the deploy layout (or traverse outside releases/) and can also break the Caddyfile stamp substitution.
# CI passes the version so a release directory traces back to a commit rather than to a clock.
VERSION="${2:-$(date -u +%Y%m%d-%H%M%S)}"

The value becomes a directory name, a symlink target, and a sed
replacement, so a separator, a traversal, or a substitution metacharacter
would place the release outside releases/ or corrupt the release stamp.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 4, 2026 21:07
@ptr727

ptr727 commented Aug 4, 2026

Copy link
Copy Markdown
Owner Author

Valid. Fixed in 421737e.

You are right that the value reaches three sinks with different rules, and the sed one is the newest and least obvious: a /, &, or \ corrupts the substitution rather than failing it. Constrained to A-Z a-z 0-9 . _ -, rejecting .. and a leading -.

Tested every shape:

'../escape'        -> rejected
'a/b'              -> rejected
'with space'       -> rejected
'amp&sed'          -> rejected
'back\slash'       -> rejected
'-leading'         -> rejected
''                 -> accepted, falls back to the timestamp
'20260804-190007'  -> accepted
'1.0.11'           -> accepted
'b5d3744'          -> accepted

The last three are the forms actually used: the timestamp default, an NBGV version, and a short commit SHA.

Both mirrors still pass 1245 URLs honored after the change.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (1)

deploy/make-release.sh:171

  • sed -i is not portable (BSD/macOS sed requires an argument to -i), but this repo’s governance states development is cross-platform by default (Windows/macOS/Linux). Using sed without in-place editing avoids local runs failing on macOS when stamping the Caddyfile.
sed -i "s/@@RELEASE@@/$VERSION/" "$STAGE/Caddyfile"
if grep -q "@@RELEASE@@" "$STAGE/Caddyfile"; then
	echo "release stamp was not substituted into the shipped Caddyfile" >&2
	exit 1
fi

The scripts use GNU and bash 4.4 constructs that a stock macOS lacks, and
the platform rule requires a narrowed platform to be recorded so it reads
as a dependency ceiling rather than an oversight.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 4, 2026 21:13
@ptr727

ptr727 commented Aug 4, 2026

Copy link
Copy Markdown
Owner Author

Declining the change, acting on the premise. Recorded in fd9ffb0.

sed -i is not the script's only GNU dependency, so replacing it would leave the same ceiling with one fewer symptom:

mv -Tf              GNU coreutils, absent from BSD mv
find -printf        GNU findutils
mapfile             bash 4.4+, macOS ships bash 3.2
sed -i              GNU sed

Any one of those fails a stock macOS run. Fixing the newest of the four would make the script look portable while still not running there, which is worse than the current state, because it invites someone to trust it.

The rule you are citing has a second clause that the repo was not honoring: a narrowed platform is recorded, so it reads as a deliberate dependency ceiling rather than an omission. That is the real finding here and it is now in deploy/README.md, naming all four constructs and the reason: every consumer is Linux already, CI builds there, the containers serve there, and the deploy account receives there.

The narrowing axis in that rule is where code executes, never where editing happens, so a macOS or Windows contributor is unaffected for editing and runs these through a container or a remote Linux host.

Making these genuinely portable is possible and is a real change with a real cost, and it would be justified by a contributor who needs to run a deploy natively on macOS, not by the syntax of one line.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (1)

checks/check-live-urls.sh:100

  • The preflight request suppresses curl errors (-s) and doesn’t check curl’s exit status. If DNS/TLS/connect fails, preflight_headers becomes empty and the script reports a misleading HTTP status failure (and may suggest auth/symlink causes). Make the preflight curl run with -sS and fail explicitly on curl errors so the output reflects transport problems correctly.
preflight_headers=$(curl -s -o /dev/null -D- -w '%{http_code}' --max-time 30 "${AUTH[@]}" "$BASE/")
preflight="${preflight_headers##*$'\n'}"
header_of() { printf '%s' "$preflight_headers" | grep -i "^$1:" | tr -d '\r' | sed 's/^[^:]*: *//'; }

A name that does not resolve produced a status code, which the preflight
then explained as a bad credential or a broken symlink.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 4, 2026 21:19
@ptr727

ptr727 commented Aug 4, 2026

Copy link
Copy Markdown
Owner Author

Valid. Fixed in aec533c.

A transport failure did surface as a status code, and the diagnosis that followed was then confidently wrong: it offered a bad credential or a dangling symlink as causes for a host that never answered. Both branches exist to explain a reply, so neither applies when there was none.

The preflight now separates the two, with curl's own message rather than a paraphrase:

FAIL preflight: https://no-such-host.invalid/ could not be reached, so nothing below was checked
     curl: (6) Could not resolve host: no-such-host.invalid

FAIL preflight: http://127.0.0.1:9/ could not be reached, so nothing below was checked
     curl: (7) Failed to connect to 127.0.0.1 port 9 after 0 ms: Could not connect to server

This is the third finding in this review of the same shape, and they are worth naming together: a check that reports the wrong cause is more expensive than one that reports nothing, because it sends someone to the wrong layer with confidence. The dangling-symlink message and the dead-watcher message were both added for that reason, and this one had the defect they were built to avoid.

Verified against all three environments afterwards, including the live staging site behind the auth gate: 1245 URLs honored on each.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged commit 4b2def3 into develop Aug 4, 2026
5 checks passed
@ptr727
ptr727 deleted the feature/two-environment-publishing branch August 4, 2026 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants