Repository navigation
PROD-2729: carry page GEO fields through sync; retry without them on a GEO validation 400 - #241
Draft
DerekAgility wants to merge 2 commits into
Draft
DerekAgility wants to merge 2 commits into
DerekAgility wants to merge 2 commits into
Conversation
…a GEO validation 400 Pages already spread the pulled page into the savePage payload, so `geo` and the new `seo` keys (title, canonicalUrl, robots, ogImage) pass through untouched, and the content batch processor's null-`seo` default only sets the five older keys. Tests now pin both, since an explicit null on save clears a stored value. When the Management API rejects a page's GEO data with a 400, the page is saved again without the offending properties (deleted, never nulled, so the target keeps its own values). The page counts as a success and the run summary carries a "GEO not copied" warning with code `geoNotCopied` and the API's message. Other 400s, non-400 errors and a failing retry behave as before. Also: verify compares `geo` on pages, and the run summary's page warnings heading no longer says "Component" only. Adds an integration round trip (static, dynamic and invalid-GEO fixture pages) that skips unless two instances and the fixture page IDs are configured. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… retry; verify item geo The MVP's item half. - Content items already spread the pulled item into the save payload, so `geo` passes through; the null-`seo` default still fills only the five older keys and never sets `geo` (comment added; tests from the page half pin it). - Item saves go through POST item/multi, which validates every item's GEO before queueing and rejects the WHOLE batch with one 400 naming "items[i].geo..." (or "$[i].geo..." from model binding). The batch processor now saves the batch again with the offending GEO properties deleted (never nulled) from the named items only, and reports each as a `geoNotCopied` warning. An unindexed message strips from every item that carries the named keys; a non-GEO 400, nothing to strip, or a failing retry fail the batch as before. Warnings reach the run summary through pushContent's warningDetails; the summary heading is now "Warnings". - `agility verify` compares an item's `geo` (fieldMismatch, path "geo"). - The skipped round-trip integration test gains an independent item block (GEO_ITEM_CONTAINER, GEO_ITEM_ID, optional GEO_INVALID_ITEM_ID), run as a sync scoped with --containers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
PROD-2729: the CLI's part of Phase 1 of the native GEO/AEO fields. Draft. Don't merge or publish until Phase 1 has passed on dev (rollout: everything on dev first, then one production window).
What the CLI has to handle
Pages can now carry a top-level
geoblock (schemaType,summary,structuredData,ai) and four newseokeys (title,canonicalUrl,robots,ogImage). The Management API treats each of them in three states: omitted means keep the stored value,nullmeans clear it, and a value replaces it.Passthrough (no change needed, now tested).
processPagespreads the pulled page into thesavePagepayload, sogeoand the newseokeys pass through untouched. Whenseois null,content-batch-processor.tsdefaults only the five olderseokeys. The new keys stay omitted (so the target keeps them) andgeosurvives the spread. Tests now pin both, because anullhere would clear the target's data.GEO validation 400 → retry without it, then report. The Management API rejects invalid GEO input with a 400 whose body is
{ statusCode: 400, exceptionMessage: "geo.structuredData: is not valid JSON (...). geo.schemaType: must be one of ..." }. On a 400 that names a GEO field path, the CLI:geofor anything undergeo.*, or just the namedseo.<key>. They are deleted, never nulled, so the target keeps its own values;code: "geoNotCopied", e.g. "GEO not copied: invalid structured data (geo left out; the target keeps its existing values). geo.structuredData: …"), which shows in the run summary and--jsonSummary;Other 400s, non-400 errors and a retry that also fails are handled as before.
ASP.NET model-binding 400s (
errors: { "$.geo.schemaType": [...] }) are recognised too.Settings profiles and model mappings (warn on sitemap or model names missing from the target) belong to Phase 2/3. Nothing to do yet.
Also:
agility verifynow comparesgeoon pages. The newseokeys were already compared as part ofseo.FailureDetailgets an optionalcode.--jsonSummaryalready had the field.Behaviour to document for the rollout
Until the Fetch API carries GEO data (rollout stage 3), a sync copies pages without
geo. Pages that already exist on the target keep their GEO data (omitted = keep), and newly created pages arrive with none. A sync also doesn't carry a deletion: a value cleared on the source is left out of the pull, so the target keeps its old value (accepted for v1). This belongs in the CLI docs and release notes when the feature launches. It isn't in the README yet, because the fields are still hidden.Tests
src/lib/pushers/page-pusher/tests/geo-fields.test.ts(new): parses both 400 shapes, maps paths to what gets left out, ignores non-GEO 400s and non-400s, deletes keys rather than nulling them.process-page.test.ts: passthrough for static and dynamic pages; nothing is added when the page has no GEO data; a 400 → retry withoutgeo→ success with a warning (all othersavePagearguments unchanged); only the namedseokey is left out; no retry for a non-GEO 400 or a 500; a failing retry fails the page.content-batch-processor.test.ts: passthrough; the null-seodefault keepsgeoand doesn't add the newseokeys;geois omitted, not nulled.check-page-zones.test.ts: verify reports ageomismatch.src/tests/geo/geo-roundtrip.integration.test.ts(new): a realagility syncround trip of a static page, a dynamic page and an invalid-GEO page between two instances. It checks that the values match on the target, that the invalid page reportsgeoNotCopied, and that its target GEO data is unchanged. It skips unlessAGILITY_TOKEN,GEO_SOURCE_GUID,GEO_TARGET_GUID,GEO_STATIC_PAGE_ID,GEO_DYNAMIC_PAGE_IDandGEO_INVALID_PAGE_IDare set. It can't pass until stage 3 is on dev and both instances are in the Management API's GEO allow-list. Run withnpm run build && npm run test:integration -- src/tests/geo.Results:
npx jest→ 133 suites, 2556 passed, 0 failed.tsc --noEmitpasses for both the app and the tests. The integration suite shows as skipped.Follow-ups (not in this PR)
agility-cli→cli), but the CLI's saves go through@agility/management-sdkwith axios' default User-Agent, so they count asapi. Fixing that needs an SDK option or a CLI-side header.Item half (MVP, 2026-10-16)
Commit 8c03e58. Content items in a list that backs a dynamic page now carry their own
geo(plan "Pick up here", part B).content-batch-processor.tsspreads the pulled item into the payload, so itemgeopasses through; whenseois null the default fills only the five older keys and never touchesgeoor the GEOseokeys. The page half's tests already pin this.POST {locale}/item/multi, and the Management API (item half,ContentItemController) validates every item's GEO before queueing and rejects the whole request with one 400:"items[3].geo.structuredData: … items[7].seo.ogImage: …"(no prefix for a single item; model binding says$[3].geo…). Without handling, one bad JSON-LD block would fail a batch of up to 100 items. Now:getItemGeoValidationErrors()(ingeo-fields.ts, shared with pages) attributes each path to its item;geoNotCopiedwarning, which now reaches the run summary and--jsonSummarythroughpushContent'swarningDetails. The heading is now "Warnings", since it lists content items too;agility verifycompares an item'sgeo(fieldMismatch, pathgeo; absent on both sides is equal).Tests:
geo-fields.test.ts+5 (indexed attribution incl. multiple paths per item,$[i]model-binding paths, single-item unprefixed, multi-item unprefixed fallback, out-of-range/non-GEO/non-400);content-batch-processor.test.ts+5 (retry strips only the named item and warns, polls the retried payloads; strips only the namedseokey; non-GEO 400 not retried; nothing to strip not retried; failing retry fails the batch);check-content.test.ts+1. The integration test gains an independent item block (GEO_ITEM_CONTAINER,GEO_ITEM_ID, optionalGEO_INVALID_ITEM_ID) that runs a sync scoped with--containersand checksgeoon the target, thegeoNotCopiedwarning and that the invalid item's target GEO is unchanged; it skips without those variables.npx jest→ 133 suites, 2567 passed;tsc --noEmitclean; eslint 0 errors (pre-existing warnings only).Depends on: the Fetch API item half emitting
geoon sync items (the CLI pulls content through@agility/content-sync), and the Management API item half (its 400 message format above is fromContentItemControlleron the PROD-2721 branch).🤖 Generated with Claude Code