Skip to content

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
mainfrom
dlo/prod-2729-cli-geo
Draft

DerekAgility wants to merge 2 commits into
mainfrom
dlo/prod-2729-cli-geo

Conversation

@DerekAgility

@DerekAgility DerekAgility commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

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 geo block (schemaType, summary, structuredData, ai) and four new seo keys (title, canonicalUrl, robots, ogImage). The Management API treats each of them in three states: omitted means keep the stored value, null means clear it, and a value replaces it.

  1. Passthrough (no change needed, now tested). processPage spreads the pulled page into the savePage payload, so geo and the new seo keys pass through untouched. When seo is null, content-batch-processor.ts defaults only the five older seo keys. The new keys stay omitted (so the target keeps them) and geo survives the spread. Tests now pin both, because a null here would clear the target's data.

  2. 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:

    • saves the page again without the offending properties: geo for anything under geo.*, or just the named seo.<key>. They are deleted, never nulled, so the target keeps its own values;
    • counts the page as a success and adds a non-blocking warning (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;
    • carries on with the sync.

    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.

  3. 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 verify now compares geo on pages. The new seo keys were already compared as part of seo.
  • The run summary heading "Page Component Warnings" is now "Warnings", since it now lists GEO notices for pages and content items.
  • FailureDetail gets an optional code. --jsonSummary already 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 without geo → success with a warning (all other savePage arguments unchanged); only the named seo key 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-seo default keeps geo and doesn't add the new seo keys; geo is omitted, not nulled.
  • check-page-zones.test.ts: verify reports a geo mismatch.
  • src/tests/geo/geo-roundtrip.integration.test.ts (new): a real agility sync round 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 reports geoNotCopied, and that its target GEO data is unchanged. It skips unless AGILITY_TOKEN, GEO_SOURCE_GUID, GEO_TARGET_GUID, GEO_STATIC_PAGE_ID, GEO_DYNAMIC_PAGE_ID and GEO_INVALID_PAGE_ID are set. It can't pass until stage 3 is on dev and both instances are in the Management API's GEO allow-list. Run with npm run build && npm run test:integration -- src/tests/geo.

Results: npx jest → 133 suites, 2556 passed, 0 failed. tsc --noEmit passes for both the app and the tests. The integration suite shows as skipped.

Follow-ups (not in this PR)

  • The Management API attributes GEO saves by User-Agent (agility-cli → cli), but the CLI's saves go through @agility/management-sdk with axios' default User-Agent, so they count as api. 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).

  1. Passthrough (no change needed, comment added). content-batch-processor.ts spreads the pulled item into the payload, so item geo passes through; when seo is null the default fills only the five older keys and never touches geo or the GEO seo keys. The page half's tests already pin this.
  2. GEO validation 400 on item saves → per-item retry. Items are saved through 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() (in geo-fields.ts, shared with pages) attributes each path to its item;
    • the batch is saved again with the offending GEO properties deleted (never nulled) from the named items only; polling and result matching use the payloads actually sent;
    • each changed item gets a non-blocking geoNotCopied warning, which now reaches the run summary and --jsonSummary through pushContent's warningDetails. The heading is now "Warnings", since it lists content items too;
    • an unindexed message on a multi-item batch strips the named keys from every item that carries them (only ever keeps target values); a non-GEO 400, nothing to strip, or a failing retry fail the batch exactly as before.
  3. agility verify compares an item's geo (fieldMismatch, path geo; 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 named seo key; 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, optional GEO_INVALID_ITEM_ID) that runs a sync scoped with --containers and checks geo on the target, the geoNotCopied warning and that the invalid item's target GEO is unchanged; it skips without those variables. npx jest → 133 suites, 2567 passed; tsc --noEmit clean; eslint 0 errors (pre-existing warnings only).

Depends on: the Fetch API item half emitting geo on sync items (the CLI pulls content through @agility/content-sync), and the Management API item half (its 400 message format above is from ContentItemController on the PROD-2721 branch).

🤖 Generated with Claude Code

DerekAgility and others added 2 commits October 15, 2026 20:05
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant