Add an AWS deployment planning skill for Prebid Server Go - #1166
ChristianPavilonis wants to merge 14 commits into
Conversation
aram356
left a comment
aram356
left a comment
There was a problem hiding this comment.
This is not a complete review, but I wanted to make sure we keep the CLI clean and consistent. We already have a prebid subcommand, so I would recommend building on top of that.
For the new command:
ts prebid server ...
And the existing command would become:
ts prebid client ...
prk-Jr
left a comment
prk-Jr
left a comment
There was a problem hiding this comment.
Summary
Adds an agent planning skill for self-hosted Prebid Server Go on AWS, a new experimental ts prebid server CLI namespace (local inspection and validation, guarded Secrets Manager writes, EC2 status), and renames ts prebid bundle to ts prebid client. The CLI subsystem is carefully built — sanitized &'static str error payloads, an injected Interaction trait so tests can't touch a terminal, secrets kept out of argv, owner-only temp payloads cleaned up on both success and failure — and the negative-path test suite is genuinely strong. All 20 CI checks pass.
One blocking item: .tool-versions pins every tool except the AWS CLI this PR adds, which is exactly the binary the credential-write path shells out to.
3 of the inline comments below carry a one-click GitHub
suggestion— use Commit suggestion (or Add suggestion to batch for several at once) to apply them as commits on the PR branch. Each was verified in an isolated worktree, individually and as a batch, againstcargo fmt --all -- --check, both host-targettrusted-server-cliclippy invocations with-D warnings, andcargo test --package trusted-server-cli. The remaining comments describe the fix in prose because the change touches multiple files, lands outside the diff hunks, or didn't survivecargo fmtin the reviewed form.
Blocking
🔧 wrench
- AWS CLI pinned to
latestwhile every other tool is exact — see inline at.tool-versions:6
Non-blocking
♻️ refactor
rpassword/serde_yaml_ngbypass workspace dependency inheritance — see inline atcrates/trusted-server-cli/Cargo.toml:27- The determinism check compares a pure function against itself — see inline at
crates/trusted-server-cli/src/commands/pbs/config.rs:378 - Hand-rolled JSON escaping round-trip in
bidder_list— see inline atcrates/trusted-server-cli/src/commands/pbs/inspect.rs:91(suggestion)
🤔 thinking
ts prebid bundle→ts prebid clientis a breaking rename with no alias — see inline atcrates/trusted-server-cli/src/run.rs:75(suggestion)- Transport failure on
put-secret-valuedoesn't flag the outcome as uncertain — see inline atcrates/trusted-server-cli/src/commands/pbs/secrets.rs:173 - A dated
Status: Implementedspec is rewritten retroactively — see inline atdocs/superpowers/specs/2026-06-17-prebid-bundle-cli-design.md:5 - The
ts prebid servernamespace is undocumented indocs/— see inline atdocs/guide/cli.md:274 - Missing
python3makes two tests pass for the wrong reason — see inline atcrates/trusted-server-cli/tests/pbs_cli.rs:23 identifier()rejects AWS profile names containing.— see inline atcrates/trusted-server-cli/src/commands/pbs/config.rs:104
⛏ nitpick
- Unparenthesized
&&/||on the noninteractive-write gate — see inline atcrates/trusted-server-cli/src/commands/pbs/secrets.rs:112(suggestion)
🌱 seedling
- Unbounded recursion over operator YAML — see inline at
crates/trusted-server-cli/src/commands/pbs/config.rs:318
📝 note
- Identity failure aborts the whole status report; resource-query failure degrades — see inline at
crates/trusted-server-cli/src/commands/pbs/status.rs:26
👍 praise
- Negative-path test discipline — see inline at
crates/trusted-server-cli/tests/pbs_cli.rs:39
Cross-cutting / body-level findings
- 📌 Three separable concerns in one PR — this bundles (a) agent-only markdown under
.claude/skills/with zero runtime impact, (b) a ~1800-LOC experimental CLI subsystem that writes AWS credentials, and (c) a breaking rename of an already-shipped command. They have different audiences, different risk profiles, and different revert stories: the rename is the one most likely to need a fast follow-up or a release note, and it's currently welded to a large feature branch. Not a change request on this PR — but if the rename landed separately it could be communicated and reverted on its own cadence. Flagging underAGENTS.md's "every change should impact as little code as possible".
CI Status
- integration tests (Fastly EC lifecycle): PASS
- integration tests: PASS
- browser integration tests: PASS
- CodeQL: PASS
- cargo test (ts CLI, native): PASS
- vitest: PASS
- format-typescript: PASS (required)
- Analyze (javascript-typescript): PASS
- cargo fmt: PASS (required)
- cargo test (axum native): PASS
- Analyze (rust): PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo test: PASS (required)
- format-docs: PASS (required)
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- cargo test (cross-adapter parity): PASS
- CLAUDE.md symlink guard: PASS
- prepare integration artifacts: PASS
- Analyze (actions): PASS
No failed, cancelled, or pending checks.
jevansnyc
left a comment
jevansnyc
left a comment
There was a problem hiding this comment.
Nothing major so posting in here as single comment:
Breaking rename with no alias — [run.rs:76] ts prebid bundle became ts prebid client. Any existing script gets error: unrecognized subcommand 'bundle'. Confirmed against the built binary. Either add a hidden alias or call the break out in the PR description.
.tool-versions aws plugin name is wrong — [.tool-versions:6] The entry is aws 2.36.45, but asdf/mise call that plugin awscli. asdf install fails on the exact onboarding path the docs point at. CI is unaffected since the workflows grep only rust/nodejs/viceroy.
Regional module has no provider pin — [modules/regional/terraform.tf:3] No AWS provider version constraint, and [.gitignore:3] excludes its lock file, yet RUNBOOK.md tells operators to init/test inside the module. So the module test runs against an unpinned provider that will drift away from the root's = 6.64.0.
Confirm prompt rejects long input instead of declining — [pbs/mod.rs:228] Terminal::confirm caps the answer at 16 bytes and returns "input exceeds size limit" rather than treating it as a no. A 17-character answer makes the operator re-enter the secret through the hidden prompt.
CPU alarm pages on stopped hosts — [modules/regional/monitoring.tf:13] The per-instance high-CPU alarm sets treat_missing_data = "breaching", so a stopped or replaced instance pages as saturated.
ONE QUESTION for Christian:
The test plan lists terraform fmt, init, and validate, but not terraform test, even though both .tftest.hcl files ship and the README/RUNBOOK instruct operators to run them. Worth confirming those two suites actually ran. validate leaves module inputs unknown and evaluates very little of the plan.
prk-Jr
left a comment
prk-Jr
left a comment
There was a problem hiding this comment.
Summary
Reviewed commit 74bdc5b57e5a8d3cca8174d9ccc7684f87466cbe.
The secret-write boundary handles account checks, payload privacy, confirmation, and uncertain outcomes carefully. The discovery command, however, reads the retired server configuration layout and misses server bidders in current valid configurations.
Blocking
- 🔧 Read server demand from the current auction schema — see inline at
crates/trusted-server-cli/src/commands/pbs/inspect.rs:134–140.
Validation
The unmodified production PBS module was imported into an isolated harness. Inspecting a current-format provider/bidder fixture returned an empty server-candidate list, false endpoint presence, null test mode, and zero override rules. This was a module harness, not a build of the complete CLI.
Terraform formatting and validation, five root mock tests, two regional mock tests, Compose configuration, shell syntax, and both example descriptor checks passed. No live AWS calls, deployments, or secret writes were performed. Full Rust/JS/adapter gates rely on remote CI.
CI Status
- integration tests: PASS
- integration tests (Fastly EC lifecycle): PASS
- browser integration tests: PASS
- CodeQL: PASS
- cargo test (axum native): PASS
- cargo test (ts CLI, native): PASS
- format-typescript: PASS (required)
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- Analyze (rust): PASS
- cargo test: PASS (required)
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- format-docs: PASS (required)
- Analyze (actions): PASS
- cargo fmt: PASS (required)
- Analyze (javascript-typescript): PASS
- CLAUDE.md symlink guard: PASS
- cargo test (cross-adapter parity): PASS
- prepare integration artifacts: PASS
- vitest: PASS
- Analyze (javascript-typescript): PASS
aram356
left a comment
aram356
left a comment
There was a problem hiding this comment.
Summary
Adds an explicitly-invoked AWS planning skill for Prebid Server Go, an experimental ts prebid server CLI namespace (inspect / check / secrets set / status), and a locally-checked two-region EC2/Compose Terraform example. Reviewed at 18d1b4877.
Verification performed in an isolated worktree rather than by reading alone: built and ran the CLI live, ran ./scripts/test-cli.sh (239 tests pass), cargo fmt --all -- --check and target-matched clippy (both clean), exercised aws configure get history semantics against a real AWS CLI v2, ran the shipped Terraform through fmt/validate/test, and reproduced each defect below end-to-end.
The secret-handling path holds up under scrutiny: payloads stay out of argv, temporary request files are owner-only and removed on every error path via Drop, AWS CLI history is checked on both cli_history and default.cli_history (I confirmed the two-key probe catches the [default]-inherited case), and account / replica / ARN verification all fail closed. The redaction tests genuinely hold.
Two blocking items below. Neither is a live production defect: one is a removed command with no compatibility path, the other is a regression guard that does not guard.
2 of the inline comments carry a one-click GitHub
suggestion. Both were applied in a scratch worktree and verified in isolation (fmt, clippy, full 239-test CLI suite, byte-exact post-verify drift check). The remaining comments describe fixes in prose because they span multiple assertions or files.
Blocking
🔧 wrench
security_unit_test.tftest.hcldoes not defend the invariants it is named for — see inline atdeploy/pbs-example/modules/regional/tests/security_unit_test.tftest.hcl:36ts prebid bundleremoved with no alias — silent breaking change — see inline atcrates/trusted-server-cli/src/run.rs:75
Non-blocking
♻️ refactor / 🤔 thinking / ⛏ nitpick
pinned_image()accepts uppercase digests that Docker rejects — see inline atcrates/trusted-server-cli/src/commands/pbs/config.rs:275- Retry guidance is wrong when a request token is reused with different content — see inline at
crates/trusted-server-cli/src/commands/pbs/aws.rs:79 - I/O errors name no path, so a multi-file descriptor failure is unactionable — see inline at
crates/trusted-server-cli/src/commands/pbs/mod.rs:150 - ALB egress rule contradicts its own description — see inline at
deploy/pbs-example/modules/regional/security.tf:18 --deploymentis the only PBS flag with empty help text — see inline atcrates/trusted-server-cli/src/commands/pbs/secrets.rs:23
Cross-cutting / body-level findings
-
📌 Committed Terraform lock file carries a single platform hash —
deploy/pbs-example/.terraform.lock.hcl:7records oneh1:hash. On a non-matching platformterraform initsilently rewrites the file, and aterraform testafter restoring it fails with "does not match any of the checksums recorded in the dependency lock file". This contradictsRUNBOOK.md:8("use the committed AWS provider lock file") andRUNBOOK.md:70("review any lock-file change"): operators on other platforms get a spurious diff on every run, which trains them to rubber-stamp lock changes. Regenerate withterraform providers lock -platform=darwin_arm64 -platform=darwin_amd64 -platform=linux_amd64 -platform=linux_arm64. -
🌱 No ALB access logging —
deploy/pbs-example/modules/regional/load_balancing.tf:1-13has noaccess_logsblock anywhere in the tree. For an internet-facing ALB in a deliberately "production-shaped" reference there is no request-level forensic record.drop_invalid_header_fields = trueandidle_timeout = 30are both set, which makes the omission stand out. Either add anaccess_logsblock or state inDEPLOYMENT_PLAN.mdthat it is deliberately out of scope — the current silence reads as an oversight. -
🌱 No VPC endpoints, so SSM and Secrets Manager traffic exits via NAT — there is no
aws_vpc_endpointin the tree. The instance role attachesAmazonSSMManagedInstanceCoreand reads Secrets Manager (modules/regional/secrets.tf:42-62), but with no interface endpoints forssm,ssmmessages,ec2messages,secretsmanager, orkms, that control-plane and credential traffic traverses the public internet and incurs per-GB NAT cost on every secret fetch. Notable for a design whose stated benefit is private hosts with regionally isolated credentials. -
📝
AmazonSSMManagedInstanceCoreis the one broad IAM grant — the inline policy atmodules/regional/secrets.tf:47-69is tight (Resourceenumerates the declared secret ARNs, KMS is conditionally scoped to one key, no wildcards). The AWS-managed SSM policy is the deliberate exception and carries"Resource": "*". Standard practice and hard to avoid, but worth a comment in an example that markets least-privilege. -
⛏ Six new markdown files are not prettier-clean —
crates/trusted-server-cli/README.md, fourreferences/*.md, anddeploy/pbs-example/DEPLOYMENT_PLAN.mdfailprettier --checkunderdocs/.prettierrc(mostly misaligned table pipes). No CI gate covers this:.github/workflows/format.yml:121-125scopesformat-docstodocs/, and nothing in CI touches.claude/,crates/**/README.md, ordeploy/. Raised only because the other new markdown files in this PR are clean, so the inconsistency is internal. Do not extend the CI gate for this. -
⛏ Terraform 1.16.2 is pinned in docs and HCL but not in
.tool-versions—RUNBOOK.md:8anddeploy/pbs-example/terraform.tf(required_version = ">= 1.16.2, < 1.17.0") agree, but.tool-versionshas noterraformentry while every other toolchain in the repo is asdf-pinned.references/terraform.md:29advises selecting a reproducible Terraform version "in the repository's toolchain", so the skill's own guidance is not followed by the example it ships. -
⛏
crates/trusted-server-cli/README.md:5will be stale on merge — "The namespace is experimental and is being evaluated in PR review." Prefer a durable phrasing such as "experimental; the interface may change without a deprecation cycle". -
⛏
deploy/pbs-example/runtime/compose.yaml:6publishes on all interfaces —"${PBS_HOST_PORT:-8000}:8000"renders without a host-IP restriction, so a smoke-test PBS instance is reachable from the local network. The smoke script itself only ever talks to127.0.0.1. Consider"127.0.0.1:${PBS_HOST_PORT:-8000}:8000"for the example.
CI Status
- cargo fmt: PASS (required)
- cargo test: PASS (required)
- format-docs: PASS (required)
- format-typescript: PASS (required)
- cargo test (ts CLI, native): PASS
- cargo test (axum native): PASS
- cargo test (cross-adapter parity): PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- vitest: PASS
- CodeQL: PASS
- Analyze (rust): PASS
- Analyze (javascript-typescript): PASS
- Analyze (actions): PASS
- CLAUDE.md symlink guard: PASS
- prepare integration artifacts: PENDING
No failing checks. CI is not a factor in this verdict.
|
Addressed the remaining review feedback in The review-body items are also covered:
Local validation passed: 121 CLI tests, Rust format and target-matched clippy, both Terraform validates, five root and two regional mocked runs, six security mutation checks, both descriptor checks, Compose/render wiring, JSON/shell checks, and scoped Markdown formatting. The PR test plan now separates this evidence from earlier JS validation and deferred runtime/cloud checks. Independent correctness review has no remaining findings. No live AWS operations, secret writes, Terraform apply, or container startup were performed. Existing-state adoption and out-of-band security-group drift still need separately authorized inspection. @aram356 @prk-Jr Ready for re-review. The six threads are left open for your verification. |
Gather deployment requirements before choosing AWS services and generating Terraform or runtime files. Keep cloud execution behind separate approval and document safe testing, state ownership, and credential handling. Refs #1163
Make security tests catch broken host and network invariants, and keep local smoke checks separate from deployment bindings. Improve operator errors without exposing credential values or changing write authority. Keep the intentionally retired bundle command rejected. Record deferred cloud evidence and use reproducible Terraform tooling and provider locks.
1d3d310 to
fbb18bd
Compare
prk-Jr
left a comment
prk-Jr
left a comment
There was a problem hiding this comment.
Summary
Pass 2 of this review. 11 of the 14 pass-1 findings are fully resolved, including the one blocking item, and all 20 CI check runs pass at fbb18bd75. The remaining ten findings below are all non-blocking, so this goes out as an approval — none of them should hold the merge.
Credit where it is due on the fix commits:
- 🔧
.tool-versionspin landed.awscli 2.36.45andterraform 1.16.2are pinned, andAGENTS.mdgained the matching AWS CLI and Terraform toolchain rows. - The secret-write "outcome uncertain" path is now unified. A transport-layer failure on
put-secret-valuereturns the sameWRITE_NOT_CONFIRMEDguidance as the malformed-response branch, andtests/pbs_cli.rsasserts the full string across all four failure shapes (collision/transport/invalid-json/unverified). - The python3 precondition is asserted, so the process tests can no longer pass for the wrong reason.
rpassword/serde_yaml_ngmoved to{ workspace = true }; the tautological determinism check became a real two-load comparison test;aws.profilenow accepts periods with a field-specific error; the&&/||gate is parenthesized; thestatusidentity-vs-resource-query asymmetry is documented; and the two dated design records were annotated rather than rewritten.
3 of the inline comments below carry a one-click GitHub
suggestion. The other 7 describe the fix in prose, because they touch a tree with no automated gate (see Cross-cutting), span multiple files, or have no single-range fix.
👍 praise
aws.rssecret-handling is genuinely careful. The payload goes to the AWS CLI through an owner-onlyNamedTempFilevia--cli-input-json file://, withstdin(Stdio::null()),AWS_IGNORE_CONFIGURED_ENDPOINT_URLS=true, and a CLI-history pre-check gating everyput-secret-value. The secret never reaches argv or shell history, andtests/pbs_cli.rs:249-289proves it end-to-end against a real process rather than a mock.PbsErroris structurally leak-proof. Every variant carries&'static str, so secret-bearing parser or provider output cannot enter aReporteven by accident. The tests back this up by assertingNEVER_PRINT_ME/DUMMY_SECRETabsence in both{error:?}and the rendered human and JSON reports.config.rs:214-230catches a nasty class of bug. Cross-bidderenvcollision detection pluspbs_pathprefix-overlap detection plus a rendered-YAML occupancy check together prevent two bidders from silently clobbering each other's secret object — exactly the kind of failure that would only surface in production.
Cross-cutting / body-level findings
🤔 thinking — the entire deploy/pbs-example/ tree has no CI coverage
No workflow under .github/workflows/ references terraform or deploy/pbs (verified by grep across all workflows at this head). That leaves roughly 2,300 new lines — 20 .tf files, two .tftest.hcl suites, scripts/smoke-runtime.sh, and scripts/test-smoke-runtime.py — verified only by the PR body's checklist, run locally by the author. This is the same class as the known gap where shell harnesses sit outside the documented gate list, and it will rot silently: a later Terraform or provider bump can break validate with nothing to catch it.
Either add a terraform job gated on deploy/** paths:
- run: terraform fmt -check -recursive deploy/pbs-example
- run: terraform -chdir=deploy/pbs-example init -backend=false -input=false
- run: terraform -chdir=deploy/pbs-example validate
- run: terraform -chdir=deploy/pbs-example test -filter=tests/root_unit_test.tftest.hcl
- run: terraform -chdir=deploy/pbs-example/modules/regional init -backend=false -input=false
- run: terraform -chdir=deploy/pbs-example/modules/regional test -filter=tests/security_unit_test.tftest.hcl…or state plainly in deploy/pbs-example/README.md that these are manual checks and name who re-runs them. This is also why every Terraform comment below is prose rather than a one-click suggestion: I have no terraform binary in the review environment and no gate to lean on, so I will not post bytes I could not verify.
📌 out of scope — PR scope grew rather than narrowed
Pass 1 flagged three separable concerns. There are now four, and the PR went from 33 files / +2,941 to 74 files / +5,297 with the addition of the whole Terraform example stack. Nothing here is wrong, and splitting now almost certainly costs more than it saves — but the review surface is larger than any single pass can scratch-verify, and the CLI rename is independently revertable in a way the rest is not. Worth noting for the next one: land the Terraform example on its own.
Still open from pass 1
- #4 (hand-rolled JSON escaping) — partially resolved. The opaque-serde-error symptom is fixed; the hand-rolled escape and a now-inaccurate doc comment remain. See inline at
crates/trusted-server-cli/src/commands/pbs/inspect.rs:147. - #8 (docs-site coverage) — partially resolved. The four operations are now on the docs site; the descriptor schema and secret-write safeguards still live only in the crate README. See inline at
docs/guide/cli.md:668. - #12 (unbounded recursion) — not addressed. See inline at
crates/trusted-server-cli/src/commands/pbs/config.rs:332. - #5 (breaking rename) — addressed differently. The retirement is now deliberate and asserted rather than accidental, which is a legitimate answer to the finding. The one residual gap is that no user-facing doc records the rename; see the suggestion inline at
docs/guide/cli.md:611.
Caveat carried forward: AWS behaviour is still fake-verified only
Two AWS behavioural assumptions remain exercised only against tests/pbs_cli.rs's python fake, which is authored to satisfy them. Flagged inline at crates/trusted-server-cli/src/commands/pbs/secrets.rs:182 — not blocking, but it is the single largest gap between "tests pass" and "this works against real AWS".
Verification performed for this review
- Scratch-verified in an isolated worktree at
fbb18bd75. Green baseline first:cargo fmt --all -- --checkclean,cargo clippy --package trusted-server-cli --all-targets -- -D warningsclean, 560 CLI tests pass (0 failed, incl. all 7pbs_cliprocess tests). - The
inspect.rssuggestion was then applied in isolation and re-run through the same gate: fmt clean, clippy clean, 560/560 pass, and the post-verify patch is byte-identical to the approved patch (no drift). - The
docs/guide/cli.mdsuggestion passesprettier --checkusing the repo-pinneddocs/Prettier, also with zero drift. - Not reproduced: the CI
x86_64-unknown-linux-gnutriple. Cross-compiling from this macOS host fails incc-rs(failed to find tool "x86_64-linux-gnu-gcc"), so clippy and the tests ran onaarch64-apple-darwin. CI's own greencargo test (ts CLI, native)at this head is the authority for the Linux triple, not my run. - Not performed: anything Terraform, any real AWS call, any container start, and confirming
prebid/prebid-server@sha256:f0fee9ca…really is PBS Go v4.7.0.
aram356
left a comment
aram356
left a comment
There was a problem hiding this comment.
Summary
Second review pass, at fbb18bd75 (first pass reviewed 18d1b4877). The branch was rebased onto newer main and two commits address the previous round's feedback.
Verification was done in an isolated worktree by running code, not by reading commit messages: 515 unit + 7 integration CLI tests pass, cargo fmt --all -- --check and cargo clippy-cli are clean, the new scripts/test-smoke-runtime.py passes, and each previous finding was re-tested against the running binary. A separate pass confirmed by mutation testing that the new Terraform security assertions genuinely fail when the invariants they guard are broken.
Six of the seven previous findings are fixed. Verified individually: the four missing Terraform security assertions now exist and bite; pinned_image() rejects uppercase digests (with a new pinned_image_requires_lowercase_sha256_hex regression test); the AWS write-failure message is a shared WRITE_NOT_CONFIRMED const covering both the rejected-write and lost-response cases; I/O errors now name the failing path, so the three previously-identical messages are distinguishable; ALB egress is scoped via aws_vpc_security_group_egress_rule.alb_to_pbs; and --deployment has help text. The body-level items also landed: the lock file covers four platforms, Terraform is pinned, aws was corrected to awscli (the previous value was not a valid asdf plugin name), the markdown is prettier-clean, and the deferred ALB access logging and VPC endpoints are now documented with named owners and decision criteria.
On the seventh — the ts prebid bundle alias — the decision to reject it is respected. run.rs:196 now carries prebid_rejects_retired_bundle_command, which asserts the retired spelling must fail to parse. The previously-suggested alias was re-applied in scratch and does break that test, so it is not being re-proposed. What remains is the communication gap, raised below as a body-level finding: the removal of a user-facing command has no CHANGELOG.md entry, and this repository documents breaking changes there in detail.
One blocking finding remains, and it is narrow: a fix that was applied correctly in one file was not carried to the two operator-facing copies.
3 of the inline comments carry a one-click GitHub
suggestion. Each was applied in a scratch worktree and verified; thehost_ipchange was additionally mutation-tested to confirm it catches the regression it guards. The remaining comments describe fixes in prose because they span files or require a design decision.
Blocking
🔧 wrench
run_cli_linuxwalkthrough fails on macOS in both operator docs — see inline atdeploy/pbs-example/README.md:35anddeploy/pbs-example/RUNBOOK.md:36
Non-blocking
🤔 thinking / ⛏ nitpick
- Module lock file is gitignored, so the documented module test path resolves the provider unpinned — see inline at
deploy/pbs-example/.gitignore:3 var.pbs_portis adjustable in Terraform but hardcoded in the runtime — see inline atdeploy/pbs-example/modules/regional/variables.tf:36- Inconsistent
host_ipassertion weakens the production check — see inline atdeploy/pbs-example/scripts/test-smoke-runtime.py:26 .tool-versionscolumn alignment — see inline at.tool-versions:7
Cross-cutting / body-level findings
-
📌 The
bundletoclientrename has noCHANGELOG.mdentry. This PR removes a user-facing command and does not touchCHANGELOG.md. The[Unreleased]section carries detailed**Breaking:**entries for changes of exactly this kind, including migration instructions. Retiring the spelling outright is a defensible call andrun.rs:196now pins it as intentional, but an operator withts prebid bundlein a script or runbook currently learns about the removal from a parse error. A short**Breaking:**bullet naming the old and new spellings would close this. The in-repo notes added todocs/superpowers/in the previous round are design-history records, not the operator-facing changelog. -
📌 Nothing under
deploy/pbs-example/runs in CI.grep -rn "deploy/pbs-example\|terraform\|smoke-runtime" .github/workflows/returns no matches: neitherterraform fmt -check, nor eitherterraform testsuite, norscripts/test-smoke-runtime.pyis executed by any job. This is raised now specifically because the tests added this round are effective — the four security assertions were confirmed to fail under mutation ofhttp_tokens,encrypted,subnet_id, andssl_policy, and three more catch ALB ingress and egress widening. Tests that are proven to work but are never executed will drift silently the first time the module is edited. Either add a job gated onpaths: ['deploy/pbs-example/**']running the format check, bothinit -backend=falseplustestinvocations, and the wiring test, or state the deferral explicitly inDEPLOYMENT_PLAN.md. The "Generated files and checks" table currently lists a "Local check" for every artifact without noting that a human has to remember to run any of them. -
🤔
PbsError::InputFileechoes operator paths, and no document mentions it.crates/trusted-server-cli/src/commands/pbs/mod.rs:68renders the full path via{path:?}, including under--json. This resolves the previous round's finding that three different missing files produced one indistinguishable message, and the escaping is correct — a path containing ANSI escapes and control characters renders as literal\u{1b},\n, and escaped quotes, so terminal injection is not possible, andmissing_descriptor_inputs_report_escaped_pathscovers it. The gap is documentation.crates/trusted-server-cli/README.md:79andRUNBOOK.md:114both instruct operators to pass--file /secure/path/<bidder>.json, so a typo or permission error emits the partner name and the layout of the secrets staging directory to stderr and into any CI log. Nothing in the docs is falsified by this, but the surrounding prose is uniformly reassuring about what the tool withholds, and no document states that input paths are echoed. One clause inRUNBOOK.md's existing "keep credentials out of public logs" guidance would resolve it; narrowing the variant toPath::file_name()is the alternative. -
⛏
DEPLOYMENT_PLAN.md's "Generated files and checks" table has no row forscripts/. The table atDEPLOYMENT_PLAN.md:102-113enumerates every generated artifact with a consumer and a local check, but omits bothscripts/smoke-runtime.shand the newscripts/test-smoke-runtime.py, though both are committed artifacts and both now appear in the README and RUNBOOK walkthroughs.SKILL.md:81sets the standard: "Done when every approved artifact exists, has a named owner and check." -
📝
scripts/test-smoke-runtime.pyis the repository's only first-party Python file, andpython3is not declared in.tool-versions. It is self-contained and appropriate for an example directory, and the process-spawning harness it implements would be worse in bash. Noting it only because it becomes an undeclared prerequisite if the CI job above is added; the bareassertstatements would also be stripped underpython3 -O.
CI Status
- cargo fmt: PASS (required)
- cargo test: PASS (required)
- format-docs: PASS (required)
- format-typescript: PASS (required)
- cargo test (ts CLI, native): PASS
- cargo test (axum native): PASS
- cargo test (cross-adapter parity): PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- vitest: PASS
- browser integration tests: PASS
- integration tests: PASS
- integration tests (Fastly EC lifecycle): PASS
- prepare integration artifacts: PASS
- CodeQL: PASS
- Analyze (rust): PASS
- Analyze (javascript-typescript): PASS
- Analyze (actions): PASS
- CLAUDE.md symlink guard: PASS
All 20 checks pass. CI is not a factor in this verdict.
|
Pushed review follow-ups in f969cf9 and replied to/resolved all 16 open review threads. The body-level requests are also covered: CHANGELOG.md records the breaking command rename; the new path-scoped PBS example workflow runs Terraform and Compose wiring checks; operator docs explain input-path disclosure; the deployment-plan table names both scripts and their checks; and Python requirements and optimization behavior are explicit. Local validation passed: ./scripts/test-cli.sh, Rust formatting, host-target CLI clippy with and without all features, Terraform validation and 13 mocked tests, both descriptor checks, Compose wiring, four mutation checks, scoped Prettier, the docs build, and actionlint. All five workflow command steps also passed locally. The pushed GitHub Actions run is separate evidence and is not claimed as passed here. The reported YAML nesting crash did not reproduce with the pinned parser; process regressions now cover deep sequence and mapping rejection. Unset history behavior was verified against the real pinned AWS CLI using isolated local configuration. Live Secrets Manager writes and retries remain a documented, separately authorized runtime-owner task. No cloud API calls, Terraform apply, secret writes, or PBS container startup were performed. deploy/pbs-test/ remains untouched. |
Preserve main's managed User ID validation and origin-probe dependencies alongside the PBS CLI and planning work. Combine the CLI documentation and changelog entries, and update new command references to prebid client.
aram356
left a comment
aram356
left a comment
There was a problem hiding this comment.
Summary
Third review pass, at e07134ff1. Previous passes covered 18d1b4877 and fbb18bd75. Since round 2 there is one PR commit, "Address PBS review follow-ups and add example CI", plus a large merge of main.
Verification was done in an isolated worktree by running code. cargo fmt --all -- --check and cargo clippy-cli are clean, ./scripts/test-cli.sh passes 688 tests, every step of the new workflow was run locally, and each previous finding was re-tested against the built binary rather than read from a commit message. A separate pass re-ran mutation testing on the Terraform security assertions and confirmed all six still fail when the invariants they guard are broken, so the main merge did not weaken them.
All seven round-2 findings are fixed. Verified individually: the macOS note is present in both operator docs and goes further than requested by adding a rustc -vV host-triple fallback, which was confirmed to run successfully; .github/workflows/pbs-example.yml now covers the example tree; CHANGELOG.md:12 carries a **Breaking:** entry for the command rename; the module lock is committed with four h1: hashes byte-identical to the root lock, so -lockfile=readonly resolves on every supported platform; var.pbs_port became local.pbs_port with a comment naming both coupled runtime files; the path-disclosure behavior is now documented at crates/trusted-server-cli/README.md:113; and the host_ip assertion, .tool-versions alignment, and DEPLOYMENT_PLAN.md scripts/ rows all landed.
Two blocking findings this round. Both are the same shape: a check that does not catch what it appears to catch.
The first is a correctness regression that arrived through the main merge rather than being authored here, but inspect.rs exists only in this PR, so this is the only place it can be fixed. The second is the newly added CI job, which is currently failing.
No inline comment carries a one-click
suggestion. Each fix spans multiple call sites, a test fixture, or several lines of a subprocess invocation, so every finding is described in prose with the proposed code. The two blocking fixes were applied and verified in a scratch worktree; results are stated in their comments.
Blocking
🔧 wrench
inspectreads a bundle schema themainmerge replaced, so bundle fields are always empty on valid configs — see inline atcrates/trusted-server-cli/src/commands/pbs/inspect.rs:92- The new
PBS Terraform and Compose checksjob is failing at this head — see inline atdeploy/pbs-example/scripts/test-smoke-runtime.py:23
Non-blocking
🤔 thinking / ⛏ nitpick
- The lock guard cannot detect a missing platform hash — see inline at
.github/workflows/pbs-example.yml:62 - Failure diagnostics are swallowed, which is why the CI log shows no cause — see inline at
deploy/pbs-example/scripts/test-smoke-runtime.py:23(covered in the blocking comment) - Python version is hardcoded while Terraform is derived from
.tool-versions— see inline at.github/workflows/pbs-example.yml:43 - Documented wiring-test command omits
-I, diverging from CI — see inline atdeploy/pbs-example/README.md:46 - Stale bundle field name in the config template — see inline at
trusted-server.example.toml:515
Cross-cutting / body-level findings
-
📌 No CI validates the committed example descriptor, so a validator change can invalidate it with every job green. Three facts combine: the new workflow's
paths:filter covers onlydeploy/pbs-example/**,.tool-versions, and the workflow itself, so a change undercrates/trusted-server-cli/does not trigger it; the workflow never runsts prebid server checkat all; and no Rust test uses the committed files, becauseconfig.rs's#[cfg(test)]fixtures build their own descriptors undertempfile::tempdir(). A contributor tighteningvalid_instance,secret_arn, orpinned_imagecan therefore land a change that breaksdeploy/pbs-example/deployment.example.yamlwhile CI stays green, and the next person to followREADME.md:36finds it. I confirmed the example currently passes, so this is prevention rather than repair.DEPLOYMENT_PLAN.md:116andRUNBOOK.md:35are honest that CLI validation is deliberately the example owner's local responsibility, so this is a scoping choice rather than a defect — but it is the one real hole in the new CI's coverage, and the cheapest close needs no new job and nopaths:widening:#[test] fn validates_the_committed_pbs_example_descriptor() { let path = Path::new(env!("CARGO_MANIFEST_DIR")) .join("../../deploy/pbs-example/deployment.example.yaml"); Deployment::load(&path).expect("should validate the committed PBS example descriptor"); }
That runs inside
test-cli, which has nopaths:filter and therefore executes on every PR, so a validator tightening fails at the source of the change. -
📝
deploy/pbs-example/*.mdsits outside every format gate. The docs gate runscd docs && npm run format, whoseprettier --check .is scoped todocs/, so the five markdown files here are ungated. This is visible inDEPLOYMENT_PLAN.md's wide ownership tables, where header separators are wider than their header rows. Cosmetic, and raised only because it explains the ragged alignment rather than asking for a change; extending prettier overdeploy/is a separate decision. -
📝 No
fail-fastconcern in the new workflow. It defines a single job with nostrategy.matrix, sofail-fastdoes not apply and its absence is correct.permissions: contents: readis correctly minimal with nopull-requestsorid-tokenover-grant,timeout-minutes: 20is present, and steps stop on first failure by default. Reading the Terraform version from.tool-versionsrather than hardcoding it is the right pattern, which is what makes the hardcoded Python version next to it stand out.
CI Status
- cargo fmt: PASS (required)
- cargo test: PASS (required)
- format-docs: PASS (required)
- format-typescript: PASS (required)
- PBS Terraform and Compose checks: FAIL (not in the branch-protection required set, so failing but not merge-blocking; see the blocking finding)
- cargo test (ts CLI, native): PASS
- cargo test (axum native): PASS
- cargo test (cross-adapter parity): PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- vitest: PASS
- browser integration tests: PASS
- integration tests: PASS
- integration tests (Fastly EC lifecycle): PASS
- prepare integration artifacts: PASS
- CodeQL: PASS
- Analyze (rust): PASS
- Analyze (javascript-typescript): PASS
- Analyze (actions): PASS
- CLAUDE.md symlink guard: PASS
20 of 21 checks pass. The one failure is the job this PR adds, and it is the second blocking finding.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Pushed follow-ups in 6516a67 and ae3cbac and replied to/resolved the remaining inline threads.
Current follow-up validation: 579 CLI tests passed, including failing-before/passing-after discovery regressions; Rust formatting, host-target CLI clippy, and scoped Prettier passed. The rebuilt CLI reports the current bundle selections and validates the committed descriptor. The prior Node migration also passed Python-free execution checks, Compose 2.38.2/5.5.0 checks, mutation tests, actionlint, and 13 mocked Terraform tests; its remote CI was green. New-head CI is not claimed as passed yet. No live AWS calls or PBS containers were started. deploy/pbs-test/ remains untracked and untouched. |
aram356
left a comment
aram356
left a comment
There was a problem hiding this comment.
Summary
Fourth review pass, at ae3cbacc4. Previous passes covered 18d1b4877, fbb18bd75, and e07134ff1.
Both round-3 blocking findings are fixed, and the new guards were confirmed to bite rather than merely exist.
inspect now reads core's nested modules.{bidder,user_id,analytics}: a valid config reports its real selections where round 3 reported empty lists, the retired flat shape correctly reports empty, and analytics_modules is surfaced. Simulating a future core rename of bidder fails three tests including the new parity test, so the schema coupling is now defended rather than assumed.
The PBS Terraform and Compose checks job is green. The Compose fix removes the version-dependent --no-env-resolution reliance; running the old command against the runner's exact Compose v2.38.2 reproduces the round-3 failure, and the new command passes on both v2.38.2 and v5.5.1, so the fix addresses the actual cause.
validates_the_committed_pbs_example_descriptor closes the example-drift gap: uppercasing one hex character of the committed digest makes it fail, and it runs in test-cli, which has no paths: filter.
The round-3 lock-guard observation is also resolved. Removing a platform hash still leaves init -lockfile=readonly at exit 0, but terraform validate and terraform test both exit 1 on the next lines of the same step, so a partially regenerated lock can no longer pass CI.
Terraform mutation testing re-run at this head: all six security assertions (IMDSv2, root-volume encryption, private-subnet placement, TLS policy, ALB egress scoping, ALB ingress CIDRs) still fail when their invariant is broken. Nothing was weakened by the Node migration.
On the Python-to-Node migration: node:assert/strict cannot be stripped, which structurally removes the previous bare-assert concern; the extensionless CommonJS shims execute correctly; env -u NODE_OPTIONS -u NODE_PATH blocks the --require preload vector; the environment scrub still holds under hostile PBS_BIND_ADDRESS/PBS_HOST_PORT; and no stale Python references remain anywhere. The inline fake-AWS script moved into a committed tests/fixtures/pbs-aws.cjs, with test.yml installing the pinned Node for both CLI jobs.
Local verification: cargo fmt --all -- --check and cargo clippy-cli clean, 578 unit plus 9 integration tests passing, and secret redaction re-proven with sentinels in provider endpoint userinfo, query strings, profile_config, bid-parameter overrides and account_id — zero occurrences across stdout and stderr in both output modes.
Approving. The five inline comments are all non-blocking nits; none needs to land before merge. Two of them are worth a glance because they affect someone following the docs: the CLI README's clippy command does not reproduce the CI gate and fails on the macOS host that same README targets, and inspect --config has a default that no document mentions.
| ```bash | ||
| ./scripts/test-cli.sh | ||
| cargo fmt --all -- --check | ||
| cargo clippy --package trusted-server-cli --all-targets --target x86_64-unknown-linux-gnu -- -D warnings |
There was a problem hiding this comment.
⛏ nitpick — This command does not reproduce the CI gate, and it fails on the host this same README targets at line 42.
Two issues:
- It omits
--all-features. The CI lint for this crate runs--all-features, so a feature-gated lint failure passes locally and fails in CI. - The hardcoded
x86_64-unknown-linux-gnuneeds a cross toolchain. Line 42 tells macOS readers to userun_cli_macos, so a macOS reader reaching line 128 gets:
error occurred in cc-rs: failed to find tool "x86_64-linux-gnu-gcc": No such file or directory
warning: build failed
The repo already has a host-agnostic alias that matches the gate exactly (.cargo/config.toml:71):
clippy-cli = "clippy -p trusted-server-cli --all-targets --all-features -- -D warnings"
I ran cargo clippy-cli on this host: clean, no --target needed.
| cargo clippy --package trusted-server-cli --all-targets --target x86_64-unknown-linux-gnu -- -D warnings | |
| cargo clippy-cli |
| /// Explicit source file; omitted fields/defaults and remote overrides remain unresolved. | ||
| #[arg(long, default_value = "trusted-server.toml")] | ||
| config: PathBuf, |
There was a problem hiding this comment.
⛏ nitpick — This default is not mentioned in any document, so a bare ts prebid server inspect reads the live operator-owned trusted-server.toml without the reader expecting it.
Every doc presents --config as something you supply: crates/trusted-server-cli/README.md:48 and references/configuration-and-secrets.md:27 both show inspect --config <path>, and none states what happens when it is omitted.
The behavior is harmless — inspect is read-only, preserves the source, and redacts account, endpoint and bid-parameter values, all of which is tested. The surprise is just that the command silently picks a file. Worth one clause where --config is first introduced, for example "omitting --config reads trusted-server.toml in the working directory".
No code change needed; the flag's own help text already carries the default, so this is a documentation gap only.
| #[serde(default)] | ||
| bidder: Vec<String>, | ||
| #[serde(default)] | ||
| user_id: Vec<String>, |
There was a problem hiding this comment.
⛏ nitpick — user_id as a plain Vec<String> collapses two states core treats as different.
Core declares it Option<Vec<String>> (crates/trusted-server-core/src/integrations/prebid.rs:513-515), where omission "selects the generator's curated preset" and [] selects none. docs/guide/cli.md documents that distinction explicitly. Here both render as identity_modules: [], so a reader cannot tell "no identity modules will ship" from "the curated preset will ship".
This is disclosed rather than misleading: the report emits "Omitted fields/defaults are not expanded" and the README says it reports explicitly supplied values only. Note the parity test normalizes the difference with .unwrap_or_default(), so it will not flag this.
If you want the report unambiguous, the existing *_explicit idiom already used for enabled_explicit and timeout_ms_explicit fits:
#[serde(default)]
user_id: Option<Vec<String>>,and report both identity_modules and an identity_modules_explicit boolean. Entirely optional — the current behavior is defensible for a tool whose stated contract is "explicit values only".
|
|
||
| | Path | Consumer | Local check | | ||
| | ---------------------------------------------------- | --------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | ||
| | `README.md` | Example user | Safe walkthrough and stop boundary review | |
There was a problem hiding this comment.
⛏ nitpick — Six committed files have no row here, so they have no named owner or check.
To be fair to the table, most apparent gaps are covered by its grouped rows (terraform.tf, providers.tf, variables.tf and main.tf, modules/regional/), which I checked before raising this. The genuinely unlisted tracked files are:
dns.tf,locals.tf,outputs.tftests/root_unit_test.tftest.hclterraform.tfvars.example.gitignore
Two of those matter more than the rest. outputs.tf renders the generated descriptor and bindings that the "Terraform-rendered generated descriptor and bindings" row depends on, and .gitignore is what keeps runtime/secret-bindings.generated.json out of Git — the mechanism behind the plan's own stop boundary. SKILL.md sets the bar as "every approved artifact exists, has a named owner and check", so these are in scope by the plan's own standard.
Folding them into the existing grouped rows would cover it, for example extending the Terraform row to terraform.tf, providers.tf, variables.tf, dns.tf, locals.tf, outputs.tf and adding one row for the root test file plus one for .gitignore and terraform.tfvars.example. No phantom rows in the other direction — every listed path exists.
|
|
||
| The `PBS example checks` GitHub Actions workflow runs Terraform formatting, both readonly-lock initializations, validation, and both mocked test suites. It also checks JSON, shell syntax, and Compose wiring. Changes to this example, `.tool-versions`, or the workflow trigger it. CI reads the Node.js version from `.tool-versions` and checks Docker Compose availability. The wiring command clears inherited Node.js preload options and module paths, and the script passes only `PATH` and `HOME` plus its explicit test inputs to subprocesses. It also parses `runtime/secret-bindings.example.json`. The CLI test suite separately loads and checks the committed deployment descriptor and its referenced inputs on every PR. The example owner must still run the descriptor command above when adapting inputs. | ||
|
|
||
| The Terraform tests use mocked AWS providers and explicit plan mode. The deployment descriptor and binding file contain fictional identifiers for local validation only. The wiring test renders Compose JSON and exercises the smoke script with fake lifecycle and health commands. It verifies the deployment all-interface binding, smoke-only loopback binding, and forced dummy input selectors without starting containers. |
There was a problem hiding this comment.
⛏ nitpick — "contain fictional identifiers" is true of every value here except one that is deliberately real.
The pinned prebid/prebid-server@sha256:f0fee9ca… is the genuine upstream v4.7.0 digest, and it must stay real: config.rs enforces binding.verified_image == pbs.image, so an operator who "replaces the fictional values" in both files consistently still ends up with an unpullable image reference, and one who replaces only one of them fails the equality check.
RUNBOOK.md already handles this well — line 11 enumerates exactly which inputs are fictional (account, profile, certificate, hosted-zone, AMI, CIDR) and pointedly excludes the digest, and line 13 says to confirm the v4.7.0 digest before release. Only this sentence is loose by comparison.
Narrowing it to match the RUNBOOK's phrasing would close the gap, for example: "contain fictional account, profile, secret and instance identifiers for local validation only; the pinned image digest is the real upstream v4.7.0 digest and should not be replaced."
This branch has not been deployed
5 participants
Summary
ts prebid: browser bundle generation ists prebid client, while self-hosted Prebid Server operations are underts prebid server.deploy/pbs-example/.The earlier post-merge inspection compatibility blocker is resolved at the current head. The CLI tests now pass against the current configuration schema.
Changes
.claude/skills/planning-prebid-aws/ts prebid server.crates/trusted-server-cli/src/commands/pbs/crates/trusted-server-cli/src/run.rsts prebid.crates/trusted-server-cli/tests/pbs_cli.rsand module testscrates/trusted-server-cli/README.mdandexamples/pbs/deploy/pbs-example/ts prebid bundlereferences withts prebid client..tool-versionsCLI layout
Client
This generates the publisher-specific browser bundle. It replaces the former
ts prebid bundlecommand.Server
ts prebid server inspect --config <file>ts prebid server check --deployment <file>ts prebid server secrets set <bidder> --deployment <file> --region <region>ts prebid server status --deployment <file>Add
--jsonanywhere underts prebid serverfor machine-readable output.Secret values stay out of process arguments and reports. AWS request payloads use owner-only temporary files on Unix and are removed on normal success and error paths. Raw AWS stderr is withheld. Abrupt termination can leave temporary files, so operators must use protected temporary storage. Windows ACL behavior has not been validated.
Example deployment
deploy/pbs-example/is a committed, locally checked example. It models:us-east-1andus-west-2.v4.7.0pinned to a verified image digest.The example uses fictional account, certificate, hosted-zone, AMI, instance, CIDR, and bidder values. It is not deployable as-is. It has no remote Terraform backend, WAF, runtime secret loader, deployment command, rollback command, or load-test evidence.
Boundaries
The deployment descriptor supports
ec2-composeonly. The planning skill may recommend ECS or another approved architecture, but that requires separate CLI support.There are no
deployorrollbackserver subcommands. This PR does not provision infrastructure, install a runtime secret loader, replace containers, adopt sandbox state, activate bidders, or change caller traffic. The example files document those deferred operations rather than presenting them as implemented.The planning skill now treats AWS RTB Fabric as an optional outbound path per bidder and region. It records partner participation and acceptance, PBS endpoint mapping, regional quotas and timeouts, cost, fallback, monitoring, and Terraform link lifecycle limitations. It does not provision gateways or links, automate partner acceptance, or replace the current EC2/Compose descriptor.
Live AWS operations still require explicit operator authorization. No real AWS calls or secret writes were made during implementation.
Open questions
ec2-compose, or should this skill stay focused on the currently supported Compose/EC2 path? ECS would improve managed task replacement and deployment behavior, but it adds a second runtime and CLI contract.ts prebid server statusdescriptor accepts explicit instance IDs, not ASG membership.Test plan
Current follow-up validation at
1d3d3107./scripts/test-cli.sh: 121 tests passed, including seven PBS process tests against a fake AWS executable.cargo fmt --all -- --checkcargo clippy --package trusted-server-cli --all-targets --target x86_64-unknown-linux-gnu -- -D warningsterraform fmt -check -recursive deploy/pbs-exampleterraform validatein the root and regional module using Terraform 1.16.2 and AWS provider 6.64.0.terraform -chdir=deploy/pbs-example test -filter=tests/root_unit_test.tftest.hcl, five passed.terraform -chdir=deploy/pbs-example/modules/regional test -filter=tests/security_unit_test.tftest.hcl, two passed.terraform providers lock -platform=darwin_arm64 -platform=darwin_amd64 -platform=linux_amd64 -platform=linux_arm64: generated four platform hashes from signed AWS 6.64.0 packages.crates/trusted-server-cli/examples/pbs/deployment.yamlanddeploy/pbs-example/deployment.example.yaml.runtime/secret-bindings.example.json, andbash -n deploy/pbs-example/scripts/smoke-runtime.sh.deploy/pbs-example/scripts/test-smoke-runtime.py: real Compose rendering with fake lifecycle/health commands; production binding remains all-interface, smoke binding is loopback, and inherited selectors cannot replace dummy inputs. No containers started.docs/.prettierrc; no CI-scope expansion.deploy/pbs-test/remains untracked and untouched.Terraform tests ran with empty AWS configuration and credentials, metadata access disabled, and every external provider mocked. The parent reran the combined CLI, formatting, lint, and infrastructure checks before committing.
Earlier implementation evidence
The earlier implementation ran JavaScript format, Vitest with 901 passing tests, and
node build-all.mjs, plus CLI help smoke checks. Those JS checks were not rerun for this follow-up, which changes no JS or adapter behavior. Current-head remote CI is separate from the local evidence above.Deferred evidence
Checklist
disable-model-invocation: true.unwrap()calls.Closes
Closes #1163