Spec: Server side ad templates creative-opportunities cli

[prk-Jr]

Summary

• Spec for ts-config, a standalone CLI that validates, matches, lints, and checks creative-opportunities.toml without requiring a Rust toolchain — directly addressing the JS auditor onboarding gap raised in Build a standalone validation tool for creative-opportunities.toml #701
• Defines a two-tool architecture: Rust binary (validate/match/lint/check, mirrors runtime glob and slot-ID logic) and Node.js tool (generates slot config from live publisher pages via Playwright)
• Covers all design decisions, edge cases, error type scaffold, JSON output contracts, and concrete test shapes for both Phase 1A (Rust) and Phase 1B (Node.js)

Changes

File	Change
docs/superpowers/specs/2026-05-19-ts-config-cli-design.md	New: full design spec for ts-config CLI (1440 lines)

Closes

Closes #701

Test plan

• cargo test --workspace
• cargo clippy --workspace --all-targets --all-features -- -D warnings
• cargo fmt --all -- --check
• JS tests: cd crates/js/lib && npx vitest run
• JS format: cd crates/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

Spec-only PR — no production code changed. WASM build and manual testing not applicable.

Checklist

• Changes follow CLAUDE.md conventions
• No unwrap() in production code — use expect("should ...")
• Uses tracing macros (not println!)
• New code has tests
• No secrets or credentials committed

[ChristianPavilonis]

the claude code package will be removed, in favor of the ts cli.

the ts cli / edgezero cli will have auditing and config store primitives, so I'd hold off and rework this when those are in place

[aram356]

Summary

Spec is detailed and well-iterated, but two structural concerns make this hard to land as-is: (1) it appears to be superseded by #669 "Add the Trusted Server CLI", which targets main and introduces a ts config namespace; (2) the PR is stacked on server-side-ad-templates-impl rather than main, so the claim of "Phase 1A ships as PR A independently" doesn't actually hold. Several spec-internal issues are flagged inline.

Blocking

🔧 wrench

• Spec collides with PR #669 (ts CLI): PR #669 introduces a ts CLI with ts config and ts audit subcommands, targeting main and currently open. This spec proposes a separate ts-config binary in a separate crates/trusted-server-cli/ crate. Christian's prior inline comment ("hold off and rework this when [the ts cli / edgezero cli] are in place") points to the same collision. Either rework so the four subcommands are designed as ts config validate|match|lint|check under the namespace from #669, or explicitly cite #669 in the spec and justify why two CLI binaries are intentional.
• PR base is server-side-ad-templates-impl, not main (PR metadata): A spec-only PR should be reviewable and mergeable independently. As stacked, this PR can't land until the much larger feature branch lands first, and reviewers have to mentally subtract the 30+ unrelated files in the base diff to see this PR's actual content. The PR body itself claims "Phase 1A ships as PR A independently" — that's contradicted by the stacked base. Rebase onto main.
• Copy-paste drift defense is insufficient (inline at §4.4): see inline comment.

❓ question

• Why a top-level ts-config binary rather than ts config <subcmd>? Given #669 establishes the ts namespace, this needs an answer in the spec. If the answer is "because EdgeZero/ts isn't merged yet," document the migration path so we don't ship a binary that's renamed/removed within one release cycle.
• --config vs $TS_CONFIG_PATH conflict semantics (inline at §4.1): see inline comment.

Non-blocking

🤔 thinking

• 2122-line spec for a Phase 1 CLI (file-level): actual binary scope (4 subcommands, ~12 rules, ~600 lines of Rust) is much smaller than the spec implies. ~140 lines of JavaScript shim and four full HTML fixtures in §4.2 belong in the Node.js package's docs once implementation lands. Recommend splitting into a Phase 1A spec (~700 lines, Rust binary) and Phase 1B spec (~900 lines, Node.js generator) so each phase is independently reviewable.
• APS "no-overlap" wording is ambiguous (inline at §4.2 lines 795-797): see inline comment.
• setInterval runs without early-exit (inline at §4.2 lines 648-654): see inline comment.

