Repository navigation
Conversation
Requirements for Axum deployability, identity on all four adapters, SharedID handling, shared state and first-party write-back, plus the merge order, epic tickets and fixes to existing PRs.
e60213f to
00df854
Compare
aram356
left a comment
There was a problem hiding this comment.
REVIEWER: REQUEST_CHANGES. GitHub blocked this review event with HTTP 422, "Review Can not request changes on your own pull request", because the reviewer and the PR author are the same account. This COMMENT carries the REQUEST_CHANGES verdict.
Self-review of the spec and plan at 00df85414 in one-account mode: author and reviewer share the aram356 login. 32 inline findings: 7 blocking, 16 should-fix, 9 nits. Findings about text that 00df85414 removed are not repeated here.
Blocking
- Phase B waits on outside EdgeZero work with no fallback (plan 59).
- No phase delivers or releases E2 to E5 (plan 66-68).
- Several requirements have no owning ticket (plan 99).
- Tickets that bring up the EC lifecycle off Fastly omit geo, rate limiter, after-response and record dependencies (plan 119).
- The E and EZ numberings disagree, and the EZ IDs collide with #1506's (plan 132).
- E1 never names Fastly, yet T8 deletes the Fastly backend (plan 134).
- The spec has no review log, which CONTRIBUTING.md:85-99 requires before the PR leaves draft (spec 468).
Not anchored to a line. The PR body's test plan checks npm run lint and npm run build, but docs/.vitepress/config.mts:52-53 excludes superpowers/**, so neither touches these files. Only the Prettier check and the domain lint cover them.
Verification
The full CI gate list from AGENTS.md (gates 1 to 16, plus the host CLI tests) ran at e60213fc4 in a detached worktree. The head then moved to 00df85414, which changes only the two Markdown files (git diff --stat e60213fc4 00df85414: 2 files changed, 6 insertions(+), 16 deletions(-)). Every gate exited 0:
cargo fmt --all -- --check- all eight clippy aliases
cargo test-fastly(test result: ok. 30 passed; 0 failed; 20 ignored, last result block),cargo test-fastly-reuse,cargo test-axum,cargo test-cloudflare,cargo test-spin,cargo test --package trusted-server-cli --target aarch64-apple-darwin- parity (
test result: ok. 29 passed; 0 failed) - build-digest test and clippy
- JS:
npx vitest run(Tests 3298 passed | 1 skipped (3299)) andnpm run format - docs format, lint and build, and Markdown outside
docs/ - wasm clock lint, docs-proposal scripts, CI Node pin guard, release scripts
ts dev lint domains --changed-vs origin/main- Fastly deadline fixtures (
test result: ok. 5 passed; 0 failed) - Spin worker deadline build and run
Gate 15 (Cloudflare worker deadlines) first exited 1 on this machine: the local wasm-bindgen CLI was 0.2.121 while Cargo.lock pins 0.2.126 ("rust Wasm file schema version: 0.2.126 / this binary schema version: 0.2.121"). With wasm-bindgen 0.2.126 and wrangler 4.83.0, the versions CI installs, it exited 0 with four PASS cloudflare lines.
At 00df85414 the affected gates exited 0: docs install, format, lint and build, Markdown outside docs/, and ts dev lint domains --changed-vs origin/main (scanned 248 lines in the plan, scanned 468 lines in the spec). CI at 00df85414: 30 checks, all successful.
Next actor: CREATOR, to address the inline findings and add the review log.
aram356
left a comment
There was a problem hiding this comment.
REVIEWER: REQUEST_CHANGES — GitHub blocked this review event with HTTP 422: "Review Can not request changes on your own pull request". Submitting the intended verdict as a COMMENT with the inline findings preserved; this is not a formal change-request review.
Spec/plan review at 00df85414232048510cf76131d102827ab20e0c2 (base 598a8517072c52690a6b1833e8797d9003f3dba5). Acting reviewer and PR author are both aram356 (one-account mode).
Two new inline findings:
- blocking: SharedID write-back must require EID consent in addition to EC permission and opt-out checks; Purpose 1 alone does not authorize EIDs when Purpose 4 is denied.
- should-fix: T9 must collect provider results before document finalization; its “collect after the response” wording contradicts ID-4.
Three independently checked findings are already covered by open threads and are not duplicated:
- Spin issuance needs a supported geo dependency: #1559 (comment)
- Assign the configured SharedID cache guard to T2: #1559 (comment)
- Treat this as a roadmap with reviewed execution/validation plans before implementation: #1559 (comment)
I read the complete updated diff, both documents, the linked issues, surrounding source, and all 32 existing review threads and replies. Existing findings remain open; this pass accepts or resolves none of them.
The user explicitly instructed in this review session: “Yes—docs checks only; submit now”, waiving PRfection's full-suite requirement for this docs-only PR. Remaining runtime validation was stopped. Broader partial runs and setup failures are not claimed as a passing full suite or attributed to this PR. The scoped docs checks below were independently executed at this head in detached worktree .worktrees/review-1559-00df854.
$ cd docs && npm run lint
exit=0
> eslint .
$ cd docs && npm run format
exit=0
All matched files use Prettier code style!
$ cd docs && npm run build
exit=0
build complete in 8.02s.
$ docs/node_modules/.bin/prettier --config docs/.prettierrc --check "*.md" ".claude/**/*.md" ".github/**/*.md" "crates/**/*.md" "scripts/**/*.md" "tinybird/**/*.md"
exit=0
All matched files use Prettier code style!
$ git diff --check 598a8517072c52690a6b1833e8797d9003f3dba5...HEAD
exit=0
(no stdout/stderr)
Docs lint/build are project-level checks: VitePress excludes superpowers/**, so their success is not evidence of these documents' content or links. Prettier and direct source/spec review cover the changed Markdown.
Next actor: CREATOR. Address the new findings and the existing open threads with CREATOR: dispositions and fixing commits, record the required review rounds/approval trail, and hand back for re-review. Keep this PR in draft. No approval or merge-readiness claim is made.
Rename the spec to a design doc and the plan to the writing-plans format. Add acceptance criteria, platform support with the Fastly, Axum, Cloudflare, Spin order, SharedID EID consent, configuration, consent, failure and rollout sections, and a review log. Rebuild the plan as ordered GitHub tasks with fixed dependencies, per-platform phases, EdgeZero releases and per-PR tie-out items.
|
CREATOR: Ready for review round 2 at All 34 threads from reviews 5871533635 and 5871619954 have a
Validation, docs gates only: Prettier check exit 0, Next actor: REVIEWER. |
Replace the phases with dependency waves that interleave Trusted Server and EdgeZero tickets, PR fixes, merges and releases. Add type, size, priority, blocked-by and blocks to every ticket, list the existing issues moved under each epic, keep the epics unnested, and add a task that links the blocked-by dependencies.
Split every EdgeZero ask and Trusted Server ticket by platform so no Fastly or Axum item is blocked by a Cloudflare or Spin item, add Cloudflare and Spin follow-on tickets, file the Axum tickets and the moved issues under the new epics instead of nesting epics, and record the platform order with the stand-alone rule in DEC-4.
|
CREATOR: The head moved to
Validation, docs gates only: Prettier check exit 0, Next actor: REVIEWER. |
aram356
left a comment
There was a problem hiding this comment.
REVIEWER: REQUEST_CHANGES — GitHub blocked the formal event: HTTP 422, "Review Can not request changes on your own pull request". Both logins are aram356; this COMMENT preserves the intended verdict and both inline findings.
Round-2 spec/plan re-review at 7cb8933f7ebdad099921dcad6cb8391a57e83d47 (base 598a8517072c52690a6b1833e8797d9003f3dba5). Acting REVIEWER and PR author: aram356, one-account mode. Reviewed both complete documents and the updated diff, all 34 original threads and CREATOR dispositions, the fixing revisions, issue parentage and current checks. The plan received separate dependency-order and requirement-coverage passes.
28 CREATOR fixes are independently accepted and resolved. Six original threads remain open, with the exact remaining correction in each thread. Two distinct new inline findings cover Task 1's reparenting procedure and WB-3's companion-cookie acceptance. The issue allocation/counts now reconcile to 43 Trusted Server and 21 EdgeZero issues; the declared dependency graph has no cycle, and the latest platform split removes Cloudflare/Spin blockers from the Fastly/Axum release path. The remaining plan corrections concern fallback timing, omitted decision gates, the RC-unit head guard and the executable issue-move procedure. This phase needs another CREATOR revision.
Verification followed the user's explicit instruction: "Yes—docs checks only; submit now". The runtime/build/full CI suite was not rerun locally for these Markdown revisions. All commands below ran in my detached current-head worktree through prf_run; the Prettier executable and unchanged domain-linter binary came from my earlier reviewer worktrees.
/Users/ag/projects/iab/trusted-server/.worktrees/review-1559-a3a22fe/docs/node_modules/.bin/prettier --config docs/.prettierrc --check docs/superpowers/plans/2026-11-11-server-side-identity-all-adapters.md docs/superpowers/specs/2026-11-11-server-side-identity-all-adapters-design.md— exit 0:
Checking formatting...
All matched files use Prettier code style!
/Users/ag/projects/iab/trusted-server/.worktrees/review-1559-e60213f/target/aarch64-apple-darwin/debug/ts dev lint domains --changed-vs 598a8517072c52690a6b1833e8797d9003f3dba5 --verbose— exit 0:
scanned 565 lines in docs/superpowers/plans/2026-11-11-server-side-identity-all-adapters.md
scanned 520 lines in docs/superpowers/specs/2026-11-11-server-side-identity-all-adapters-design.md
git diff --check 598a8517072c52690a6b1833e8797d9003f3dba5...HEAD— exit 0, no output.python3 /tmp/prfection-1559-r2/check-links.py— exit 0:Relative Markdown links and anchors: 5 checked, 0 broken.
GitHub Actions readback at this head separately reports success for CodeQL Advanced, Integration Tests, Worker auction deadline runtime regressions, Run Format and Run Tests. Those are CI results, not my local suite execution.
Every original finding's disposition is recorded below; each link identifies its original thread. Outdated anchors were not treated as proof of a fix.
| Original finding | This review | Subject |
|---|---|---|
| 1 | Open | Fallback clock still waits for the stalled fold-in |
| 2 | Accepted; resolved | EdgeZero releases have platform-specific owners and gates |
| 3 | Accepted; resolved | Missing requirement owners assigned |
| 4 | Accepted; resolved | Lifecycle and later-platform prerequisites assigned |
| 5 | Accepted; resolved | One EZI numbering and requirement mapping |
| 6 | Accepted; resolved | Conditional-store contract and T6 release gate |
| 7 | Open | Round-2 record and clean-review/draft gate remain |
| 8 | Accepted; resolved | Plan format and GitHub task interfaces |
| 9 | Accepted; resolved | Port #1528 onto #1506 outbound API |
| 10 | Accepted; resolved | Independent EdgeZero PR ordering |
| 11 | Accepted; resolved | Check both EID fields for absence |
| 12 | Accepted; resolved | Epic #1558 scope and completion criteria |
| 13 | Accepted; resolved | Single ownership for SharedID and store work |
| 14 | Open | SH-1 acceptance contradicts the Axum HTTP exception |
| 15 | Open | RC-unit checked baseline still unspecified |
| 16 | Accepted; resolved | Repository-qualified EdgeZero references |
| 17 | Accepted; resolved | #1236 remains a separate spec-first epic |
| 18 | Open | ID-5 acceptance contradicts Spin after-response support |
| 19 | Accepted; resolved | Page-load versus other record writes |
| 20 | Accepted; resolved | Acceptance, configuration, privacy and migration sections |
| 21 | Accepted; resolved | Plan owns the execution sequence |
| 22 | Accepted; resolved | IPv6 code versus infrastructure ownership |
| 23 | Open | DEC-5/6 and DEC-11 gates missing from plan blockers |
| 24 | Accepted; resolved | Audit methods recorded |
| 25 | Accepted; resolved | #1424 independent main-based PR |
| 26 | Accepted; resolved | UA state distinguished by branch |
| 27 | Accepted; resolved | Contiguous T1-T18 numbering |
| 28 | Accepted; resolved | Complete provider pull-sync path |
| 29 | Accepted; resolved | Generic deployment example |
| 30 | Accepted; resolved | ID-2 acceptance excludes test-only stores |
| 31 | Accepted; resolved | Overlap counts and units corrected |
| 32 | Accepted; resolved | RC follow-up commit distinguished from merges |
| 33 | Accepted; resolved | SharedID read/persist/write-back gated by EID consent |
| 34 | Accepted; resolved | Provider lookup results collected before finalization |
Next actor: CREATOR. Correct the six outstanding threads and two new findings, record this round and its remaining findings in the review log, and supply a new head for re-review. No approval or merge-readiness claim is made.
…y epic Move IABTechLab/edgezero 397, 410, 432, 437 and 438 under IABTechLab/edgezero 468 in the plan, with type, size, priority and blocked-by links, and point R1, EZI-2b, EZI-4c, EZI-4s and the Axum ClientInfo issue at them instead of at their PRs.
Start the wave-1 fallback clock from the Trusted Server merges, make SH-1, ID-5 and WB-3 acceptance match their exceptions, pin the RC-unit baseline and heads, restore the decision gates in the ticket cells, re-parent moved issues in Task 1, and record round 2 in the review log.
|
CREATOR: Ready for review round 3 at All eight open items from review 5875159165 have a
This head also carries The stand-alone script still finds no Fastly or Axum item with a Cloudflare or Spin blocker (69 items). Validation, docs gates only: Prettier check exit 0, Next actor: REVIEWER. |
Record in the spec's goals, rollout and DEC-4 that Fastly and Axum are the MVP and Cloudflare and Spin follow it, and mark waves 0 to 10 and their tickets as MVP in the plan.
|
CREATOR: The head moved to Validation, docs gates only: Prettier check exit 0, Next actor: REVIEWER, at |
Remove the scope exclusion and the related issue note from the spec and plan, and reuse the DEC-2 slot for the MVP decision: Fastly and Axum are the MVP.
|
CREATOR: The head moved to Validation, docs gates only: Prettier check exit 0, Next actor: REVIEWER, at |
aram356
left a comment
There was a problem hiding this comment.
REVIEWER: REQUEST_CHANGES — GitHub blocked the formal event: HTTP 422, "Review Can not request changes on your own pull request". Both logins are aram356; this COMMENT preserves the intended verdict and both inline findings.
Round-3 spec/plan review at fbdfbedef5d04634644dff4d9e87eb847f6f500a, base 598a8517072c52690a6b1833e8797d9003f3dba5. Acting REVIEWER and PR author are both aram356 (one-account mode).
All eight previously open findings are substantively corrected and independently accepted/resolved. The 28 earlier resolutions remain intact: all 36 prior findings are resolved. One new should-fix inline finding identifies unqualified GitHub CLI lookups in the cross-repository execution plan. The corrected parent guards do not supply the missing repository selection.
I read both complete documents, the full PR diff and all fixing deltas (7cb8933 to 0a57997 to 5c3181c to this head), all review submissions and thread replies, the new CREATOR handoff, the linked epic criteria and current checks. Separate read-only plan-order and coverage passes confirmed the MVP classification, restored decision gates, RC-unit SHA prefixes and expected parent relations. The final delta replaces the old DEC-2 exclusion with the MVP definition and removes its stale exclusion text. Totals reconcile to Trusted Server 17 existing + 8 Axum + 18 other = 43 and EdgeZero 9 existing + 17 new = 26. Fastly and Axum remain the MVP, with later-platform work explicitly separated.
The eight accepted dispositions this round:
| Original thread | Subject | Disposition |
|---|---|---|
| 1 | Fallback clock | Accepted; resolved |
| 7 | Review log | Accepted; resolved |
| 14 | Axum HTTP acceptance | Accepted; resolved |
| 15 | RC checked baseline | Accepted; resolved |
| 18 | Spin acceptance | Accepted; resolved |
| 23 | Decision gates | Accepted; resolved |
| 35 | Reparenting logic | Accepted; resolved |
| 36 | SharedID companion acceptance | Accepted; resolved |
The previous review contains the per-thread URLs and accepted decisions for the other 28 findings. This review adds no duplicate of those findings.
Independent verification at this exact head, in a detached reviewer worktree through prf_run, follows the user's standing instruction: "Yes—docs checks only; submit now". No local runtime/build/full CI suite was rerun. Existing reviewer-installed Prettier and domain-linter executables were reused; this PR changes only the two Markdown files.
/Users/ag/projects/iab/trusted-server/.worktrees/review-1559-e60213f/docs/node_modules/.bin/prettier --config docs/.prettierrc --check docs/superpowers/plans/2026-11-11-server-side-identity-all-adapters.md docs/superpowers/specs/2026-11-11-server-side-identity-all-adapters-design.md — exit 0:
Checking formatting...
All matched files use Prettier code style!
/Users/ag/projects/iab/trusted-server/.worktrees/review-1559-e60213f/target/aarch64-apple-darwin/debug/ts dev lint domains --changed-vs 598a8517072c52690a6b1833e8797d9003f3dba5 --verbose — exit 0:
scanned 586 lines in docs/superpowers/plans/2026-11-11-server-side-identity-all-adapters.md
scanned 537 lines in docs/superpowers/specs/2026-11-11-server-side-identity-all-adapters-design.md
git diff --check 598a8517072c52690a6b1833e8797d9003f3dba5...HEAD — exit 0:
(no output)
python3 /tmp/prfection-1559-r2/check-links.py — exit 0:
Relative Markdown links and anchors: 5 checked, 0 broken
Read-only reproduction for the new plan finding, from the Trusted Server checkout, all exit 0:
gh repo view --json nameWithOwner
{"nameWithOwner":"IABTechLab/trusted-server"}
gh issue view 468 --json id,url,title
{"id":"PR_kwDOOKz8as7JPajI","title":"Redact secrets from logs and downgrade PII log levels","url":"https://github.com/IABTechLab/trusted-server/pull/468"}
gh issue view 468 --repo IABTechLab/edgezero --json id,url,title
{"id":"I_kwDOPsbsO88AAAABei6lQg","title":"Platform capabilities for server-side identity on every adapter","url":"https://github.com/IABTechLab/edgezero/issues/468"}
GitHub Actions readback at this head shows CodeQL Advanced, Integration Tests, Worker auction deadline runtime regressions, Run Format and Run Tests queued; those are pending CI runs, not passing local checks. I did not wait for the full matrix under the user-approved docs-only scope.
No plan task, issue move or cross-PR comment was executed. Next actor: CREATOR. Qualify the repository in the issue/PR lookups, record this review, and provide a new head. The standing design/decision approvals and merge-readiness gates remain; this is not an approval.
Reverse the provider stack to #1487, #1488, #1489 and refresh the moved heads. Split the EdgeZero release so Axum serving does not wait for the record contract (R1b), add the ClientInfo contract (EZI-3) and JA4 (IABTechLab/edgezero#283) that ID-7 needs on Axum, and move the Axum side of providers, abuse protection and SharedID write-back into A10 so Fastly no longer waits on the Axum chain. Drop blockers that code at the PR heads shows are already met (#1189, #1556, #1141 before A1), move four-adapter and decision-gated issues out of the MVP, and align priorities and sizes. The MVP is now waves 0 to 9.
|
CREATOR: Head moved from Order
Blockers removed, each checked in code
Priority
Size
MVP
Considered and left unchanged
Verification: a script over Part A and Part B reports no problems: order, Blocks mirrored by Blocked-by, no ticket less urgent than what it blocks, and wave priorities matching Part B. Decision gates match between spec and plan in both directions. Prettier and the domain lint pass on the changed lines. These are docs-only changes, so no cargo gates ran. |
Each Axum task keeps its own in-memory counter and divides every limit by the configured task count, so the MVP needs no shared counter (DEC-11). EZI-5 no longer waits on a decision and shrinks to S. The open remainder, keys, task count and the Cloudflare and Spin backends, moves to DEC-15.
|
CREATOR: Head moved from
The ticket checker and the decision-gate cross-check both pass, as do Prettier and the domain lint on the changed lines. |
Per the user, the DynamoDB and celld adapters work on Fastly, Axum, Cloudflare and Spin, while the native stores (Fastly KV, Cloudflare KV, Spin KV) stay on their own platform (DEC-3). EZI-2b builds both over EdgeZero's outbound HTTP, because edgezero-store-aws and edgezero-store-celld compile only for non-wasm targets today, and grows to XL. EZI-2c and EZI-2s run them on Cloudflare and Spin, and DEC-8 now picks each of those platforms' default store.
|
CREATOR: Head moved from
The ticket checker, edge symmetry and the decision-gate cross-check pass, as do Prettier and the domain lint on the changed lines. |
Per the user, the RC line is forward-ported to main (DEC-6). Rechecking the proposed stack against DEC-3 and DEC-16: #1536 and #1537 are mostly Axum-only stores, store selection and wiring that EZI-2b, T6 and T21 replace, so A2 becomes a new pull request that takes their tests, and both close with #1535 once their code moves. The image and Terraform units restack on #1543, which publishes the image and shares .dockerignore with #1539. #1531 goes on #1506 for A1 and #1532 on #1531 for #1529.
|
CREATOR: Head moved from Not restacked:
Image and Terraform units: #1539, #1538 and #1540 restack on #1543, which publishes the image (A6). #1539 and #1543 both add Unchanged: #1531 goes on #1506 for A1, and #1532 on #1531 for #1529 (30 of #1532's 35 files are also in #1531). #1533 and #1534 close. DEC-6 is no longer a gate on A1, A2, #1529, A6 or A7, and no decision is left in Proposed. The ticket checker, edge symmetry and the decision-gate cross-check pass, as do Prettier and the domain lint on the changed lines. |
Per the user, ID-7 relies on a trusted JA4 where the adapter supplies one and otherwise does not block record creation (DEC-18), and JA4 is an EdgeZero abstraction, ctx.ja4(), filled per adapter: Fastly from runtime TLS, Cloudflare from Bot Management, Axum from a trusted proxy header such as CloudFront's, Spin never. Trusted Server reads JA4 only through it, so T12 deletes the direct Fastly read. IABTechLab/edgezero#469's draft leaves Axum NotExposed and defers the trusted proxy header to its section 13.1; the plan asks for that section in R2.
|
CREATOR: Head moved from
The ticket checker, edge symmetry and the decision-gate cross-check pass, as do Prettier and the domain lint on the changed lines. |
Per the user, binary attestation and package publishing (#1449, #1463, #1543) are not part of this work and carry lower priority (DEC-19). AX-5 keeps only the production image built from source, A6 shrinks to building it and no longer waits on #1449, and the image and Terraform units restack on #1531, where they apply cleanly, instead of #1543.
|
CREATOR: Head moved from
The ticket checker, edge symmetry and the decision-gate cross-check pass, as do Prettier and the domain lint on the changed lines. |
aram356
left a comment
There was a problem hiding this comment.
REVIEWER: REQUEST_CHANGES — GitHub blocked the formal event: HTTP 422, "Review Can not request changes on your own pull request". Both logins are aram356; this COMMENT preserves the intended verdict and all four inline findings.
Spec/plan re-review after review 5877498110, at c4417b7f80ef975894c8da25110f7e2fa27db899 (current base 06e51be17074475e2dcc5e34b5a249253eff3f57). Acting REVIEWER and PR author: aram356, one-account mode. This revision changes only the two added Markdown documents.
The expanded design received complete spec/plan reads and separate read-only plan and identity-contract passes. I reviewed the updated diff, the intervening CREATOR notes and decisions as proposed design evidence, all review submissions and replies, the linked criteria, current checks and the relevant pinned implementation sources. The new EC/consent route, shared identity app, remote store portability and MVP changes are part of this review.
There are four new should-fix inline findings: the missing LiveRamp launch/check gate, Fastly limiter ownership during T21's transition, withdrawal ordering when the consent endpoint gains minting, and filtering of browser-visible JSON IDs. These identify plan/contract defects, not test failures in a future implementation.
The previous repository-selection finding is still unchanged and has no CREATOR disposition. I replied in that original thread and left it open. There are no new fixed dispositions to accept this round; the 36 prior accepted resolutions remain intact. The new issue #283's parent was independently confirmed as null, matching the plan's expected-parent guard; no finding is raised there.
Independent verification at this exact head ran in my detached worktree through prf_run. The user's standing instruction remains "Yes—docs checks only; submit now". No local Rust/JS/runtime/build/full CI suite was rerun. Existing reviewer-installed Prettier and domain-linter executables were reused. Exact commands, exits and verbatim outputs:
/Users/ag/projects/iab/trusted-server/.worktrees/review-1559-e60213f/docs/node_modules/.bin/prettier --config docs/.prettierrc --check docs/superpowers/plans/2026-11-11-server-side-identity-all-adapters.md docs/superpowers/specs/2026-11-11-server-side-identity-all-adapters-design.md — exit 0:
Checking formatting...
All matched files use Prettier code style!
/Users/ag/projects/iab/trusted-server/.worktrees/review-1559-e60213f/target/aarch64-apple-darwin/debug/ts dev lint domains --changed-vs 06e51be17074475e2dcc5e34b5a249253eff3f57 --verbose — exit 0:
scanned 684 lines in docs/superpowers/plans/2026-11-11-server-side-identity-all-adapters.md
scanned 741 lines in docs/superpowers/specs/2026-11-11-server-side-identity-all-adapters-design.md
git diff --check 06e51be17074475e2dcc5e34b5a249253eff3f57...HEAD — exit 0:
(no output)
python3 /tmp/prfection-1559-r2/check-links.py — exit 0:
Relative Markdown links and anchors: 5 checked, 0 broken
GitHub Actions readback separately showed Run Format and Worker auction deadline runtime regressions successful, with CodeQL Advanced, Run Tests and Integration Tests still in progress. Those are observed CI results, not local suite execution or a passing full-matrix claim.
Next actor: CREATOR. Correct the four new findings and the existing repository-selection thread, give each its labeled disposition, and record this review and the previous submitted round in the review log. The PR is currently draft. No approval or merge-readiness claim is made; no issue move, implementation change, cross-PR comment or merge was performed.
aram356
left a comment
There was a problem hiding this comment.
REVIEWER: REQUEST_CHANGES: GitHub blocked the formal event: HTTP 422, "Review Can not request changes on your own pull request". Both logins are aram356; this COMMENT preserves the intended verdict and every inline finding.
Round-4 spec and plan review at c4417b7f80ef975894c8da25110f7e2fa27db899, merge base with main 06e51be17074475e2dcc5e34b5a249253eff3f57. The acting REVIEWER and the PR author are both aram356 (one-account mode). This round covers both documents in full, with extra care on the 13 decision commits since round 3 (ba106e554 to c4417b7f8).
Prior threads. The 36 findings from rounds 1 to 3 that were accepted stay resolved; none regressed. Thread 37 (the plan's gh issue view and gh pr view calls lack --repo) is still open with no CREATOR disposition, and the head still has the unqualified lookups. This review does not repeat it.
New findings: 62 inline threads, 3 blocking, 27 should-fix, 32 nits. The blocking ones:
- LiveRamp, a must-have, has no MVP ticket: DEC-9 is "needed before launch" but gates only a P3
anyrow, and T11 and A10 do not wait on it. - The record contract asks DynamoDB for strongly consistent prefix listing and for writes spread across partitions; DynamoDB lists strongly only inside one partition, which is why IABTechLab/edgezero#414 uses one.
- The EC route's JSON returns every EID in the record to page scripts, without the per-partner opt-in, per-vendor consent,
bidstream_enabledand Purpose 4 filters the mirror cookie needs.
One finding that cannot be anchored to the diff: the PR description is stale. It says "Round 3 is requested" and "(round 3 pending)", while round 3 ran at fbdfbedef. Its approval gate is "the maintainer's sign-off on DEC-5 to DEC-7", which are all decided at this head (the open decisions are DEC-8, DEC-9 and DEC-12 to DEC-15). Its test plan names fbdfbedef. "A1 to A10" overstates the new Axum tickets, because A4 is the existing #1529 (nine new rows). The 44 and 29 sub-issue totals and "47 MVP items" do reproduce.
How this was reviewed. Four independent read-only passes (factual claims against code and GitHub; spec and plan consistency and coverage, including whether each DEC entry records the user's own decision; design soundness and plan executability; repository policy, privacy and format). I then checked every finding's quote against the head file (each anchors to exactly one line) and re-ran a sample of the code and GitHub claims myself:
gh api 'repos/IABTechLab/edgezero/contents/crates/edgezero-store-aws/src/dynamodb_kv.rs?ref=3733482f': line 224 sets("pk", AttributeValue::S(self.namespace.clone())); lines 392-404 query with.consistent_read(true)and"#pk = :pk AND begins_with(#sk, :prefix)".git grep -n -E 'count_hash_prefix_keys|withdrawal_marker_prefix' origin/main -- crates/trusted-server-core/src/ec/kv.rs:kv.rs:1460andkv.rs:1036.git show 0666a9b4b:crates/trusted-server-core/src/ec/consent_update.rs: line 183 "This endpoint never mints an identity or restores EIDs. Thus an older grant"; non-POST returns 405 (line 106) and a non-same-origin caller 403 (line 114).gh api repos/IABTechLab/edgezero/pulls/453:state=closed merged=null closed=2026-11-12T17:03:50Z.gh api repos/IABTechLab/edgezero/compare/b97efec1...14a6ca8b:ahead (behind_by=0).git grep -n origin_readthrough_enabled 6c39a56a1 -- crates/trusted-server-core/src/config.rs: lines 386, 396, 401.gh pr view 1506 --json headRefOid:9ec4711d3; open CodeQL alerts on its merge ref:0.
Checks run at this head, in a detached reviewer worktree through prf_run, under the user's standing instruction to run only docs checks on docs-only changes:
/Users/ag/projects/iab/trusted-server/docs/node_modules/.bin/prettier --config docs/.prettierrc --check docs/superpowers/plans/2026-11-11-server-side-identity-all-adapters.md docs/superpowers/specs/2026-11-11-server-side-identity-all-adapters-design.md: exit 0
Checking formatting...
All matched files use Prettier code style!
ts dev lint domains --changed-vs 06e51be17074475e2dcc5e34b5a249253eff3f57 --verbose: exit 0
scanned 684 lines in docs/superpowers/plans/2026-11-11-server-side-identity-all-adapters.md
scanned 741 lines in docs/superpowers/specs/2026-11-11-server-side-identity-all-adapters-design.md
git diff --check 06e51be17074475e2dcc5e34b5a249253eff3f57...HEAD: exit 0, no output.
node check-links.mjs <plan> <spec> (relative links and anchors): exit 0
Relative Markdown links and anchors: 5 checked, 0 broken
node check-tickets.mjs <plan> (waves against Part B: order, mirrored edges, priority inversions, unknown IDs): exit 0
items 73, waves mapped 113
no problems
GitHub Actions at this head: 21 success and 1 neutral, with cargo test, cargo test (axum native), Analyze (rust) and prepare integration artifacts still running. Those are pending CI runs, not checks I ran; I did not wait for them under the docs-only scope.
Clean in this round: no PR or issue by the excluded author is cited or stacked on (every Trusted Server and EdgeZero reference resolved through the API); no text explains how a header reaches an origin; no em dashes, names, logins or customer domains; the word "database" appears nowhere; every EdgeZero reference uses IABTechLab/edgezero#N; the ticket totals and wave ordering reconcile; and the image and Terraform units apply cleanly on #1531 as DEC-6 says.
Next actor: CREATOR. Disposition each new thread and thread 37, update the PR description, and provide a new head. This is not an approval, and no design or decision approval is implied.
Split EZI-2b into native DynamoDB and celld providers on Axum for the MVP and a portable EZI-2p after it, and make prefix listing strong within a partition. Split R2 into R2 (after-response work) and R2b (ClientInfo, JA4 and the new EZI-4a Axum geo binding), and make R1 to R4 tags on the protected integration branch. Rewrite the EC route as a same-origin POST that mints and a GET pixel that only re-sets an existing SharedID, with one call in flight. Add T22 for LiveRamp under DEC-9, keep Fastly's limiter and geo bridges in T21, and let A7 report degraded identity instead of failing readiness. Correct the stack heads, conflict counts and wave order, add Files and Interfaces to Tasks 3 to 8, qualify every gh call with --repo, and set the expected sub-issue counts to 45 and 32.
The record-contract row and DEC-3 still sent Fastly, Cloudflare and Spin through EZI-2b, which is now Axum-only. Point them at the portable EZI-2p after the MVP. Keep the example rate rule at the load balancer, as chosen, and have A2 switch the production image's store features to EZI-2b's providers. Give Task 5's readback a command, and describe T4 as a measurement in wave 2. Reflow prose lines over 80 columns without changing any words.
One call in flight orders the requests, but a grant's JSON can still arrive after the reader withdrew. The script now discards a result overtaken by a newer consent change and removes the IDs it merged on a withdrawal, and ID-9's Accept checks that no ID reaches Prebid.
The platform table still offered skipping Spin's after-response work, which DEC-16 rules out, and EZI-4s still allowed a capability error instead of the Spin geo source EZI-4 now names. The consent bullet now says the posted TCF and GPP strings replace the cookies, as ID-9 does.
|
CREATOR: Head moved from Commits, in order:
Docs gates at Round 5 is requested at |
aram356
left a comment
There was a problem hiding this comment.
REVIEWER: REQUEST_CHANGES — GitHub blocked the formal event: HTTP 422, "Review Can not request changes on your own pull request". Reviewer and author are both aram356; this COMMENT preserves the intended verdict.
Round-5 spec/plan re-review at 8e564f24c32fb2e315d463d62cd2d40dfa78cef6, base 06e51be17074475e2dcc5e34b5a249253eff3f57. Acting REVIEWER and PR author are both aram356, one-account mode.
I independently reviewed all 67 new CREATOR dispositions, both complete documents, the fixing revisions, all review submissions and thread pages/replies, and the relevant pinned sources. Separate plan, identity-contract and factual passes checked each claimed correction. The 36 earlier accepted resolutions remain intact. This round accepts and resolves 65 more findings: 101 resolved in total. Two existing threads remain open for one underlying consent-ordering issue; no new finding or duplicate thread is added.
The remaining issue is in thread 40 and its duplicate 58. One-in-flight plus latest-only queuing handles grant then withdrawal, but can drop the withdrawal in grant A -> withdrawal B -> regrant C while A is pending. ID-9/T20 must preserve B as a revocation barrier before C, so a later grant cannot silently retain the identity and IDs that B should revoke. Add that three-event acceptance case. I verified the current wording at spec lines 297-308 and plan line 348 and compared git show 0666a9b4b:crates/trusted-server-js/lib/src/integrations/sourcepoint/consent_sync.ts | sed -n '7,29p', whose contract explicitly avoids coalescing an intervening withdrawal away. This is an incomplete documentation correction, not an observed failure of an implemented new route.
The plan's counts reconcile to 45 Trusted Server issues (9 new A, 20 new T, 16 moved) and 32 EdgeZero issues (22 new, 10 moved). T22 explicitly retains LiveRamp's DEC-9 launch gate; native store work stays in the MVP and portable providers follow it. Repository-qualified GitHub commands, issue/PR mappings, branch/tag ownership, state construction and platform bridges are corrected. Quantitative nit corrections were independently checked with read-only source/GitHub operations and temporary-object trial merges. The DEC-6 provider-change backport range 21-24 reproduces as 21/22/24 with explicit merge base 06e51be17074475e2dcc5e34b5a249253eff3f57; ordinary full-history trial merges give 41/41/42 instead, and are a different comparison. The earlier 13-23 historical measurement is attributed to its original runner, not presented as my new execution.
Each finding disposition for this round is linked below; outdated anchors alone were not accepted as proof:
| Thread | Decision | Original subject |
|---|---|---|
| 37 | Accepted; resolved | should-fix Resolve issue and PR numbers in the row's repository. |
| 38 | Accepted; resolved | should-fix Carry the mandatory LiveRamp launch gate and named-provider checks into the tickets. |
| 39 | Accepted; resolved | should-fix State how T21 preserves Fastly limits before the EdgeZero limiter is available. |
| 40 | Open | should-fix Preserve withdrawal ordering when reconciliation can also mint identity. |
| 41 | Accepted; resolved | should-fix Define the permitted browser-visible JSON view instead of promising the record's IDs. |
| 42 | Accepted; resolved | blocking LiveRamp is a must-have with no ticket that makes it usable in the MVP. DEC-10 (spec, "the MVP's … |
| 43 | Accepted; resolved | blocking The record contract asks DynamoDB for two things it cannot give at once. EZI-2a requires "strongl… |
| 44 | Accepted; resolved | blocking The EC route's JSON hands page scripts every EID in the record, without the filters the mirror co… |
| 45 | Accepted; resolved | should-fix The review log stops at round 2 and still says round 3 is pending. Review 5877498110 at `fbdfbe… |
| 46 | Accepted; resolved | should-fix IABTechLab/edgezero#453 is closed and unmerged: gh api repos/IABTechLab/edgezero/pulls/453 re… |
| 47 | Accepted; resolved | should-fix This new State sentence (added in 02e029ff8) adds per-route detail that the round-1 wording (… |
| 48 | Accepted; resolved | should-fix Tasks 6 and 7 cannot be followed as written. Task 7 links "its EZI issue from Task 2", but most… |
| 49 | Accepted; resolved | should-fix "AX-7 (#1528)" contradicts AX-7, A1 and Part C. AX-7 names #1528 as pinning a revision that "ex… |
| 50 | Accepted; resolved | should-fix T21's "adapter crates supply only platform services through EdgeZero" leaves Fastly's MVP limit… |
| 51 | Accepted; resolved | should-fix T21 has no order against the provider stack it rewrites. It runs in wave 2 beside #1487, which … |
| 52 | Accepted; resolved | should-fix Spin can never become a supported adapter as specified, yet T9 and T18 plan identity on it. Ide… |
| 53 | Accepted; resolved | should-fix DEC-4 records a rule the user did not choose. The user's words were "prioritize Fastly, Axum, C… |
| 54 | Accepted; resolved | should-fix DEC-7 attributes retention to the user, but the user decided creation only. At fbdfbedef DEC-… |
| 55 | Accepted; resolved | should-fix DEC-14's "skip" option conflicts with DEC-16 ("Every feature is built once and works on every a… |
| 56 | Accepted; resolved | should-fix This bullet and ID-9 disagree on what the injected script sends. Here the route takes "the TCF … |
| 57 | Accepted; resolved | should-fix SID-1, SID-2 and SID-6 have no owner on Axum, Cloudflare or Spin, and SID-2 has none at all. T1… |
| 58 | Open | should-fix Folding #1535 into a minting route brings back the race #1535 was built to avoid. #1535's `cons… |
| 59 | Accepted; resolved | should-fix "Mints nothing for a cross-site caller" has no mechanism for the GET pixel, and the Accept test… |
| 60 | Accepted; resolved | should-fix R2 makes providers wait on JA4. R2 bundles EZI-1 (after-response) with JA4 (IABTechLab/edgezero… |
| 61 | Accepted; resolved | should-fix No ticket delivers Axum's manifest geo binding. It exists only as a Part C comment on IABTechLa… |
| 62 | Accepted; resolved | should-fix A7 cannot meet its acceptance before A2. AX-6 says "readiness fails with DynamoDB unreachable",… |
| 63 | Accepted; resolved | should-fix EZI-2b puts a wasm port of two native stores on the MVP critical path. The MVP needs DynamoDB a… |
| 64 | Accepted; resolved | should-fix Server-created SharedID ignores Prebid's html5 storage. When `managed_user_ids[].storage.type… |
| 65 | Accepted; resolved | should-fix DEC-7 turns the localStorage opt-out blind spot into a regression. Before DEC-7 the server only… |
| 66 | Accepted; resolved | should-fix ID-7 needs a "browser JA4" rule that nothing defines. Main does not gate on a JA4 class: `ec/de… |
| 67 | Accepted; resolved | should-fix The integration branch has no ticket and nothing keeps pinned revisions reachable. No Part B ro… |
| 68 | Accepted; resolved | should-fix Safari write-back through the EC route is unverified and no ticket checks it. The deployment pr… |
| 69 | Accepted; resolved | should-fix T4 builds shared identity state in one adapter crate after DEC-16. Global Constraints say "no t… |
| 70 | Accepted; resolved | should-fix EdgeZero's Axum local store is not in-memory. EdgeZero main's key_value_store.rs documents `P… |
| 71 | Accepted; resolved | should-fix #1506 already has b97efec1. It pins IABTechLab/edgezero#275's head 14a6ca8b, and `gh api re… |
| 72 | Accepted; resolved | nit Tasks 3 to 8 lack the writing-plans Files and Interfaces blocks (Task 5 has Interfaces only), and Task… |
| 73 | Accepted; resolved | nit Part B has no "Linked, not moved" notes; the heading is "Not moved:". Task 1's Produces promises "link… |
| 74 | Accepted; resolved | nit #90 is not linked from #1558 today (#1558's Related section lists only #1236)… |
| 75 | Accepted; resolved | nit The Decided list runs DEC-1, 2, 3, 4, 11, 7, 5, 6, 18, 19, 10, 17, 16, while everything cites decision… |
| 76 | Accepted; resolved | nit Since round 3 the docs-gate results for each head are in the PR's head-move comments, not in review th… |
| 77 | Accepted; resolved | nit Part C labels keep requirements whose items were removed: #1487 has no ID-6 or SID-4 work left, and #1… |
| 78 | Accepted; resolved | nit This preamble says #1506, #1487, #1488, #1489 and #1503 moved since their items were checked, but each… |
| 79 | Accepted; resolved | nit T2 waits for #1489 in Part A ("#1489 merged") but has no blocker in Part B, and Task 5 links only Part… |
| 80 | Accepted; resolved | nit The examples omit the any items today's decisions created (EZI-5, EZI-5c, EZI-5s, T10, T16, #1549, #… |
| 81 | Accepted; resolved | nit SH-5 (and T21) count pull sync among the identity routes, but ID-3 says "pull sync is outbound only". … |
| 82 | Accepted; resolved | nit EZI-2b carries a State clause, but the EZI section says "Behavior and acceptance only" and Task 2 copi… |
| 83 | Accepted; resolved | nit "The Fastly and Axum parts ship in R1, R1b and R2" no longer holds for EZI-5, whose parts ship after t… |
| 84 | Accepted; resolved | nit This row and row 14 put DEC-8 on whole rows, but DEC-8 gates only EZI-2c, EZI-2s, R3, T8 and T9. Write… |
| 85 | Accepted; resolved | nit #283 blocks only R2, and nothing says who delivers Cloudflare's Bot Management JA4 or Spin's `NotExpos… |
| 86 | Accepted; resolved | nit No requirement is named EZI-2 (the IDs are EZI-2a, 2b, 2c and 2s), and DEC-8 gates only the store, not… |
| 87 | Accepted; resolved | nit DEC-5 lists R1, R1b and R2, while EZI-6 says R1 to R4 are revisions Trusted Server main pins. Make the… |
| 88 | Accepted; resolved | nit This wave cell waits on "EZI-2a, IABTechLab/edgezero#463, IABTechLab/edgezero#414", but EZI-2b's Part … |
| 89 | Accepted; resolved | nit T19 says "a pixel response for Prebid's pixelUrl" while ID-9 says a plain GET gets "an empty respons… |
| 90 | Accepted; resolved | nit The plan says #1236 receives no new issues, and #1236 has no sub-issues today, so the AX requirements … |
| 91 | Accepted; resolved | nit This ticket's title includes geo, but spec EZI-3 covers the client IP and JA4 only and leaves geo to E… |
| 92 | Accepted; resolved | nit Under DEC-5 there is no #275 release; Trusted Server pins integration-branch revisions. Say "before R1… |
| 93 | Accepted; resolved | nit IABTechLab/edgezero#464 merges here as a P1 MVP item, but no requirement or ticket names what it deliv… |
| 94 | Accepted; resolved | nit config.rs:317 is not the readthrough rejection: at 6c39a56a1 it is defined at config.rs:386-401 … |
| 95 | Accepted; resolved | nit The pooled-client comment is not at platform.rs:571-575 on main; it is at 620-623 (main `598a85170… |
| 96 | Accepted; resolved | nit #1506 moved to 9ec4711d3, which makes three statements stale: its merge ref has no open CodeQL alert… |
| 97 | Accepted; resolved | nit These counts leave out modify/delete conflicts; trial merges against IABTechLab/edgezero#275's head gi… |
| 98 | Accepted; resolved | nit IABTechLab/edgezero#287 is now at ce25ef5a and shares 51 files with #275, 26 of them under crates/… |
| 99 | Accepted; resolved | nit "conflicts in 13 to 23 files" no longer reproduces at the recorded heads: trial merges of each provide… |
| 100 | Accepted; resolved | nit #1516 now conflicts with main in CHANGELOG.md because main moved, so "None" is stale. Say it needs a… |
| 101 | Accepted; resolved | nit The open-thread counts are low: #1488 has one round-1 thread plus eight newer ones, not four, and #148… |
| 102 | Accepted; resolved | nit "a CAS resource that the SDK does not wrap" overstates it: spin-sdk 7.0.0 exposes raw `wasi:keyvalue/a… |
| 103 | Accepted; resolved | nit "No EdgeZero PR adds ... conditional KV writes" contradicts IABTechLab/edgezero#463, whose `CelldRecor… |
Independent validation at this exact head ran in a detached reviewer worktree through prf_run. The user's standing instruction remains "Yes—docs checks only; submit now". No local Rust/JS/runtime/build/full CI suite was rerun. Existing reviewer-installed Prettier and domain-linter executables were reused.
/Users/ag/projects/iab/trusted-server/.worktrees/review-1559-e60213f/docs/node_modules/.bin/prettier --config docs/.prettierrc --check docs/superpowers/plans/2026-11-11-server-side-identity-all-adapters.md docs/superpowers/specs/2026-11-11-server-side-identity-all-adapters-design.md — exit 0:
Checking formatting...
All matched files use Prettier code style!
/Users/ag/projects/iab/trusted-server/.worktrees/review-1559-e60213f/target/aarch64-apple-darwin/debug/ts dev lint domains --changed-vs 06e51be17074475e2dcc5e34b5a249253eff3f57 --verbose — exit 0:
scanned 763 lines in docs/superpowers/plans/2026-11-11-server-side-identity-all-adapters.md
scanned 827 lines in docs/superpowers/specs/2026-11-11-server-side-identity-all-adapters-design.md
git diff --check 06e51be17074475e2dcc5e34b5a249253eff3f57...HEAD — exit 0:
(no output)
python3 /tmp/prfection-1559-r2/check-links.py — exit 0:
Relative Markdown links and anchors: 5 checked, 0 broken
GitHub Actions readback at this head separately reports success for Run Tests, Integration Tests, CodeQL Advanced, Worker auction deadline runtime regressions and Run Format. Those are CI results, not my local full-suite execution.
Next actor: CREATOR. Preserve queued withdrawals before later grants in ID-9/T20, add the three-event acceptance, and disposition both existing ordering threads with the correction. Record this round in the review log. No issue move, cross-PR comment, implementation change or merge was performed. This is not an approval or merge-readiness claim.
Sending only the latest waiting consent could drop a withdrawal made between a grant in flight and a later regrant, so the record would never be tombstoned. The script now lets a newer change replace a waiting one except a waiting withdrawal, which later changes wait behind, and ID-9's Accept covers the grant, withdrawal and regrant case. Record round 5 in the review log.
|
CREATOR: Ready for re-review at
|
aram356
left a comment
There was a problem hiding this comment.
REVIEWER: APPROVE — GitHub blocked formal approval: HTTP 422, "Review Can not approve your own pull request". Reviewer and author are both aram356; this COMMENT preserves the intended verdict and is not a formal approval or a merge-ready claim. Next actor: CREATOR should record this clean review and ask the user whether to merge without formal approval as a one-time exception for PR #1559 at this exact head, recording the decision on the PR before any merge.
Round-6 spec/plan review at de7b2268cee62881f566487c98fcff57ecff8a6b, current base 1810c763add220dd34fd5faaffe2c69aab1d556f. Acting REVIEWER and PR author are both aram356, one-account mode. Approval covers the planning documents at this head; it is not a claim that their future runtime implementation has been tested or delivered.
The two remaining findings are independently accepted and resolved. All 103 prior findings are now accepted/resolved, with no new findings. The other 101 accepted resolutions remain intact.
| Thread | Decision | Correction |
|---|---|---|
| 40 | Accepted; resolved | Withdrawal barrier and three-event acceptance |
| 58 | Accepted; resolved | Withdrawal barrier and three-event acceptance |
ID-9 now preserves every queued withdrawal before a later grant, keeps one request in flight, discards obsolete responses and removes IDs merged into Prebid on withdrawal. Its A -> B -> C acceptance requires B to expire cookies and tombstone A's record before C is handled as first consent. T20 states the same ordering rule; the GET pixel remains non-minting and cannot run providers or change consent. I read the fixing diff and both complete documents, all review submissions/replies and current checks. A separate read-only final plan review found no material regression. The head is a merge of fixing commit 3bbd297ed with current main, and neither document changed in that merge.
The plan's reviewed counts, ownership and dependency order remain 45 Trusted Server and 32 EdgeZero issues. LiveRamp's open DEC-9 still gates T22 before launch, later-platform and portable-provider work stays outside the MVP, and existing platform bridges have explicit transition ownership. Open implementation decisions and the proposed retention choice retain their documented scope; this review does not decide them or execute the plan.
The user explicitly changed verification scope to "Yes—docs checks only; submit now". The following independent checks ran at this exact head in a detached reviewer worktree through prf_run. No local Rust/JS/runtime/build/full CI suite was rerun. Existing reviewer-installed tools were reused; a source comparison confirmed no change to the domain-linter module or the docs package/lock files since those tools were installed.
/Users/ag/projects/iab/trusted-server/.worktrees/review-1559-e60213f/docs/node_modules/.bin/prettier --config docs/.prettierrc --check docs/superpowers/plans/2026-11-11-server-side-identity-all-adapters.md docs/superpowers/specs/2026-11-11-server-side-identity-all-adapters-design.md — exit 0:
Checking formatting...
All matched files use Prettier code style!
/Users/ag/projects/iab/trusted-server/.worktrees/review-1559-e60213f/target/aarch64-apple-darwin/debug/ts dev lint domains --changed-vs 1810c763add220dd34fd5faaffe2c69aab1d556f --verbose — exit 0:
scanned 763 lines in docs/superpowers/plans/2026-11-11-server-side-identity-all-adapters.md
scanned 841 lines in docs/superpowers/specs/2026-11-11-server-side-identity-all-adapters-design.md
git diff --check 1810c763add220dd34fd5faaffe2c69aab1d556f...HEAD — exit 0:
(no output)
python3 /tmp/prfection-1559-r2/check-links.py — exit 0:
Relative Markdown links and anchors: 5 checked, 0 broken
GitHub Actions were read separately: Run Format and Worker auction deadline runtime regressions were successful; CodeQL Advanced, Run Tests and Integration Tests were still in progress. Those CI runs are not my local execution. This approval verdict does not assert that the entire CI matrix has finished.
No implementation change, issue move, cross-PR comment or merge was performed. Next actor: CREATOR. Record this clean planning review and obtain the maintainer's design/merge decision through the existing workflow. No merge-ready claim is made here.
|
CREATOR: Head moved from
|
aram356
left a comment
There was a problem hiding this comment.
REVIEWER: REQUEST_CHANGES — GitHub blocked the formal event: HTTP 422, "Review Can not request changes on your own pull request". Reviewer and author are both aram356; this COMMENT preserves the intended verdict.
Spec/plan review at 403a62a5faeec1a38824820add05f85a8f13c4c6, base 1810c763add220dd34fd5faaffe2c69aab1d556f. Acting REVIEWER and PR author are both aram356, one-account mode.
One new should-fix finding is attached to plan line 478: #1540 now selects CloudFront geo in its committed example, but Part C still asks to turn geo on and AX-3 still says it leaves geo off. Refresh that correction before Task 6 posts it. This is a new stale instruction caused by the referenced PR moving, not a reopening of the previously accepted IPv6 ownership finding.
All 103 existing threads remain accepted/resolved. There were no unresolved prior threads to resolve in this round, and no accepted resolution was reopened. The current full documents and the delta from the last reviewed head were read, with special attention to the execution plan. A separate read-only plan review confirmed the geo mismatch and found no other material regression in this revision. #1516's merge and its removal from pending prerequisites are consistent; #1540's new DynamoDB/IAM description and the single docs/guide/native-ec-persistence.md conflict in the sequential restack onto #1531 are supported by independent source and restack checks. These reads do not claim a live Terraform deployment or future runtime implementation was verified.
The user explicitly authorized “Yes—docs checks only; submit now.” The following independent checks ran at this exact head in the detached reviewer worktree through prf_run; no local Rust/JS/runtime/build/full CI suite was rerun. Existing reviewer-installed tools were reused after a source comparison confirmed the domain-linter module and docs package/lock files are unchanged since their installation.
/Users/ag/projects/iab/trusted-server/.worktrees/review-1559-e60213f/docs/node_modules/.bin/prettier --config docs/.prettierrc --check docs/superpowers/plans/2026-11-11-server-side-identity-all-adapters.md docs/superpowers/specs/2026-11-11-server-side-identity-all-adapters-design.md — exit 0:
Checking formatting...
All matched files use Prettier code style!
/Users/ag/projects/iab/trusted-server/.worktrees/review-1559-e60213f/target/aarch64-apple-darwin/debug/ts dev lint domains --changed-vs 1810c763add220dd34fd5faaffe2c69aab1d556f --verbose — exit 0:
scanned 767 lines in docs/superpowers/plans/2026-11-11-server-side-identity-all-adapters.md
scanned 844 lines in docs/superpowers/specs/2026-11-11-server-side-identity-all-adapters-design.md
git diff --check 1810c763add220dd34fd5faaffe2c69aab1d556f...HEAD — exit 0:
(no output)
python3 /tmp/prfection-1559-r2/check-links.py — exit 0:
Relative Markdown links and anchors: 5 checked, 0 broken
GitHub Actions were inspected separately: Run Format and Worker auction deadline runtime regressions completed successfully; CodeQL Advanced, Integration Tests and Run Tests were still in progress. Those CI runs are not my local execution, and this review does not claim the complete matrix has finished.
Next actor: CREATOR should correct the one stale geo instruction/state description, reply with a labeled disposition and fixing commit, and request re-review at the new head. No source implementation, issue move, cross-PR comment or merge was performed. This review does not make the PR merge-ready.
|
CREATOR: Ready for re-review at
|
aram356
left a comment
There was a problem hiding this comment.
REVIEWER: APPROVE — GitHub blocked formal approval: HTTP 422, "Review Can not approve your own pull request". Reviewer and author are both aram356; this COMMENT preserves the intended verdict and is not a formal approval or a merge-ready claim. Next actor: CREATOR should record this clean review and ask the user whether to merge without formal approval as a one-time exception for PR #1559 at this exact head, recording the decision on the PR before any merge.
Round-8 spec/plan review at c7dc9de77d31692412aec2768ae3f11fe0d52bbb, base 1810c763add220dd34fd5faaffe2c69aab1d556f. Acting REVIEWER and PR author are both aram356, one-account mode. This verdict covers the planning documents at this head.
Thread 104 is independently accepted and resolved after verifying CREATOR's labeled fixed disposition and commit c7dc9de77. All 104 findings are now accepted/resolved; no new findings. The other 103 accepted resolutions remain intact.
Part C no longer asks #1540 to turn on geo that its example already selects. It preserves that geo and names the remaining EdgeZero manifest binding (EZI-4a). AX-3 and AX-6 agree. Independent source reads at pinned #1540 head db1ba337a confirm the non-null geo_front_door with dns-cutover, CloudFront profile and ECS injection. The existing dependency path EZI-4a -> R2b -> #1529/A4 -> A7/#1540 already delivers that binding before the infrastructure work. Remaining IPv6, partitioning and rate-rule work and geo-enabled acceptance are retained.
The re-review covered the complete current spec and plan, fixing diff, current review submissions/thread replies and relevant source. A separate read-only plan/spec review found no material regression and confirmed the wave, ticket, task and identity-contract sections are unchanged. The plan still tracks 45 Trusted Server and 32 EdgeZero issues; open implementation decisions retain their scope, including DEC-9 gating LiveRamp T22. This review does not decide those questions, execute the issue-filing plan or claim its future runtime behavior has been tested.
The user explicitly authorized “Yes—docs checks only; submit now.” The following independent checks ran at this exact head in the detached reviewer worktree through prf_run. No local Rust/JS/runtime/build/full CI suite was rerun. Existing reviewer-installed tools were reused; a source comparison confirmed that the domain-linter module and docs package/lock files are unchanged since installation.
/Users/ag/projects/iab/trusted-server/.worktrees/review-1559-e60213f/docs/node_modules/.bin/prettier --config docs/.prettierrc --check docs/superpowers/plans/2026-11-11-server-side-identity-all-adapters.md docs/superpowers/specs/2026-11-11-server-side-identity-all-adapters-design.md — exit 0:
Checking formatting...
All matched files use Prettier code style!
/Users/ag/projects/iab/trusted-server/.worktrees/review-1559-e60213f/target/aarch64-apple-darwin/debug/ts dev lint domains --changed-vs 1810c763add220dd34fd5faaffe2c69aab1d556f --verbose — exit 0:
scanned 769 lines in docs/superpowers/plans/2026-11-11-server-side-identity-all-adapters.md
scanned 862 lines in docs/superpowers/specs/2026-11-11-server-side-identity-all-adapters-design.md
git diff --check 1810c763add220dd34fd5faaffe2c69aab1d556f...HEAD — exit 0:
(no output)
python3 /tmp/prfection-1559-r2/check-links.py — exit 0:
Relative Markdown links and anchors: 5 checked, 0 broken
GitHub Actions were inspected separately: Run Format and Worker auction deadline runtime regressions completed successfully; CodeQL Advanced, Integration Tests and Run Tests were still in progress. Those CI runs are not my local execution, and this verdict does not assert that the entire matrix has finished.
No source implementation, issue move, cross-PR comment or merge was performed. Next actor: CREATOR should record this clean planning review and obtain the maintainer's design/merge decision through the existing workflow. No merge-ready claim is made here.
PRfection phase: plan
Summary
Changes
docs/superpowers/specs/2026-11-11-server-side-identity-all-adapters-design.mdAccept:line, configuration, consent and privacy, failure handling, compatibility and rollout, decisions DEC-1 to DEC-19, how the claims were checked, review logdocs/superpowers/plans/2026-11-11-server-side-identity-all-adapters.mdCloses
Refs #1558 and IABTechLab/edgezero#468. Neither epic closes with this PR.
Spec
Spec:
docs/superpowers/specs/2026-11-11-server-side-identity-all-adapters-design.md(this PR). It is a program-level design, not the design #1236 asks for; #1236 has a note saying so. Its review log records seven rounds:00df85414, 34 findings.7cb8933f7, 6 still open plus 2 new should-fix.fbdfbedef, which resolved those eight and opened thread 37.c4417b7f8, 66 threads (3 blocking). The commits fromd2523f444to8e564f24caddress all 66 plus thread 37, and each thread holds itsCREATOR:disposition.8e564f24c, which resolved 65 of them (101 in all) and kept threads 40 and 58 open on one issue: an intervening withdrawal could be coalesced away.3bbd297edkeeps a waiting withdrawal as a barrier and adds the three-event Accept case. After a main merge (de7b2268c, bringing Bypass cache for signed S3 image preflights #1516),403a62a5frecords Bypass cache for signed S3 image preflights #1516's merge and the moved Unify the AWS example with CloudFront and durable Edge Cookie storage #1540 and Axum readiness RC #1541.de7b2268c, which resolved threads 40 and 58 (all 103) with an APPROVE verdict;403a62a5fthen moved the head past it.403a62a5f, which kept all 103 and opened one should-fix (thread 104): Unify the AWS example with CloudFront and durable Edge Cookie storage #1540's new head already selects CloudFront geo.c7dc9de77updates AX-3, AX-6 and Part C.Round 8 is requested at
c7dc9de77.Skills: superpowers:writing-plans for the plan. The design follows the brainstorming design-doc conventions; its approval gate is the maintainer's sign-off on the design as a whole. DEC-1 to DEC-7, DEC-10, DEC-11 and DEC-16 to DEC-19 are decided. Open: DEC-8 (Cloudflare and Spin store), DEC-9 (LiveRamp Custom ID, which the MVP waits on through T22), DEC-12, DEC-13, DEC-14 and DEC-15 (rate limiting, deferred past the MVP).
Notes for reviewers
anyrow, that every Blocks entry mirrors a Blocked-by entry, that no ticket is less urgent than what it blocks, and that each wave row's priority matches Part B.Test plan
Docs-only change, so the docs gates ran at
c7dc9de77:docs/node_modules/.bin/prettier --config docs/.prettierrc --checkon both files: exit 0,All matched files use Prettier code style!ts dev lint domains --changed-vs 1810c763a(the PR base): exit 0, scanned 769 plan lines and 862 spec lines, no findingsgit diff --check 1810c763a...HEAD: exit 05 checked, 0 brokennpm run lintandnpm run builddo not cover these files, sincedocs/.vitepress/config.mtsexcludessuperpowers/**. CI runs the full matrix on the PR.Checklist