Skip to content

Report deployed git version in x-ts-version - #1213

Open
dhruv8sh wants to merge 9 commits into
mainfrom
feat/git-version-header
Open

dhruv8sh wants to merge 9 commits into
mainfrom
feat/git-version-header

Conversation

@dhruv8sh

@dhruv8sh dhruv8sh commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • x-ts-version now reports the deployed git version: the tag, else the branch, else exactly the first 6 characters of the commit. It is compiled in from the build-time TRUSTED_SERVER__GIT_VERSION (set by the deploy pipeline), falling back to local git, and is omitted when neither is available. The validated value is always re-emitted under the same name (empty when unknown), so an unvalidated ambient value never reaches the binary.
  • The Fastly service version it used to carry moves to x-ts-fastly-version; an inherited origin value is removed when FASTLY_SERVICE_VERSION is unset or invalid. Under Viceroy, main today answers with x-ts-version: 0, which is the Fastly version, not a Trusted Server one. Breaking for anything that reads x-ts-version as the Fastly version number.
  • Every adapter sends x-ts-version, including the Fastly GET /health fast path that deploy health checks probe and Fastly responses whose finalization is skipped because settings fail to load. Operator response_headers still apply last.

Changes

File Change
crates/trusted-server-core/build.rs Resolve the version (override, then local git) and always re-emit it as TRUSTED_SERVER__GIT_VERSION, read by constants::TS_GIT_VERSION; rerun-if-env-changed=TRUSTED_SERVER__GIT_VERSION; watch the worktree HEAD, the current branch's ref, refs/tags and packed-refs (existing paths only; other branches and refs/remotes skipped)
crates/trusted-server-core/build_support/git_version.rs Pure resolver shared with the test via #[path]; visible-ASCII rule
crates/trusted-server-core/tests/git_version_resolve.rs Resolver tests: precedence, 6-char rule, blanks, non-ASCII and inner-space overrides
crates/trusted-server-core/src/constants.rs HEADER_X_TS_FASTLY_VERSION, TS_GIT_VERSION
crates/trusted-server-core/src/version_header.rs git_version_header_value, apply_git_version_header (+ private testable variants)
crates/trusted-server-core/src/lib.rs Register version_header
crates/trusted-server-adapter-fastly/src/middleware.rs Git version to x-ts-version, FASTLY_SERVICE_VERSION to x-ts-fastly-version (removing an inherited value when unset or invalid); doc comments
crates/trusted-server-adapter-fastly/src/main.rs /health fast path and the finalize-skipped path set x-ts-version
crates/trusted-server-adapter-{axum,cloudflare,spin}/src/middleware.rs Call apply_git_version_header before operator headers
crates/trusted-server-adapter-axum/tests/routes.rs, crates/trusted-server-adapter-spin/src/app.rs /health route tests assert the header
docs/guide/getting-started.md, docs/guide/edgezero.md How to set TRUSTED_SERVER__GIT_VERSION when deploying, and a post-deploy x-ts-version check
docs/guide/first-party-proxy.md Example no longer overrides X-TS-Version
CHANGELOG.md Breaking-change entry
docs/superpowers/{specs,plans}/2026-09-25-git-version-header*.md Design and plan

Closes

Closes #1212

Related: #325 (introduced x-ts-version as the Fastly version)

