Skip to content

feat(HNT-2487): validate inbound crawl messages and the publisher list - #20

Draft
mmiermans wants to merge 1 commit into
claude/hnt-2086-domain-typesfrom
claude/hnt-2086-message-validation
Draft

feat(HNT-2487): validate inbound crawl messages and the publisher list#20
mmiermans wants to merge 1 commit into
claude/hnt-2086-domain-typesfrom
claude/hnt-2086-message-validation

Conversation

@mmiermans

@mmiermans mmiermans commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Goal

HNT-2487

Validate the two inbound Pub/Sub job messages and the agent's publisher list at the boundary so a malformed payload fails fast rather than reaching a handler that trusts its static type. This slice adds validateCrawlArticleMessage, validateCrawlArticleDiscoveryMessage, validateLiveArticle, validatePublisherList, and the MessageValidationError they throw, each narrowing an untyped payload to its typed domain shape or throwing with the offending field named.

This builds on the domain-types slice by importing the message, corpus item, and publisher list types that these validators narrow untyped payloads to. See docs/ARCHITECTURE.md in #14 for the message contracts these validators enforce.

Implementation decisions

Decision Approach Why
Hand-write the validators rather than adopt a schema library Small guards narrow an untyped payload field by field and throw an error naming the offending field The inbound Pub/Sub messages carry only a handful of required fields, so small hand-written guards that name the offending field are simpler than adding a schema library. A schema library adds a dependency, and the compiler-plugin options add toolchain complexity, neither of which is justified at a validation surface this small. The validation library evaluation in #8 weighed Zod, Valibot, Typia, and AJV with TypeBox, leaned toward Zod if a library were adopted, and noted that the content-monorepo backend lambdas hand-write their types. Hand-writing wins here because the surface is that small, and naming the offending field keeps a poison payload easy to diagnose from the logs.
Report a validation failure as a kind distinct from a transient failure The validators throw MessageValidationError, which the Pub/Sub validate hook reports under the validation-error kind and nacks, while at agent startup the same error aborts the process At the subscriber boundary a malformed message must be told apart from a transient handler failure so it is not mistaken for something a redelivery would fix, matching the validate hook contract in the Pub/Sub package. At agent startup a bad publisher list should fail fast so a config error aborts rather than the agent quietly crawling nothing.
Validate shape and types without normalizing the payload The guards reject a malformed value but return it unchanged, doing no trimming or normalization Keeping the boundary a pure check leaves URL normalization to the single Redis key builder that hashes a URL for deduplication, so trimming lives in one place rather than being half-applied at the edge. This matches the consumer-boundary validation intent recorded for HNT-2487.
Allow a blank publisher and excerpt on a corpus item Accept an empty string for those two fields while requiring the others non-empty An empty excerpt is a real production state for a few genuine corpus items, so rejecting a blank excerpt would drop valid live articles. A blank publisher is different: production publishers are non-empty because the Corpus API back-fills an empty publisher with a hostname fallback on create, so allowing a blank here is defensive robustness against malformed or dev-seeded input rather than a legitimate production value. It stays harmless because publisher is an opaque pass-through the crawler echoes back to updateApprovedCorpusItem unchanged, so a blank never overwrites a real stored value.
Check status and language for string shape only, not the exact union value Require a non-empty string and trust the producing agent for the value Rejecting a value outside today's union would break a rolling deploy the moment a new language or status is added, against the additive-only evolution the Article Crawler tech spec calls for.
Reject a non-positive interval_minutes or refresh_interval_minutes Guard both against zero and negatives at the boundary, beyond the type check A non-positive interval makes every page look perpetually stale to the discovery worker, and the optional refresh window gates re-fetch in the article worker, so a zero or negative value would silently defeat deduplication.

@mmiermans
mmiermans force-pushed the claude/hnt-2086-domain-types branch from c389458 to 5da2693 Compare July 6, 2026 17:08
@mmiermans
mmiermans force-pushed the claude/hnt-2086-message-validation branch from 0b762e8 to b59806c Compare July 6, 2026 17:08
@mmiermans mmiermans changed the title feat(HNT-2086): validate inbound crawl messages and the publisher list feat(HNT-2487): validate inbound crawl messages and the publisher list Aug 6, 2026
@mmiermans

Copy link
Copy Markdown
Collaborator Author

Remove the interval_minutes validation with the field

We are replacing the per-page crawl interval with a single global setting, read by the scheduler for its due-check and by the discovery worker for its page freshness check. The field leaves CrawlArticleDiscoveryMessage in #19, so the validation for it leaves here: the requireNumber(obj, 'interval_minutes', label) call and the positive-value guard come out of validateCrawlArticleDiscoveryMessage, and the returned object narrows to { url, contexts }. validatePublisherList picks that up for free, since it validates each page through the same function. The three specs covering a missing, non-numeric, and non-positive interval_minutes go with it, along with the field in the VALID_DISCOVERY fixture.

Nothing is lost by dropping the boundary check. The guard exists because a non-positive interval would make every page look perpetually stale, and that risk moves to config load, where both services already fail fast on a non-numeric value and can range-check a single constant once at startup instead of on every message. The refresh_interval_minutes handling in validateCrawlArticleMessage is a separate field on a separate message and is untouched.

The reason for the change: all 3,399 pages in publishers.json carry interval_minutes: 20, with no page differing, so the per-page capability has never been exercised. What it does do is make the one value we genuinely tune expensive to tune, since a cadence change means re-running the generator and reviewing a diff across every page entry. The architecture doc calls the page crawl interval the main lever on Zyte volume, so it is both the most likely setting to change and the hardest to change.

Two consequences worth being explicit about. Page jobs no longer carry their own window, so a job already on the queue is evaluated against the worker's current setting rather than the value in force when it was enqueued; that is acceptable for a uniform, rarely changed value but it is a real property being given up. And the capability is deferred rather than dropped: if per-page or per-topic differentiation is wanted later, and the FPS "Zyte Cost Optimization" page does propose slower crawling for evergreen topics and overnight, the field and its validation can be reintroduced deliberately then.

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