Repository navigation
Conversation
Move the EdgeZero pin from 8efad3c8 to 657bfdcb. The branch head replaces the edgezero_runtime_env selector store with logical-ID resource links and removes runtime_env_config, so the Fastly entry point now opens trusted_server_config and trusted_server_secrets by logical ID and derives the config key from Fastly's staging signal through EdgeZero's target key rule. Expose the local Viceroy secret store under trusted_server_secrets, drop the runtime selector store from fastly.toml and the integration template, and regress that shape in the config test. Update the Fastly and CLI guides for deterministic config keys, logical-ID links, and the application release that managed deploys now require.
Move the EdgeZero pin from 657bfdcb to 74207863. Upstream fixed the Fastly config key to the logical store ID for production, staging, and local Viceroy alike, removing the `_staging` suffix and `store_key_for_target`. Staging isolation now comes only from the physical Config Store the staging environment selects. Replace `RuntimeStoreConfig::for_target(staging)` with `logical()`, drop the staging signal from both entry points, and update the CLI guide, spec, and plan to describe the fixed key.
The only upstream change since 74207863 hardens immutable release publication in the Fastly CLI deploy path. No public API or runtime contract changed, so this is a lockfile-only update.
The only upstream change since dbb95018 aligns the immutable release verifier with the complete set of adapter-manifest references in edgezero.toml. No public API or runtime contract changed, so this is a lockfile-only update.
The only upstream change since fba2076a scopes the Fastly release verifier and the config push preflight to the selected adapter. A Fastly config push no longer reads the Spin or Cloudflare manifests. No public API or runtime contract changed, so this is a lockfile-only update.
Upstream now parses the live Fastly resource-link types (`config`, `kv-store`, `secret-store`), documents the custom entry point migration, and moves the release verifier into edgezero-adapter, which adds serde, serde_json, sha2, and walkdir edges under its cli feature. No public API used by Trusted Server changed, so this is a lockfile-only update.
The two upstream commits since 4531aeec touch only the deploy action script and a demo lockfile. No crate source changed, so this is a lockfile-only update.
The two upstream commits since 7162b7e2 only reorganize the EdgeZero CI test workflows. No crate source changed, so this is a lockfile-only update.
Upstream replaces the Compute-unsupported version diff snapshot with per-collection reads and drops the parent service ID comparison from the staged-source guard and staging rollback. No public API used by Trusted Server changed, so this is a lockfile-only update.
The only upstream change since fd45db1f corrects the Google Pub/Sub logging snapshot path to the Fastly API's `logging/pubsub` and adds `logentries` to the swept endpoint kinds. No public API used by Trusted Server changed, so this is a lockfile-only update.
The only upstream change since 6258b6b2 switches the Fastly deploy to the current `service resource-link` and `service version` command spellings, removing the CLI deprecation notices. No public API used by Trusted Server changed, so this is a lockfile-only update.
# Conflicts: # crates/trusted-server-adapter-fastly/src/app.rs # crates/trusted-server-adapter-fastly/src/main.rs # crates/trusted-server-integration-tests/fixtures/configs/viceroy-template.toml # fastly.toml
The only crate change since bb4e0040 canonicalizes the Fastly configuration snapshot by sorting object keys before ordering rows, so the drift check no longer fails on Fastly's random JSON key order. No public API used by Trusted Server changed, so this is a lockfile-only update.
The only upstream commit since cf9a96a0 adds a Cargo target cache to EdgeZero's CI. No crate source changed, so this is a lockfile-only update.
The build script asked Cargo to rerun on every file under lib/, including node_modules, which emitted 33,932 rerun-if-changed directives in a 2.8 MB output that Cargo re-checked on every build and invalidated the crate after any npm install. Watch the TypeScript sources, package manifests, and bundler configuration explicitly instead. Measured on an Apple Silicon host with a warm cache: a no-op `cargo check -p trusted-server-js` drops from 1.1s to 0.15s, the script emits 7 directives, touching a node_modules file no longer reruns it, and touching lib/src or package-lock.json still does.
The only upstream commit since 119fbf80 fixes a shellcheck finding in EdgeZero's CI cache action. No crate source changed, so this is a lockfile-only update.
Replace the mutable `branch =` pin on all six EdgeZero dependencies with `rev = "12c3215c2637d961a27eefa12e235033fadb4c4a"`, the commit the lockfile already resolved, so `main` stays reproducible and no `cargo update` or upstream force-push can move it. Issue #1195 tracks replacing the rev with the release tag once the upstream PR merges. Assert the legacy physical `ts_secrets` store is absent from both Viceroy configurations, so a squash merge that resurrects the block fails instead of writing entries into a store nothing opens. Correct the `RuntimeStoreConfig::logical` doc comment: the crate does read the process environment elsewhere, so state the real invariant, that no store selector reaches the runtime. Document the Config Store selector in the Fastly guide alongside the secrets one, and drop the leftover instruction not to create a `trusted_server_secrets` link, which the deployment now creates itself.
|
Addressed all findings from the latest review in a1710a0. Blocking, mutable branch pin. All six EdgeZero dependencies now use Non-blocking, leftover guidance contradicts the new model. The two lines at Non-blocking, Config Store selector undocumented for Fastly. The Config Store section now exports The two inline P3 items are answered on their threads. Verification. |
prk-Jr
left a comment
prk-Jr
left a comment
There was a problem hiding this comment.
Summary
The logical Fastly store bindings match the pinned EdgeZero implementation. One deployment-documentation correction is required; no confirmed runtime regressions were found.
The inline comment carries one GitHub suggestion that can be applied with Commit suggestion.
Blocking
- 🔧 Correct the application-release instructions — see inline at
docs/guide/cli.md:227–232.
Non-blocking
None.
CI Status
All 20 reported checks pass:
- PASS: integration tests
- PASS: browser integration tests
- PASS: integration tests (Fastly EC lifecycle)
- PASS: CodeQL
- PASS: cargo test (ts CLI, native)
- PASS: cargo check (cloudflare native + wasm32-unknown-unknown)
- PASS: Analyze (javascript-typescript) (both reported runs)
- PASS: cargo test (cross-adapter parity)
- PASS: cargo fmt
- PASS: cargo test (axum native)
- PASS: Analyze (actions)
- PASS: format-typescript
- PASS: cargo test
- PASS: CLAUDE.md symlink guard
- PASS: format-docs
- PASS: cargo check/build/test (spin native + wasm32-wasip1)
- PASS: Analyze (rust)
- PASS: prepare integration artifacts
- PASS: vitest
The documentation suggestion was scratch-checked with Prettier; the reviewer worktree was restored clean.
dhruv8sh
left a comment
dhruv8sh
left a comment
There was a problem hiding this comment.
Summary
Reviewed head a1710a05 against EdgeZero at the pinned 12c3215.
The runtime change is correct and small: the Fastly entry point opens trusted_server_config / trusted_server_secrets by logical ID and reads the config entry under the logical ID for every target, which matches how EdgeZero links stores at that commit and matches edgezero.toml. The blocking issues are in the operator docs: one staging instruction can overwrite production config, and there is no upgrade path for services already deployed from main.
Findings already raised by earlier reviews are not repeated here (branch pin → rev =, EDGEZERO_MANIFEST, Config Store selector example, ts_secrets assertion, runtime-env doc comment, and the package-fastly-application-release action name thread at cli.md:232).
Blocking
🔧 wrench
- Staging config push docs can overwrite production config — see inline at
docs/guide/cli.md:245 - No upgrade path for services on
main;fastly compute publishstill documented — see inline atdocs/guide/fastly.md:325
Non-blocking
⛏ nitpick / 🏕 camp site / 📝 note
--staging/--keywording — see inline atdocs/guide/cli.md:255- Design spec still says branch dependency — see inline at the design spec
scripts/smoke-fastly.shstill targetsts_secretsand fails — see cross-cutting- Pin is one commit behind PR 381 head — see cross-cutting
- Stale "derived staging key" test messages — see cross-cutting
Cross-cutting / body-level findings
- 🔧
docs/guide/edgezero.md:19(outside diff) still says--staging"writesLOGICAL_ID_stagingin the same physical store", contradicting the newcli.md. - 🏕
scripts/smoke-fastly.sh:93,134-142anddocs/guide/fastly.md:386still use[local_server.secret_stores.ts_secrets], which nothing opens any more. Running the smoke on this head fails (expected HTTP 500 ... received 200, exit 1). To be fair: it fails identically onmain(a4e01eb5) — the basefastly.tomlalready contains the three secrets and the awk only strips the appended unindented blocks, so the base values still resolve — and renaming the store alone does not fix it. Fix both (rename + strip/override base entries in the copiedfastly.toml) or file a follow-up; CI does not run this script. - 📝 Pin is one upstream commit behind PR 381. Upstream head is
61e8444("address managed Fastly deployment review"), which changesconfig gcstore resolution (manifest defaults, rejects empty selectors) and touches provision and the release verifier. Low impact for Trusted Server, but worth bumping or noting in #1195. - ⛏
crates/trusted-server-cli/src/run.rs:482,494,505,518(outside diff) still say "derived staging key" in test messages.
Verification
Isolated worktree at a1710a05:
| Check | Result |
|---|---|
cargo fmt --all -- --check |
pass |
cargo clippy-fastly |
pass |
cargo build -p trusted-server-adapter-fastly --release --target wasm32-wasip1 |
pass (unchecked in the PR test plan) |
cargo test-fastly -p trusted-server-adapter-fastly runtime_store_config |
1 passed |
integration local_fastly tests |
3 passed |
./scripts/smoke-fastly.sh (PR head / main) |
fails the same way on both |
EdgeZero source read at 12c3215: resolve_config_store_and_key, validate_fastly_config_key, ConfigPushArgs --staging, and the custom-entrypoint migration guide.
CI Status
All 20 checks pass on a1710a05, including required cargo fmt, cargo test, format-typescript, and format-docs.
…-381 # Conflicts: # crates/trusted-server-adapter-fastly/src/app.rs # crates/trusted-server-integration-tests/fixtures/configs/viceroy-template.toml # docs/guide/fastly.md
Correct the application-release instructions: the action is `package-application-release-fastly`, and `deploy-fastly` consumes a prebuilt archive rather than building one. Warn that `config push --staging` on Fastly selects neither a different store nor a different key. The physical store is whatever EDGEZERO__STORES__CONFIG__<ID>__NAME resolves to in the shell running the push, so a staged push into the production store overwrites the live entry. Set the selector explicitly in both examples. Add an upgrade section for services deployed before logical store links. Only a managed deploy creates the `trusted_server_secrets` link, so `fastly compute publish` activates a version whose link is missing and every publisher route returns 500 while /health still answers 200. Replace that command in the getting-started guide and AGENTS.md, and annotate the manifest deploy command that EdgeZero never runs for a store-declaring app. Fix the Fastly smoke script. It appended and stripped `ts_secrets` blocks, which nothing opens now, and the base manifest defines the same three keys under indented headers, so removing an appended block still left a resolvable value and no missing-secret case could fail. Match either indentation and strip the base entries on copy. Repin to upstream c2e862f4, eight commits ahead of 12c3215c, and update the test for the key-selector change it brings: with no --key the CLI now writes to EDGEZERO__STORES__CONFIG__<ID>__KEY instead of ignoring it. Also correct the contradicting staging-key sentence in the EdgeZero guide, the design spec's branch wording, and two stale test messages.
|
Addressed the review in 86695c3, and resolved the conflict with Conflict resolutionThree files conflicted. Each needed a decision rather than a side:
Cross-cutting findings
Pin was eight commits behind, not one. Repinned to That repin surfaced a behavior change worth flagging. Stale test messages in Verification
|
dhruv8sh
left a comment
dhruv8sh
left a comment
There was a problem hiding this comment.
Summary
Moves the Fastly runtime to logical-ID store links from EdgeZero PR 381, drops the edgezero_runtime_env selector store, renames the Viceroy secret store to trusted_server_secrets, routes deploy docs through ts deploy --application-release, and narrows the trusted-server-js build-script watch list. The runtime change is clean and well tested; one operator guide outside the diff still documents the removed model, which blocks.
2 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. The remaining findings describe the fix in prose because the change touches files outside the diff and can't be auto-applied.
Blocking
🔧 wrench
- Configuration guide still documents the removed
edgezero_runtime_envmodel — see Cross-cutting below (docs/guide/configuration.md:2799-2908)
Non-blocking
⛏ nitpick
- Upgrade note misstates what a leftover
__KEYselector does — see inline atdocs/guide/fastly.md:352-354 - Managed-path upgrade bullet omits
EDGEZERO_MANIFESTand splits inline code — see inline atdocs/guide/fastly.md:346-348
🏕 camp site / 📝 note
- Stale core doc comments on Fastly store resolution — see Cross-cutting below
- PR description and plan still name the old pin — see Cross-cutting below
👍 praise
- Build script watches only bundle inputs — see inline at
crates/trusted-server-js/build.rs:18-32 - Smoke missing-secret cases can now actually fail — see inline at
scripts/smoke-fastly.sh:34-54
Cross-cutting / body-level findings
-
🔧 Configuration guide still documents the removed
edgezero_runtime_envmodel —docs/guide/configuration.md:2799-2908("Fastly Runtime Config Store", "Initial setup with a service-specific store", "Selecting another blob key") is unchanged and still tells operators that the runtime readsEDGEZERO__SERVICES__<SERVICE_ID>__…selectors fromedgezero_runtime_env, to link that store, to publish withfastly compute publish(line 2870), that a__KEYoverride "does not change that write destination", and to select another production key via--keyplus a runtime__KEYentry. With the new pin all of that is wrong: the runtime never reads the selector store,fastly compute publishactivates a version without thetrusted_server_secretslink (every publisher route 500s, per this PR's own upgrade notes), and the Fastly CLI rejects a non-logical--key/__KEY(validate_fastly_config_key). The PR's spec also promisesedgezero_runtime_envreferences are removed from current operator docs; only the link to this anchor was dropped fromfastly.md.Proposed fix (apply manually — the file is outside the diff, so it can't be a
suggestion): replace everything from## Fastly Runtime Config Storeup to, but not including,### Local development(which is still accurate) with:## Fastly Runtime Config Store After the EdgeZero cutover, the Fastly adapter always dispatches through the EdgeZero entry point. The former `edgezero_enabled` and `edgezero_rollout_pct` canary keys are no longer read. `[stores.config].default` in `edgezero.toml` supplies the logical config store ID, currently `trusted_server_config`. The Fastly runtime opens the store by that logical ID and reads the entry under the same key on every target. Store selection is a deploy-time input: a managed `ts deploy --adapter fastly` links the physical store named by `EDGEZERO__STORES__CONFIG__TRUSTED_SERVER_CONFIG__NAME` to the service version under the logical ID. No selector is read from a Config Store at runtime, and on Fastly the CLI rejects any `--key` or `__KEY` selector other than the logical ID. Keep the `__NAME` selector set in the shell or deploy job for every push, so `ts config push` writes into the same physical store the deployment links. Isolate staging by selecting a different physical store. See [Secret Stores](/guide/fastly#secret-stores) and the [CLI lifecycle commands](/guide/cli#lifecycle-commands) for the full deploy and staging flow.
-
🏕 Stale core doc comments on Fastly store resolution —
crates/trusted-server-core/src/settings_data.rs:44-60: the docs ondefault_config_store_nameanddefault_config_keystill say Fastly resolves from service-scoped entries inedgezero_runtime_env, and that a__KEYoverride is ignored by a plain push. Neither holds after this PR. Proposed text (apply manually — outside the diff):/// Resolves the `EdgeZero` app-config store name from the process environment. /// /// Native adapters such as Axum use this wrapper. Fastly reads no selector at /// runtime: it opens the store under the logical ID that a managed deployment /// links to the selected physical store. Without an override, this wrapper /// also returns the manifest default logical store ID. ... /// Resolves the app-config blob key from the process environment. /// /// Native adapters such as Axum use this wrapper. Fastly always reads the /// logical store ID as the key, and its CLI rejects any other `--key` or /// `__KEY` selector.
-
📝 PR description and plan still name the old pin — the description says the EdgeZero dependencies pin
12c3215c…, butCargo.toml/Cargo.lockpinc2e862f436421cdcfbac620c06995f262d6788c9since 86695c3 (the spec is correct).docs/superpowers/plans/2026-09-16-edgezero-fastly-store-selectors.md:117still showsbranch = "fix/fastly-environment-store-selectors". For reference: upstream PR 381 is still open atb537495a, 3 commits ahead of the pin with c2e862f still an ancestor, so the rev is reachable today; #1195 tracks moving to a release tag.
CI Status
- integration tests: PASS
- browser integration tests: PASS
- integration tests (Fastly EC lifecycle): PASS
- CodeQL: PASS
- cargo test (ts CLI, native): PASS
- format-docs: PASS (required)
- Analyze (javascript-typescript): PASS
- cargo test (cross-adapter parity): PASS
- cargo fmt: PASS (required)
- cargo test (axum native): PASS
- Analyze (actions): PASS
- format-typescript: PASS (required)
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo test: PASS (required)
- prepare integration artifacts: PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- Analyze (rust): PASS
- CLAUDE.md symlink guard: PASS
- vitest: PASS
prk-Jr
left a comment
prk-Jr
left a comment
There was a problem hiding this comment.
Summary
The runtime change is small and correct. RuntimeStoreConfig::logical() opens trusted_server_config / trusted_server_secrets by logical ID and reads the config entry under the logical key. That matches what c2e862f links and what validate_fastly_config_key enforces, and local Viceroy plus the integration fixtures now take the same path. One new blocking item is in the upgrade docs. I also agree with the open blocking finding on configuration.md from the earlier review of this head, and I don't repeat it here.
1 of the inline comments below carries a one-click GitHub
suggestion. Use Commit suggestion to apply it.
Blocking
🔧 wrench
- Upgrade checklist doesn't move store selectors into the deploy environment — see inline at
docs/guide/fastly.md:349-351
Cross-cutting / body-level findings
- 📝 Agreeing with existing open items on this head (not repeated inline): the stale
docs/guide/configuration.md"Fastly Runtime Config Store" section (blocking), the stalesettings_data.rsdoc comments, the__KEYrationale atfastly.md:352-354, theEDGEZERO_MANIFESTomission atfastly.md:346-348, and the PR description still naming pin12c3215cwhileCargo.toml/Cargo.lockpinc2e862f4. When you rewriteconfiguration.md, its "Local development" paragraph needs updating too. Atc2e862f,push_config_entries_localwrites[local_server.config_stores.<logical>]regardless of__NAME, so the<resolved-name>wording and the "service-scoped runtime selectors" sentence are both stale.
Local verification (head 870135e8)
cargo fmt --all -- --check: PASScargo clippy-fastly,cargo clippy-cli,cargo clippy-axum: PASScargo test-fastly: PASS (adapter 194 passed, includingruntime_store_config_opens_logical_store_ids_and_key; core 2887 passed)cargo test -p trusted-server-cli --target aarch64-apple-darwin: PASS (includingconfig_push_writes_the_runtime_key_override_without_the_key_flag)cargo test-axum,cargo check-cloudflare,cargo check-spin: PASS- Integration crate
common::configtests (3): PASS - I traced deploy link reconciliation in the pinned EdgeZero source. I did not run it against a live Fastly service.
CI Status
- integration tests: PASS
- browser integration tests: PASS
- integration tests (Fastly EC lifecycle): PASS
- CodeQL: PASS
- cargo test (ts CLI, native): PASS
- format-docs: PASS
- Analyze (javascript-typescript): PASS
- cargo test (cross-adapter parity): PASS
- cargo fmt: PASS
- cargo test (axum native): PASS
- Analyze (actions): PASS
- format-typescript: PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo test: PASS
- prepare integration artifacts: PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- Analyze (rust): PASS
- CLAUDE.md symlink guard: PASS
- vitest: PASS
…-381 # Conflicts: # Cargo.lock # Cargo.toml # crates/trusted-server-adapter-fastly/src/main.rs
Correct the upgrade checklist: on Fastly the CLI rejects any config key other than the logical store ID, so a leftover `__KEY` selector makes `ts config push` and `ts deploy` fail rather than writing to a key the runtime ignores. My earlier note had this backwards. Add the selector-migration steps the checklist was missing. Service-scoped `EDGEZERO__SERVICES__<SERVICE_ID>__STORES__…__NAME` entries must move to the unscoped variables in the deploy environment, because an unset selector falls back to the logical ID and links whichever account-level store carries that name, possibly another service's. Also call out `trusted_server_kv`, which the managed deploy links even though the Fastly runtime never opens it. Name `EDGEZERO_MANIFEST` in the managed-deploy step, matching the CLI and getting-started guides. Replace the stale "Fastly Runtime Config Store" section in the configuration guide. It still described the removed `edgezero_runtime_env` selector model, `fastly compute publish`, and `--key` overrides. Its local development paragraph also claimed `--local` writes under the resolved `__NAME`; it writes under the logical ID. Update the matching doc comments in settings_data.rs. Read the sandbox reuse bounds from the application's own config store under its logical ID. They previously came from `edgezero_runtime_env`, which is deprecated and which this branch's pin no longer exposes a constant for. The service-scoped key prefix goes with it, since it only existed to namespace entries inside that shared store.
|
Addressed both reviews in fa0353b and resolved the conflict in c0dc08c. The conflict was a genuine fork, not a textual one
Sandbox bounds moved off the deprecated store
FindingsAll three applied with your wording. On the Verification
To be precise about that last one: |
The skill's guardrails told operators to always pass `--service-id` to `fastly compute publish`. That command now strands a service: it clones the active version's resource links, so the new version lacks the logical links the runtime opens, app config fails to load, and every publisher route returns 500 while `/health` still answers 200. Point the guardrail at the managed deploy instead, and keep the `--service-id` advice for whichever deploy command is used, since a checked-in `fastly.toml` can still pin a dead service.
prk-Jr
left a comment
prk-Jr
left a comment
There was a problem hiding this comment.
Summary
The Fastly runtime bindings match the pinned EdgeZero logical-resource-link and config-key contracts. No runtime correctness regression was found in this review, and all 22 reported GitHub checks pass. The two documentation observations below are nonblocking follow-ups.
Non-blocking
- 🤔 Document the managed-deploy token prerequisite — see inline at
docs/guide/cli.md:224. - 🤔 Preserve config used by legacy staged versions — see inline at
docs/guide/fastly.md:369–371.
Validation
Reviewed the changed files and exact EdgeZero revision 0645339d1848332f4805259d29e3b4b881fccad3. Both proposed documentation replacements pass Prettier; both changed shell scripts pass bash -n. Rust and JS suites were not rerun locally because the remote checks passed. The isolated review worktree is clean.
CI Status
- integration tests: PASS
- browser integration tests: PASS
- integration tests (Fastly EC lifecycle): PASS
- CodeQL: PASS
- cargo test (ts CLI, native) (macos-latest): PASS
- cargo test (axum native): PASS
- CLAUDE.md symlink guard: PASS
- cargo test: PASS (required)
- format-typescript: PASS (required)
- Analyze (rust): PASS
- format-docs: PASS (required)
- cargo fmt: PASS (required)
- Analyze (javascript-typescript): PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- cargo test (cross-adapter parity): PASS
- cargo test (ts CLI, native) (ubuntu-latest): PASS
- prepare integration artifacts: PASS
- vitest: PASS
- Analyze (actions): PASS
- Analyze (python): PASS
- Analyze (javascript-typescript): PASS
| bare `ts deploy --adapter fastly` without a release is accepted only for | ||
| store-free applications. The CLI loads `edgezero.toml` from the working | ||
| directory unless `EDGEZERO_MANIFEST` names another file, and it rejects a | ||
| manifest outside the release root, so select the release's own manifest: |
There was a problem hiding this comment.
🤔 Nonblocking follow-up — Document the managed-deploy token prerequisite. The setup guides configure a Fastly CLI profile, but the pinned EdgeZero managed deploy calls require_token() and reads a nonempty FASTLY_API_TOKEN directly from the process environment. Following the profile setup alone therefore makes these commands fail with FASTLY_API_TOKEN must be set in the environment. Please name that prerequisite beside the commands. Suggested wording:
manifest outside the release root, so select the release's own manifest.
Managed deployment also requires a nonempty `FASTLY_API_TOKEN` in the process
environment; signing in with `ts auth login` or a Fastly CLI profile alone
does not supply the token used by the adapter's direct API calls. Export the
token before running:| - Delete the stale `edgezero_runtime_env` link and any `*_staging` config | ||
| entries once no active version reads them. Nothing in Trusted Server reads | ||
| that store any more. If you set the optional `TS__SANDBOX__*` reuse bounds |
There was a problem hiding this comment.
🤔 Nonblocking follow-up — Preserve config used by legacy staged versions. A production upgrade can leave an older staged version deployed. Its version-linked selector still reads <logical-store-id>_staging, so removing that shared entry when no production-active version uses it can make the staged publisher routes return 500. Please retain the legacy entries until active, staged, and retained rollback versions no longer depend on them. Suggested wording:
- Delete the stale `edgezero_runtime_env` link and any `*_staging` config
entries only after no active, staged, or rollback version depends on them.
This release no longer reads that store. If you set the optional
`TS__SANDBOX__*` reuse bounds
dhruv8sh
left a comment
dhruv8sh
left a comment
There was a problem hiding this comment.
Summary
Moves the Fastly runtime to open its config and secret stores by logical ID, with deploy-time resource links replacing the edgezero_runtime_env selector store. It also narrows the trusted-server-js build-script watch set. The runtime change is clean and the EdgeZero claims check out at the pinned rev: validate_fastly_config_key rejects non-logical keys, and managed deploys require --application-release. One operator-safety gap in the upgrade docs needs to close before merge.
2 of the inline comments below carry a one-click GitHub
suggestion. Use Commit suggestion (or Add suggestion to batch for both) to apply them as commits on the PR branch. The remaining comments describe the change in prose because they are questions or judgement calls for the author.
Blocking
🔧 wrench
- A
--stagingconfig push now overwrites the production config when__NAMEis unset (see inline atdocs/guide/fastly.md:363)
Non-blocking
⛏ nitpick / 🤔 thinking
- CLI test comment understates the Fastly behaviour (see inline at
crates/trusted-server-cli/tests/config_store_defaults.rs:139) limit_keys_are_unscoped_app_config_keysonly asserts constants (see inline atcrates/trusted-server-adapter-fastly/src/sandbox.rs:361)deploy = "fastly compute publish"is kept but documented as unused (see inline atedgezero.toml:56)
👍 praise
- Narrowed
rerun-if-changedset inbuild.rs(see inline atcrates/trusted-server-js/build.rs:22)
CI Status
- Analyze (javascript-typescript): PASS
- Analyze (rust): PASS
- Analyze (python): PASS
- Analyze (actions): PASS
- CodeQL: PASS
- CLAUDE.md symlink guard: PASS
- browser integration tests: PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- cargo fmt: PASS (required)
- cargo test: PASS (required)
- cargo test (axum native): PASS
- cargo test (cross-adapter parity): PASS
- cargo test (ts CLI, native) (ubuntu-latest): PASS
- cargo test (ts CLI, native) (macos-latest): PASS
- format-docs: PASS (required)
- format-typescript: PASS (required)
- integration tests: PASS
- integration tests (Fastly EC lifecycle): PASS
- prepare integration artifacts: PASS
- vitest: PASS
| - Re-push staging app config under the logical key into the staging Config | ||
| Store. Entries previously pushed under `<logical-store-id>_staging` are | ||
| ignored. |
There was a problem hiding this comment.
🔧 wrench: A --staging config push now overwrites the production config when __NAME is unset.
On main, ts config push --adapter fastly --staging wrote the separate <logical-store-id>_staging key, so a staged push could not touch production's entry. With this PR, --staging only changes which physical store is selected through EDGEZERO__STORES__CONFIG__TRUSTED_SERVER_CONFIG__NAME. When that variable is unset, the push falls back to the logical-ID store, which is normally the one production links, and it overwrites production's entry. Production instances pick it up on their next start. EdgeZero allows staging and production to share a store on purpose (args.rs: "Production and staging may select the same or different physical stores"), so nothing on the CLI side stops this.
cli.md explains the hazard, but this upgrade checklist is what existing operators will follow, and an unchanged runbook that ran --staging safely before this PR would now push staging config to production. The checklist should say so explicitly:
| - Re-push staging app config under the logical key into the staging Config | |
| Store. Entries previously pushed under `<logical-store-id>_staging` are | |
| ignored. | |
| - Re-push staging app config under the logical key into the staging Config | |
| Store, with `EDGEZERO__STORES__CONFIG__TRUSTED_SERVER_CONFIG__NAME` set to | |
| that store. Entries previously pushed under `<logical-store-id>_staging` are | |
| ignored. Do not rerun an existing `ts config push --adapter fastly --staging` | |
| runbook unchanged: `--staging` no longer selects a separate key, so without | |
| the `__NAME` selector it writes the production entry in the logical-ID store, | |
| and production instances load it on their next start. |
Separately, consider having ts config push --staging on Fastly refuse to run when the __NAME selector is unset. That would turn this from a documentation warning into a guard. It can be a follow-up issue.
| // runtime that reads it cannot diverge. The Fastly runtime reads the logical id | ||
| // for every target, so an operator deploying to Fastly should leave this unset, | ||
| // which the Fastly guide now states. |
There was a problem hiding this comment.
⛏ nitpick: This comment understates the Fastly behaviour. The Fastly CLI doesn't just recommend leaving __KEY unset, it rejects any value other than the logical ID (validate_fastly_config_key). The comment also doesn't say that this test runs the Axum adapter, which is why the override is honoured here.
| // runtime that reads it cannot diverge. The Fastly runtime reads the logical id | |
| // for every target, so an operator deploying to Fastly should leave this unset, | |
| // which the Fastly guide now states. | |
| // runtime that reads it cannot diverge. This test runs the Axum adapter: the | |
| // Fastly runtime reads the logical id for every target, and the Fastly CLI | |
| // rejects any other `__KEY` value, so the override only applies off Fastly. |
| "EDGEZERO__SERVICES__0000000000000000000000__TS__SANDBOX__TIMEOUT_MS", | ||
| "should scope the wait timeout the same way" | ||
| ); | ||
| fn limit_keys_are_unscoped_app_config_keys() { |
There was a problem hiding this comment.
⛏ nitpick: This test asserts that string literals start with TS__SANDBOX__ and don't contain EDGEZERO__SERVICES__, so it can only fail if someone edits the constants and the test together. The behaviour that matters is that read_raw_limits opens DEFAULT_CONFIG_STORE_ID and looks the keys up unscoped, and this test doesn't cover it. I'd drop it rather than keep a test that can't catch a regression. Your call.
| # service version under its logical ID. Publishing with the Fastly CLI directly | ||
| # clones the active version's links and omits the logical links the runtime | ||
| # opens. | ||
| deploy = "fastly compute publish" |
There was a problem hiding this comment.
🤔 thinking: The new comment says this command is unused because EdgeZero always takes the managed path for an app with stores, and the docs now say never to deploy with fastly compute publish. If EdgeZero's manifest schema doesn't require deploy, removing the line is safer than keeping a command someone could copy into a runbook. If the schema does require it, the comment is fine as it is.
| // output changes. `lib` as a whole is deliberately not watched: it holds | ||
| // `node_modules`, and enumerating that tree emitted tens of thousands of | ||
| // directives that Cargo re-checked on every build. | ||
| for watched in [ |
There was a problem hiding this comment.
👍 praise: Replacing the recursive lib/ walk with explicit inputs is a good fix: 7 directives instead of 33,932, and an npm install no longer invalidates the crate. Every watched path exists, which matters because a missing path would make Cargo rerun the script on every build.
This branch has not been deployed
4 participants
Summary
EDGEZERO__STORES__<KIND>__<ID>__NAMEdeployment selector to the target version under its logical ID, and the runtime opens stores by that ID.trusted_server_configandtrusted_server_secretsby logical ID in the Fastly entry point and read the config entry undertrusted_server_configfor every target. The upstream branch removedruntime_env_configand theedgezero_runtime_envConfig Store it read, and fixed the Fastly config key to the logical ID; staging isolation comes from the physical store the staging environment selects.trusted_server_secretsand drop theedgezero_runtime_envselector store, so local runs exercise the same store-opening path as a deployed version.ts deploy --adapter fastlypasses a verified application release.trusted-server-jsbuild script. It previously emitted a rerun directive for every file underlib/, includingnode_modules, so any npm install invalidated the crate and Cargo re-checked 33,932 paths on every build.The EdgeZero dependencies pin the immutable commit
0645339d1848332f4805259d29e3b4b881fccad3on the unmerged upstream branch withrev =, somainstays reproducible andcargo updatecannot move it. Issue #1195 tracks replacing the rev with the release tag after the upstream PR merges.Changes
Cargo.toml,Cargo.lock0645339dwithrev =.main.rs,app.rsRuntimeStoreConfig::from_envwithlogical(): logical store IDs and the logical config key for every target. A unit test covers the bindings.fastly.toml, Viceroy integration fixture, template-cache scripttrusted_server_secrets; remove theedgezero_runtime_envmapping.--application-releaserequirement for managed deploys.crates/trusted-server-js/build.rslib/walk with explicitrerun-if-changeddirectives forlib/src, the package manifests, and the bundler configuration. No-opcargo check -p trusted-server-jsdrops from 1.1s to 0.15s and the script emits 7 directives instead of 33,932.Closes
Closes #1174
Test plan
cargo test-fastly && cargo test-axumcargo test-cloudflare && cargo test-spincargo clippy-fastly && cargo clippy-axumcargo clippy-cloudflare && cargo clippy-cloudflare-wasmcargo clippy-spin-native && cargo clippy-spin-wasmcargo check-fastly && cargo check-axum && cargo check-cloudflare && cargo check-spincargo fmt --all -- --check./scripts/test-cli.shcargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test integration local_fastly --target aarch64-apple-darwincargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test parity --target aarch64-apple-darwincd crates/trusted-server-js/lib && npm run buildcd crates/trusted-server-js/lib && npm testcd crates/trusted-server-js/lib && npm run formatcd docs && npm run formatcargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1fastly compute serveChecklist
unwrap()added in production code