Skip to content

chore(main): release 0.71.0 - #1

Open
github-actions[bot] wants to merge 1 commit into
mainfrom
release-please--branches--main
Open

chore(main): release 0.71.0#1
github-actions[bot] wants to merge 1 commit into
mainfrom
release-please--branches--main

Conversation

@github-actions

Copy link
Copy Markdown

This release is too large to preview in the pull request body. View the full release notes here: https://github.com/rlei-odes/Nexus-Stack/blob/release-please--branches--main--release-notes/release-notes.md

@github-actions
github-actions Bot force-pushed the release-please--branches--main branch 4 times, most recently from 17e9c8d to 411220f Compare March 29, 2026 12:41
@github-actions github-actions Bot changed the title chore(main): release 0.31.0 chore(main): release 0.40.0 Apr 1, 2026
@github-actions
github-actions Bot force-pushed the release-please--branches--main branch from 411220f to 7c81562 Compare April 1, 2026 20:49
@github-actions github-actions Bot changed the title chore(main): release 0.40.0 chore(main): release 0.43.0 Apr 2, 2026
@github-actions
github-actions Bot force-pushed the release-please--branches--main branch from 7c81562 to 472599e Compare April 2, 2026 21:52
@github-actions github-actions Bot changed the title chore(main): release 0.43.0 chore(main): release 0.46.0 Apr 11, 2026
@github-actions
github-actions Bot force-pushed the release-please--branches--main branch from 472599e to ee2b94f Compare April 11, 2026 10:08
@github-actions github-actions Bot changed the title chore(main): release 0.46.0 chore(main): release 0.52.0 Apr 22, 2026
@github-actions
github-actions Bot force-pushed the release-please--branches--main branch 2 times, most recently from cb0b86b to 3026ff3 Compare April 23, 2026 18:36
rlei-odes pushed a commit that referenced this pull request May 1, 2026
…r sync; fix flow-sync targetNamespace

Live SSH probing of the deployed stack (commit 4a012b1, run 24951698827)
surfaced THREE distinct architectural defects in the Kestra-bootstrap
block that all stem from PR stefanko-ch#486 making wrong assumptions about Kestra
OSS v1.0:

1. `/api/v1/secrets/system/<NAME>` PUT → **HTTP 404**.
   Kestra OSS does not have a writable runtime secret API. The
   `/api/v1/namespaces/<ns>/secrets` GET endpoint exists but reports
   `"readOnly": true`. The only secret-feed in OSS is the
   `EnvVarSecretProvider` which reads `SECRET_<NAME>=<base64-value>`
   environment variables at container start. Both the standalone
   GITEA_TOKEN PUT block AND the bulk Infisical→Kestra PUT loop in
   PR stefanko-ch#486 have therefore NEVER worked — they returned 404 silently
   while the FAILED counter stayed at 0 because the prerequisite
   folder-discovery step also no-op'd (see #3).

2. `system.flow-sync` registration → **HTTP 422**.
   Direct API probe response:
       "Invalid entity: sync.targetNamespace: must not be null"
   Kestra v1.0's SyncFlows plugin requires `targetNamespace`
   explicitly. The PR stefanko-ch#486 design assumed the namespace would be
   derived from the subdir layout, which is not how the plugin works.

3. The Infisical→Kestra bulk sync loop **silent no-op**.
   Even if #1 were fixed, the loop body never ran because `jq` is
   not installed on the Hetzner VM. The bash-s heredoc executed
   `jq -r '.folders[]?.name' "$FOLDERS_BODY" 2>/dev/null` on the
   remote; jq is missing → "command not found" → `2>/dev/null`
   swallows the error → `|| echo ""` makes FOLDER_LIST empty →
   zero loop iterations → Pushed=0 / Failed=0 / FetchFailed=0
   (which our existing assertions read as "all good").

This commit replaces the broken architecture wholesale:

- **scripts/deploy.sh** — the standalone GITEA_TOKEN PUT block and
  the bulk Infisical→Kestra API-PUT block (the entire `bash -s`
  heredoc that wrote to /api/v1/secrets/...) are removed. New
  block: build SECRET_<NAME>=<base64-value> lines on the runner
  (jq is available there) by reading every Infisical folder + root
  path, plus a special-case SECRET_GITEA_TOKEN entry (the Gitea
  token is generated post-Gitea-start and isn't in Infisical at
  build_folder() time). Lines are written to a delimited block
  in /opt/docker-server/stacks/kestra/.env on the server, then
  Kestra is force-recreated so the env-var-based secret provider
  picks them up. Re-wait for Kestra ready (auth-aware loop) before
  registering flows.

- **scripts/deploy.sh** SyncFlows YAML — adds `targetNamespace:
  tutorials` (required) and `includeChildNamespaces: true` so
  subdirs under `kestra/flows/` extend the namespace
  (`kestra/flows/sub1/x.yaml` → `tutorials.sub1`).

- **examples/workspace-seeds/kestra/flows/tutorials/r2-taxi-pipeline.yaml**
  → flat to **examples/workspace-seeds/kestra/flows/r2-taxi-pipeline.yaml**
  to match the SyncFlows targetNamespace + includeChildNamespaces
  semantics. Internal description-block path updated.

- **CLAUDE.md / examples/README.md** — diagram and table updated for
  the flat layout. Diagram now shows `kestra/flows/r2-taxi-pipeline.yaml`
  with an explanatory paragraph on the SyncFlows config that maps it
  into namespace `tutorials`.

Operator-visible changes:

- One Kestra cold-restart added per spin-up (~2–4 min). The restart
  is unavoidable because Kestra reads SECRET_* only at process
  startup, not on signal-reload.
- `Configuring Kestra Git sync...` now shows secondary progress lines:
  `Building Kestra secret env from Infisical...`,
  `Wrote N Kestra SECRET_* env-vars to .env (skipped M invalid keys)`,
  `Restarting Kestra to load secrets...`,
  `Waiting for Kestra to come back up...` (with the 10-iteration
  liveness output and HTTP-status snapshots).