Test plan

  • cargo test-fastly && cargo test-axum && cargo test-cloudflare && cargo test-spin
  • All eight clippy aliases (clippy-fastly, -axum, -cloudflare, -cloudflare-wasm, -spin-native, -spin-wasm, -cli, -codegen)
  • cargo fmt --all -- --check
  • Parity: cargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test parity
  • JS tests: cd crates/trusted-server-js/lib && npx vitest run (1130 passed; no JS changes. Run with NODE_OPTIONS=--no-webstorage locally, because Node 26's built-in localStorage shadows jsdom's. The repo pins Node 24.12.0, where this doesn't apply.)
  • JS format: cd crates/trusted-server-js/lib && npm run format
  • Docs format: cd docs && npm run format
  • WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
  • Manual testing via fastly compute serve: offline end-to-end under Viceroy (below)

End-to-end under Viceroy. Release Wasm served with fastly compute serve --file <wasm> and a local config pushed with ts config push --adapter fastly --local, set up the same way as scripts/smoke-fastly.sh. Each case curls GET /health and GET / and asserts 200. x-geo-info-available on / proves the response went through apply_finalize_headers. Viceroy reports FASTLY_SERVICE_VERSION=0.

Case Build /health x-ts-version / x-ts-version / x-ts-fastly-version
Override TRUSTED_SERVER__GIT_VERSION=v9.9.9-test v9.9.9-test v9.9.9-test 0
Warm-cache rebuild …=v9.9.9-test2, no clean; Cargo: "the env variable TRUSTED_SERVER__GIT_VERSION changed" v9.9.9-test2 v9.9.9-test2 0
No env, on a branch unset feat/git-version-header same 0
No env, detached unset; rebuild triggered by the worktree HEAD 6-char hash same 0
No env, tagged HEAD unset; local tag the tag same 0
Non-git tree copy without .git, unset absent absent 0

It also checks the rebuild triggers. A new tag alone triggers a rebuild through refs/tags. Moving the current branch off a tagged commit triggers one through the branch ref and switches the value from the tag to the branch. A simulated fetch (git update-ref refs/remotes/...) and a ref update on another branch both leave core Fresh. An override of v1-été prints a cargo:warning and falls back to local git.

Checklist

  • Changes follow AGENTS.md conventions
  • No unwrap() in production code — use expect("should ...")
  • Uses tracing macros (not println!) — this repo uses log per AGENTS.md; the only println! calls are the cargo: directives in build.rs
  • New code has tests
  • No secrets or credentials committed

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
…rsion on Fastly

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
@dhruv8sh dhruv8sh self-assigned this Sep 25, 2026
@dhruv8sh
dhruv8sh requested review from ChristianPavilonis, aram356 and prk-Jr and removed request for ChristianPavilonis September 25, 2026 11:49
@aram356 aram356 added this to the 202610 milestone Sep 28, 2026

@ChristianPavilonis ChristianPavilonis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review summary

Reviewed cd704a7aacb4df866c471cfafe3cf6d130ad5fcb against a4e01eb55fe940bd02b2426dccd46050704c54c5. Requesting changes for the two P2 correctness issues documented inline. Both were reproduced in temporary fixtures without editing repository files.

Validation

The 10 resolver tests, 7 core header tests, and focused Fastly, Axum, Cloudflare, and Spin tests passed. Consecutive override builds emitted the requested versions without cleaning the target cache; removing the override restored the local branch value. All 20 reported CI checks pass.

The second pass exercised the shipped header helper with repository-locked dependencies and the unchanged build script in a packed-ref Cargo fixture. No duplicate review feedback was present. Full Viceroy HTTP smoke testing and live Cloudflare/Spin deployment were not independently repeated.

Comment thread crates/trusted-server-core/src/version_header.rs
Comment thread crates/trusted-server-core/build.rs Outdated

@prk-Jr prk-Jr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Summary

The compiled-in version and shared header helper provide consistent version reporting across adapters, and the Fastly service-version rename is documented. Two cases can still report a misleading deployed version: retaining an origin's header when the build version is unknown, and resolving Git metadata from an unrelated parent repository.

Verdict: REQUEST_CHANGES

Two inline comments contain one-click GitHub suggestion blocks. The third is a nonblocking improvement to a documented cache-invalidation limitation.

Blocking

  • 🔧 Remove inherited version headers when the build version is unknown — see crates/trusted-server-core/src/version_header.rs:37–39.
  • 🔧 Reject Git metadata from an unrelated parent repository — see crates/trusted-server-core/build.rs:35.

Non-blocking

  • 🤔 Improve invalidation when the current branch ref is packed — see crates/trusted-server-core/build.rs:46–48.

Verification

All 10 resolver tests passed locally. Each one-click suggestion independently passed formatting, all eight target-matched clippy aliases, Fastly/Axum/Cloudflare compile checks, all four adapter test aliases, and the 14-test parity suite. Post-verification patches matched the approved suggestion bytes exactly. Targeted scratch probes verified inherited-header removal and correct repository-root handling for normal checkouts, linked worktrees, exported sources, and explicit overrides.

CI Status

Comment thread crates/trusted-server-core/src/version_header.rs
Comment thread crates/trusted-server-core/build.rs
Comment thread crates/trusted-server-core/build.rs
Remove an existing x-ts-version when the compiled-in version is unknown or invalid, so a proxied origin header is never reported as the deployed version.

Ignore git metadata unless the repository top level is the workspace root, and watch the nearest existing ancestor of a packed current-branch ref so the next commit re-runs the build script.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>

@ChristianPavilonis ChristianPavilonis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review summary

Reviewed e6904260ab48917114d9b134bc7f5fbcfe826f6f against a4e01eb55fe940bd02b2426dccd46050704c54c5, from feat/git-version-header into main.

Inspected all 17 changed files and traced build inputs, header finalization, adapter conversions, configuration precedence, and consumers. The header rename is intentionally breaking and documented. No new actionable issues remain at this revision.

Safety proof

  1. Version resolution survives the tested cache and repository transitions. Executed 16 passing probes in temporary Cargo fixtures using the unchanged build script and resolver. These covered override changes/removal, tag creation, packed-branch advancement, detached HEAD, linked worktrees, exported sources, and unrelated parent repositories. Proven for these cases.
  2. Finalized responses report the compiled version rather than an inherited origin value. All 10 core header tests passed, including valid replacement and unknown/invalid removal. Focused tests passed for all four adapters, the Fastly header split, Fastly /health, Axum's service path, and Spin's startup-fallback /health. Proven for these paths.

Findings

No meaningful findings introduced by this PR remain at the reviewed revision.

Validation and review context

Passed:

cargo test-fastly version_header
cargo test-fastly --test git_version_resolve
cargo test-fastly version_headers_split_git_and_fastly_versions
cargo test-fastly health_response_reports_git_version_only
cargo test-axum emits_git_version_header
cargo test-axum --test routes health_reports_git_version
cargo test-cloudflare emits_git_version_header
cargo test-spin emits_git_version_header
cargo test-spin startup_error_router_answers_health_with_200
cargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test parity
cargo fmt --all -- --check
cargo clippy-fastly
cargo clippy-axum
cargo clippy-cloudflare
cargo clippy-cloudflare-wasm
cargo clippy-spin-native
cargo clippy-spin-wasm

Parity: 14 passed. The lint batch initially timed out during Cloudflare WASM compilation; the remaining aliases passed on rerun. Build-script fixtures executed cargo run --offline --quiet --manifest-path <temporary-workspace>/Cargo.toml; all 16 probes passed.

  • CI: gh pr checks 1213 reports only one passing JavaScript/TypeScript CodeQL check at this head. The earlier revision's full green suite does not validate this revision.
  • Existing feedback: Reviewed bodies, inline comments, replies, issue comments, and thread resolution. All five prior threads are resolved; the three distinct defects are fixed and independently checked.
  • Residual risk: Full CI and live Fastly, Cloudflare, and Spin HTTP deployment checks were not repeated. Missing Git metadata watch paths retain the documented invalidation limitation.
  • No repository files were modified. The worktree remained clean, and head and base were rechecked before submission.

@prk-Jr prk-Jr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Summary

Reviewed e6904260ab48917114d9b134bc7f5fbcfe826f6f against merge base a4e01eb55fe940bd02b2426dccd46050704c54c5. No actionable findings remain. The compiled-in version is shared across adapters, the Fastly service-version rename is documented, and operator response-header precedence is preserved.

The latest commit addresses the previous inherited-header, unrelated-parent-repository, and packed-branch-ref findings. Header regression tests and temporary Cargo fixtures independently verified those behaviors.

Verdict: APPROVE

Validation

  • 27 distinct focused Rust tests passed: 10 resolver tests, 10 shared header tests, and 7 adapter middleware/health tests.
  • Cross-adapter parity suite: 14 passed.
  • All eight target-matched clippy aliases passed, including CLI and codegen.
  • cargo fmt --all -- --check passed.
  • 15 temporary Cargo build/version probes passed, covering overrides, tags, packed refs, detached HEAD, linked worktrees, and exported sources inside unrelated repositories.

The initial sandboxed Viceroy run failed during macOS certificate loading; the retry with keychain access passed. The complete Rust test suites, standalone JS tests, docs formatting, and live deployment smoke tests were not repeated in this pass. Tracked repository files remain unchanged.

CI Status

  • Analyze (javascript-typescript): PASS.
  • Other CI gates: not run at this head according to the reported check set. Only the above check is reported; no required checks are reported. Earlier revisions' CI results do not validate this revision.

@aram356 aram356 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Summary

The resolver, the shared version_header helper, and the round-2 fixes hold up. All 14 CI-equivalent steps pass locally at e6904260a. The blocker is the merge state: the branch conflicts with main, so GitHub's test, format, and integration workflows have never run on this head.

3 of the 6 inline comments below carry a one-click GitHub suggestion. Use Commit suggestion (or Add suggestion to batch to apply several at once). The other three describe the fix in prose, because it touches several files or lines outside the diff.

Blocking

🔧 wrench

  • Rebase onto main so CI can run on the merged result: see Cross-cutting below

Non-blocking

🤔 thinking

  • Fastly startup failures omit x-ts-version: see inline at crates/trusted-server-adapter-fastly/src/main.rs:72
  • An ambient TS_GIT_VERSION compiles in when the version is unknown: see inline at crates/trusted-server-core/build.rs:112
  • Document the build input and the header split for operators: see inline at CHANGELOG.md:12

♻️ refactor

  • Keep the testable helpers private: see inline at crates/trusted-server-core/src/version_header.rs:18

⛏ nitpick

  • The OVERRIDE_ENV comment overstates the CI checkout limitation: see inline at crates/trusted-server-core/build.rs:11
  • The spec header is stale: see inline at docs/superpowers/specs/2026-09-25-git-version-header-design.md:3

Cross-cutting / body-level findings

🔧 Rebase onto main so CI can run on the merged result

GitHub reports this PR as conflicting with main. While that's true it can't build the pull_request merge ref, so Run Tests, Run Format, Integration Tests, and CodeQL Advanced have not run for e6904260a. The only check on this head is the dynamic CodeQL JavaScript analysis. Both approvals call out the same gap. Three files conflict:

  • crates/trusted-server-core/build.rs: #879 made main() load edgezero.toml and emit TRUSTED_SERVER_DEFAULT_CONFIG_STORE_ID, with edgezero-core as a build-dependency. Keep both bodies. Putting main's manifest block first keeps its cargo::error= exit ahead of the version lookup.
  • crates/trusted-server-adapter-fastly/src/main.rs: imports only. Keep cache_policy::{EdgeCacheHeader, cache_control_headers_have_directive} from #1179 next to constants::HEADER_X_TS_VERSION. health_response is unchanged on main.
  • CHANGELOG.md: keep both sides' ### Changed entries.

The merged build.rs header and main() shape:

use std::env;
use std::path::{Path, PathBuf};
use std::process::Command;

use edgezero_core::manifest::ManifestLoader;

use git_version::{Candidates, is_usable, resolve_git_version};

// ... OVERRIDE_ENV, git(), rerun_if_exists(), resolve_from_local_git() unchanged ...

fn main() {
    println!("cargo:rerun-if-changed=build.rs");
    println!("cargo:rerun-if-changed=build_support/git_version.rs");
    println!("cargo:rerun-if-env-changed={OVERRIDE_ENV}");

    // main's edgezero.toml block, unchanged, ending with:
    // println!("cargo:rustc-env=TRUSTED_SERVER_DEFAULT_CONFIG_STORE_ID={default_store_id}");

    // this PR's override / local-git resolution, unchanged
}

I resolved the conflicts this way in a scratch merge with main at 182fdf45c. It passes fmt, clippy-fastly, test-fastly, test-fastly-reuse, test-axum, check-cloudflare, test-cloudflare, test-spin, and parity (17/17), and the merged build script emits both TRUSTED_SERVER_DEFAULT_CONFIG_STORE_ID and TS_GIT_VERSION. Once merged, both halves derive the workspace root from CARGO_MANIFEST_DIR: one with ancestors().nth(2), the other with parent()?.parent()?. Computing it once and passing it to resolve_from_local_git would remove the duplicate.

CI Status

  • Analyze (javascript-typescript): PASS
  • Run Tests, Run Format, Integration Tests, CodeQL Advanced: not run at this head (no merge ref while the PR conflicts)
  • Local results at e6904260a: cargo fmt --check, all eight clippy aliases, test-fastly, test-axum, test-cloudflare, test-spin, and parity (14/14) PASS. The docs and root Markdown Prettier checks PASS. There are no JS changes, so vitest was not run.

Comment thread crates/trusted-server-adapter-fastly/src/main.rs
Comment thread crates/trusted-server-core/build.rs Outdated
Comment thread CHANGELOG.md Outdated
Comment thread crates/trusted-server-core/src/version_header.rs Outdated
Comment thread crates/trusted-server-core/build.rs Outdated
Comment thread docs/superpowers/specs/2026-09-25-git-version-header-design.md Outdated
Comment thread crates/trusted-server-core/src/constants.rs Outdated
Always re-emit the validated value under the same name so an unvalidated ambient value never reaches option_env!, and treat an empty value as unknown.

Report x-ts-version on Fastly responses whose finalization is skipped because settings fail to load, drop an inherited x-ts-fastly-version when FASTLY_SERVICE_VERSION is unset or invalid, make the version_header test helpers private, and document the build input and header split for operators.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>

# Conflicts:
#	CHANGELOG.md
#	crates/trusted-server-adapter-fastly/src/main.rs
#	crates/trusted-server-core/build.rs
@dhruv8sh

Copy link
Copy Markdown
Collaborator Author

Merged main in 959e503. build.rs now runs main's EdgeZero config-store and template build digest steps first, then compiles in the git version; CRATE_INPUTS already watches build.rs, so only build_support/git_version.rs and the override variable are added as rerun inputs. Kept both import sets in the Fastly main.rs and both ### Changed entries in CHANGELOG.md. The full gate set passes after the merge, including test-fastly-reuse, the build-digest test and lint, and parity.

Also in 50ad004: an inherited x-ts-fastly-version is now removed when FASTLY_SERVICE_VERSION is unset or invalid, so an origin value is never reported as ours, matching the x-ts-version rule.

@dhruv8sh
dhruv8sh requested a review from aram356 October 10, 2026 06:29

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Report the deployed git version in x-ts-version

4 participants