♻️ refactor

• MatchResult enum exists only in the CLI (inline at §4.4 line 1121): see inline comment.
• JSON contract omits kind-specific fields (inline at §6 line 1383): see inline comment.
• Hash-suffix collision terminal condition unbounded (inline at §4.2 line 770): see inline comment.
• No stable ordering for errors[] / warnings[] (cross-cutting): HashMap iteration order means two CI runs against the same file produce diff churn. Specify lexicographic ordering by field then kind, or by slot-file order then field, in the §6 JSON contract. Diff-friendly output matters once this lands in CI.
• lint output dedup vs validate rules (cross-cutting §4.1, §6): lint's kind enumeration is a superset of validate. If a file has an invalid slot ID, both should report it once — but spec is silent on whether lint --json puts validate-level errors in errors[] or warnings[]. Recommend: validate-level errors always in errors[]; lint-only findings always in warnings[]; no duplication.

🌱 seedling

• No per-URL Playwright navigation timeout (inline at §4.2 line 666): see inline comment.

📌 out of scope

• Runtime should also enforce config invariants once KV-backed config lands: today matches_path silently treats an uncompilable pattern as false (creative_opportunities.rs:84-91) — fine while config is baked into the WASM binary via include_str!(), but harmful once it comes from KV (mentioned as a non-goal in §3). The CLI is pre-flight; runtime needs its own enforcement once that surface opens.
• Confirm generator's slug derivation is a strict subset of [A-Za-z0-9_-]+: §7 says slot IDs are an HTML-injection safety surface. The validate step will catch any drift, but a one-line invariant statement in §4.2 ("tokenizer output is a strict subset of [A-Za-z0-9_-]+ by construction") would close the loop and let reviewers verify by inspection.

⛏ nitpick

• §1 prose: "GPT slot definition injected into <head>" (inline at line 11)
• Malformed markdown at §8 "Pattern glob normalization" (inline at line 1479)
• §11 verification step numbering collides (inline at line 2117)

CI Status

• browser integration tests: PASS (1m54s)
• integration tests: PASS (1m34s)
• prepare integration artifacts: PASS (2m1s)

[aram356]

⛏ nitpick — "GPT slot definition injected into <head>"

creative-opportunities.toml doesn't call defineSlot; the runtime injects __ts_ad_slots and the GPT integration consumes it client-side. As written, this implies the TOML schema drives GPT directly. Tighten the prose so readers don't form that mental model.

[aram356]

❓ question — --config vs $TS_CONFIG_PATH conflict semantics undefined

Spec says --config takes precedence when specified. But: if --config /missing.toml is passed and $TS_CONFIG_PATH=/exists.toml is set, does the tool fall back to $TS_CONFIG_PATH, or surface the explicit path's not-found error?

Standard convention is explicit > env with no fallback (so the not-found error is on /missing.toml), but the spec doesn't say so. State this explicitly — otherwise implementers will guess, and the behavior becomes hard to change later.

[aram356]

🤔 thinking — setInterval runs for settle + 2000ms with no early-exit

With default settle = 6000, the interval runs ~8s × 20Hz = 160 fires. All patches are idempotent via __ts_patched, so this is wasted scheduler work — not a correctness bug. Cleaner pattern:

const interval = setInterval(() => {
  patchGoogletag()
  patchCmdPush()
  patchApstag()
  if (allPatched()) clearInterval(interval)
}, 50)

…where allPatched() checks the three __ts_patched flags. Optional polish.

[aram356]

🌱 seedling — no per-URL Playwright navigation timeout

Spec calls waitForTimeout(settle) after { waitUntil: 'load' } but doesn't set a Playwright navigation timeout. Default is 30s — fine for one URL, but a multi-URL crawl with 10 dead URLs hangs CI for 5 minutes.

