Skip to content

Add full-surface documentation refresh spec - #1049

Open
aram356 wants to merge 3 commits into
mainfrom
spec-docs-refresh
Open

aram356 wants to merge 3 commits into
mainfrom
spec-docs-refresh

Conversation

@aram356

@aram356 aram356 commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

Important

Retargeted on 2026-09-21: the base changed from rc/202608 to main and the head was force-pushed from the rc-scoped refresh (1c6b23a54, preserved at branch spec-docs-refresh-rc) to a docs-only transplant verified against main. The three earlier approvals were given for the rc-scoped diff and predate this change — please re-review.

Brings the documentation content of the refresh to main now, rather than waiting for the rc/202608 release merge. Scope is documentation surfaces only: the docs site, top-level and crate READMEs, the four adapter smoke scripts the guides teach, and the documentation-snippet compile test. The refresh's Rust doc-comments, route-contract tests, startup diagnostics, and CI workflow changes are not included; they land with the release merge from spec-docs-refresh-rc.

What differs from the rc-scoped version reviewers approved

Everything was re-verified against main, and the guides were re-pointed where the branches diverge:

  • The rc-only ts audit ad-templates and ts config ad-templates command families are removed from the CLI guide. Every remaining documented command was probed against a ts binary built from this branch.
  • check-documentation.sh is not included: main's rustdoc does not yet pass with warnings denied (29 intra-doc-link errors that the rc-scoped refresh fixes). testing.md documents the two documentation checks that do run on main.
  • scripts/README.md lists only scripts that exist here; CI-gate links point at AGENTS.md (CLAUDE.md is a symlink on main).
  • Conflicts with main's newer CHANGELOG.md, README.md, TESTING.md, and auction README resolved to main's side; configuration.md keeps both main's initial-deployment steps and the refresh's secret-field migration guidance.

Verification

  • cd docs && npm run lint && npm run format && npm run build — clean; the build fails on dead links and passed.
  • cargo test --test documentation_snippets — the documented integration fixture compiles against main's core.
  • ./scripts/smoke-axum.sh — boots main's Axum adapter end to end.
  • A claims scan extracted every referenced script path, repository path, and route from the transplanted docs and checked each against this tree; the only remaining flags are deliberate (the sentence stating docs/public/CNAME intentionally does not exist, and the gitignored dist/ directory).

Relation to the release branch

The full rc-scoped refresh (with the code changes and the review history above) is preserved at spec-docs-refresh-rc and merges into rc/202608 with the release. When rc later merges to main, its versions supersede these files and restore the rc-only material. Merging this PR publishes the docs site from main via deploy-docs.yml.

Follow-ups

@aram356 aram356 self-assigned this Aug 20, 2026
@aram356
aram356 marked this pull request as draft August 20, 2026 06:53
@aram356 aram356 added this to the 202608 milestone Aug 20, 2026
@aram356
aram356 requested a review from jevansnyc August 20, 2026 16:07
@aram356
aram356 changed the base branch from main to rc/202608 August 20, 2026 17:39
@aram356
aram356 force-pushed the spec-docs-refresh branch 2 times, most recently from 392c994 to 087e1a7 Compare August 21, 2026 03:45
@aram356
aram356 force-pushed the spec-docs-refresh branch 3 times, most recently from f11ad3c to 0ddbb88 Compare August 28, 2026 21:28
@aram356
aram356 removed the request for review from jevansnyc September 1, 2026 02:36
@aram356 aram356 modified the milestones: 202608, 202609 Sep 1, 2026
@aram356
aram356 marked this pull request as ready for review September 8, 2026 19:21
@aram356
aram356 requested a review from prk-Jr September 11, 2026 20:44

@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

A large, unusually careful documentation refresh that also carries runtime Rust, CI workflow, shell script, config template, and governance changes. The documentation itself is high quality — I mechanically cross-checked every documented route and every TOML key in the changed docs against the source and found zero fabricated routes or config keys, which is a rare result for a 6,000-line docs change.

