feat(web): admin-core consumer migration — web canary (Unit 5)#915
Merged
Conversation
Plan for unit 5 of feat-104 admin-core consumer migration. Scoped to two modes: strapi (default) + dual-read. Admin-mode rendering deferred to U5b. Built via /ce-plan + /ce-doc-review with 5 reviewer personas. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the dual-read parity canary's mode flag scaffolding: - FORGE_CONTENT_API server env var with two values: strapi (default) + dual-read. U5b adds admin-with-fallback / admin later. - ADMIN_GRAPHQL_URL server env var with warn-only host allowlist (jesusfilm.org / railway.app / local / localhost), mirroring NEXT_PUBLIC_CANONICAL_ORIGIN's refine shape. - content-api-mode.ts: ContentApiMode union, getContentApiMode() reading from env at module scope (ISR-safe — no headers/cookies), normalizeContentApiMode() defensive layer for unknown inputs. - Top-of-file deletion checklist cross-referencing the U4 bridge and the harness's own checklist at packages/graphql/src/parity/ index.ts. - 12 unit tests covering happy paths, edge cases, and the module-cache-vs-mutate semantic. Plan: docs/plans/2026-05-08-001-feat-consumer-migration-web-canary-unit-5-plan.md Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stands up the server-side admin GraphQL surface for U5's dual-read canary: - admin-client.ts: anonymous Apollo singleton mirroring client.ts. AbortSignal.timeout(3000) constructed inside the fetch override per-call (NOT module scope — would share one signal across all calls and fire 3s after process boot). - fragments/admin-experience.ts: GetAdminExperienceBySlug operation via adminGraphql() factory. Selects the 10 ExperienceLocale fields the parity bridge consumes, including blocks (JSON scalar — shape validation lands at normalizeAdmin via BlocksSchema). - Re-export from fragments/index.ts. - 2 unit tests: singleton check + per-call AbortSignal foot-gun guard. Plan: docs/plans/2026-05-08-001-feat-consumer-migration-web-canary-unit-5-plan.md Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements the consumer-side bridge between U2's admin/Strapi clients
and the U4 parity harness in @forge/graphql/parity:
- runDualReadComparison(outcome) takes a tagged-union per-side fetch
outcome (ok/error/timeout) and routes to one of five log events:
diff, admin_timeout, harness_error (with subkind), both_failed,
strapi_failed_admin_succeeded (the U5b advance gating signal).
- Input adaptation at the boundary, not by changing harness:
* Strapi: synthesize locale from urlLocale if response lacks it
* Admin: remap metaDescription → description (admin schema field
name vs harness's AdminExperienceLocaleInput field name)
- R13 enforcement: production log payload carries ONLY counts per
channel + RFC6901 JSON-Pointer paths + timings. Raw ValueDiff and
SemanticDiff strapi/admin field values are STRIPPED — content
fields (titles, descriptions, URLs) must not reach Vercel/Railway
log search and bypass CMS access control.
- Dev-only opt-in: FORGE_PARITY_DEBUG=1 includes diffSamples with raw
values for first 3 diffs. Production strips unconditionally.
- 14 tests covering happy path, locale synth, metaDescription remap,
R13 enforcement (negative+positive), all 5 events, all 4 harness
error subkinds, JSON-parseability, and event-name union pinning.
Plan: docs/plans/2026-05-08-001-feat-consumer-migration-web-canary-unit-5-plan.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires U2's admin client and U4's parity bridge into content.ts: - New internal helper fetchSlugExperience(locale, slug) called only from resolveSlugPage's slug-equality case at line 376. The homepage path (resolveHomepage) and legacy-homepage call still use getExperienceByFilters directly — out of U5 scope. - strapi mode: identical to current behavior (calls getExperienceByFilters with the slug filter). - dual-read mode: Promise.all over both fetches (each timed via performance.now). Hands the orchestrated outcome to the parity bridge's runDualReadComparison. Returns Strapi to the user; admin failures or timeouts never affect user-facing render. - AbortError / TimeoutError on the admin side classified as the bridge's "timeout" outcome; other errors classified as "error" so the bridge can emit the right log event. - watchExperienceFragment now selects `locale` so normalizeStrapi has the field it requires (bridge synthesis remains as defense-in-depth). - typeof window guard on env reads in admin-client.ts and content-api-mode.ts so server-only env vars don't throw via t3-oss/env-nextjs's Proxy when these modules are transitively imported by client components (mirrors client.ts pattern). - Test infra: vitest.setup.ts + .env.ci + .env.example get ADMIN_GRAPHQL_URL placeholders. content.test.ts gains parallel mocks for admin-client, content-api-mode, parity-bridge. - 10 new test scenarios cover: strapi-mode no-admin-touch, dual-read happy path, admin throws (typed ApolloError shape), admin AbortError → timeout classification, Strapi-fails-admin-OK gating signal, both-fail propagation, resolveSlugPage shape stability across modes, the 5-mode regression snapshot, dual-read with admin shadow, and the WatchExperience-fragment-has-locale pin. Plan: docs/plans/2026-05-08-001-feat-consumer-migration-web-canary-unit-5-plan.md Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…Operation Adds the U5 admin-side query to the canonical consumer-migration inventory so future graphql() callsite sweeps catch it without drift. Plan: docs/plans/2026-05-08-001-feat-consumer-migration-web-canary-unit-5-plan.md Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Defense-in-depth on the U5 host allowlist. Admin's auth host (PR #909) returns 404 on /api/* by design — pointing ADMIN_GRAPHQL_URL there would silently emit forge.parity.harness_error events on every dual-read request instead of failing at boot. Mirrors the rejection pattern in packages/graphql/src/parity/live-config.ts:24 (REJECTED_HOSTS). Surface: if ADMIN_GRAPHQL_URL hostname is in the reject set, z.url().refine() throws at boot with a clear message pointing at admin.jesusfilm.org as the correct host. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
🚅 Deployed to the forge-pr-915 environment in forge
5 services not affected by this PR
|
Three simplification fixes from /simplify pass: 1. content.ts: drop the `as DualReadOutcome["admin"]` cast and the double `isAbortTimeoutError` call in fetchAdminSlugExperience by restructuring to direct branches with an `elapsed()` helper. Behavior identical; type narrowing now exact. 2. content.ts: drop the message-substring fallback in isAbortTimeoutError. Per CLAUDE.md (AWS NoSuchKey classification): never branch on the error MESSAGE — match error.name first. The substring fallback would misclassify any GraphQL error mentioning "timeout" as forge.parity.admin_timeout, polluting the canary's gating signal. 3. env.ts: extract `softHostAllowlistRefine(varName, exacts, suffixes)` helper used by both ADMIN_GRAPHQL_URL (server) and NEXT_PUBLIC_CANONICAL_ORIGIN (client). Same shape, different lists. ~25 LOC saved; behavior identical (still warn-only). Plus minor: extract `PARITY_ROUTE = "[slug]" as const` in parity-bridge.ts (7 literal sites consolidated) and inline the single-element STRAPI_ALIAS_MODES array into a flat test case in content.test.ts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves 7 actionable findings from /ce-code-review (12 reviewers, autofix mode, plan-explicit). Each fix corroborated by 2+ reviewers. P1 — boot-fail risk: env.ts FORGE_CONTENT_API z.enum widened from [strapi, dual-read] to all four origin-R7 values. An operator pre-setting "admin" or "admin-with-fallback" no longer bricks boot. content-api-mode.ts now wires normalizeContentApiMode (dead code → load-bearing) — U5b values coerce to "strapi" with a warn until U5b ships admin-mode rendering. P1 — log content-leak risk (R13 defense-in-depth): the parity bridge's FORGE_PARITY_DEBUG check now requires BOTH the explicit flag AND NODE_ENV !== "production". A production typo of FORGE_PARITY_DEBUG=1 becomes a no-op. Also registered FORGE_PARITY_DEBUG in env.ts schema for boot-time visibility. P1 — bridge sync-throw risk: content.ts wraps runDualReadComparison in try/catch. If the bridge throws (circular ref, throwing toString, JSON.stringify failure on BigInt), the user still gets Strapi — forge.parity.canary_failed event logs the error. P2 — admin_missing event: split the comparator_unknown bucket so admin returning null while Strapi has data (typical during backfill) emits forge.parity.admin_missing distinctly. Real comparator failures stay in harness_error/comparator_unknown. Restores R-18a gating-signal fidelity. P2 — Apollo networkError walk: isAbortTimeoutError extended to walk error.networkError (Apollo Client v4 surfaces transport errors there) and its cause chain. Real timeouts no longer misclassify as harness_error/ admin_fetch_error. Extracted hasTimeoutOrAbortName helper. P2 — test coverage: 11 new tests covering Apollo result.error path (distinct from rejection), TimeoutError name, networkError walk, networkError.cause chain walk, U5b values coerced with warn, R13 defense-in-depth holds when both flags set, the new admin_missing event, both-null and Strapi-null-admin-OK edge cases, and bridge sync-throw recovery emits forge.parity.canary_failed. PARITY_LOG_EVENTS now pins 7 events (was 5): admin_missing and canary_failed added. Plan: docs/plans/2026-05-08-001-feat-consumer-migration-web-canary-unit-5-plan.md Run artifact: /tmp/compound-engineering/ce-code-review/20260508-160116-a106c03a/ Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implementation shipped, /ce-code-review applied, 307/307 tests pass. Plan retires; ongoing operator tasks (flipping FORGE_CONTENT_API in production envs, monitoring forge.parity.* log events, advancing through R-18a thresholds) belong to runbook + U5b. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Round-2 ce-code-review verified all round-1 findings resolved and
surfaced 9 new findings. 4 safe_auto + 5 gated_auto/manual applied; 1
manual deferred (admin_missing subkind discrimination needs admin-side
cooperation).
P2 fixes:
- canary_failed log payload now carries `timings: {strapiMs, adminMs}`,
matching the ParityLogPayload contract every other parity event uses.
Operator dashboards filtering on timings.* no longer hit undefined.
- FORGE_CONTENT_API enum is now whitespace+case-tolerant via
z.preprocess(trim+lowercase) → enum, so "DUAL-READ" or "dual-read \n"
(common operator-typo failure modes) coerce cleanly instead of
bricking boot. The runtime narrower in content-api-mode.ts handles
unknown values; this fix removes the gap where the schema rejected
before the narrower could run.
P3 fixes:
- parity-bridge.ts now reads FORGE_PARITY_DEBUG via the typed env (not
process.env), so the registered z.enum schema is actually load-bearing
at the runtime read site instead of cosmetic boot-time validation.
- admin-client.ts timeoutFetch now combines a caller-supplied init.signal
with the 3s timeout via AbortSignal.any() (Node 20.3+), preserving
external cancellation. Falls back to timeout-only on older runtimes.
- Deletion-checklist comment in parity-bridge.ts updated from "five
parity log event names" to seven (admin_missing + canary_failed
added in round 1 weren't propagated to the deletion list).
- .env.example documents FORGE_PARITY_DEBUG with allowed values "0"|"1",
the production-safety note, and the retire-with-U5 marker.
- normalizeContentApiMode admin-with-fallback test now asserts the warn
message body (was call-count-only).
- 5 internal types in parity-bridge.ts (ParityLogEvent, HarnessErrorSubkind,
SideOutcome, StrapiExperienceResponse, AdminExperienceResponse,
ParityLogPayload) dropped from the export surface — zero non-test
consumers and the deletion contract is cleaner with a smaller
export footprint.
Manual fix added as comment-only (no concrete shape):
- fetchStrapiSlugExperience now warns future maintainers in a docblock
that adding strapi-side timeout classification requires updating the
bridge's branch table (currently has a defensive narrowing return for
the unreachable case).
Manual deferred:
- admin_missing event commingles backfill gap / Pothos shield-plugin
null / replication lag without subkind discrimination. Requires
admin-side response classification to fix; deferred to U5b operator
triage protocol.
Run artifact: /tmp/compound-engineering/ce-code-review/round-2/
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Railway's @forge/web deploy was failing because ADMIN_GRAPHQL_URL is a
NEW required env var that the service hadn't been configured with. The
default mode (FORGE_CONTENT_API=strapi) never invokes the admin client
— it's byte-identical to current main — so requiring a new env var just
to deploy was an unnecessary regression.
Fix: make ADMIN_GRAPHQL_URL optional in env.ts. The Zod refines (auth-
host hard-reject + soft-allowlist warn) still run when a value IS
provided, but absence no longer bricks boot. admin-client.ts coalesces
to empty string so the Apollo HttpLink construction stays the same.
Operational behavior:
- FORGE_CONTENT_API=strapi (default) + ADMIN_GRAPHQL_URL unset:
admin client never invoked. Identical to main.
- FORGE_CONTENT_API=dual-read + ADMIN_GRAPHQL_URL unset:
admin queries fail with non-URL fetch error → caught by
fetchAdminSlugExperience → emits forge.parity.harness_error subkind
admin_fetch_error. Operator notices in logs and configures the var.
This is the round-2 reliability finding rel-r2-1 ("ADMIN_GRAPHQL_URL is
required without .optional() or .default(); boot fails when unset, even
in default strapi mode where admin-client is never invoked"). I rated it
P2 in round 2 but skipped applying — that was the wrong call. This PR's
Railway deploy proved it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…er-2 review discipline Captures two high-yield learnings from PR #915's review and recovery cycle: 1. **Opt-in scaffolding env vars must be `.optional()`** (docs/solutions/runtime-errors/required-env-var-without-default-broke-railway-deploy-20260511.md) ADMIN_GRAPHQL_URL was specced as required z.url() with no default. The U5 plan correctly anticipated the failure mode AND wrote the mitigation ("deploy env var to all environments BEFORE PR merge"), but the mitigation was treated as a routine documentation note and the boot-fail-fast became a deploy block. The fix moves the precondition from operator deploy-checklist discipline into the schema: required vars are reserved for code paths the default mode consumes; opt-in scaffolding vars must be optional so default mode has zero new env-var prerequisites. Includes prevention rules, a vi.stubEnv test pattern that would catch the same trap, and the "reliability persona presumption-of-correctness" decision rule for env-var findings flagged at P2+ confidence 75+. 2. **Tier-2 ce-code-review is mandatory before push** (docs/solutions/workflow-issues/ce-code-review-tier-2-mandatory-before-push-20260511.md) Knowledge-track learning: when /ce-work Phase 2 ends with tests green, the instinct to push is the signal that shipping-workflow.md is being skipped. Tier-2 triggers (>=400 LOC + multi-dir, >=1000 LOC, or sensitive surface) make multi-persona review non-optional. Captures the full shipping checklist (simplify, tier-1 vs tier-2 escalation, Residual Work Gate, Operational Validation Plan) + the per-finding routing table from Stage 5 step 6b + a counter-example (PR #902 ran it correctly). Plus CLAUDE.md "Known Patterns" entries pointing at both docs so future agents see them at session start. Both docs cross-link to each other — the Railway boot-fail is a concrete instance of the broader Tier-2-before-push discipline. Related: docs/solutions/auth/better-auth-secret-must-not-fallback-to-hardcoded-value.md, docs/solutions/developer-experience/env-matrix-drift-from-runtime-requirements-20260421.md, docs/solutions/platform/admin-manager-enrichment-trigger-endpoint-20260506.md, docs/solutions/best-practices/review-fix-round-2-sibling-call-site-regressions-20260421.md Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ur-imazing
added a commit
that referenced
this pull request
May 11, 2026
…attern Compound capture of the architectural pattern shipped in this PR: - Three-layer auth coordination (resolver authScopes + service guard + permission matrix) with three explicit widening modes - Paired authScopes + unauthorizedResolver:()=>null for field-level strip without populating response.errors[] (load-bearing for the U5 parity comparator on PR #915) - Service-mediated public→abac bridge via t.prismaField on objectRef - Centralized public-resolvers regression test as substitute for admin-schema-drift CI's blindness to authScopes changes - Smoke-script-as-vitest-substitute for full-pipeline assertions Cross-links pothos-relation-abac-filter-required-for-nested-types.md (sibling) and dual-client-gql-tada-multi-schema-codegen-pattern- 20260507.md (predecessor — explains why drift CI is auth-blind). Captured via /ce-compound with 4 parallel research subagents plus session-history search across 5 prior sessions.
Ur-imazing
added a commit
that referenced
this pull request
May 11, 2026
* docs(plans): u2 admin PUBLIC widenings plan
Plan for Unit 2 of the consumer-side Strapi → admin migration:
widen videoBySlug/video/videos to PUBLIC + Video.locales publish
filter, field-level strip on Experience/ExperienceLocale (with
nullability flip + unauthorizedResolver), reference data widening,
new watchSetting query matching the apps/web consumer shape.
Built via /ce-plan with two rounds of /ce-doc-review applied.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(admin): u1 widen video reads to PUBLIC + Video.locales publish filter
- videoBySlug, video(id), videos: authScopes flipped from
hasPermission:read:videos to public:true. PUBLIC consumer apps
(web/mobile/tv) can now read anonymously.
- Video.locales relation gains principal-aware query callback
mirroring Experience.locales — anonymous and VIEWER callers see
only PUBLISHED locales, EDITOR/ADMIN see all.
- video.service.ts: drop hasPermission(read:videos) guards on
list/getById/getBySlug. Resolver authScopes is the single auth
contract for these three. getByCoreId keeps its guard (Core
sync internal, not GraphQL-exposed).
- video.service.test.ts: flip PUBLIC=>Forbidden assertions to
PUBLIC=>resolution for list/getById. Add explicit
PUBLIC=>Forbidden test for getByCoreId pinning the
Core-sync-only contract.
Part of consumer-migration Unit 2.
* feat(admin): u2 field-level PUBLIC strip on Experience + ExperienceLocale
- Experience.{isTemplate, ownerId, archivedAt, createdAt, updatedAt}:
add authScopes:{hasPermission:read:experiences} + unauthorizedResolver:()=>null.
Anonymous callers receive null; EDITOR/ADMIN see real values.
- ExperienceLocale.{isHomepage, createdAt, updatedAt}: same strip.
isHomepage added (analogous to Experience.isTemplate — editorial-state
flag that anonymous callers shouldn't be able to enumerate).
- unauthorizedResolver overrides Pothos's default-throw so the U5 parity
comparator does not see errors[] entries for stripped-field selections.
SDL impact: zero on Experience/ExperienceLocale signatures (Pothos builder
defaults to nullable fields in this codebase; all strip candidates were
already nullable in the SDL). The schema.graphql diff captures the
U1 description prose updates (Video queries and Video.locales) that were
not regenerated in the U1 commit.
Part of consumer-migration Unit 2.
* feat(admin): u3 widen reference data to PUBLIC + add public-resolvers regression test
- languages, countries, keywords: flip authScopes from loggedIn:true
to public:true. Brings the resolver layer in line with the matrix
(read:reference is already PUBLIC at permissions.ts:83). Consumer
homepage language pickers + search filters depend on these.
- New: apps/admin/src/graphql/public-resolvers.regression.test.ts
walks every t.prismaField/t.field declaration under
src/graphql/{types,queries}/ and asserts the manifest of intended-
PUBLIC resolvers (experienceBySlug, searchExperiences, search,
sceneRecommendations, video, videoBySlug, videos, languages,
countries, keywords) carries authScopes: public:true. Also asserts
no PUBLIC resolver outside the manifest — accidental widenings
fail CI.
- classification.test.ts: add file-existence assertion for the
regression test as one-way meta-defense. Deleting either fails
the other.
- SDL impact: zero (authScopes directives stripped pre-commit).
Part of consumer-migration Unit 2.
* feat(admin): u4 watchSetting public query + service
- New WatchSettingService at apps/admin/src/services/watch-setting.service.ts.
Two Prisma reads, both gated by status:PUBLISHED + archivedAt:null
for anonymous-safe content:
* homepage: ExperienceLocale where isHomepage=true + locale matches
* template: Experience where isTemplate=true, joined to its locale row
Multi-row tiebreak orderBy updatedAt desc + structured warning log.
Strict-null locale fallback (matches Strapi v5 singleType+i18n default).
- New Query.watchSetting(locale: String!): WatchSetting resolver with
authScopes:public:true. Returns documentId +
homepageExperience: ExperienceLocale +
defaultTemplateExperience: ExperienceLocale, matching the shape
apps/web/src/lib/content.ts:48-63 consumes from Strapi today.
- Consumer-side coordination: apps/web's WatchExperience fragment is
currently 'on Experience' against Strapi; when the homepage cuts
over to admin (U5b/U6), it must rewrite to 'on ExperienceLocale'.
Documented in the resolver and service comments.
- WatchSetting is a builder.objectRef (not a prismaObject) and its
inner fields use t.prismaField. The classification.test.ts walker
inspects prismaObject + t.relation, so this service-mediated bridge
is invisible to it by construction. Service is the gate.
- public-resolvers.regression manifest extended with 'watchSetting'.
- SDL: additive new type + query. admin-graphql-env.d.ts regenerated.
Part of consumer-migration Unit 2.
* refactor(admin): simplify u2 per simplify-skill review
- Export isEditorOrAdmin(user) from src/auth/principal.ts. Replaces 4
inline duplicates (Video.locales + Experience.locales relation
callbacks). Above the extract-on-third-use threshold.
- Extract STRIPPED_FOR_PUBLIC constant in experience.ts. Collapses 8
call sites of the {nullable, authScopes, unauthorizedResolver}
triplet to spreads. Renaming the permission key now requires one
edit, not eight. Load-bearing comment about Pothos default-throw
lives on the constant.
- WatchSettingService.get: parallelize the homepage + template Prisma
reads via Promise.all. Halves wall-clock latency on the consumer
homepage hot path. No data dependency between the two.
- Promote WatchSettingShape to an exported type from the service.
Consumed by the Pothos objectRef so the two shape declarations
cannot silently diverge.
- Add a comment on findTemplateLocale explaining the asymmetry with
findHomepageLocale (the isTemplate flag lives on the parent
Experience, not the locale; multi-row warning shape deferred).
- classification.test.ts: move existsSync import to top-of-file with
the other node:fs imports.
No behavior change. SDL byte-identical. 1678 admin tests pass.
* refactor(admin): apply ce-code-review autofix findings
Tier-2 review (10 reviewers) surfaced 5 safe_auto + 14 residual findings.
Auto-applied:
- watch-setting.service.ts: multi-row tiebreak log was misleading
because take:2 caps matches.length at 2. Replaced count with
count_min + capped_at_take so operators see the bounded-anomaly
semantics, not a false-exact count. Test assertion updated.
- watch-setting.service.ts: clarifying comment on documentId
derivation — corrects a Round 2 correctness reviewer's
false-positive concern about template-id leakage by documenting
that findTemplateLocale already returns the locale row, not the
parent.
- public-resolvers.regression.test.ts: SOURCE_DIRS now also scans
src/graphql/mutations/. Today every mutation is hasPermission-
gated, so the manifest-exhaustiveness check stays green; the
expansion catches a future accidental PUBLIC mutation.
- public-resolvers.regression.test.ts: unit-attribution comments
on the manifest were ambiguous (U1/U3/U4 vs the brief's U2 unit).
Cleared up — all video/reference/watchSetting widenings are part
of this single PR's commit phases, not separate brief units.
- experience.service.ts + experience.search.ts: consolidate the
two pre-existing isPrivileged duplicates onto the shared
isEditorOrAdmin helper from auth/principal.ts (finishing the
prior simplify pass that only addressed the U2-introduced
duplicates).
Residual findings (P1/P2) surfaced for owner review:
- A1 (P1): Pothos default-allow for new resolvers without authScopes
- AC1/AC2 (P1): WatchExperience fragment cutover risk at U5b/U6
- T1-T5 (P1/P2): pipeline-level test gaps (vitest transitive-graphql
double-instance issue blocks full-pipeline tests in admin today)
- A3 (P2): Video.dubs publish filter not added
- S1 (P2): Video parent itself has no publish filter
Run artifact at /tmp/compound-engineering/ce-code-review/20260511-114055-34ac3303/
* test(admin): close P1/P2 review-loop test gaps + drop dead user params
Resolving the highest-value residuals from the autofix-mode code review:
Test gaps closed:
- apps/admin/src/auth/principal.test.ts (new): isEditorOrAdmin
predicate covered for null/PUBLIC/VIEWER/EDITOR/ADMIN/SYSTEM/
WORKFLOW_TRIGGER + an unknown-role default-deny assertion.
Single source of truth for the editorial-tier check.
- apps/admin/src/graphql/types/watch-setting.test.ts (new):
resolver wiring assertion — locale arg pass-through to the
service plus thin pass-through of the service return shape.
- apps/admin/src/graphql/schema.test.ts: add watchSetting to the
Query root entry-point assertion + new fieldsOf('WatchSetting')
structural test covering documentId / homepageExperience /
defaultTemplateExperience.
Dead-param cleanup (KT1/C4/M3):
- VideoService.list/getById/getBySlug now have no parameter
(auth contract lives at the resolver). getByCoreId retains its
guard. video.ts resolver call sites updated; the dashboard SSR
caller in live-data.ts threads requireSession() and no longer
passes user.
- video.service.test.ts rewritten: per-tier tests dropped (no
longer meaningful when the service takes no user), with one
signature-drift regression test that fails if a future
contributor re-adds a user param without re-wiring callers.
SDL description fix (AC3):
- WatchSetting.documentId description corrected to reflect that it
reflects the homepage Experience OR the template Experience, not
exclusively the homepage one.
Known deferred residuals (carried in the PR description):
- A1 (P1): Pothos default-allow for new resolvers with no
authScopes — structural change to builder defaultStrategy
deferred to U7.
- AC1/AC2 (P1): WatchExperience fragment cutover risk — U5b/U6.
- T1 (P1): full-pipeline response.errors test — blocked by
vitest transitive-graphql double-instance issue documented in
scene-recommendations.test.ts. Static regression test + isEditor
unit test cover the predicate; the runtime-pipeline assertion
remains a known gap.
- A3/S1 (P2): Video.dubs + Video parent publish filtering — needs
data-model verification + editor impact review; deferred.
* docs(plans): u2 admin public widenings plan status active → completed
* docs(solutions): u2 — pothos PUBLIC widening with field-level strip pattern
Compound capture of the architectural pattern shipped in this PR:
- Three-layer auth coordination (resolver authScopes + service guard
+ permission matrix) with three explicit widening modes
- Paired authScopes + unauthorizedResolver:()=>null for field-level
strip without populating response.errors[] (load-bearing for the
U5 parity comparator on PR #915)
- Service-mediated public→abac bridge via t.prismaField on objectRef
- Centralized public-resolvers regression test as substitute for
admin-schema-drift CI's blindness to authScopes changes
- Smoke-script-as-vitest-substitute for full-pipeline assertions
Cross-links pothos-relation-abac-filter-required-for-nested-types.md
(sibling) and dual-client-gql-tada-multi-schema-codegen-pattern-
20260507.md (predecessor — explains why drift CI is auth-blind).
Captured via /ce-compound with 4 parallel research subagents plus
session-history search across 5 prior sessions.
* chore(admin): u2 — trim verbose inline comments
Cuts ~340 lines of comment narrative across 15 files in the U2 PR.
Preserves every file:line citation, date, decision, and rejected
alternative per the readability contract; drops restated context,
ceremonial section dividers, and JSDoc essays that paraphrase the
code below them.
Behavior unchanged: typecheck clean, 115 test files / 1687 tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(admin): u2 — regen schema.graphql after description trim
Pothos `description:` strings on Video.locales, VideoEdition,
VideoLocale, and WatchSetting flow into the SDL. Trim commit
4b57552 shortened them; this regen aligns the committed
schema.graphql with the source. admin-graphql-env.d.ts is
unchanged because gql.tada types ignore description text.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ur-imazing
added a commit
that referenced
this pull request
May 14, 2026
When two queries spread the same gql.tada fragment, the consumer type alias should anchor on FragmentOf<typeof fragment> rather than ResultOf<typeof QueryA>[path]. gql.tada projects fragment types through each query's selection set independently, so query anchored aliases produce structurally-equivalent but nominally distinct types — and the as <Type> cast that bridges them silently masks future query-level drift. Cross-package applicability: same risk re-emerges in U9's @forge/admin-graphql when admin fragments spread across multiple admin queries. Session history (4 prior sessions) showed the fragment-anchoring principle was established in packages/graphql/src/parity/ normalizers but never applied to content.ts; U5b adapter work where correct anchoring belonged was deferred out of PR #915. Surfaced by ce-code-review (KT-01 finding) and fixed in commit 37c5e2d on this branch. File path: docs/solutions/logic-errors/ gql-tada-fragment-anchor-cast-drift-same-fragment-multi-query-20260514.md Related: dual-client-gql-tada-multi-schema-codegen-pattern (narrow additive refresh recommended).
Ur-imazing
added a commit
that referenced
this pull request
May 14, 2026
…n scaffolding revert + Video widenings) (#939) * docs: capture web→admin rebuild brainstorm, plan, and harness defects writeup Three durable artifacts that motivate and structure the rebuild: - Brainstorm requirements doc (R1-R20 + deferred Open Questions from ce-doc-review) — adopts admin's schema as the only data source for apps/web, with admin's prod widening untouched and packages/graphql frozen for mobile/TV continuing Strapi access. - Plan (22 implementation units across 7 phases) — branch foundation, 4 admin widenings inside the branch (deviation from R17), new packages/admin-graphql package, web data-layer rebuild, local fixture seeding, admin→web revalidation webhook, final verification. - Parity-harness defects writeup — the three stacked blockers from the 2026-05-14 prod-cutover smoke attempt that triggered the pivot away from migration framing. * feat(ce): pause apps/web UI feature work on main during rebuild (U1) Adds an Active Freeze section at the top of CLAUDE.md so contributors see the freeze before reading project overview. References the plan file for scope and names the rebase trigger (critical fixes touching apps/web/src/lib/, apps/web/src/app/, shared types, or packages/graphql). * feat(web): revert migration scaffolding (U2) Strips the web-side scaffolding that exists only because of the Strapi→admin migration framing. After this commit, web is back on Strapi-only reads through packages/graphql's graphql() factory — clean baseline before the rebuild begins. Deleted: - parity-bridge + tests (canary log emitter) - content-api-mode + tests (FORGE_CONTENT_API mode reader) - admin-client + tests (the canary's separate Apollo client; rebuilt in U13 against the new admin-only package) - fragments/admin-experience.ts (the only admin-bound web fragment) - __tests__/content-mode-regression.test.ts - docs/admin-core-migration/cutover-runbook.md (origin R4) Also removes files orphaned by the env-var + class deletions: - MaintenanceFallback.tsx + slug-page test that exercised FORGE_DISABLE_WATCH_ROUTES - slug-segment error.tsx + test (added in PR #933 specifically for WatchPageAdminError; dead with that class gone) Modified: - content.ts — strips fetchAdminSlugExperience, fetchSlugExperience dispatcher, WatchPageAdminError, mode-keyed unstable_cache, related event logging; collapses resolveSlugPage back to direct Strapi reads - content.test.ts — drops admin mocks and the cutover describe block - fragments/index.ts — drops adminExperienceBySlugOperation re-export - [slug]/page.tsx — drops dual-read branching + maintenance fallback - env.ts — removes FORGE_CONTENT_API, FORGE_PARITY_DEBUG, FORGE_DISABLE_WATCH_ROUTES blocks and host-allowlist helpers; ADMIN_GRAPHQL_URL and WEB_ADMIN_API_KEYS stay .optional() for now (flipped to required in U13) apps/admin/src/domain/package.json (the ESM/CJS workaround) stays untracked; it deletes in U3 alongside the parity directory that depends on it (doc-review F1). * feat(graphql): trim to Strapi-only (U3) Strips every admin-side artifact and the parity harness from the shared GraphQL package. After this commit, packages/graphql emits only the Strapi graphql() factory and its types. Mobile and TV continue consuming it unchanged; web has nothing in here for now (it gets the new packages/admin-graphql in U9). Deleted: - src/parity/ entire directory (15 source files + fixtures + tests) - scripts/capture-parity-fixture.ts, scripts/run-batch-verification.ts - src/admin.ts (the adminGraphql() factory) - src/admin-graphql-env.d.ts (admin gql.tada introspection output) - src/fragments/admin/ entire directory (19 admin block fragments + barrel + watch-experience root; relifted into the new package in U9 from git history) - src/__tests__/dual-client.types.ts (type-isolation guard; replaced by single-package equivalent in U11) - apps/admin/src/domain/package.json (ESM/CJS workaround; its only consumer was src/parity/normalize-admin.ts, gone above) - Now-empty parent directories (scripts/, src/__tests__/, src/fragments/admin parent) cleaned for tidiness Modified: - src/index.ts — collapsed to Strapi exports only (graphql, readFragment, FragmentOf, ResultOf, VariablesOf) - package.json — dropped ./admin, ./admin/fragments, ./parity exports; dropped @forge/admin devDep, zod devDep, p-limit dep; dropped run-batch-verification script. @forge/cms kept (build- graph signal even though no direct imports) - tsconfig.json — dropped the 'admin' schema entry from the gql.tada plugin config; only 'strapi' remains - CLAUDE.md, AGENTS.md — rewritten for single-schema state with forward pointers to the upcoming @forge/admin-graphql Pre-flight grep confirmed mobile and TV import only the Strapi graphql() factory + ResultOf type from @forge/graphql — no admin references anywhere. R18 holds. Verified: pnpm --filter @forge/graphql typecheck passes; codegen regenerates only graphql-env.d.ts; mobile, TV, web all typecheck against the trimmed package. Out of scope for U3 (handled later): - turbo.json generate task still lists admin schema inputs/outputs (U10 owns the per-package CI split) - apps/admin/AGENTS.md and src/scripts/print-schema.ts still reference adminGraphql / admin-graphql-env (retargeted to the new package in U9-U12) * feat(admin): tear down PARITY_BEARER scaffolding (U4) With the parity harness deleted in U3, PARITY_BEARER has no consumer. Admin's posture toward CONSUMER_BEARER, PUBLIC, EDITOR, and ADMIN principals stays exactly the same — this commit removes dead code authenticating a caller that no longer exists. Deleted: - src/auth/parity-bearer.ts + parity-bearer.test.ts Modified: - src/auth/principal.ts — dropped PARITY_BEARER role + PARITY_BEARER_PRINCIPAL factory; refreshed JSDoc - src/auth/permissions.ts — dropped read:experience-templates key, matrix entry, PARITY_BEARER_PERMISSIONS set, and the PARITY_BEARER early-return branch - src/auth/permissions.test.ts, principal.test.ts — drop PARITY_BEARER coverage - src/graphql/context.ts — drop parity-bearer branch from resolution chain (now workflow → consumer → public) - src/graphql/context.test.ts — drop 5 PARITY_BEARER tests - src/graphql/plugins/rate-limit.ts — drop the role === 'PARITY_BEARER' branch. CONSUMER_BEARER routing to consumer:<key> is unchanged (doc-review S1 verification) - src/graphql/plugins/rate-limit.test.ts — drop 3 PARITY_BEARER tests; CONSUMER_BEARER + anonymous + authenticated coverage preserved (9 tests pass) - src/graphql/types/experience.ts — drop the experienceTemplates Pothos field (PARITY-only) - src/services/experience.service.ts — drop listTemplateLocales method + PARITY_BEARER bypass comment in getBySlug - src/services/experience.service.test.ts — drop PARITY_BEARER getBySlug test + listTemplateLocales describe block - src/config/env.ts — drop PARITY_API_KEYS from Zod schema + runtimeEnv; revert assertBearerCsvsDisjoint to two-way (WORKFLOW_API_KEYS vs WEB_ADMIN_API_KEYS) - src/config/env.test.ts — collapse three-way disjointness tests to two-way - .env.example — drop PARITY_API_KEYS references - schema.graphql — regenerated; experienceTemplates field gone Doppler follow-up (operator runs manually): - doppler secrets delete PARITY_API_KEYS --project forge-admin --config dev - doppler secrets delete PARITY_API_KEYS --project forge-admin --config prd Verified: pnpm --filter @forge/admin typecheck passes; pnpm --filter @forge/admin test passes (137 files, 2186 tests + 1 todo); zero PARITY_BEARER references in apps/admin/src. Reference: docs/solutions/architecture-patterns/parity-bearer-narrow-carveout-pattern-20260513.md (the pattern doc framed this as throwaway scaffolding and is the canonical teardown recipe). * feat(admin): widen Video.parents, Video.children, and Video.locales(locale) (U5, U6) Three additive PUBLIC-surface widenings on the Video type, all needed by the upcoming web rebuild: - Video.parents: [VideoRelation!] — new relation through the VideoRelation join table. Anonymous callers see only relations whose target Video has at least one PUBLISHED locale and deletedAt IS NULL; EDITOR/ADMIN see all. Powers the web watch- page sibling carousel. - Video.children: [VideoRelation!] — same shape as parents. - Video.locales(locale: String) — added optional locale arg to the existing relation. Filters to a single locale when provided; default behavior (all locales) preserved when omitted. Lets web fetch single-locale fields (description, snippet, imageAlt) without overfetching every locale. Chose Option A (relation arg) over Option B (new videoBySlug(slug, locale) overload) — Option A composed cleanly with no nullable-shape friction. Pattern: mirrors the principal-aware relation filter at apps/admin/src/graphql/types/experience.ts:92-99 (Experience.locales). Filter logic extracted to exported helpers (videoLocalesFilter, videoParentsFilter, videoChildrenFilter) so the new principal- aware regression test can exercise them as pure functions — Pothos doesn't expose query callbacks via introspection. New: VideoRelation Pothos prismaObject — minimal projection (id, order, parent, child) of the join table. Preserves relation ordering for downstream renderers. Classified as public-shape. Changes: - src/graphql/types/video.ts — add parents/children relations + VideoRelation type; locale arg on locales; export filter helpers - src/graphql/classification.test.ts — register the new public- shape relations and VideoRelation in RELATION_TARGETS - schema.graphql — regenerated (additive only) - src/graphql/types/video.principal-filter.test.ts (new) — 18 unit tests per principal × per relation × per locale-arg variant Reference: docs/solutions/graphql/pothos-public-widening-multi-layer-coordination-20260511.md Doc-review S1 verification: identifyForRateLimit unchanged; CONSUMER_BEARER still routes to consumer:<key>; all 9 rate-limit tests pass. Verified: pnpm --filter @forge/admin typecheck passes; pnpm --filter @forge/admin test passes (138 files, 2219 tests, 1 todo). Public-resolvers regression manifest unchanged (field-level relations don't appear in the top-level resolver manifest). * feat(web): refactor template/homepage routing to watchSetting (U7, U8) Two web-side cleanups in apps/web/src/lib/content.ts that remove web's reliance on the isTemplate and isHomepage Experience fields. Web's user-facing rendering is unchanged — only the field used to decide template-vs-experience routing and homepage resolution. U7 — Option B (selected for security posture preservation): - resolveSlugPage now reads watchSetting first and uses watchSetting.defaultTemplateExperience.slug as the single source of truth for template-route decisions. The previous asNonTemplateExperience / asTemplateExperience helpers and the INVALID_DEFAULT_TEMPLATE / INVALID_HOMEPAGE_EXPERIENCE validation throws are gone — we trust admin's data model rather than re-asserting field-flag invariants. - isTemplate dropped from the WatchExperience fragment selection. - Admin-side Experience.isTemplate stays STRIPPED_FOR_PUBLIC (verified via grep) — admin's auth posture is preserved per doc-review S3. U8 — Drop the legacy isHomepage fallback in resolveHomepage: - resolveHomepage reads watchSetting.homepageExperience as the only source. The getExperienceByFilters(locale, isHomepage: eq: true) fallback path is gone. - isHomepage dropped from the WatchExperience fragment selection. Behavior invariant: a slug that previously rendered as a template still does; the home route renders the same homepage Experience. Only the routing-decision code path changes. Tests rewritten in content.test.ts: - New: homepage Experience returned from watchSetting - New: missing-homepage error when watchSetting.homepageExperience is null - New: explicit-experience match when slug differs from template - New: template-route routing when slug matches defaultTemplateExperience.slug - Removed: INVALID_DEFAULT_TEMPLATE / INVALID_HOMEPAGE validation tests (no longer applicable) Surfaced but intentionally left alone: - apps/web/src/app/api/revalidate/route.ts retains an isTemplate? field on the Strapi webhook payload type. Not used by any logic; the whole route gets reshaped to admin's webhook in U21. Out of scope here. - apps/web/src/components/ExperienceError.tsx still has dictionary entries for the deleted error strings. Dead text, no cost; not worth widening scope to remove. Verified: pnpm --filter @forge/web typecheck passes; pnpm --filter @forge/web test passes (400 tests, 33 files, 3 todo). * fix: address ce-code-review P1 findings + small cleanups Two P1 findings from ce-code-review on the branch + three small cleanups bundled together. P1 — fix CI: stale admin-graphql-env.d.ts references would fail the graphql-generate job (project-standards ps-001, anchor 100): - turbo.json: drop apps/admin/schema.graphql + admin-graphql-env.d.ts from the generate task's inputs/outputs. The Strapi side stays; admin codegen returns when packages/admin-graphql lands in U9-U10. - .github/workflows/ci.yml: drop admin-graphql-env.d.ts from the graphql-generate diff check. CI now matches the trimmed package shape post-U3. P1 — fix security: web still reads Strapi, which has no read-side filter for isTemplate. U7's Option B refactor removed the client-side asNonTemplateExperience guard (security sec-001 + adversarial adv-001, cross-reviewer agreement, anchor 75): - apps/web/src/lib/content.ts: re-introduce template-exclusion at the Strapi query layer via isTemplate: { eq: false } in the experiences filter. Defense-in-depth lives until U13 cuts web over to admin (which strips isTemplate from PUBLIC). P3 cleanups (maintainability M-001, M-002): - apps/admin/src/services/experience.service.ts: stale comment referenced the deleted asNonTemplateExperience helper. Rewritten to describe the current contract (templates hidden from PUBLIC + CONSUMER_BEARER via the service-layer where-clause). - apps/web/src/app/api/revalidate/route.ts: drop unused isTemplate field from StrapiWebhookPayload type. No logic ever read it; left over from the dual-read era. Deferred to U13 (advisory, not blocking): - adv-002 (WEB_ADMIN_API_KEYS declared but unused) — env var stays for U13's Apollo wiring; the existing comment in env.ts documents its intended consumer. Removing now and re-adding in U13 is churn. Verified: pnpm --filter @forge/web typecheck passes; pnpm --filter @forge/web test passes (400 tests, 33 files, 3 todo). Admin tests unaffected; admin SDL unchanged. * chore: regenerate pnpm-lock.yaml after U3 dep removals packages/graphql/package.json dropped @forge/admin, zod, p-limit in U3 (parity harness removal) but the lockfile wasn't regenerated, causing CI's pnpm install --frozen-lockfile to fail with ERR_PNPM_OUTDATED_LOCKFILE across commit-lint, format, and affected jobs. Pure lockfile-only diff: removes the 9 entries those three packages contributed to packages/graphql's resolved deps. * fix: address ce-code-review findings (proper run) ce-code-review (proper Stage 1-6 flow this time) on the branch surfaced 20 actionable findings + 7 advisory acknowledgements. Auto-resolve with best-judgment applied 13 fixes; 7 acknowledged as intentional design. P1 fixes: - apps/web/src/env.ts — ADMIN_GRAPHQL_URL and WEB_ADMIN_API_KEYS now carry inline 'Placeholder — wired in U13' comments so they don't read as dead code (M-01) - apps/web/src/lib/content.ts — WatchExperience type anchored to FragmentOf<typeof watchExperienceFragment>. Both GET_WATCH_EXPERIENCE and GET_WATCH_SETTINGS query projections now reference the same fragment-derived type, eliminating gql.tada cast drift between two ResultOf paths (KT-01) P2 fixes — documentation drift: - apps/admin/AGENTS.md — SDL-emission section rewritten to drop the stale admin-graphql-env.d.ts regen step; notes the admin codegen consumer lands in U9 (PS-001) - CLAUDE.md (root) — 'Admin-side change flow' rewritten to reflect packages/graphql is Strapi-only; admin codegen will live in future packages/admin-graphql under U9 (PS-002) - packages/graphql/CLAUDE.md + AGENTS.md — forward references to @forge/admin-graphql annotated as '(future — lands in U9)' (adv-003) P2 fixes — runtime correctness + safety: - apps/web/src/lib/fragments/watch-experience.ts — isTemplate re-added to the fragment for defensive validation - apps/web/src/lib/content.ts — INVALID_HOMEPAGE_EXPERIENCE_MESSAGE and INVALID_DEFAULT_TEMPLATE_MESSAGE runtime guards restored in resolveHomepage and resolveSlugPage. The watchSetting-based routing logic stays; isTemplate is consulted defensively only. Prevents silent template-as-homepage / non-template-as-template misconfigurations from rendering (adv-002) - apps/web/src/lib/content.ts — TODO(U14) prepended to the isTemplate Strapi-filter workaround comment so the deletion trigger is in-code (M-03) - apps/web/src/lib/content.ts — templateSlug comparison normalized via toLowerCase() on both sides; case-mismatched template slugs no longer silently mis-route (adv-005) P2 fixes — type safety: - apps/admin/src/graphql/types/video.ts — explicit return type annotations on videoLocalesFilter / videoParentsFilter / videoChildrenFilter using Prisma.VideoLocaleWhereInput / Prisma.VideoRelationWhereInput unioned with Record<string, never>. Prisma where shape now machine-checked (KT-04) P3 fixes: - apps/admin/src/graphql/types/video.ts — videoLocalesFilter locale predicate tightened to typeof+length>0 so empty-string locale arg behaves like 'no filter' (C3) Test additions: - apps/web/src/lib/content.test.ts — two new tests covering resolveSlugPage's null-template and null-streamingUrl return branches that were untested after U7/U8 (T-01, T-02) - apps/admin/src/graphql/types/video.principal-filter.test.ts — ADMIN+locale test case + empty-string locale edge case (T-03) Acknowledged (no code change — intentional): - adv-001: deploy-ordering CONSUMER_BEARER fall-through (operational, documented in plan U13) - KT-03: admin-graphql-env.d.ts CI drop (intentional per U3; new CI job lands in U10) - M-04: filter helpers co-located with Pothos schema (observational) - M-02: unconditional getWatchSettings prefetch (intentional simplicity; cached at route level) - AC-004: codegen gap until U9 (temporary, documented) - C2: slug==template-slug Experience-lookup skip (intentional; related concern fixed by adv-005) - adv-004: Video.parents cross-locale enumeration (pre-existing pattern consistent with Video root resolver gates) Verified: pnpm --filter @forge/web typecheck passes; pnpm --filter @forge/web test passes (402 tests, 33 files, 3 todo); pnpm --filter @forge/admin test passes (2221 tests + 1 todo). Pre-existing finding KT-02 (Apollo result cast bypasses Apollo v4 types) carried forward as a known pre-existing pattern across the codebase; not addressed in this PR. Review artifact: /tmp/compound-engineering/ce-code-review/20260514-152723-dc41ab8f/ * docs(solutions): compound gql.tada cast-drift learning When two queries spread the same gql.tada fragment, the consumer type alias should anchor on FragmentOf<typeof fragment> rather than ResultOf<typeof QueryA>[path]. gql.tada projects fragment types through each query's selection set independently, so query anchored aliases produce structurally-equivalent but nominally distinct types — and the as <Type> cast that bridges them silently masks future query-level drift. Cross-package applicability: same risk re-emerges in U9's @forge/admin-graphql when admin fragments spread across multiple admin queries. Session history (4 prior sessions) showed the fragment-anchoring principle was established in packages/graphql/src/parity/ normalizers but never applied to content.ts; U5b adapter work where correct anchoring belonged was deferred out of PR #915. Surfaced by ce-code-review (KT-01 finding) and fixed in commit 37c5e2d on this branch. File path: docs/solutions/logic-errors/ gql-tada-fragment-anchor-cast-drift-same-fragment-multi-query-20260514.md Related: dual-client-gql-tada-multi-schema-codegen-pattern (narrow additive refresh recommended).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stand up the dual-read parity canary for the admin-core consumer migration. One web data-access function (the slug-page Experience branch in
apps/web/src/lib/content.ts) now fans out to admin'sexperienceBySlugGraphQL query in parallel with Strapi whenFORGE_CONTENT_API=dual-read. The user always sees Strapi; admin runs alongside for parity-signal collection via the U4 harness, emitting a structured log line per request.This is U5 of feat-104 admin-core consumer migration. Plan:
docs/plans/2026-05-08-001-feat-consumer-migration-web-canary-unit-5-plan.md. Origin brief:docs/brainstorms/2026-05-05-consumer-migration-implementer-brief-requirements.md.Scope (and what's deferred)
U5 ships only two modes:
strapi(default, byte-identical to currentmain) anddual-read(canary). The twoadmin-rendering modes from origin R7 (admin-with-fallbackand pureadmin) are explicitly deferred to a follow-up unit (U5b). Reasoning:dual-readprovides that.admin-mode rendering through the harness's normalizers conflates parity-comparison (lossy by design) with rendering (must be lossless). The admin →WatchExperienceshape adapter is the load-bearing migration work; it gets its own unit.Out of scope for U5: homepage,
/watch/[collection]/[video]/[locale], video-template fallback, sibling carousel — those depend onvideos/watchSettingqueries still gated onread:videos.Implementation
env.ts,content-api-mode.tsFORGE_CONTENT_APIserver flag (accepts all 4 origin-R7 values; U5b values coerce to "strapi" with a warn until U5b ships);ADMIN_GRAPHQL_URLwith hard-reject forauth.jesusfilm.org(PR #909) and warn-only suffix allowlist.FORGE_PARITY_DEBUGregistered.admin-client.ts,fragments/admin-experience.tsclient.ts, per-callAbortSignal.timeout(3000)(NOT module-scope — foot-gun called out)parity-bridge.tslocaleif missing; remap adminmetaDescription → description. R13 enforcement: production log payload carries counts + JSON-Pointer paths only — rawValueDiff.{strapi,admin}field values are STRIPPED. R13 defense-in-depth: requires bothFORGE_PARITY_DEBUG=1ANDNODE_ENV !== "production". Dev opt-in viaFORGE_PARITY_DEBUG=1content.ts,fragments/watch-experience.tsfetchSlugExperience(locale, slug)branched once at the smallest divergence point.Promise.allover both fetches, each timed viaperformance.now. Bridge call wrapped in try/catch — sync throw never breaks user render. 7 log events:diff,admin_timeout,harness_error,strapi_failed_admin_succeeded,both_failed,admin_missing(NEW — backfill gap signal),canary_failed(NEW — bridge-throw recovery)10 commits + plan + inventory entry (12 commits total). +2600/-50 LOC across 17 files.
Code review (Tier 2)
/ce-code-review mode:autofixran 12 reviewer personas (correctness, testing, maintainability, project-standards, agent-native, learnings-researcher, security, performance, api-contract, reliability, adversarial, kieran-typescript). 7 actionable findings applied:normalizeContentApiModenow load-bearingrunDualReadComparisonin try/catch; emitsforge.parity.canary_failedon bridge sync errorcomparator_unknownso backfill-gap (admin null + Strapi has data) is distinguishable from real comparator failuresisAbortTimeoutErrornow walkserror.networkErrorand its cause chain; restores timeout classification for Apollo v4result.errorpath, TimeoutError name, networkError walk, defense-in-depth, U5b values coerce-with-warn, bridge sync-throw recoveryRun artifact:
/tmp/compound-engineering/ce-code-review/20260508-160116-a106c03a/Verification
pnpm --filter @forge/web typecheckpnpm --filter @forge/web testpnpm --filter @forge/web lintpnpm --filter @forge/web buildpnpm --filter @forge/graphql generateFORGE_CONTENT_API=dual-read, Strapi up, admin local 404)forge.parity.harness_errorwithsubkind: admin_fetch_error, HTTP 200 to user, parallel timing measured separately, payload shape per specKey decisions
headers()/cookies()would silently disable Next.js Full Route Cache.dual-read. Diff signal lands in the same trace as the request; user-facing budget is bounded.AbortSignal.timeoutconstructed inside the fetch override (per-call). Module-scope construction would share one signal across all calls.auth.jesusfilm.orghard-rejected inADMIN_GRAPHQL_URLallowlist. Mirrors the harness'sREJECTED_HOSTSpattern (packages/graphql/src/parity/live-config.ts:24). Boot fails fast on misconfig.FORGE_CONTENT_API=adminin prep for U5b doesn't brick boot; falls back to"strapi"with a console.warn until U5b ships admin-mode rendering.WatchExperiencefragment now selectslocale. Required bynormalizeStrapi; bridge synthesizes fromurlLocaleas defense-in-depth.runDualReadComparison's sync errors and emitsforge.parity.canary_failed. The user always gets Strapi.Post-Deploy Monitoring & Validation
Log queries (Vercel/Railway log search):
event:"forge.parity.diff"diffCountsnear-zero across all channelsvalueorstructuralcount → admin schema driftevent:"forge.parity.admin_missing"event:"forge.parity.admin_timeout"event:"forge.parity.harness_error"subkind:"admin_blocks_validation"→ admin write-side validator gap; any sustained subkind → escalateevent:"forge.parity.both_failed"ORevent:"forge.parity.strapi_failed_admin_succeeded"event:"forge.parity.canary_failed"Validation window: First week post-flag-flip in any env. Owner: Urim (per memory: end-to-end ownership of admin migration). The plan defers numeric thresholds for advancing to admin mode to U5b.
Rollback trigger: Any
forge.parity.canary_failedevent fires → flipFORGE_CONTENT_API=strapiand redeploy. R17 no-redeploy rollback ships in U7.Mitigation: Set
FORGE_CONTENT_API=strapiin Railway service settings + redeploy. The default mode is byte-identical to current main.Known follow-ups (NOT blocking U5)
/api/graphqlreturns 404 fromlocalhost:3003. Caused byapps/admin/src/proxy.ts:79-81blocking/api/*from the auth host (PR feat(admin): host auth on JesusFilm SSO domain #909 design). Locallylocalhost:3003IS the auth host perapps/admin/src/auth/origins.ts:4. Production has separateauth.jesusfilm.org(auth) andadmin.jesusfilm.org(app), so the canary works there. Local-dev fix options (separate PR): (a) run admin on a second port for the app surface, or (b) add a dev-onlyDEV_ALLOW_GRAPHQL_FROM_AUTH_HOSTflag.watchExperienceFragment. If any nested relation truncates silently, parity diffs will surface false positives. Hardening pass — separate PR.docs/runbooks/. Worth lifting into a discoverable runbook for U7.Plan history
Built via
/ce-plan→/ce-doc-review(5 reviewer personas, 17 actionable findings, scope revision applied) →/ce-work(4 implementation units shipped serially with inline verification) →/simplify(3 simplification fixes) →/ce-code-review mode:autofix(12 reviewers, 7 fixes applied).