Fix: specify page.setDefaultNavigationTimeout(navTimeout) with a documented default (suggest 15s) and a --nav-timeout flag. Note in §4.2 and the CLI interface block.

[aram356]

♻️ refactor — hash-suffix collision terminal condition unbounded

"Lengthen by 2 hex chars at a time until unique" has no documented stopping point. In practice 6 hex (24 bits) handles every realistic case (birthday-bound collision probability is ~0.003% at 1000 slots), but the loop is theoretically unbounded.

Fix: specify an upper bound — e.g. "extend up to 16 hex (64 bits); error out if still colliding." That makes the error path documented rather than implicit, and keeps the generator deterministic in the (effectively unreachable) failure case.

[aram356]

♻️ refactor — MatchResult enum exists only in the CLI; exposes drift surface

Runtime matches_path returns plain bool (creative_opportunities.rs:81). This spec introduces a richer MatchResult { Match, NormalisedMatch, UncompilablePattern } only in the CLI. That's exactly the drift-prone surface §4.4 warns about — every future change to runtime matching semantics has to be mirrored manually here, and the round-trip test only catches drift exercised by the production config.

Fix: expose matches_path_with_normalisation from creative_opportunities.rs as the canonical entry point (returning the enum); have runtime call a .is_match() / .was_match() helper on it; let the CLI consume the same enum directly. If wasm32-wasip1 blocks sharing the module today (per §4.4's EdgeZero note), document that blocker explicitly — but don't introduce a CLI-only variant of the type that defines matching behavior.

[aram356]

🔧 wrench — copy-paste drift defended only by comments and single-config round-trip test

The spec correctly identifies that the CLI duplicates validate_slot_id and matches_path_with_normalisation from creative_opportunities.rs. The defense is:

1. // NOTE: keep in sync comments in both files
2. A round-trip test that runs the CLI against the production creative-opportunities.toml (currently 3 slots, all clean)

The round-trip test cannot detect drift in any rule the production config doesn't exercise. The spec mentions "the test fixture matrix in §9... should be run against both the CLI and the equivalent runtime logic in creative_opportunities.rs tests" — but only as aspirational future work, not Phase 1A scope.

Fix: promote that fixture-matrix proposal to Phase 1A scope. Concrete shape:

• tests/fixtures/parity/<kind>.toml for each kind value in §6 (one fixture per invalid_slot_id, uncompilable_pattern, missing_leading_slash, non_string_targeting, etc.)
• A shared loader in both creative_opportunities.rs::tests and the CLI's integration tests that iterates the directory and asserts identical pass/fail outcomes per fixture.

Without this, "CLI passes ≠ runtime accepts" can ship silently.

[aram356]

♻️ refactor — kind-specific JSON fields not documented in the contract

§4.1 examples show glob_normalised warnings carrying original and effective keys (line 222-227). The §6 contract states all entries carry slot/field/kind/message — implying that's the complete shape.

Fix: either (a) enumerate kind-specific fields per kind value in a small table, or (b) add an explicit "additional kind-specific keys may be present" clause and pin which kinds carry which extras. As written, a downstream JSON consumer building strictly against §6 will silently miss the original/effective fields and break when they appear.

[aram356]

⛏ nitpick — malformed markdown escapes

This paragraph mixes literal underscores, escaped asterisks, and code spans in ways that won't render cleanly:

/20**follows the same rule as/b**: the ** immediately follows … /20\*\*get the correct match outcome … the generated output uses/20_directly. Theequivalent_patternlint fires if both/20\*_and/20_appear

Intent reads, but the rendered HTML will mangle the underscores and asterisks. Reformat with consistent code spans (/20**, /20*, etc.) and proper spacing around them.

[aram356]

⛏ nitpick — verification step numbering collides

Phase 1B CI uses steps 8–12 (lines 2109-2113); Manual then restarts at 8 (this line). Renumber Manual as 13–17 so the section reads top-to-bottom without ambiguity.