- Final outcome line: `✓ Seeded flow tutorials.r2-taxi-pipeline
  registered in Kestra` (or a yellow warning naming the failure mode
  if it's not).
@github-actions github-actions Bot changed the title chore(main): release 0.52.0 chore(main): release 0.55.0 May 1, 2026
@github-actions
github-actions Bot force-pushed the release-please--branches--main branch from 3026ff3 to c665456 Compare May 1, 2026 14:32
rlei-odes pushed a commit that referenced this pull request May 10, 2026
Three Copilot findings, all addressed. One was a real R4-violation that
my round-1 docstring had handwaved away.

1. R4 violation — base64 in argv (real bug):
   The push script piped `printf '%s' '<b64>' | base64 -d | docker
   exec -i ...`. The b64 string was a positional arg to printf, so
   `ps -ef` on the nexus host during the brief exec window would
   expose it — and b64 of S3 secret keys is trivially decodable to
   the secrets themselves. My round-1 reasoning ("argv to printf is
   ok because the *decoded* secret never reaches docker argv") was
   wrong: argv is argv, regardless of which command in the pipeline
   carries it.

   Fix: switched to heredoc form. The b64 travels via `cat <<'NEXUS_
   FS_PUSH_EOF' | base64 -d | docker exec -i ...` — bash writes the
   heredoc body directly to cat's stdin; no fork in the pipeline
   carries the secret in argv visible to remote `ps -ef`. Single-quoted
   delimiter disables variable expansion. Two-tier validation: the
   existing base64-alphabet regex stays the primary guard; an
   additional "delimiter not in payload" check is defence-in-depth
   against a future alphabet-rule widening that could allow
   collision (currently unreachable since the b64 alphabet excludes
   underscores; marked with pragma: no cover).

2. pipefail not enabled (correctness drift):
   Without `set -o pipefail`, a base64 -d failure (corrupt input,
   missing binary) would be masked by docker exec's exit status —
   silent empty-write to config.json while reporting WRITE_OK=true.
   Now `set -o pipefail` is the second line of the rendered script,
   and the pipeline's exit status is captured into $WRITE_RC for the
   subsequent if-check.

3. Test docstring drift (cosmetic, fixed by #1):
   `test_render_filestash_push_script_uses_stdin_for_b64` claimed
   "via stdin to base64 -d" but tested the printf-argv form. Renamed
   to `test_render_filestash_push_script_uses_heredoc_not_argv_for_b64`
   and rewrote the assertion to scan ALL lines: any line containing
   the b64 must equal exactly the b64 (i.e. the heredoc-body line),
   not a printf/echo line that would leak it to argv.

Two new tests:
- `test_render_filestash_push_script_pipefail_enabled` — pins #2
- The renamed `_uses_heredoc_not_argv_for_b64` — pins #1 with a
  much stricter assertion (per-line scan, not just substring).

Coverage: 99.25%, 401 tests passing.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
Round 2 Copilot findings on PR stefanko-ch#530 (6 comments):

- R2 #1: Wetty ssh-agent start now validates the agent is responsive
  (socket present + 'ssh-add -l' rc != 2) before setting AGENT_STARTED=1.
  A silent ssh-agent failure no longer falsely reports success.
- R2 #2a: Wetty ssh-add wrapped in 'if ... ; then KEY_ADDED=1' (no
  unconditional set) so a real ssh-add failure leaves KEY_ADDED=0 in
  RESULT_WETTY for operator visibility.
- R2 #2b: Wetty .env append wrapped in 'if printf ... ; then' with
  fail-fast emitting auth_sock_written=0 on write failure (legacy
  unconditional append + AUTH_SOCK_WROTE=1 lied about success).
- R2 #3: Wetty inline step comments renumbered 2-6 to align with
  the docstring framing (1 = mkdir/chmod precondition; 2-6 produce
  flags).
- R2 #4: _infisical_provision_admin returns rc=1 when ProvisionResult
  has no usable credentials (token / project_id dropped, e.g. invalid
  base64). Avoids deploy.sh printing 'OK' while eval'ing empty
  INFISICAL_TOKEN= / PROJECT_ID= lines.
- R2 #5: render_pg_ducklake_hook docstring now says '30s wall-clock'
  to match the SECONDS-bounded loop (was '15 iterations of 2s').
- R2 #6: _setup_wetty_ssh_agent returns rc=1 when auth_sock_written
  is False — surfaces the fail-fast paths to deploy.sh as a soft
  failure even when the script returns a parseable RESULT line.

Regression tests added:
- test_render_wetty_agent_script_validates_agent_responsiveness
- test_render_wetty_agent_script_fail_fast_on_env_append_failure
- test_render_wetty_agent_script_ssh_add_failure_leaves_key_added_zero
- test_render_wetty_agent_script_step_numbering_aligns_with_docstring
- test_cli_setup_wetty_ssh_agent_returns_1_when_auth_sock_not_written
- test_cli_setup_wetty_ssh_agent_happy_path_returns_0
- test_cli_infisical_provision_admin_returns_1_when_creds_dropped
- test_cli_infisical_provision_admin_returns_0_with_full_creds

Suite: 1015 passed (was 1007), mypy --strict clean, ruff clean.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
Round 3 Copilot findings on PR stefanko-ch#530 (4 comments):

- R3 #1 (init-r2-state.sh bearer header in arglist): dismissed —
  out of scope. The whole script (12+ instances pre-dating this PR)
  uses 'curl -H Authorization: Bearer ...'. Migrating the file-wide
  pattern to mode-600 'curl --config' tempfile is a separate refactor;
  this PR's changes (pre-cleanup loop + per_page=100 fix) follow the
  established convention. To be tracked as a follow-up issue.
- R3 #2 (deploy.sh:452 stderr message): correctness drift. Reword
  rc=1 path from 'produced no parseable result' → 'soft-failed'.
  After R2 #6 the rc=1 path also covers auth_sock_written=0.
- R3 #3 (r2-tokens CLI docstring): correctness drift. Docstring
  said both 'rc=0 even when per-token failures' AND 'rc=1 with at
  least one delete failure'. Implementation does the latter
  ('return 0 if result.is_success else 1'); docstring rewritten to
  match.
- R3 #4 (Wetty .pub guard): real bug edge case. The keygen-skip
  gate only checked $KEY_PATH; if .pub was missing/corrupted (manual
  cleanup, partial write, fs corruption), keygen would skip but
  later 'cat $KEY_PATH.pub' would yield empty PUBKEY, silently
  no-oping the authorized_keys append. Now the gate is OR-of-missing
  AND we 'rm -f' both files first if half-present (ssh-keygen
  refuses to overwrite an existing private key).

Regression test added:
- test_render_wetty_agent_script_regenerates_on_half_present_keypair

Suite: 1016 passed (was 1015), mypy --strict clean, ruff clean,
bash -n scripts/deploy.sh OK.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
Round 4 Copilot findings on PR stefanko-ch#530 (4 comments):

- R4 #1 (setup_wetty_ssh_agent check=False masks transport failure):
  real bug. Wrapper ran ssh.run_script with check=False, so an SSH
  transport failure (rc=255, connection drop) would fall through to
  parse_wetty_agent_result → None → CLI rc=1 (soft fail), violating
  the documented rc=2 'transport failure' contract. Switched to
  check=True so CalledProcessError propagates to the CLI handler's
  rc=2 branch. Safe because the rendered script ALWAYS terminates
  with exit 0 (fail-fast paths emit a parseable RESULT line + exit 0
  before bailing).
- R4 #2 (deploy.sh:452 wording 'produced no parseable result'):
  already fixed in R3 commit 9952233 → 'soft-failed — see stderr
  above'. Copilot reviewed against the prior head; reply points to
  the existing fix.
- R4 #3 + R4 #4 (init-r2-state.sh new pre-cleanup curl Authorization
  headers in argv): same theme as R3 #1 — file-wide refactor pattern.
  The whole script (12+ pre-existing instances) uses the same form;
  applying mode-600 'curl --config' to ONLY my 2 new additions while
  leaving 11 existing ones unchanged would be inconsistent. Will be
  tracked as a dedicated security-hardening follow-up.

Regression test added:
- test_setup_wetty_ssh_agent_propagates_transport_failure

Suite: 1017 passed (was 1016), mypy --strict clean, ruff clean.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
Round 5 Copilot findings on PR stefanko-ch#530 (3 comments):

- R5 #1 (test_secret_sync.py:784 grep assertion mismatch): dismissed —
  Copilot misread the assertion. The trailing " in the Python single-
  quoted string literal is the bash double-quote closer, not part
  of the regex. Renderer at secret_sync.py:406 emits
  'grep -qE "^${KEY_PREFIX}GITEA_TOKEN="' — exactly what the test
  asserts; tests pass.
- R5 #2 (Wetty authorized_keys substring vs full-line match): real
  bug. Comment claimed 'full-line match' but 'grep -qF "$PUBKEY"'
  is a substring grep — a longer existing line containing $PUBKEY
  as substring would false-positive (skip the append). Switched to
  'grep -qFx' (-x = whole-line match) to match the documented
  invariant. Without this, an attacker's authorized_keys entry that
  happens to share a prefix with the wetty pubkey could prevent the
  legitimate pubkey from being appended on re-deploy. Existing
  R-pubkey-dedup test updated + asserts the -F-only form is gone.
- R5 #3 (r2_tokens.py:141 'client_factory' comment vs 'client'
  parameter): correctness drift. Comment referenced a non-existent
  'client_factory' arg; the actual injection seam is the 'client'
  parameter. Reworded the comment.

Suite: 1017 passed, mypy --strict clean, ruff clean.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
Round 6 Copilot finding on PR stefanko-ch#530 (1 comment):

- R6 #1 (parse_provision_result docstring): correctness drift.
  Docstring claimed unparseable RESULT is 'treated as transport-
  failure by the CLI dispatcher', but the actual chain is
  parse_provision_result returns None → provision_admin substitutes
  ProvisionResult(status='not-ready') → CLI maps to rc=1 (soft-fail,
  warn-and-continue), NOT rc=2. Reworded to spell out the full call
  chain + clarify that real transport failures (CalledProcessError /
  OSError) are caught BEFORE this parser and route to rc=2.

Suite: 1017 passed, mypy --strict clean, ruff clean.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
Round 7 Copilot finding on PR stefanko-ch#530 (1 comment):

- R7 #1 (setup_wetty_ssh_agent stderr-forwarding comment): correctness
  drift. Comment said 'Forward non-RESULT stderr lines' but the code
  iterates completed.stdout (SSHClient.run_script defaults to
  merge_stderr=True, so stderr is folded into stdout). Reworded to
  spell out the merge behaviour explicitly: the script's stderr ⚠
  warnings are folded into stdout, the parseable RESULT_WETTY line
  is skipped, everything else is forwarded.

Suite: 1017 passed, ruff/mypy clean.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
Round 8 Copilot findings on PR stefanko-ch#530 (3 comments):

- R8 #1 (__main__.py:2511 'Available:' help text): correctness drift.
  Help text printed for an unknown command was missing the new
  subcommands added across this PR series — 'infisical provision-admin',
  'r2-tokens list/cleanup', 'setup wetty-ssh-agent', and the kestra
  variant of 'secret-sync --stack'. Operators mistyping a command
  couldn't discover them. All four added with their env/arg shapes.
- R8 #2 (services.py:1486 pg_ducklake failure hint): correctness
  drift. Hint said 'check container logs' but the failing
  'docker exec ... psql ...' call has stdout/stderr → /dev/null,
  so the actual psql error never reaches docker logs OR the deploy
  log. Replaced the hint with the exact ssh + docker exec command
  the operator can re-run manually to surface the error (kept the
  /dev/null redirection in the production path so a credential leak
  in the SQL doesn't end up in CI logs).
- R8 #3 (secret_sync.py:85 StackTarget docstring): correctness drift.
  Said jupyter/marimo write 'plaintext KEY=value lines' but the
  renderer emits dotenv-style 'KEY="<escaped-value>"' (sed-escaped
  + double-quoted). Reworded to spell out the actual on-disk
  format + clarify why kestra's base64 form skips quoting.

Suite: 1017 passed, mypy --strict clean, ruff clean.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
…round 1)

Round 1 Copilot findings on PR stefanko-ch#531 (5 actionable + 1 dismissed):