The blocking findings are not about the docs. They are three things riding along inside a "docs refresh": a runtime observability regression on the Spin adapter, a rewrite of the repository's sensitive-data policy that legalizes an existing violation, and a rewrite of the project's governance charter.

2 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 this branch. Both were scratch-verified in an isolated worktree at this head, individually and together. The remaining comments describe the fix in prose because the change is a scope/ownership decision or spans multiple sites and can't be auto-applied.

Note on the stated diff size

The PR description cites 107 commits / 140,494 insertions, which is measured against an older base. Against current rc/202608 (07dfc1c6dddf69345ded17bd2d40a3d01bb39bcf) the real diff is 109 commits, 203 files, +6,003 / −3,631. This review is against that diff, not the description.

Blocking

🔧 wrench

  • Spin startup diagnostic drops the error-stack report — see inline at crates/trusted-server-adapter-spin/src/app.rs:509 (suggestion)
  • Sensitive-data policy rewritten to sanction a value the same PR keeps — see inline at fastly.toml:10
  • ProjectGovernance.md rewrites the chartered governance of an IAB Tech Lab project — see inline at ProjectGovernance.md:13

❓ question

  • Seven commands documented under "CI Gates" are run by no PR-triggered workflow — see inline at .github/workflows/documentation-checks.yml:3

Non-blocking

🤔 thinking / ♻️ refactor / 📌 out of scope / ⛏ nitpick / 📝 note

  • 🤔 Spin release build lost its boot-proving env overrides with no CI replacement — see inline at .github/workflows/test.yml:201
  • 🤔 documentation_snippets.rs hardcodes dep versions and leans on ambient cargo cache — see inline at crates/trusted-server-integration-tests/tests/documentation_snippets.rs:40
  • 🤔 [trusted_client_ip] removed from the operator template with no decision-record entry — see inline at trusted-server.example.toml:145 (LEFT side)
  • ♻️ Same current_context() truncation on Cloudflare — see inline at crates/trusted-server-adapter-cloudflare/src/app.rs:362 (suggestion)
  • 📌 spin is not pinned in .tool-versions — see inline at .tool-versions:4
  • uncomment_block heuristic still mis-handles prose — see inline at crates/trusted-server-core/src/config.rs:540
  • The new .env.dev [ec] entry is a no-op — see inline at .env.dev:8
  • 📝 OpenRTB codegen binary is now completely silent — see inline at crates/trusted-server-openrtb-codegen/src/main.rs:25 (LEFT side)

Cross-cutting / body-level findings

🤔 Actions are pinned to mutable release tags, not commit SHAs

I verified every pin against the GitHub releases API. All thirteen exist and are the current latest release, so there are no broken or hallucinated pins:

action pinned latest
actions/checkout v7.0.1 v7.0.1
actions/setup-node v7.0.0 v7.0.0
actions/cache v6.1.0 v6.1.0
actions/upload-artifact v7.0.1 v7.0.1
actions/download-artifact v8.0.1 v8.0.1
actions/configure-pages v6.0.0 v6.0.0
actions/upload-pages-artifact v5.0.0 v5.0.0
actions/deploy-pages v5.0.1 v5.0.1
actions-rust-lang/rustfmt v1.1.2 v1.1.2
github/codeql-action v4.37.9 current
browser-actions/setup-chrome v2.2.0 v2.2.0
fastly/compute-actions v14 v14

The upload-artifact v7 / download-artifact v8 major mismatch is genuine upstream, not an authoring error. actions-rust-lang/setup-rust-toolchain stays at v1.17.0 while v2.0.0 exists — a safe, deliberate non-bump.

The remaining concern is only that git tags are mutable and can be repointed by a compromised maintainer; SHA pinning is the hardening standard, and this PR bills itself partly as a hardening pass. docs/internal/audits/documentation-refresh-decisions.md:93 explicitly chose "exact release tags", so this is a recorded disagreement rather than a defect — flagging so the tradeoff is visible.

Verified clean on the rest of the supply-chain surface: zero ${{ github.event.* }} interpolations anywhere in .github/workflows/ or .github/actions/ (no script-injection sinks), and every workflow declares least-privilege permissions: (contents: read, with pages: write + id-token: write scoped to deploy-docs.yml alone).

📝 gam.md / kargo.md are published but unlinked — intentional, recording so it isn't "fixed" later

Both were dropped from the sidebar in docs/.vitepress/config.mts yet still build (.vitepress/dist/guide/integrations/{gam,kargo}.html) with zero inbound links from any published page. I read both: they are tombstones ("Trusted Server does not ship a direct Google Ad Manager integration…"), whose purpose is to catch stale external links, so absence from navigation is correct. No action requested — noting it so a future reader doesn't re-link them. (Anchored here rather than inline because the removal is a deletion spanning several config.mts hunks.)

👍 Praise

  • Route-contract tests across all four adapters (crates/trusted-server-adapter-fastly/src/app.rs:1592-1631, and tests/routes.rs in the cloudflare / spin / axum adapters) pin the exact (method, path) set, including the legacy /admin/keys/* aliases. This is precisely the regression net this area needed.
  • crates/trusted-server-adapter-cloudflare/src/platform.rs:585-598 corrects a doc claim that was wrong. The old text said config and KV "are sourced from the edgezero handles that run_app injects". I verified grep -n "fn stores" across all four adapters returns only crates/trusted-server-adapter-fastly/src/app.rs:1336, so the new text ("the Cloudflare application does not implement Hooks::stores()") is the accurate one. Correcting a doc toward the less flattering truth is the right instinct.
  • Zero fabricated routes or config keys. I extracted all 27 backticked route paths from the changed guides and every key = from every TOML fence in every added/modified doc, then grepped them against the full Rust source. The only hit was thumbnail, which is a user-chosen profile-map key rather than a struct field.
  • The secret_store template removals are correctcrates/trusted-server-core/src/settings.rs:291-305, 913-915, 1922-1924 deprecate-and-ignore secret_store, server_side_key_secret_store, and credential_secret_store with a warning, so dropping them from the template is right.
  • The deploy-docs provenance assertion actually works. I ran GITHUB_SHA=aaaa…aaaa npm run build in the worktree and confirmed grep -R --fixed-strings --quiet "$GITHUB_SHA" .vitepress/dist succeeds.
  • docs/public/CNAME deletion is safe — it contained the literal placeholder your-custom-domain.com, not a live domain.
  • scripts/smoke-fastly.sh correctly avoids the tracked-manifest secret hazard — it copies fastly.toml into a per-run temp project (scripts/smoke-fastly.sh:33-34) before ts config push --local writes secrets into it, so the tracked file is never written.
  • The .env synthetic → EC migration is correctgrep "counter_store\|opid_store\|SYNTHETIC" crates/trusted-server-core/src/settings*.rs returns nothing, so the removed keys really are dead.
  • cache-dependency-path fixed from package.jsonpackage-lock.json in both format.yml and test.yml; $GITHUB_OUTPUT quoted throughout; multi-line run: blocks moved into repository scripts.

Local verification

Run from an isolated worktree detached at 9dfb956cda2674b2ad04796321a8f6edcd26337b.

Gate Result
cargo fmt --all -- --check PASS
cargo fmt --manifest-path crates/trusted-server-integration-tests/Cargo.toml -- --check PASS
cargo clippy-spin-native PASS (no warnings)
cargo clippy-cloudflare PASS
cargo clippy-cloudflare-wasm PASS
cargo test-spin PASS — 38 passed, 0 failed
cargo test-cloudflare PASS — 24 + 23 passed, 0 failed
cd docs && npm ci && npm run format PASS — "All matched files use Prettier code style!"
cd docs && npm run lint PASS
cd docs && npm run build PASS — build complete in 6.02s, no dead links

Both suggestion blocks were applied and verified individually against a clean tree, then together as a batch (fmt + target-matched clippy + cargo test-spin + cargo test-cloudflare all green in every configuration). The worktree was restored to the PR head afterwards.

Not run locally (cost, and green on CI): cargo test-fastly, cargo test-axum, the parity suite, the full clippy alias chain, vitest.

Verified vs. unverified

Scratch-verified: the Spin and Cloudflare diagnostic findings (fix applied, fmt/clippy/tests green); the policy and governance findings (both sides of the diff plus the decision record read in full); the CI-gate-enforcement question (grep across all workflows plus live gh pr checks); the Spin release-build finding (workflow diff and smoke matrix contents); the action pins (all 13 checked against the GitHub releases API); the [trusted_client_ip] finding (settings.rs:2739-2810, all four adapter call sites, doc coverage at both base and head, and absence from every decision record); and the config.rs, .env.dev, .tool-versions, openrtb-codegen, and gam/kargo findings. All praise items verified as stated.

Unverified / judgement: the documentation_snippets.rs finding — I reasoned about --offline resolution and the shared CARGO_TARGET_DIR from reading the test; I did not construct a cold-cache run to prove it fails. The spin pinning finding relies on this PR's own evidence document for the Spin 4.1.0 claim; I could not run the Spin CLI here.

Concerns I raised and then killed by checking — listing these so they don't get re-raised in a later pass: bogus or nonexistent action version pins (all real and current); the docs/public/CNAME deletion breaking a custom domain (it was a placeholder); scripts/install-wrangler.sh's strict test "$(wrangler --version)" = "$version" (the adapter first success (cloudflare) check is green, so it works); the [ec] and secret_store template removals being wrong (both correct against the code); and a "docs-parity standalone crate with its own lockfile" (git ls-files | grep -i docs.parity returns nothing — it was withdrawn by commit 9dfb956c, "Remove intrusive documentation tooling").

Recommendation

Three of the four blockers are scope problems rather than code problems. The cleanest path forward:

  1. Split ProjectGovernance.md into its own PR for Task Force / IAB Tech Lab sign-off. Deleting chartered commitments (biweekly cadence, published minutes, continuous release) is not a change a code review can approve.
  2. Split the CLAUDE.md sensitive-data policy rewrite into its own PR so the new typed-exception system gets explicit maintainer sign-off from someone other than the exception's own owner — and either remove the service_id or have the exception re-approved independently.
  3. Apply the one-line Spin diagnostic suggestion (and optionally the matching Cloudflare one).
  4. Answer the "CI Gates" labelling question.

What remains after that split is a strong, accurate documentation refresh I would be glad to see merged.

CI Status

All 22 reported checks pass. Branch protection reports no required checks on spec-docs-refresh.

  • .github/dependabot.yml: PASS
  • Analyze (actions): PASS
  • Analyze (javascript-typescript): PASS
  • Analyze (rust): PASS
  • CodeQL: PASS
  • adapter first success (axum): PASS
  • adapter first success (cloudflare): PASS
  • adapter first success (fastly): 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
  • cargo test: PASS
  • cargo test (axum native): PASS
  • cargo test (cross-adapter parity): PASS
  • cargo test (ts CLI, native): PASS
  • format-docs: PASS
  • format-typescript: PASS
  • integration tests: PASS
  • integration tests (Fastly EC lifecycle): PASS
  • prepare integration artifacts: PASS
  • vitest: PASS
  • Documentation checks: not run — the workflow this PR adds is workflow_dispatch-only, which is the subject of the ❓ finding above.

Comment thread crates/trusted-server-adapter-spin/src/app.rs Outdated
Comment thread fastly.toml Outdated
Comment thread ProjectGovernance.md Outdated
Comment thread .github/workflows/documentation-checks.yml Outdated
Comment thread .github/workflows/test.yml Outdated
Comment thread crates/trusted-server-adapter-cloudflare/src/app.rs Outdated
Comment thread .tool-versions Outdated
Comment thread crates/trusted-server-core/src/config.rs Outdated
Comment thread .env.dev Outdated
Comment thread crates/trusted-server-openrtb-codegen/src/main.rs

@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.

Summary

Reviewed at e7687a1b075847bf50c7bdc64bfc99b737a53c97 against 07dfc1c6dddf69345ded17bd2d40a3d01bb39bcf. I have not repeated findings already covered by existing review threads.

One follow-up worth tracking: trusted_client_ip.shared_secret is the secret value itself and is serialized into the app-config blob. Redacted only masks debug and display output, and this field is not registered for secret-store resolution. Changing that storage contract is outside this documentation refresh, but it should get a separate issue with a compatibility and migration plan.

Comment thread docs/roadmap.md Outdated
@aram356

aram356 commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator Author

Note

Attribution, added 2026-09-20 in response to this comment: the review below was generated by an automated Claude Code review agent and posted from my account. Its "Verdict" is the tool's output, not an independent maintainer sign-off, and its "Correction to something I said earlier" refers to the tool's own prior automated pass, not to anything a person said on this PR.

Review feedback

Reviewed at e7687a1b0. All 21 checks pass and the earlier rounds have been addressed thoroughly, so this is one blocking item plus scope observations — not a defect list.

Blocking: onboarding is the only content removed without a destination

Everything else that shrank in this PR was deduplicated into something better. testing.md 367→142 is a genuine improvement — it correctly deletes cargo test -p trusted-server-core, which fails to link because it carries no --target, and keeps the useful filter examples. integrations-overview.md 403→82 became a complete inventory table. Good changes.

Onboarding is different. It goes 160→42 lines, moves to docs/internal/onboarding.md, and srcExclude: ['internal/**'] at docs/.vitepress/config.mts:52-56 means it is no longer published at all. Combined with two pre-existing gaps on the base branch:

  • it is not in the sidebar (this predates the PR);
  • neither README.md nor CONTRIBUTING.md mentions onboarding;
  • after the move, the only inbound links are docs/internal/audits/documentation-refresh-decisions.md:35 and the plan/spec files — i.e. the records explaining why it was hidden.

Net effect: after merge a new engineer has no discoverable onboarding entry point.

The containment rationale is right — contacts and meeting times should not be public. The execution over-applied it. The material that did not survive is the part that is not sensitive and has no other home: what the system does, how a request flows through it, and what the ad-tech vocabulary means. Nothing in the refreshed reference set absorbed it; rust-analyzer, asdf, and "good first issue" appear nowhere in the new docs.

I have put up #1165 as a draft stacked on this branch showing one way to close it: a published docs/guide/onboarding-new-engineer.md plus a sidebar entry, leaving your internal maintainer page untouched. Take it, adapt it, or replace it — but something public should ship with this.

Scope

The title says documentation refresh; the diff is 202 files, of which 144 are non-doc (+2641/-1146) including adapter source, workflows, fastly.toml, and seven new shell scripts.

I checked whether that hides behaviour changes and it largely does not — the TypeScript edits in core/render.ts, registry.ts, types.ts, globals.ts and integrations/creative/* are comment-to-JSDoc conversions with zero non-comment lines changed, and the Rust is characterization tests plus the startup-diagnostic fix reviewers already requested. The smoke scripts are shellcheck-clean and wired into integration-tests.yml.

That is solid work. It is also why this needed four review rounds: reviewers had to audit a runtime surface to approve a docs change, and two real bugs (the Spin global logger suppressing production logging, the current_context() truncation) were found by reading code rather than by any of the ledgers. Splitting governance and the CLAUDE.md policy edits out mid-review was the right call; applying that instinct earlier would have made this two or three PRs. 100 commits also means commit-by-commit review is not really available.

Not asking you to re-cut it now — flagging it for the next one of this size.

Smaller points

  1. Spin smoke is manual and time-bounded. The matrix at integration-tests.yml:123 is [axum, fastly, cloudflare]; the PR body carries a local Spin receipt expiring 2026-10-07, about three weeks out. The decision record is admirably blunt that no CI workflow proves the Spin artifact boots, and Pin spin in .tool-versions and add Spin to the adapter-first-success smoke matrix #1164 is filed — just worth not letting that receipt lapse silently.

  2. roadmap.md:35 page-bids method — still open from @ChristianPavilonis, and it is correct: both /__ts/page-bids and /_ts/page-bids are GET.

  3. Comment-to-JSDoc conversions lose reasoning in a few places. In render.ts, the old comment on sanitizeCreativeHtml explained why sanitizedLength always equals originalLength and removedCount is always 0 — they exist for structural consistency with the shared result type and carry no signal. The JSDoc replacement drops that, and it is the kind of thing the next reader will otherwise misread as a bug. Worth keeping the "why" sentence where the original had one.

Correction to something I said earlier

I previously flagged {{RUST_VERSION}} in getting-started.md as rendering literally. That was wrong. docs/.vitepress/config.mts installs a markdown parse hook that reads .tool-versions and substitutes every {{TOOL_VERSION}} at build time, so the published page reads "Rust 1.95.0". The placeholder never appears as a literal in the config, which is why my grep missed it. The mechanism is also the better design — versions cannot drift from the pinned toolchain.

Verdict

Merge it once the onboarding placement is settled. That is a one-line srcExclude decision plus a page, not a content dispute. The reference layer here is a real improvement: four adapter guides, telemetry, tsjs, settings and route parity, ten crate READMEs, and the Equativ/Kargo/GAM fiction removed. What it does not add is anything that teaches — no request trace, no glossary, no triage path — but that is the next PR, not a reason to hold this one.

@aram356

aram356 commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator Author

Latest feedback is addressed in 5c043a3.

  • Restored public Onboarding at the established /guide/onboarding URL, with the verified Fastly request path, repository vocabulary, code map, development loop, and triage pointers.
  • Kept maintainer onboarding outside the VitePress build and aligned the decision record, design, and executed plan.
  • Corrected both page-bids roadmap methods to GET.
  • Restored the sanitizeCreativeHtml rationale for the structural length/count fields.
  • The stacked draft Add a new-engineer setup page alongside the onboarding guide #1165 is superseded by this restoration; it was closed on 2026-09-20 (the earlier claim here that it was already closed was wrong).

Verification on the final tree: full scripts/check-documentation.sh passed; all 949 Vitest tests passed; Prettier passed for the root README and JS/docs trees.

@aram356
aram356 requested a review from dhruv8sh September 16, 2026 19:34
@jevansnyc

Copy link
Copy Markdown
Collaborator

The PR description no longer matches the branch. It still describes the docs-parity tooling that 9dfb956 removed: the "Settings parity (WP3)" block, the settings --check / generate --check / check --all gates, the docs-parity lock hash, and the CLI golden provenance. The "Final documentation-refresh acceptance" section cites head f682c05d and a 107-commit, 140,494-insertion range. The head is 5c043a32, and the diff against rc/202608 is 202 files, +6,113 / -3,602. The WP8 row says cargo doc -D warnings and doctests run in CI, but CLAUDE.md on this branch now lists them under "Manual documentation gates" and says CI does not block on them.

The description becomes the merge record, so please rewrite it against the current head. A short summary of what changed plus the test plan is enough. The generated ledger sections (SHA-256 manifests, "immutable inputs", artifact digests, the Spin receipt with an expiry date) can go; the decision record under docs/internal/audits/ already carries that material.

@jevansnyc

Copy link
Copy Markdown
Collaborator

On the "Review feedback" comment from 2026-09-14 (#1049 (comment)): it is posted from the PR author's account but written as a third-party review, down to "Verdict: Merge it". It also has a "Correction to something I said earlier" about {{RUST_VERSION}} rendering literally, and nobody said that anywhere on this PR. That reads like an automated review pasted verbatim. If a tool produced it, say which one at the top. A merge verdict under the author's name is confusing for anyone reading this thread later.

Two related items:

@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

Full-surface documentation refresh plus deterministic parity enforcement: 202 files, ~6.1k insertions / ~3.6k deletions against rc/202608. Reviewed the whole diff — the Rust and TypeScript changes are overwhelmingly doc comments, new #[cfg(test)] route-contract tests, and test-fixture hardening; the only runtime deltas are additive startup diagnostics on Cloudflare and Spin. Public APIs, routes, status codes, and successful-request behaviour are unchanged. All 21 GitHub checks pass on the reviewed head.

1 of the inline comments below carries a one-click GitHub suggestion — use Commit suggestion to apply it as a commit on the PR branch.

Non-blocking

♻️ refactor

  • Canonical CI gate list omits the two template-cache harness steps — see inline at CLAUDE.md:382

Cross-cutting / body-level findings

  • 📝 Spin startup diagnostic now writes directly to stderrcrates/trusted-server-adapter-spin/src/logging.rs replaces log::error! with a direct writeln!(io::stderr(), ...). This is a deliberate, well-documented deviation from the repo's "use log macros, not println!/eprintln!" convention: the EdgeZero Spin logger initializer is a no-op, so the boot failure would otherwise be invisible, and writing directly avoids claiming the process-global logger. Flagging it only so the exception is visible to future reviewers — no action requested.
  • 📝 Cloudflare startup errors now log the full Report debugcrates/trusted-server-adapter-cloudflare/src/app.rs adds worker::console_error! with {error:?} under #[cfg(target_arch = "wasm32")]. The response body still exposes only user_message(), and this matches the pre-existing Spin behaviour, so it is not a new disclosure class. worker is an unconditional cfg(target_arch = "wasm32") dependency, so the cfg gate compiles on both the native and --features cloudflare wasm legs.
  • 👍 Smoke scripts never mutate the tracked working treescripts/smoke-fastly.sh copies fastly.toml/edgezero.toml into an isolated mktemp -d project and symlinks crates/, so ts config push --local and the seeded ts_secrets blocks land only in the throwaway workspace. smoke_remove_workspace additionally refuses to rm -rf any path outside the generated prefix. That is a meaningfully safer shape than "mutate and restore on trap", and the three sibling scripts follow the same pattern. Secret values are obviously synthetic (smoke-admin-password-32-bytes-ok, etc.).
  • 👍 Route-registration contract tests across all four adapterscomplete_route_registration_set_matches_the_prechange_contract in the Fastly/Axum/Cloudflare/Spin route suites pins the exact (method, path) set, and the matching startup_error_route_set_... tests pin the fallback router. Combined with the existing parity suite this makes a silent route/method regression during a docs-driven refactor very hard to land.
  • 👍 docs/public/CNAME removal and deploy-docs.yml provenance assertion — dropping the your-custom-domain.com placeholder is correct (it would have hijacked the Pages custom domain), and the new grep -R --fixed-strings "$GITHUB_SHA" .vitepress/dist step is backed by a real emitter: docs/.vitepress/config.mts injects provenanceBanner() into every page's parsed markdown and validates the SHA shape, so the assertion cannot silently pass on an unstamped build.
  • 📝 Docs build is enforced on PRs — worth recording since the new Documentation checks workflow is workflow_dispatch-only: the dead-link-failing VitePress build still runs on every PR via format.yml's format-docs job ("Build with VitePress (fails on dead links)"). TESTING.md and the new ## Manual documentation gates section both state the manual-only scope explicitly, so the split is intentional and documented.

CI Status

  • cargo fmt: PASS
  • cargo test: PASS
  • cargo test (axum native): PASS
  • cargo test (cloudflare) / 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): PASS
  • vitest: PASS
  • format-typescript: PASS
  • format-docs: PASS
  • prepare integration artifacts: PASS
  • integration tests: PASS
  • integration tests (Fastly EC lifecycle): PASS
  • browser integration tests: PASS
  • adapter first success (axum): PASS
  • adapter first success (fastly): PASS
  • adapter first success (cloudflare): PASS
  • Analyze (rust): PASS
  • Analyze (javascript-typescript): PASS
  • Analyze (actions): PASS
  • CodeQL: PASS
  • Documentation checks: not run (workflow_dispatch only, by design)

No required checks are configured on this branch.

Comment thread CLAUDE.md Outdated

@jevansnyc jevansnyc 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.

checks out sans 2 minor comments which are fine to move forward with outside of housekeeping

@aram356

aram356 commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator Author

Housekeeping from the two 2026-09-17 comments is done:

  • The PR description is rewritten against the current head (1c6b23a): the withdrawn docs-parity tooling sections, stale head SHA, old diff counts, and ledger material are gone, and the rustdoc commands are described as manual gates, matching CLAUDE.md.
  • The 2026-09-14 "Review feedback" comment now opens with an attribution note: it was generated by an automated Claude Code review agent and posted from my account, and its verdict and self-correction are the tool's, not a maintainer's.
  • The "independent review found no Critical or Important issues" line is removed from the status comment; it referred to an automated pass I cannot attribute precisely, so it is dropped rather than named.
  • Add a new-engineer setup page alongside the onboarding guide #1165 is now actually closed as superseded (the earlier claim that it was already closed was wrong, and the status comment says so).
  • prk-Jr's gate-list suggestion is applied in 1c6b23a, adding the two template-cache harness steps to the canonical list.
  • ChristianPavilonis's shared_secret storage-contract follow-up is filed as Resolve trusted_client_ip.shared_secret through the secret store instead of the app-config blob #1186.

…aces

Transplant the reader-facing content of the rc/202608 documentation
refresh (PR #1049) onto main so the docs ship ahead of the release merge:

- Rebuilt guides (adapters, configuration, testing, creative processing,
  integrations inventory, telemetry, tsjs, CLI, API reference), the
  restored public onboarding page, ten crate READMEs, and the internal
  decision and evidence records
- The four adapter smoke scripts plus scripts/README.md, and the
  documentation-snippet compile test
- docs/public/CNAME placeholder removed; VitePress config updated

Re-pointed at main where the release branch differs: the rc-only
`ts audit ad-templates` and `ts config ad-templates` command families are
removed from the CLI guide (every remaining documented command probed
against a main-built binary); CI-gate links target AGENTS.md; the
documentation-checks aggregate script is left out because main's rustdoc
does not yet pass with warnings denied, and testing.md lists the two
commands that do run here; scripts/README.md lists only scripts present
on main. Merge conflicts with main's newer CHANGELOG, README, TESTING,
and auction README resolved to main's side.

Verified: VitePress lint/format/build (dead links fail the build),
documentation_snippets compiles against main's core, smoke-axum.sh boots
main's adapter, and a claims scan checked every referenced script, repo
path, and route against this tree.
@aram356
aram356 changed the base branch from rc/202608 to main September 21, 2026 02:38
@aram356

aram356 commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator Author

Retargeted per maintainer decision: base is now main and the head is force-pushed to a docs-only transplant verified against main (853eb45). The rc-scoped refresh that ChristianPavilonis, prk-Jr, and jevansnyc approved is preserved unchanged at spec-docs-refresh-rc (tip 1c6b23a) and remains the content that merges into rc/202608 at release time. The approvals above predate this retarget and applied to that rc diff, so please re-review against the new, smaller diff; the description lists exactly what changed relative to the approved version. #1187, which briefly carried this transplant, is closed in favor of this PR.

Comment thread docs/internal/audits/documentation-refresh-decisions.md Outdated
Comment thread docs/internal/audits/documentation-refresh-evidence.md Outdated
Comment thread scripts/smoke-common.sh
Comment thread scripts/smoke-axum.sh Outdated
Comment thread docs/guide/cli.md Outdated
Comment thread docs/guide/configuration.md
Comment thread CONTRIBUTING.md Outdated
- Remove the documentation-refresh decision and evidence records: they
  declare rc/202608 as the delivery target and describe code decisions
  (Spin logger, workflow scoping, policy split) that this docs-only diff
  does not carry; both stay accurate on spec-docs-refresh-rc
- Keep the full ts audit and ad-templates command index: PR #823 landed
  on main and the merge from main restored those subcommands, so every
  documented chain is re-verified against a freshly built binary
- Add the missing js_asset_proxy integration to the configuration
  inventory (with a field-level section), the integrations overview, and
  both API-reference tables; the deploy-validated set has 15 entries
- Correct CONTRIBUTING.md: TESTING.md is an auction-orchestration runbook
  that repeats the adapter aliases, not a link index, and the gate link
  points at AGENTS.md
- Give each smoke script a distinct default origin port outside every
  claimed adapter port range, and make smoke_stop_process capture child
  PIDs via pgrep, wait for them to exit after signaling, and escalate to
  KILL on timeout; pgrep is now a declared requirement in all four

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.

5 participants