Upgrade EdgeZero to v0.0.6 - #940
Conversation
Point the edgezero-* dependencies at the feature/edgezero-deploy-actions branch (PR #316) and adapt Trusted Server to its API changes: - Wire the new ts CLI subcommands surfaced by edgezero-cli: active-version, healthcheck, and rollback, plus deploy --stage and a --version flag, with argument-parsing coverage. - Migrate TrustedServerAppConfig to the AppConfigMeta::secret_fields() method that replaces the removed SECRET_FIELDS associated constant.
Status update — P0/P1 trackingHead: `96dd1f72` (main merged in twice since the original commit; edgezero dep unchanged, no conflicts). P0.2 — CI + output-line contract
P1.3 — edgezero library ↔ action lockstep (recorded pair)The `ts` CLI is built from this repo's `Cargo.toml` (edgezero library rev), and the deployer separately pins `stackpop/edgezero/.github/actions/deploy-fastly@` (edgezero action ref). These MUST be the same edgezero commit so library and action can't diverge.
P0.1 — re-pin off the moving branch (blocker, still open)edgezero #316 is not yet merged and no new tag exists. Cannot re-pin until it lands; PR stays draft until then. |
Re-resolve the six edgezero-* deps from 145f1699 to bb441162 (current tip of feature/edgezero-deploy-actions, PR #316). The deploy staging flag was renamed there from --stage to --staging, standardizing on the same verb healthcheck/rollback/config-push already use; update the deploy CLI parse test to match. No production dispatch change is needed — ts passes the edgezero-cli arg structs through, so the renamed flag is picked up automatically.
Re-resolve the six edgezero-* deps from bb441162 to 908e229a (current tip of feature/edgezero-deploy-actions, PR #316), and adapt the ts CLI to its surface changes: - Wire the new `ts config gc` subcommand (reclaims orphaned config-store chunk entries) to edgezero_cli::run_config_gc, with parse coverage for the preview default, destructive --yes/--older-than sweep, and the --dry-run/--yes conflict. - Lock the hardened deploy staging behavior: --stage was renamed to --staging and deploy passthrough is now last=true, so a stray --stage fails closed at parse time instead of routing a staging-intended deploy to production. Add tests for the rejection and for post---- passthrough capture.
…ain sync) with the #940 re-merge
Re-resolve the six edgezero-* deps from 908e229a to 5f3d648c (current tip of feature/edgezero-deploy-actions, PR #316). The upstream change is an internal review-addressing pass (redact config-store errors, fix version parse, log cleanup, docs) confined to the Fastly adapter CLI; no ts CLI surface change, so no run.rs adaptation is needed.
prk-Jr
left a comment
There was a problem hiding this comment.
Summary
Tight, well-tested upgrade: six edgezero deps repointed at the deploy-actions branch, SECRET_FIELDS migrated to secret_fields(), and the new active-version / healthcheck / rollback / config gc subcommands wired through with argument-parsing coverage for each. Dispatch is a clean passthrough and every new command has a doc comment. Two things block: the dependency reference is mutable while upstream #316 is still open, and the operator docs weren't updated for the new (partly destructive) surface.
Blocking
🔧 wrench
-
Mutable
branch =dep on an unmerged upstream PR (Cargo.toml:57-62): upstreamstackpop/edgezero#316is confirmedopen/merged: false, but this PR isisDraft: falseandMERGEABLE. See the inline comment for therevpin suggestion. -
docs/guide/cli.mdnot updated for the new CLI surface: four subcommands ship undocumented —active-version,healthcheck,rollback, andconfig gc.config gcis the one that matters most: with--yesand--older-thanit destructively sweeps config-store entries, and the only place its semantics are written down right now is a one-line clap doc string and three parse tests. The "Lifecycle commands" block (docs/guide/cli.md:79-90) and "Configuration commands" block (:43-77) both enumerate the available commands, so they read as complete and are now silently wrong.The same file also carries a version claim that this PR invalidates (
docs/guide/cli.md:67-70):EdgeZero v0.0.4 only overrides leaves already present in the TOML; add newly introduced fields to existing configs before relying on their overrides.
The dependency is no longer v0.0.4. Either confirm the behavior still holds on the new tree and drop the version qualifier, or update it to whatever gets pinned at merge time.
Non-blocking
🤔 thinking
- Unrelated dependency downgrades in the lock refresh (
Cargo.lock):windows-sys 0.61.2 → 0.48.0anditertools 0.13.0 → 0.10.5. Details and a scopedcargo updateinline.
🌱 seedling
ts --versioncan't identify the embedded edgezero (crates/trusted-server-cli/src/run.rs:16): follow-up, not this PR.
📝 note
secret_fields()port (crates/trusted-server-core/src/config.rs:115): mechanical and correct; the empty-set caveat above it is unchanged.
👍 praise
deploy_rejects_renamed_stage_flag_before_separator(crates/trusted-server-cli/src/run.rs:306): catches a silent staging→production routing hazard created by the upstream flag rename pluslast = truepassthrough.
CI Status
All 19 checks green on fe5767e — reported from GitHub, not re-run locally:
- fmt: PASS
- clippy (fastly / axum / cloudflare native + wasm / spin native + wasm): PASS
- rust tests (core, axum, cloudflare, spin, ts CLI, cross-adapter parity): PASS
- integration + browser integration + Fastly EC lifecycle: PASS
- js tests (vitest) / format-typescript / format-docs: PASS
- CodeQL (actions, javascript-typescript, rust): PASS
Replace the mutable branch = "feature/edgezero-deploy-actions" reference with rev = "5f3d648c", the exact commit the lockfile already resolved, so cargo update or an unlocked resolve cannot silently retarget the branch tip. Rebuild the lock from main with a scoped update of the six edgezero packages. The re-resolve of the edgezero subtree unifies wide version ranges onto co-present lower majors (prost-build/prost-derive onto itertools 0.10.5, colored/winapi-util onto windows-sys 0.48.0); restore those five dependency edges to main's picks so the only non-edgezero lock changes are syn 3.0.3 and toml_edit 0.25.12, both required by the new edgezero tree. Verified with cargo metadata/check --locked and the host CLI test suite.
Cover active-version, healthcheck, rollback, deploy --staging with the -- passthrough boundary, and the destructive config gc surface (preview default, --yes with a required --older-than window, and gc's different --no-env meaning). Drop the stale EdgeZero v0.0.4 qualifier from the env-overlay note; the leaves-only behavior is unchanged at the pinned rev.
The dependency is no longer pinned at v0.0.4. The leaves-only overlay behavior is unchanged at the pinned rev (apply_env_overlay still only overrides keys already present in the parsed tree), so keep the behavior claims and remove the version qualifier.
prk-Jr
left a comment
There was a problem hiding this comment.
Summary
Re-review of the three commits added since the last pass (f90f6384, bf47680d, 40dd9b37). Both prior findings are resolved: the six deps moved from a mutable branch = ref to rev = 5f3d648c…, and the unrelated lock downgrades are gone from the diff. Verified independently — 5f3d648c is an ancestor of the branch tip b3e02111 (behind_by: 0), and the v0.0.5 tag is genuinely diverged from the pin (117 behind / 134 ahead), so the "no tag contains it" reasoning holds.
What remains blocking is the upstream merge gate plus one gap in the new staging docs: the documented deploy --staging → healthcheck --staging sequence never mentions the config leg, so following it literally stages a version pointed at a config key nothing wrote.
4 of the inline comments below carry a one-click GitHub
suggestion— use Commit suggestion (or Add suggestion to batch) to apply them as commits on the PR branch. The remaining comments describe the fix in prose because the change is "wait for upstream", needs a new test function, or touches files outside this diff.
Blocking
🔧 wrench
- Upstream #316 still open; branch-only reachability and squash-merge consequences — see inline at
Cargo.toml:57 - Staged-deploy docs omit the config leg — see inline at
docs/guide/cli.md:120-123
Non-blocking
♻️ refactor
--retry 3is 3 total attempts, not 3 retries — see inline atdocs/guide/cli.md:140-141andcrates/trusted-server-cli/src/run.rs:182config_gc_previews_by_defaultnever assertsyes— see inline atcrates/trusted-server-cli/src/run.rs:489
🤔 thinking
secret_fields()comment names a blocker this upgrade removes — see inline atcrates/trusted-server-core/src/config.rs:115
⛏ nitpick
- No test for the new top-level
--version— see inline atcrates/trusted-server-cli/src/run.rs:16
Cross-cutting / body-level findings
-
♻️
40dd9b37missed two livev0.0.4qualifiers. The commit dropped the stale version qualifier acrossdocs/guide/*.md, but two non-archival sites still restate exactly the same claim:trusted-server.example.toml:124— the operator-facing template: "Keep this leaf present when using the EdgeZero v0.0.4 environment override."crates/trusted-server-cli/tests/config_env_overlay.rs:47— "EdgeZero v0.0.4 environment overlays cannot create missing TOML leaves, …"
Neither file is in this diff, so neither can carry a
suggestion— apply manually, droppingv0.0.4the same way the docs did. (docs/superpowers/plans/2026-07-15-gam-ts-cohort-attribution.md:835also mentions it, but that is a dated archived plan describing a past experiment; leaving it is defensible.)Separately, the genericization itself checks out: at the pinned rev
apply_env_overlaystill only overrides keys already present in the parsed tree, andenv_overlay_only_overrides_existing_keysasserts an env var for an absent key is silently ignored. So this is a by-design property, not a v0.0.4 bug, and dropping the version qualifier is the right call. -
📝 Two
synmajors and twotoml_editmajors now compile. The lock carriessyn 2.0.118alongside the newsyn 3.0.3(pulled byedgezero-macros), andtoml_edit 0.23.10+spec-1.0.0alongside0.25.12+spec-1.1.0. Both duplicates are host-build-only — proc-macro expansion and CLI-side TOML editing — with nowasm32-wasip1runtime exposure, so this is compile-time cost rather than shipped weight. Nothing to change in this PR; it collapses when the rest of the tree catches up. -
👍 Two things worth keeping. First, the rev pin was done as a scoped lock rebuild — restore main's lock, run the narrow update, then restore the five edges the resolver had needlessly walked back — and verified with
cargo metadata --lockedrather than asserted. That is exactly the right way to answer "is this lock diff minimal?". Second, theconfig gcdocumentation catches a real footgun rather than paraphrasing--help: it spells out that--older-thanis a safety assertion about the whole physical store, and thatgc's--no-envmeans something different from every otherconfigsubcommand's--no-env(it reselects the target store instead of skipping the app-config overlay). On a destructive command, that distinction is the one an operator would otherwise learn the hard way.
CI Status
All 19 reported checks PASS at 40dd9b37:
- cargo fmt: PASS (required)
- cargo test: PASS (required)
- format-docs: PASS (required)
- format-typescript: PASS (required)
- cargo test (axum native): PASS
- cargo test (cross-adapter parity): PASS
- cargo test (ts CLI, native): PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- vitest: PASS
- integration tests: PASS
- integration tests (Fastly EC lifecycle): PASS
- browser integration tests: PASS
- prepare integration artifacts: PASS
- CodeQL: PASS
- Analyze (rust): PASS
- Analyze (javascript-typescript): PASS
- Analyze (actions): PASS
Verification performed for this review
Every suggestion below was applied in an isolated worktree at 40dd9b37, verified alone and then batched, with a byte-exact pre/post patch snapshot each time:
cargo fmt --all -- --check |
prettier 3.8.1 (docs/node_modules) |
clippy -D warnings |
tests | |
|---|---|---|---|---|
| each suggestion alone | PASS | PASS | PASS | 165 pass |
| all four batched | PASS | PASS | PASS | 165 pass |
No drift between the approved patch and the post-verification tree on any run. Scope note: clippy and tests were run as --package trusted-server-cli --target aarch64-apple-darwin rather than the full six-alias chain, because both Rust suggestions are assertion lines inside that crate's own #[cfg(test)] module and cannot reach an adapter target.
# Conflicts: # docs/guide/cli.md
…ording A staged deploy only redirects the staged version's config selector at the <logical-store-id>_staging key; it does not copy the production blob. Add the config push/diff --staging step to the staging sequence so following the guide no longer stages a version whose config key nothing wrote, and note the --key conflict. Reword the healthcheck defaults: --retry 3 is 3 total attempts (attempts = retry.max(1) upstream), not 3 retries after a first try.
Add parse coverage for config push/diff --staging including the --key conflict, assert healthcheck's path default and reword the retry assertion to total attempts, assert config gc's yes default since --yes is the only destructive gate, and pin the top-level --version flag via ErrorKind::DisplayVersion.
The pinned edgezero rev ships nested/array secret extraction, so the secret_fields comment no longer names it as the blocker; what remains is spelling out the paths in this hand-written impl (it does not inherit the derive's) plus operator migration. Drop the stale v0.0.4 qualifiers the earlier docs pass missed in the example config and the env-overlay test.
prk-Jr
left a comment
There was a problem hiding this comment.
Summary
Repoints the six edgezero workspace deps from tag = "v0.0.4" to an immutable rev = "5f3d648c" pin on the unmerged feature/edgezero-deploy-actions branch, wires the new ts lifecycle subcommands, and documents them. The Rust side is in good shape: the lockfile claims in the PR body check out, the SECRET_FIELDS -> secret_fields() migration is behavior-preserving, and no production wire format moved across the 39k-line upstream jump. The remaining blockers are the upstream merge gate and one operator-docs gap that leaves a rollback unreachable.
3 of the inline comments below carry a one-click GitHub
suggestion— use Commit suggestion (or Add suggestion to batch for several at once) to apply them as commits on the PR branch. All three aredocs/guide/cli.mdprose and were verified against the pinned prettier indocs/node_modules(3.8.1), individually and batched, with no drift.
Blocking
🔧 wrench
- Upstream
stackpop/edgezero#316is still open; the pin is immutable but branch-scoped — see inline atCargo.toml:57 - The documented rollback lifecycle gives an operator no way to obtain
--rollback-to— see inline atdocs/guide/cli.md:140-153
Non-blocking
♻️ refactor
config gc's destructive-target warning enumerates--no-envbut omits--store— see inline atdocs/guide/cli.md:94-100deploypassthrough is a breaking change, documented as if it were always the rule — see inline atdocs/guide/cli.md:114-118
👍 praise
- The reworded
secret_fields()comment is now accurate — see inline atcrates/trusted-server-core/src/config.rs:113-116
Cross-cutting / body-level findings
-
📝 Lockfile claims audited and confirmed; the "second lockfile" CI risk does not exist on this branch. All six
Cargo.tomldeps carry the samerev = "5f3d648c3c6c38fc6e6b22b5c65c66177363aad8".Cargo.lockhas 8 matching source lines (the six plus transitiveedgezero-adapterandedgezero-macros), all identical, and zero remainingtag=/v0.0.4references.cargo metadata --locked --offlineexits 0, so the lock is consistent with the manifest and needs no re-resolve. The body's "onlysyn 3.0.3andtoml_edit 0.25.12are non-edgezero changes" is true — the rest of the lock diff is source-line repointing plus mechanicaltoml_edit->toml_edit 0.23.10+spec-1.0.0/0.25.12+spec-1.1.0disambiguation, and every new transitive (winnow 1.0.3,toml_datetime 1.1.1,toml_writer,toml_parser 1.1.2,indexmap 2.14.0) already existed in the lock. The earlier-roundwindows-sys 0.61.2->0.48.0anditertools 0.13.0->0.10.5regressions are gone. On the separate-lockfile concern raised in an earlier round:crates/trusted-server-integration-tests/Cargo.lockdoes not exist on this branch — the crate is a workspace member using{ workspace = true }deps, so it shares the root lock. The only other tracked lockfile iscrates/trusted-server-openrtb-codegen/Cargo.lock, which has no edgezero edges, is untouched here, and has no--lockedCI gate. No lockfile-mismatch gate is at risk. -
📝 Production wire formats and runtime behavior are unchanged across the upstream jump. Diffed
9e661ae..5f3d648directly, since neither the PR body nor CI answers this. Config blob compatibility holds:BlobEnvelopekeepsENVELOPE_VERSION_V1and the same four fields (only errorDisplay/Debuggained hash redaction), andPOINTER_KIND = "fastly_config_chunks",CHUNK_KEY_INFIX = ".__edgezero_chunks.",FASTLY_CONFIG_ENTRY_LIMIT = 8_000,CHUNK_PAYLOAD_TARGET = 7_000are all unchanged — which matters becausecrates/trusted-server-core/src/settings_data.rsduplicates those constants in this repo's own resolver.edgezero-core/src/manifest.rschanged only in style andenv_config.rsis not in the diff at all, soEnvConfig::store_key("config", ...)still resolves the staging selector a staged deploy redirects.EdgeError::status()maps identically in both revs, so no status-code drift. New surface is additive with no-op defaults (Router::with_stateplus a per-requestextensions.extend(state_extensions.clone()), empty here; the Fastlydispatch_with_handlesextendclosure is|_req, _extensions| {}on the default path). One thing deliberately not raised as a finding: edgezero'sConfigOutOfDateerror now emits a redactedfield_pathand a literal<app-cli>placeholder, which would be an operator-visible 503-body regression — except this repo never uses edgezero'sAppConfig<C>extractor, loading config through its ownconfig_payload.rs->BlobEnvelope->Settings::from_json_valuepath withTrustedServerError::Configuration. Not reachable. -
📝 What was and was not verified locally. Run in an isolated worktree at this exact head:
cargo fmt --all -- --checkclean; pinned prettier 3.8.1 fromdocs/node_modules(notnpx, which resolves a different version and reports false failures)--check .clean at baseline, for each suggestion alone, and for all three batched;cargo metadata --locked --offlineexit 0;./scripts/test-cli.shexit 0;cargo test -p trusted-server-cli --target aarch64-apple-darwin --lib168 passed, 0 failed, with all 14 newrun::tests::*present and green. Byte-exact pre/post patch snapshots showed no drift on every verification run. Relied on green CI at this head for the six-alias clippy chain,cargo test-fastly/-axum/-cloudflare/-spin, the parity suite, and vitest — no Rust suggestion is proposed, so none was required for scratch verification. Not verifiable from here at all: the runtime behavior of every lifecycle command (deploy --staging,healthcheck,rollback,active-version,config gc), which needs a live Fastly service and API token. All coverage in this PR and in this review is clap parse-level only.
CI Status
- cargo fmt: PASS (required)
- cargo test: PASS (required)
- format-docs: PASS (required)
- format-typescript: PASS (required)
- cargo test (axum native): PASS
- cargo test (cross-adapter parity): PASS
- cargo test (ts CLI, native): PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- vitest: PASS
- integration tests: PASS
- integration tests (Fastly EC lifecycle): PASS
- browser integration tests: PASS
- prepare integration artifacts: PASS
- CodeQL: PASS
- Analyze (rust): PASS
- Analyze (javascript-typescript): PASS
- Analyze (javascript-typescript): PASS
- Analyze (actions): PASS
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Summary
Reviewed dd8bcdde7e7c5773ebda09411e37525d877ca2f6 against d97bda69f89b15bc61f30f28ad12cc2767924897. The dependency migration and CLI wiring are otherwise consistent, and the relevant local checks and CI pass. I left one inline P1 finding on the staged-config runtime path.
# Conflicts: # crates/trusted-server-cli/tests/config_env_overlay.rs # crates/trusted-server-core/src/config.rs # docs/guide/cli.md
stackpop/edgezero#316 was squash-merged and released as v0.0.6, so the six deps move from the interim rev = "5f3d648c" pin to tag = "v0.0.6" (c8d6e414). The squash makes this different bytes rather than a mechanical reference swap: the CLI argument surface is byte-identical to the prior pin, the documented behaviors (healthcheck attempts clamp, staging selector-only mirror, leaves-only env overlay) were re-verified at the tag, and the full local gate list was re-run. The lock is rebuilt from main with a scoped update and the five resolver-unified edges restored; the only non-edgezero changes are syn 3.0.4 and toml_edit 0.25.12.
…-upgrade # Conflicts: # crates/trusted-server-cli/tests/config_env_overlay.rs
Apply review round 4: capture the production rollback target with active-version before deploying (nothing emits it afterwards), label the lifecycle trio Fastly-only, state where healthcheck's <version> comes from, name --store alongside --no-env as a way a destructive gc sweep can retarget, and present the deploy -- separator as the migration it is. Document the staging config limitation honestly: a staged version currently loads the production blob because the Fastly entry point does not read the version-linked edgezero_runtime_env selectors. The non-hack fix needs edgezero to expose its runtime-env loader as public API, filed as stackpop/edgezero#349 with the downstream wiring tracked in #1082; the stale previous-version rollback hint is stackpop/edgezero#350. Add parse coverage for config gc --store.
|
Need to merge stackpop/edgezero#351 and stackpop/edgezero#352 |
# Conflicts: # Cargo.lock # Cargo.toml
Summary
Upgrades the six edgezero workspace dependencies from
tag = "v0.0.4"totag = "v0.0.6", the release containing the squash-merge of stackpop/edgezero#316 (deploy actions + Fastly staging lifecycle), and adapts Trusted Server to its API changes.edgezero-adapter-{axum,cloudflare,fastly,spin},edgezero-cli, andedgezero-coreattag = "v0.0.6"(c8d6e414); the lock is rebuilt from main with a scoped update, so the only non-edgezero lock changes aresyn 3.0.4andtoml_edit 0.25.12(both required by the edgezero tree).tsCLI subcommands surfaced byedgezero-cli—active-version,healthcheck,rollback,config gc— plusdeploy --staging,config push/diff --staging, and a top-level--versionflag, with argument-parsing coverage.config gcsurface indocs/guide/cli.md: the staged-deploy config leg, capture-before-deploy rollback ordering, Fastly-only labeling, the destructiveconfig gc --yes/--storesemantics, and thedeploy--passthrough migration.TrustedServerAppConfigto theAppConfigMeta::secret_fields()method that replaces the removedSECRET_FIELDSassociated constant.Known limitation (staging config selectors)
A staged version currently loads the production config blob: Trusted Server's Fastly entry point bypasses
edgezero_adapter_fastly::run_appand never reads the version-linkededgezero_runtime_envselectors that a staged deploy redirects. The clean fix needs edgezero to expose its runtime-env loader as public API (the store name and key derivation are upstream internals; duplicating them here would drift):EnvConfigAPI for custom Fastly entry points; fix open as Expose runtime_env_config for custom Fastly entry points stackpop/edgezero#351run_rollback's error hint references a deployprevious-versionoutput that does not exist; fix open as Fix rollback hint to reference a capture route that exists stackpop/edgezero#352docs/guide/cli.mdcarries a known-limitation callout warning operators not to rely on--stagingto validate config changes until this lands.Upstream merge resolution
#316 was squash-merged — the different-bytes case flagged in review — so the repin was treated as a content change: the CLI argument surface is byte-identical between the prior
5f3d648cpin andv0.0.6, the documented behaviors (healthcheckattempts = retry.max(1)clamp, selector-only staging mirror, leaves-only env overlay) were re-verified at the tag, and the full local gate list was re-run. Lockstep reminder for the deployer (outside this repo): movedeploy-fastly@<ref>tov0.0.6when adopting the actions.Verification
All CI gates re-run locally against
v0.0.6and pass:cargo fmt --all -- --check— cleancargo clippy— fastly, axum, cloudflare native + wasm, spin native + wasm — cleanparity(13) — all passcargo metadata --lockedOut of scope
deploy-fastly/healthcheck-fastly/rollback-fastly) in this repo's workflows is separate, unstarted work.ts --versionoutput (reviewer suggestion) is a follow-up.Closes #939