- R1 #1 (firewall.py stale-cleanup MISSING): real bug. When
  firewall_rules is empty (zero_entry mode) OR an operator removes
  a single service's rule from Tofu, the next deploy did NOT clean
  up the stale stacks/<svc>/docker-compose.firewall.yml files.
  stack-sync would still rsync them to the server and compose_runner
  would still '-f'-layer them on every 'docker compose up' — host
  port mappings persisted even though the operator had already
  removed them from Tofu. Legacy bash had the same hole; this PR
  plugs it. write_overrides() now walks stacks/*/docker-compose.firewall.yml
  and deletes any file NOT in the just-written set. RedPanda's
  rendered config/redpanda-firewall.yaml is also removed when
  RedPanda has no firewall ports. configure() always calls
  write_overrides() (even in zero_entry mode) so the cleanup pass
  runs unconditionally. New 'remove_stale=False' opt-out exists for
  the rare back-compat caller. 5 new regression tests covering
  remove-on-zero-entry, remove-on-rule-removed, redpanda-config-removal,
  no-stacks-dir short-circuit, OSError aggregation on unlink.

- R1 #2 (Prefect manifest paths wrong): real bug. The seeded
  prefect.yaml referenced 'prefect/requirements.txt' and
  'prefect/flows/...' but workspace seeds land under nexus_seeds/
  prefix in the user's Gitea repo (per stefanko-ch#501). After git_clone the
  cwd is the repo root, so the actual paths are
  'nexus_seeds/prefect/requirements.txt' and
  'nexus_seeds/prefect/flows/nyc_green_taxi_pipeline.py'. Without
  this fix the Prefect deploy fails on first attempt because the
  paths don't resolve. Updated both the requirements_file and
  the entrypoint, plus the manifest's leading comment to spell
  out the seeded location and the 'cd nexus_seeds/prefect && prefect
  deploy' invocation pattern.

- R1 #3 (Prefect R2 env vars unplumbed): real bug. The Green-Taxi
  flow does os.environ['R2_ENDPOINT'] / 'R2_ACCESS_KEY' /
  'R2_SECRET_KEY' / 'R2_BUCKET' which raises KeyError on a stock
  install — _render_prefect previously only emitted
  PREFECT_DB_PASSWORD + PREFECT_UI_API_URL, and deploy.sh's
  secret-sync only targets Jupyter/Marimo. Extended _render_prefect
  to also write the four R2_* vars from c.r2_data_*; same pattern
  Jupyter uses for HETZNER_S3_*. Empty values are kept as empty
  strings so the flow's 'configure R2 first' check (operator-side)
  can detect the case and emit a clear error.

- R1 #4 (http-fetch-to-r2 description inaccurate): correctness
  drift. Description claimed re-runs land in different keys but
  the hour-stamp granularity means same-hour reruns DO overwrite.
  Reworded to spell out 'idempotent within an hour, sliced into
  separate folders across hours' + pointer to the parallel-fetch
  seed's index-suffix pattern for true append behavior.

- R1 #5 (parallel-http-fetch zero-padding claim wrong):
  correctness drift. Inline comment claimed taskrun.iteration was
  zero-padded for sort stability, but the actual key uses the raw
  value. Reworded to acknowledge it's NOT padded (acceptable for
  the default 3-URL teaching case), with a Pebble 'string.format'
  recipe showing how to switch to '%03d' if the operator extends
  the URL list past 9.

- R1 #0 (CodeQL test_firewall.py URL substring sanitization):
  dismissed — false positive. The flagged line is a Python
  substring assertion in test code on hardcoded test data
  ('redpanda-kafka.example.com'), not URL sanitization in a
  request-handling code path. The string never participates in any
  URL parsing or routing decision; the assertion just verifies the
  template substitution wrote the expected output.

Suite: 1061 passed (was 1053, +8 new firewall cleanup tests),
firewall.py 100% coverage, mypy --strict clean, ruff clean.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
Round 2 Copilot findings on PR stefanko-ch#531 (6 actionable):

- R2 #1 (prefect.yaml description references wrong path): real bug.
  Deployment description still said 'prefect/flows/...' but the
  seeded path is 'nexus_seeds/prefect/flows/...'. Updated to point
  users at the correct location with explicit nexus_seeds/ prefix.

- R2 #2 (Marimo DuckDB seed assumes AWS_*): correctness drift.
  The 'where to go from here' section claimed DuckDB picks up
  AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY automatically — true
  in general, but Marimo's secret-sync uses HETZNER_S3_* naming.
  Replaced with explicit DuckDB SET statements (s3_endpoint,
  s3_access_key_id, s3_secret_access_key, s3_url_style) reading
  from os.environ['HETZNER_S3_*'], same pattern NYC_Taxi_Pipeline.py
  uses for the Spark side.

- R2 #3 (parallel-fetch URL collision): real bug. Kestra's each_parallel
  keys per-iteration outputs by taskrun.value (the loop value), so
  duplicate URLs in 'inputs.urls' collide on outputs.download[<dup>].
  Both download tasks write to the same key; second silently
  overwrites first. The R2 keys themselves stay distinct (idx<N>-<hash>),
  but both objects carry the second download's payload. Documented
  the uniqueness requirement explicitly in the flow description with
  a recovery suggestion (dedupe upstream OR add a leading dedup task).

- R2 #4 (examples/README normative table missing prefect/): correctness
  drift. Added prefect/ to the diagram in R1 but didn't add a row to
  the 'Stick to these names' normative table below. Added entry
  documenting: prefect.yaml manifest location, requirements.txt run-time
  install, flows/ entrypoint files, the 'cd nexus_seeds/prefect &&
  prefect deploy' invocation pattern.

- R2 #5 (deploy.sh local cleanup doesn't propagate): real bug, and
  this one supersedes my R1 #1 fix. The Python firewall.write_overrides
  pass added in R1 only deletes LOCAL stale files. stack-sync
  earlier in deploy.sh uses 'rsync_to_remote' WITHOUT '--delete=True',
  so the equivalent stale files on the SERVER persist after a Tofu
  rule removal. compose_runner keeps '-f'-layering them on every
  'docker compose up' — the host port mapping stays exposed even
  though Tofu was supposed to close it. Added an explicit ssh-side
  cleanup pass in deploy.sh AFTER firewall configure: build a sorted
  newline-separated list of expected '<svc>/docker-compose.firewall.yml'
  rel-paths locally + remotely, comm -23 to find orphans, ssh-rm
  each one. Also covers the rendered redpanda-firewall.yaml.

- R2 #6 (prefect.yaml cd path wrong inside container): correctness
  drift. Manifest comment said 'cd nexus_seeds/prefect && prefect deploy'
  but inside the prefect-worker container the workspace repo is
  cloned at /flows/$REPO_NAME/ (not the user's local checkout
  layout). Reworded to spell out both invocation contexts: from
  a local checkout vs from inside the worker container.

Suite: 1061 passed (no test changes for documentation fixes), bash
-n scripts/deploy.sh OK, ruff/mypy clean.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
Round 3 Copilot findings on PR stefanko-ch#531 (5 actionable). Theme: my R1+R2
fixes had silent-failure paths that made the cleanup work look
'green' when it had partially failed — Copilot dug deeper into the
exact code I changed and surfaced 3 real failure-mode bugs.

- R3 #1 (firewall configure CLI zero-entry path swallowed write.failed):
  real bug. The CLI's zero-entry branch returned rc=0 BEFORE checking
  write.failed, so a failed stale-cleanup unlink (OSError on remote-
  rm or local-unlink) was silently ignored. The whole point of the
  R1 cleanup is to surface failures so the workflow doesn't finish
  green with stale firewall state. Now write.failed is logged AND
  triggers rc=1 even in zero-entry mode. Regression test added.

- R3 #2 (deploy.sh FW_RC=1 was warn-and-continue): real bug. rc=1
  from the Python firewall step means at least one write OR cleanup
  failed, which means the local + remote firewall state is now
  INCONSISTENT with what Tofu requested. Continuing into the
  copy/start phases would either leave a removed port still exposed
  (failed cleanup) or skip a newly-requested port (failed render),
  and the workflow would still finish green. Changed rc=1 from
  yellow-warn to RED-abort with explicit 'state is inconsistent
  with Tofu' message.

- R3 #3 (deploy.sh orphan-removal failure was warn-and-continue):
  real bug, same theme as #2. The new orphan-cleanup ssh-rm pass
  in deploy.sh just printed a yellow warning on rm failure. But
  compose_runner picks up any docker-compose.firewall.yml on the
  remote, so a failed orphan rm leaves the host port exposed
  contrary to Tofu. Changed to: track per-orphan failures in a
  tempfile, abort the whole deploy with red error if any fail.

- R3 #4 (Prefect flow consumes empty R2_* without upfront guard):
  correctness drift / latent bug. After R1 _render_prefect writes
  empty R2_* values when r2_data_* aren't configured (optional
  datalake). Without an upfront check, the first run crashes deep
  inside boto with a confusing 'invalid endpoint URL' / SSL error.
  Added a precondition check at flow entry that raises a clear
  RuntimeError with the exact Tofu fields to set + the workflow
  invocation to re-pick up the values.

- R3 #5 (DuckDB seed cross-reference to Spark-side wrong):
  correctness drift. My R2 fix said 'see NYC_Taxi_Pipeline.py for
  the Spark-side pattern' but those SET s3_* statements there are
  for ITS OWN DuckDB bootstrap (parquet upload), not for the Spark
  read which uses s3a:// + hadoop-aws + Spark-stack settings. Fixed
  to clarify: NYC_Taxi_Pipeline uses the same DuckDB-side SET pattern
  for its bootstrap, then SWITCHES to s3a:// for Spark reads.

Suite: 1062 passed (was 1061, +1 for the new zero-entry-cleanup-
failure regression test), bash -n scripts/deploy.sh OK, mypy strict
clean, ruff clean.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
Round 4 Copilot findings on PR stefanko-ch#531 (9 unique findings):

- R4 #1 (firewall configure CLI zero-entry write.failed): already
  fixed in commit afd21de (R3). Copilot reviewed against the prior
  HEAD; the fix is already on the branch with regression test
  test_cli_firewall_configure_zero_entry_with_stale_cleanup_failure_returns_1.

- R4 #2 (deploy.sh FW_RC=1 non-blocking): already fixed in commit
  afd21de (R3). The case branch now exits 1 with red 'state is
  inconsistent with Tofu' message instead of warn-and-continue.

- R4 #3 (deploy.sh SSH listing failure silently produces empty
  REMOTE_FW_LIST): real bug. The original `|| true` form would
  swallow ssh failures (network blip, expired Cloudflare-Access
  token) and produce an empty list, making comm -23 produce no
  orphans, silently leaving stale firewall files on the server.
  Replaced with explicit rc capture: `REMOTE_FW_RC=0; ... || REMOTE_FW_RC=$?`
  + abort with red error if rc != 0. SSH 'connected, no files'
  remains rc=0 (legitimate empty list); SSH transport failure
  is now distinguished from it and surfaced.

- R4 #4 (deploy.sh redpanda-firewall.yaml rm uses `|| true`):
  real bug, same theme. `rm -f` on the remote is idempotent for
  missing files, so a non-zero rc means SSH transport failed —
  not 'file already gone'. Replaced `|| true` with explicit
  conditional + abort. Without this, a transient SSH failure during
  the redpanda cleanup would leave the external-listener config in
  place and setup_redpanda_hook would keep advertising
  redpanda-kafka.<domain> while the firewall is closed.

- R4 #5 (D1 schema seeds old defaults): correctness drift.
  control-plane/schema.sql:85-86 still seeded server_type='cax31'
  and server_location='fsn1' as initial config-row values.
  control-plane/functions/api/info.js:92-102 reads from D1 first
  with the Tofu values as a secondary fallback, so fresh control-
  plane deployments would display the OLD ARM defaults until a
  later spin-up overwrites the row. Updated to seed cx43/hel1.

- R4 #6 (docs/stacks/overview.md says 'larger sizes available in
  the Control Plane configuration'): correctness drift. The
  Control Plane web UI displays SERVER_TYPE/SERVER_LOCATION but
  has no settings API to edit them. Updated the resource-sizing
  text to point at the correct override path: GitHub repository
  variables (Settings → Secrets and variables → Actions → Variables),
  with an explicit link to setup-guide.md's 'Optional Repository
  Variables' section. The Control Plane displays the value; it
  doesn't let users edit it.

- R4 #7 (docs/stacks/marimo.md missing DuckDB seed entry):
  correctness drift. Added the new Getting_Started_DuckDB.py to
  the canonical Marimo docs page's quickstart section, alongside
  the PySpark + NYC_Taxi seeds. Each notebook gets a one-sentence
  summary explaining when to start there.

- R4 stefanko-ch#8 (Prefect download_month docstring 'retries the task once'
  vs decorator retries=2): correctness drift. retries=2 produces
  TWO retries after the initial attempt = 3 total attempts.
  Reworded docstring to match: 'up to TWO times on transient HTTP
  errors — three total attempts including the initial one'.

- R4 stefanko-ch#9 (no explicit R2_* test for _render_prefect): valid test-gap
  request. Added two tests:
  - test_render_prefect_emits_r2_credentials — populated R2 config
    → all four R2_* keys appear in stacks/prefect/.env with the
    expected values.
  - test_render_prefect_emits_empty_r2_when_unconfigured — empty
    r2_data_* fields → keys still present (so the seed flow's
    upfront precondition check sees them) but values empty.

Suite: 1064 passed (was 1062, +2 prefect R2 tests), bash -n
scripts/deploy.sh OK, mypy strict clean, ruff clean.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
…round 2)

R2 #1 — Rename misnamed orchestrator field stacks_dir → project_root
        and env var STACKS_DIR → PROJECT_ROOT. Phases derive
        project_root/stacks internally; the prior STACKS_DIR name
        collided with deploy.sh's existing STACKS_DIR=$PROJECT_ROOT/stacks
        convention and would have produced .../stacks/stacks/... paths
        when wired up in Phase 4b.

R2 #2 — Plumb host through compose_runner.run_compose_up and
        infisical.provision_admin so a non-default SSH_HOST_ALIAS
        reaches every pre-bootstrap phase. Previously _phase_stack_sync
        honored ssh_host but _phase_compose_up + _phase_infisical_provision
        called helpers that hard-coded 'nexus' via _remote.ssh_run_script's
        default — different phases would talk to different hosts.

R2 #3 — Clear self.infisical_token / self.project_id alongside the
        state mirrors at the top of run_pre_bootstrap. State guards the
        rc=1 stdout emission; self.fields guard the post-bootstrap phase
        gating (infisical-bootstrap, secret-sync) when the same instance
        is later passed to run_all. Regression test extended.

R2 #4 — Update stale section comment that still referenced the dropped
        ssh-parameter pattern (R1 #2 removed it).

R2 #5 — Update OrchestratorState comment to match current behavior:
        post-bootstrap phases gate on self.fields, not state. State
        stays as the canonical record for stdout emission;
        _phase_infisical_provision writes both.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
…round 3)

R3 #1 — _phase_service_env now uses self.gitea_repo_owner consistently
        (the required Orchestrator constructor field) instead of
        self.bootstrap_env.gitea_repo_owner. Eliminates the dual-source
        of truth: callers populating only the constructor field would
        previously have hit the workspace_coords_complete guard against
        a None bootstrap_env mirror and silently skipped the Gitea
        block append (or built a malformed http://gitea:3000/None/...
        URL). Now the orchestrator field is canonical.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
…round 4)

R4 #1 — Real bug. Plumb host through stack_sync.cleanup_disabled_stacks
        so rsync + cleanup target the same SSH alias. Previously the
        cleanup ran via _remote.ssh_run_script(s) without a host=
        override → with non-default SSH_HOST_ALIAS, rsync went to host A
        but the destructive cleanup loop ran on host B (default 'nexus').
        Same class as R2 #2; cleanup path was missed. run_stack_sync
        now forwards its host kwarg to cleanup_disabled_stacks, and the
        orchestrator's _phase_stack_sync already passes
        host=self.ssh_host.

R4 #2 — Correctness drift. Comment claimed the gitea workspace guard
        checks 'ALL six coords (repo_url, username, password,
        author_name, author_email, repo_name)', but the actual guard
        gates on 5 inputs (repo_owner, repo_name, username, password,
        email) and derives repo_url + author_name from those. Updated
        the comment to match the code.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
…round 5)

R5 #1 — Real bug. Reordered run_pre_bootstrap phases so firewall-configure
        runs BEFORE stack-sync (was: service-env → stack-sync →
        firewall-configure → compose-up; now: service-env →
        firewall-configure → stack-sync → compose-up). Previously the
        per-stack docker-compose.firewall.yml overrides were generated
        AFTER rsync had already pushed stacks/<svc>/ to the server, so
        compose-up would start containers without the new firewall
        exposure overrides on the server. Tests updated to assert the
        new order. Server-side orphan-cleanup of stale firewall
        overrides remains in deploy.sh's bash for Phase 4b.

R5 #2 — Real bug. FIREWALL_RULES_JSON is now REQUIRED env (no '{}'
        default). The firewall module treats '{}' as intentional
        zero-entry mode and triggers destructive cleanup of existing
        override files. An accidentally empty value (forgot to set,
        tofu output failure) would have silently wiped overrides.
        Operators must now pass '{}' explicitly to opt into zero-entry
        mode. Added test_cli_run_pre_bootstrap_missing_firewall_rules_returns_2.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
…round 6)

R6 #1-3 — Correctness drift. After R5 phase reordering (firewall before
          stack-sync), three docstrings/help-text strings still listed
          the old order:
          - Orchestrator.run_pre_bootstrap docstring
          - _run_pre_bootstrap CLI handler docstring
          - main() help/usage string
          All three updated to match the actual order
          (service-env → firewall-configure → stack-sync → compose-up
          → infisical-provision).

R6 #4 — Correctness drift. cleanup_disabled_stacks docstring claimed
        'Returns None on transport failure or unparseable RESULT line',
        but the default runner (_remote.ssh_run_script with check=True)
        raises CalledProcessError/TimeoutExpired on transport failure
        rather than returning None. Updated docstring to clarify:
        Returns None only when RESULT is missing/unparseable; transport
        failures propagate as exceptions for the caller (orchestrator
        wraps with try/except → status='failed' PhaseResult).
rlei-odes pushed a commit that referenced this pull request May 10, 2026
…round 7)

R7 #1 — Correctness drift. _phase_compose_up docstring still said
        'ssh nexus 'bash -s'' even after R2 #2 plumbed host through
        run_compose_up. Updated to 'ssh <host> 'bash -s'' with
        explicit note that <host> = self.ssh_host (default 'nexus',
        override via SSH_HOST_ALIAS).

R7 #2 — Correctness drift. _phase_infisical_provision docstring
        claimed 'Read the constructor's project_id / infisical_token
        fields as fallback (so a post-bootstrap-only test can still
        bypass this phase)' — but the implementation has NO such
        read-fallback. The phase only WRITES to those fields on
        success. run_pre_bootstrap also clears them at start (R1 #4
        + R2 #3). Updated docstring to describe the actual flow:
        writes to BOTH state mirrors AND self.fields on success;
        callers wanting to bypass the phase should use run_all
        directly with constructor-provided creds.

R7 #3 — Correctness drift. Test comment for
        test_phase_service_env_skips_gitea_block_on_incomplete_coords
        described 'the original lax check' (a prior pre-R3 #1
        implementation) which is no longer accurate now that
        self.gitea_repo_owner is the canonical source of truth.
        Rewrote the comment to reflect the current
        workspace_coords_complete check (5 inputs: repo_owner,
        repo_name, username, password, email) and which one the test
        deliberately leaves missing.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
…round 8)

R8 #1 — Correctness drift. test_phase_service_env_happy_path comment
        claimed the gitea-block append is skipped because
        bootstrap_env.gitea_repo_owner=None makes workspace_coords_complete
        fail. After R3 #1, the check uses self.gitea_repo_owner (set to
        'admin' by the orchestrator fixture), so the bootstrap_env clear
        alone wouldn't trip the guard. Actual skip reason: orchestrator
        fixture leaves gitea_user_username/_password/_email at None,
        which fails the all() check. Comment rewritten to reflect this.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
…round 1)

R1 #1 — ❌ Wrong (dismissed). Docker Hub login uses double-quoted
        outer ssh argument, so $DOCKERHUB_TOKEN and $DOCKERHUB_USER
        DO expand locally. The single-quotes inside are literal chars
        (verified: 'echo "echo '$X' | docker login -u '$Y' …"' →
        'echo "echo 'value' | docker login -u 'value' …"' which is
        what the remote sees and works correctly). Same pattern as
        legacy bash that's been working since Phase 1.

R1 #2 — 🔴 Real bug (linked to R1 #3 fix). GITEA_USER_USERNAME
        wasn't passed to run-pre-bootstrap CLI in deploy.sh, but
        workspace-coords derives it. The downstream issue (R1 #3)
        was that the derived value didn't reach _phase_service_env's
        gate — fixed in R1 #3. Reply to R1 #2 explains the linked fix.

R1 #3 — 🔴 Real bug. _phase_workspace_coords now also dual-writes
        self.gitea_user_username / _password / _email from the
        derived coords, not just state mirrors. In admin-fallback
        mode (no GITEA_USER_EMAIL / _PASS env), the legacy bash
        filled these from admin coords; the orchestrator must
        replicate that behavior so _phase_service_env's
        workspace-block-append guard passes. Otherwise git-integrated
        stacks silently miss GITEA_REPO_URL / GITEA_USERNAME etc. in
        their .env. Existing constructor values still win — tests
        can pre-seed alternative identities. Regression test added.

R1 #4 — 🔴 Real bug. _phase_firewall_sync now explicitly checks
        local stacks/ dir is_dir() BEFORE computing the orphan list.
        Without the guard, a missing local stacks/ directory yields
        Path.glob → empty list → every remote *.firewall.yml treated
        as orphan and rm'd. That's catastrophic if project_root is
        mis-set or the checkout is incomplete. Now fails fast with
        explicit 'would rm every remote firewall override' message.
        Regression test added.

R1 #5 — 🟠 Correctness drift. compose_restart's rendered remote
        script now suppresses docker compose's normal output
        (>/dev/null), captures stderr to a tempfile, and only emits
        it on failure (with 6-space indent for log readability).
        Matches legacy bash's '>/dev/null 2>&1 || true' semantics —
        restart loops produce minimal log output by default. Snapshot
        regenerated.

R1 #6 — 🟠 Correctness drift. _parse_owner_repo docstring claimed
        'Returns empty string on a URL that doesn't fit the pattern'
        but actually returns the input passed-through (with prefix/
        query/fragment strips). Updated to match reality + cite the
        caller's regex check as the actual gate.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
…round 2)

R2 #1 — 🔴 Real bug. _phase_firewall_sync's orphan-rm script now
        prefixed with 'set -e' so the first failed rm propagates as
        the script's exit code. Without this, a permission-denied (or
        any non-zero) rm in the middle would be MASKED by a later
        success — script exits 0, phase reports ok, but stale firewall
        overrides remain on the server. That's a security-relevant
        inconsistency: the orphan-cleanup is what closes a host port
        after Tofu removes a firewall rule. set -e + check=True on the
        ssh wrapper now ensures any failure → CalledProcessError →
        status='failed' on the phase.

R2 #2 — 🔴 Real bug. _phase_global_env now writes env_content via
        ssh-stdin streaming (subprocess.run with input=env_content)
        instead of an inline heredoc. The old approach (heredoc with
        a fixed 'NEXUS_GLOBAL_ENV_EOF' delimiter) was a heredoc-
        injection risk: any image-version value or USER_EMAIL whose
        content contained that exact delimiter on a line by itself
        would terminate the heredoc early, and the remaining content
        bytes would be interpreted as shell commands on the server.
        Streaming via stdin removes the delimiter entirely — the ssh
        command is just 'cat > <path>', env_content goes through
        stdin where bash treats it as opaque bytes (cat doesn't parse
        its stdin). New regression test:
        test_phase_global_env_no_heredoc_injection_with_malicious_image_value
        constructs an adversarial image value containing the old
        delimiter + 'rm -rf /' and asserts the content reaches stdin
        verbatim (NOT argv) and ssh_args contain no '<<' heredoc.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
…round 3)

R3 #1 — 🔴 Real bug. _phase_woodpecker_apply now uses
        'docker compose --env-file <path> up -d' instead of
        'source <path> && docker compose up -d'. Compose's
        --env-file directive parses KEY=VALUE without shell
        interpretation, so a malicious image-version value containing
        $() / backticks / ; / newlines cannot trigger remote command
        execution at source-time. (compose_runner.py still uses the
        legacy 'set -a; source ...' pattern; tracked for Phase 4c
        follow-up — out of scope for this PR.)

R3 #2 — 🔴 Real bug. _phase_global_env now validates every value
        before write: the file is consumed by TWO mechanisms
        (compose_runner's 'set -a; source' + compose's 'env_file:'
        directive). The safe intersection is 'values without shell
        metacharacters'. The validation rejects values containing
        whitespace, $, backticks, ;, &, |, <, >, !, ?, *, [], {},
        single/double quotes, or newlines/CRs. Image versions
        legitimately need alphanum + .-_:/@+ only — real values
        like 'treeverse/lakefs:1.73.0' or 'v1.2.3' pass; adversarial
        values like 'v1.0\nrm -rf /' or 'v1.$(rm -rf /)' fail with
        a clear status='failed' detail BEFORE any write. 3 new
        regression tests cover shell-unsafe rejection in image
        values, dollar-sign in values, and unsafe ADMIN_EMAIL.

R3 #3 — 🟠 Correctness drift. compose_restart.run_restart's default
        runner now passes check=False to _remote.ssh_run_script. On
        ssh transport failure (network blip, expired CF Access
        token), the runner now returns a CompletedProcess with
        non-zero rc instead of raising CalledProcessError. The
        existing parse_result fallback (None → failed=len(services))
        kicks in, the orchestrator converts that to status='partial',
        and the deploy continues — matching legacy bash's best-effort
        '|| true' restart-loop semantics. With check=True, an ssh
        blip would have aborted the entire post-bootstrap pipeline.

R3 #4 — 🟠 Correctness drift. The R2 #2 test asserted safety that
        didn't actually hold — the .env is later sourced (R3 #1
        + #2 above), so 'no heredoc injection' didn't mean 'no
        injection'. Replaced with 4 stronger tests that assert the
        new validation gate REJECTS unsafe values BEFORE writing the
        file (the actual safety property, not a partial one).
rlei-odes pushed a commit that referenced this pull request May 10, 2026
…round 4)

R4 #1 — 🟠 Correctness drift. _phase_kestra_secret_sync docstring
        claimed 'secret-sync still attempts on wait_ready timeout
        but may not succeed'. Actual behavior: returns partial
        immediately and skips run_sync_for_stack entirely (which is
        the correct semantic — pushing secrets to a Kestra whose
        basic-auth layer isn't ready would 401). Updated docstring
        to match: 'we then skip run_sync_for_stack entirely'.

R4 #2 — 🔴 Real bug. _phase_firewall_sync's remote-listing command
        `cd … 2>/dev/null && ls … | sort || true` had `|| true`
        masking failure of BOTH cd AND ls. If the remote
        /opt/docker-server/stacks dir was missing (server FS issue
        or wrong project_root expectation), cd would fail, the inner
        ls would never run, but `|| true` swallowed the non-zero
        exit → remote_overrides looked empty → no orphans detected
        → phase reported ok despite stale firewall overrides
        remaining on the server (host port stays exposed). Now
        prefixed with set -e and bare cd so any failure propagates
        to ssh's exit code → check=True → CalledProcessError →
        status='failed'. The `|| true` is now scoped to ls's
        'no matches' exit-1 only.

R4 #3 — 🟠 Correctness drift. Comment above the redpanda
        chown_script said the chmod fallback was for 'sudo isn't
        available', but BOTH primary and fallback commands use
        sudo. Corrected the comment to reflect the actual intent:
        chown to 101:101 may fail on minimal hosts where uid 101
        doesn't exist as a real user (some FS layouts disallow
        chown to a numeric uid that's not in /etc/passwd); chmod
        777 is the fallback. Both paths use sudo because the nexus
        user can't chown/chmod files owned by 101:101 from a
        previous firewall-on deploy.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
…round 5)

R5 #1 — 🔴 Real bug. _phase_global_env now validates KEYS (not just
        values) for image_versions. A malicious / invalid
        image-versions key containing ';', whitespace, newlines, or
        other shell metacharacters would survive the dash→underscore
        normalization and produce an env-file line whose left-hand
        side breaks shell parsing OR (worse) becomes a command
        injection vector when the file is later sourced by
        compose_runner. The post-normalization name must now match
        ^[A-Z_][A-Z0-9_]*$ (POSIX shell variable name); image-version
        keys from image-versions.tfvars normalize cleanly under
        normal contributor edits but malicious / typo'd values fail
        with a clear status='failed' detail BEFORE write. 2 new
        regression tests cover the rejection.

R5 #2+#3 — 🟡 Project-rule (consistency). Switched the two remaining
        `echo "$SECRETS_JSON" | …` invocations in deploy.sh
        (lines 335 + 405 — pre-bootstrap + run-all CLI calls) to
        `printf '%s' "$SECRETS_JSON" | …`. Matches the existing
        config dump-shell pattern at line 139 and the broader
        project convention (Phase 1-3 had migrated to printf for the
        echo-with-arbitrary-content edge cases). Removes the small
        chance of echo's flag/escape interpretation mangling JSON
        with backslash-escapes.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
…round 6)

R6 #1 — 🔴 Real bug. ENABLED_SERVICES_CSV used echo (always trailing
        newline) → empty service list → tr converts the lone newline
        to ',' → CSV becomes a single comma → downstream CLI's
        comma-split parser sees one empty service in the list.
        Switched to printf '%s' (no trailing newline). Empty list
        now stays empty.

R6 #2 — 🟠 Correctness drift. _phase_mirror_finalize comment said
        'only ALL-failed → partial' but the actual logic marks
        partial when EITHER (a) kestra-enabled-but-flow-not-triggered
        OR (b) any git-restart failure. Updated comment to describe
        the actual two-condition logic + clarify that
        flow_triggered=False is benign when kestra isn't enabled
        (the gate excludes that case).
rlei-odes pushed a commit that referenced this pull request May 10, 2026
…round 7)

R7 #1 — 🔴 Real bug. _phase_kestra_secret_sync (and existing
        _phase_secret_sync for jupyter/marimo) called
        _secret_sync.run_sync_for_stack() without forwarding
        self.ssh_host. The default ssh_run_script + ssh_run paths
        used host='nexus' regardless of SSH_HOST_ALIAS — secrets
        and the post-sync 'docker compose up -d' would target the
        wrong host on non-default deployments. Same class of bug
        as PR stefanko-ch#532 R2 #2 + R4 #1; the secret_sync module was the
        last unmigrated host=plumbing point.

        Fix: extended secret_sync.run_sync_for_stack with a
        host: str = 'nexus' kwarg that flows to the default
        ssh_run_script + ssh_run lambdas. Both orchestrator phases
        now pass host=self.ssh_host. Test fakes that monkeypatched
        _remote.ssh_run_script directly were updated to accept
        **kwargs (the new host= kwarg).

R7 #2 — 🟠 Correctness drift. _phase_woodpecker_apply's catch-all
        for CalledProcessError + TimeoutExpired labeled both as
        'transport (...)', but CalledProcessError from the docker
        compose ssh-run is a service-level failure (image pull
        failed, container crashed at startup, port collision, …)
        not a transport issue. Split into three distinct paths:
        (a) rsync transport ('rsync transport (CalledProcessError)')
        (b) docker compose up -d failure ('docker compose up -d
            failed (rc=N): <last stdout line>') — operator gets a
            captured tail from the compose error output for direct
            triage
        (c) genuine ssh transport timeout ('ssh transport timeout
            (TimeoutExpired)')
        Each maps to a distinct phase status='partial' detail so
        operators know whether to investigate ssh, rsync, or
        container logs.
rlei-odes pushed a commit that referenced this pull request May 10, 2026
…round 8)

R8 #1 — 🔴 Real bug, third iteration on the firewall-sync orphan
        listing (R1 #4 added the local guard, R4 #2 fixed the
        cd-vs-ls scoping, R8 #1 fixes the pipeline-error masking).
        Without 'set -o pipefail', the 'ls | sort' pipeline only
        reflects sort's exit code — a real ls error (permission
        denied, unexpected fs error) returns non-zero but sort
        still completes successfully, so the pipeline exits 0 and
        '|| true' is redundant. Real ls errors got swallowed →
        remote_overrides looked empty → phase reported ok despite
        stale overrides remaining.

        Switched to 'find -mindepth 2 -maxdepth 2 -name … -printf'.
        find returns 0 on the legitimate 'no matches' case (empty
        stdout) AND propagates non-zero on real errors (unreadable
        subdir, etc.). With 'set -euo pipefail', any failure in the
        'find | sort' pipeline propagates to the script's exit code
        → check=True → CalledProcessError → status='failed'. No
        '|| true' anywhere — every error path now surfaces. cd is
        bare so a missing /opt/…/stacks still fails fast (R4 #2
        guard preserved).
rlei-odes pushed a commit that referenced this pull request May 10, 2026
…round 1)

R1 #1 — 🔴 Real bug. _docker_hub_login interpolated dockerhub_user
        directly into the remote shell command via f-string. A
        DOCKERHUB_USER containing whitespace or shell metacharacters
        would have been parsed by the remote shell, opening a
        command-injection vector. Now wrapped with shlex.quote.

R1 #2 — 🟠 Correctness drift. run_pipeline() docstring claimed rc=1
        on partial; the CLI returns rc=0 (PR stefanko-ch#535 R0 fix). Updated
        the docstring to describe the actual contract: rc=0 covers
        both clean and partial; rc=2 only on hard failure. Added
        cross-reference to the spin-up.yml 'shell: bash -e' reason
        for the rc=0-on-partial choice.

R1 #3 — 🟠 Correctness drift. tfvars.py module docstring claimed
        malformed/unquoted lines 'fail fast with clear error'; in
        fact parse() silently doesn't match those lines and returns
        the dataclass empty-string default. Tests pin the soft-skip
        behavior. Updated docstring to describe what the code
        actually does + cite where the empty-after-parse case
        is caught downstream.

R1 #4 — 🔵 Pedantic (trivial fix). troubleshooting.md referenced
        the orchestrator's private '_phase_firewall_configure'
        symbol. Replaced with the user-visible 'firewall-configure
        phase' wording so the doc doesn't stale on internal
        renames.

R1 #5 — 🟠 Correctness drift (same root as R1 #2). Updated
        _run_pipeline CLI docstring's exit-code section to match
        the new rc=0-on-partial contract.

R1 #6 — 🟠 Correctness drift. Error handler in _run_pipeline only
        printed the exception class name, not the rc / stderr tail
        / message. CI failures were undiagnosable without a re-run
        with --debug. Now: subprocess.CalledProcessError prints
        rc + last 500 chars of stderr (or stdout), TimeoutExpired
        prints the timeout duration, SetupError prints its message
        directly, OSError + Exception print message. Still avoids
        printing exc.cmd (can carry secrets via env-var-prefixed
        argv forms).
rlei-odes pushed a commit that referenced this pull request May 10, 2026
…gate

Codecov flagged 88.6% patch coverage; new tests bring pipeline.py to
96% and tfvars.py to 100% (overall 97%). No production-code changes —
covers branches that were exercised only end-to-end before:

- diagnose_state reason surfaced through PipelineError (R2 #2)
- ConfigError → PipelineError wrap (dead-code branch today, but pinned
  via monkeypatched from_secrets_json so a future schema tightening
  doesn't silently break the wrap)
- enabled_services not-a-list defensive guard
- _b64_encode_ssh_key empty + non-empty (legacy bash newline-append)
- _ssh_keygen_cleanup empty-target skip + TimeoutExpired suppress (R2 #1)
- tfvars OSError-on-read wrapped in TfvarsError
rlei-odes pushed a commit that referenced this pull request May 10, 2026
- Required tofu outputs are now actually required (R4 #1). Previous
  safe-looking defaults were destructive in the partial-apply case:
  enabled_services=[] drives stack-sync to remove ALL remote stacks,
  firewall_rules={} puts firewall-configure into zero-entry mode
  (wiping existing per-stack overrides). state_list_ok() doesn't
  catch partial-apply (state file exists, specific outputs never
  populated). enabled_services / firewall_rules / ssh_service_token
  now lack default → TofuError on missing → wrapped to PipelineError
  with an actionable message. image_versions stays in the same
  required-list (defensive symmetry — no harm if it can't be empty).
  server_ip / persistent_volume_id stay optional (their empty values
  are non-destructive).
- TofuError from load_r2_credentials now wrapped to PipelineError
  (R4 #2). Without the wrap a malformed .r2-credentials file showed
  as 'unexpected error (TofuError)' which is wrong — it's an
  obvious operator-actionable preflight failure.
- TfvarsError from tfvars.parse same treatment (R4 #3).
- mount_persistent_volume result now captured + emits a stderr
  warning when volume_id is non-zero but mounted=False (R4 #4).
  Mount failure stays non-fatal (downstream stacks that don't need
  /opt/data can still come up healthy), so we warn but don't raise.
  The warn-vs-silent path is gated on volume_id != '0' so an
  operator who hasn't configured a volume doesn't see noise.
- 5 new unit tests pin: TfvarsError wrap, TofuError-on-creds wrap,
  required-output missing, volume-mount warning emitted, volume-mount
  warning suppressed when volume_id='0'. 1252 tests green.
@github-actions github-actions Bot changed the title chore(main): release 0.55.0 chore(main): release 0.56.0 May 10, 2026
@github-actions
github-actions Bot force-pushed the release-please--branches--main branch from c665456 to c7d93d5 Compare May 10, 2026 14:05
rlei-odes pushed a commit that referenced this pull request May 22, 2026
…udflare R2

User-directed change: R2 makes more sense than Hetzner Object
Storage for this use case because (a) the project already uses R2
for the Tofu state backend, the cloudflare/cloudflare provider is
already wired up, and the R2 token already exists; (b) R2 is
region-agnostic with zero egress fees, eliminating the EU-only-
compute caveat that the previous Hetzner-OS revision needed; (c)
Cloudflare's cloudflare_r2_bucket resource is a first-class Tofu
resource — no parallel storage system to operate.

Changes:

- Title + tl;dr + motivation: storage provider is now Cloudflare
  R2. Historical context (Hetzner-OS was previously proposed)
  retained for reviewers tracking the revision.
- Goal #1: removed the EU-only-compute default caveat. R2 has
  zero egress so non-EU compute (ash, hil, sin) is just as cheap
  to serve. Operators can keep `SERVER_PREFERENCES` EU-only for
  latency reasons but the architecture doesn't force it.
- "What this is NOT": dropped the EU-only and "we stay on
  Hetzner for data-residency" framing. Added clarification on
  R2's `EU` jurisdiction hint for callers who do want a stricter
  geography pin.
- Storage layout diagram: Cloudflare R2 (EU jurisdiction) header.
- Tofu code-changes row: cloudflare_r2_bucket (not
  minio_s3_bucket). No new provider plumbing — cloudflare/
  cloudflare is already configured.
- Decision Points #1, #3, #5: revised. #1 names R2 explicitly.
  #3 names cloudflare_r2_bucket. #5 names R2's built-in
  versioning + lifecycle.
- Cost analysis: rewrote for R2 pricing. Free tier (10 GB
  storage + 1M Class A + 10M Class B ops) covers small classes
  entirely. Above-free-tier is $0.015/GB storage + free egress.
  Honest comparison vs Hetzner-OS — R2 is more expensive per GB
  but the zero-egress advantage dominates the moment any non-EU
  compute pull happens.
- Risks table: removed the "R2 might be better" self-reflection
  row (resolved by this revision). Added "R2 ops free-tier
  exhaustion" as the new structural risk to monitor.
- Open Question #5: resolved with cloudflare_r2_bucket pattern.
- Appendix B (rclone config): updated to Cloudflare R2 form
  (provider = Cloudflare, region = auto,
  endpoint = https://<account_id>.r2.cloudflarestorage.com).
@github-actions github-actions Bot changed the title chore(main): release 0.56.0 chore(main): release 0.66.0 May 22, 2026
@github-actions
github-actions Bot force-pushed the release-please--branches--main branch 2 times, most recently from c6514c4 to 0ba66cf Compare May 23, 2026 07:47
@github-actions github-actions Bot changed the title chore(main): release 0.66.0 chore(main): release 0.68.0 May 26, 2026
@github-actions
github-actions Bot force-pushed the release-please--branches--main branch 4 times, most recently from a738fd2 to e0f6dfe Compare May 27, 2026 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants