feat(sources): add RSS feed support - #175
Closed
gracexmatin wants to merge 95 commits into
Closed
Conversation
…er demo to supplement The main spec now specifies the complete project: a native read-only type: rss source (two fixed tables, per-feed TTL cache + conditional requests, partition-per-feed execution, visible per-feed degradation, typed config, rss_scan UDTF, html_to_markdown UDF, compatibility strategy) and the auto_news_base skill. The Python-fetcher demo is repositioned as a non-gating exploratory probe whose pain log seeds the parser-compatibility fixture corpus.
…d-mapping annotations to RSS design - Decision 16: detect declared vs parsed dialect, verify spec-required fields, surface deviations in queryable feeds.conformance_notes - Decision 17: dialect -> unified-schema mapping as documented contract, shipped in docs/rss.md and as semantics-overlay column descriptions - feeds table: +dialect, +dialect_declared, +conformance_notes - Field Mapping table (RSS 2.0 / RSS 1.0 / Atom 1.0 / JSON Feed) - Testing + acceptance criteria extended accordingly
…ence loop into main spec
…rlier-draft comparisons Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- system-context diagram + 5-step interaction walkthrough before any decision - decisions grouped into 7 logical blocks (data model, freshness, execution, fault tolerance/conformance, configuration, parsing, surfaces) - v1 retained as normative reference for schemas, alternatives, acceptance
…tecture - decisions become one-line statements; rationale moves to topical sections (Scan Execution, Freshness and Caching, Parsing/Sanitation/Conformance, ...) - mermaid architecture + sequence diagrams replace ASCII art - fully standalone: schemas, field mapping, failure modes, acceptance criteria all in-document; adds Research Findings and Observability
…l lifecycle The archive the skill renders gains a wire-faithful news_items table alongside news_chunks, so search results stay citable after entries fall out of the live window and history can be re-chunked/re-embedded. Adds a skill-lifecycle contract (subscription edits are config-only; parameter changes rebuild from retained content; re-runs are idempotent and diff-first) plus matching testing/acceptance criteria. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…provider in composition diagram Goals now promise post-assembly maintainability and citation durability; Decisions gains a "Downstream contract" group indexing the two-table archive, subscription-agnostic artifacts, and idempotent rendering. The composition diagram makes the provider and the engine/user-space boundary explicit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per review: every feed Skardi reads is declared in configuration first. Registration is zero-I/O and the feeds health table covers the preview need. Recorded in Non-goals, Decisions, and Future Extensions; the skill's subscribe-time preview becomes register-then-verify. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The conversion is a bridge between the provider's wire-faithful HTML and markdown-aware chunking, so it lives inside chunk()'s existing mode dispatch as a pre-pass — no new user-facing function registered. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Absorb review feedback on silent incompleteness after exhausted retries: - Stamp every items row with window_status (fresh | revalidated | stale-error) so stale-window serving is visible in the result stream - Prescribe the anti-join absence check in SQL Interfaces and state the polling contract explicitly (nobody polls feeds; consumption is reactive) - Make feeds a pure observation surface (never fetches) and re-arm the TTL on failure too (negative caching, bounded failure fuse); two-step the preview/verify flows accordingly (scan items, then read feeds) - End sync's ingest pipeline with a closing health SELECT whose rows are the response (empty = all healthy, report-only); declare the multi-statement pipeline extension as an M3 dependency - Record result-level warnings as a deferred alternative - Extend unit/mock-HTTP/e2e tests and acceptance criteria (4, 13, 14) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…on contract Feed URLs are agent-authored configuration, so a subscription URL is attacker-influenceable: a prompt-injected agent could add an internal or cloud-metadata address as a "feed". The spec was silent on this. Egress (SSRF): the fetcher default-denies hosts resolving into reserved ranges (loopback/link-local/private/CGNAT/ULA), re-validates on each redirect hop, and connects to the validated IP against DNS rebinding. New logic kept local to the RSS fetcher — no existing helper filters by resolved IP (llm_extract gates by scheme, not address). An egress allowlist for intentional internal feeds is deferred to Future Extensions. Stored content: item content stays wire-faithful HTML; the sanitization obligation is stated to sit with any consumer that renders it — Skardi neither executes nor sanitizes it. Threaded through Decisions, Fetcher, a new Security section, Failure Modes, Acceptance, Testing, Repo Shape, and Documentation Commitments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…duals Follows the egress/SSRF pass (0b000e2) with the remaining content-axis threats surfaced in review. Parse-time DoS: the response-size cap is enforced on the decompressed stream (a compressed payload cannot inflate past it) and the parser runs with DTD/entity expansion disabled (billion-laughs class). New Failure Modes rows and fixture/mock-HTTP tests cover both. Content trust residuals the provider cannot fix, stated as contracts and delegated to the consumer: - LLM consumption: feed content is untrusted input to any reasoning agent; prompt-injection mitigation is the consumer/harness's (treat-as-data, least-privilege, human-confirm side effects). Provider contributes consequence-limiting only (egress + config-diff visibility). - Content authenticity: guid/link/author/published are feed-asserted and unverifiable; item identity is feed-scoped and archive ingest is append-only, so a feed cannot collide across feeds or rewrite history. Enclosure/link fetching: the provider never fetches them; any future feature that does must route through the same egress policy. The bundled semantics overlay now flags content/summary as untrusted so an agent discovers the trust boundary from the schema. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Marking feed content "untrusted" is not a mitigation — external content is untrusted by definition, an injected agent ignores a label, and the semantics overlay is for data meaning, not trust posture. Dropped the schema-level untrusted-content flag and the "untrusted / delegate to consumer" framing. Each content-risk note now names the actual defense and the layer that owns it: - XSS: contextual output encoding at the render point (the standard, sink-dependent defense the provider cannot perform for the consumer). - Prompt injection: containment, not prevention — least-privilege reading agent + deterministic action gating; egress and human-visible config edits are the provider-side rails. - Authenticity: feed-asserted fields are unverifiable, but identity is feed-scoped and archive ingest is append-only (structural), with provenance surfaced. Parse-time DoS remains the one content risk the provider bounds mechanically (unchanged). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…icit A bare LIMIT (no ORDER BY) short-circuits partition launch, so it serves a nondeterministic subset of feeds — and because fetch and health refresh ride on the scan, pruning also bounds the side effects: un-launched feeds keep aging past the one-TTL observation bound a full scan restores. Documented rather than mechanically "fixed": LIMIT is the caller's work bound, and honoring it is the point — this is requested truncation, not concealed failure. The result's feed column is the scan's coverage manifest; completeness-sensitive reads (the absence check, sync's ingest) use no bare LIMIT; ORDER BY … LIMIT is Top-K and prunes nothing. Absence diagnosis gains a third case (not-scanned, next to legitimately-empty and dead), and the semantics overlay teaches the bare-LIMIT caveat so agents learn it from the schema alone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
feed-rs exposes the parsed dialect (Feed.feed_type: Atom/JSON/RSS0/RSS1/ RSS2) but family-level only, and its XML dispatch keys on exactly root element + version attribute — so in-band declared-vs-parsed comparison is tautological. dialect's value set drops to feed_type granularity (atom loses the version split), dialect_declared keeps the version detail (atom-0.3 vs atom-1.0), and the conformance mismatch axis moves to Content-Type vs parsed family. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…re-faithful "Repair naked ampersands" was four words wide open to the classic corruption footgun: text-level rewriting mangles CDATA and already-valid references. Three layers close it: - Behavioral spec: the repair is lexical, not textual — CDATA sections, comments, and PIs pass through untouched; only an & that cannot open a valid reference is rewritten (the five predefined entities and numeric charrefs are left alone; undefined HTML names become &-escaped). - Staged retry: sanitation runs as a cumulative ladder — re-encode, strip control characters, repair ampersands — re-parsing after each rung and stopping at the first success, so a feed receives the minimal repair set and conformance_notes records exactly that set. - Contract: every rung is a byte-level no-op on well-formed input, and fixtures rescued by sanitation pin their expected extracted content — parse-success alone would miss silent corruption. Acceptance criterion 16. wire-faithful is now defined at its storage-contract site: faithful to the HTML fragment the parser extracted — transport encoding removed at extraction, repairs pre-extraction and queryable, nothing altered after — a fragment-level fidelity claim, not byte-identity with the wire, covering items and the news_items archive alike. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…uously updating" The motivation promised continuity without saying who supplies it; the scheduler cut lived only in Non-goals. Now named at the promise and at the operating surface: the provider fetches only when read — query-time freshness is self-serve — while unattended continuity (archive capture, health observation) is the caller's cadence, a scheduled skardi sync running faster than the fastest feed's window roll. The missed-cadence cost becomes a Failure Modes row — entries that scroll out between scans are a permanent capture gap, diagnosed by aging feeds.last_fetch — and the rendered skill README teaches the scheduling note. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-repo The archive DDL and pipelines render from skardi-skills yet embed the engine's feeds/items column names, and a rendered pipeline, once in a user's context, outlives both repositories — version skew is this design's default failure mode, not an edge case (unlike Open Connector's compiled-in source packs). Four layers close it: - Declared surface (v1): feeds/items evolve additively; an enumerated list of breaking changes bumps an integer rss surface version. - Visible at registration (v1): the active version reaches logs and table metadata — the Open Connector convention (visible, never silent). - Pinned consumer fixture (v1): a canonical render is vendored into the engine's fixture corpus as a representative consumer; acceptance criteria 6/11 run against it in engine CI, so engine-initiated breakage fails in-repo with no cross-repo plumbing. - Load-time handshake (M3, with the skill): rendered pipelines stamp requires: rss/<version>; the loader refuses a mismatch with an error naming both versions and the re-render remedy. Standing cross-repo CI rejected: HEAD×HEAD only — protects neither released pairings nor already-rendered artifacts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ider
Reverse the wire-faithful HTML storage decision: items.content/summary
(and the archived news_items rows) now hold Markdown produced by a
deterministic HTML-to-Markdown pass that runs provider-internally at
extraction time, between field extraction and Arrow conversion.
Why: the live consumers of item content are prompts, fts5, and
embeddings - nobody renders feed HTML - so markup fidelity taxed every
read (token noise, markup in indexes, per-query conversion) while
serving no one. Converting once at extraction makes results
prompt-ready, lets pipelines use the already-implemented
chunk('markdown') mode, and narrows the rendering surface: the
converter emits no raw HTML, so stored content carries no executable
markup. The old design is recorded under Alternatives Considered with
the accepted loss stated (source HTML not retained; history cannot be
re-converted, only re-chunked/re-embedded).
The chunk('html') bridge mode loses its only in-spec consumer and moves
to Future Extensions; M2 sheds its engine change. New acceptance
criterion 18 pins the converter contract (no raw HTML out, determinism,
plain-text passthrough) via hostile-markup fixtures.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Code review (fix round 1) flagged unknown_fields_are_rejected as only checking .is_err(), which would also pass on an unrelated parse error. Strengthen both cases to assert the serde_yaml error text names the misspelled field, matching the bar open_connector/config.rs's own deny_unknown_fields tests hold themselves to. Co-Authored-By: Claude <noreply@anthropic.com>
The Task 1 review caught a weak assertion this plan mandated verbatim: unknown_fields_are_rejected checked only is_err(), which passes when an unrelated error fires. Fix the offending snippet and lift the rule into Global Constraints so the remaining tasks inherit the bar open_connector/config.rs:607-619 already sets. Also record that cargo is not on this machine's default PATH. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Task 17's corpus measured both against feed-rs 2.4 rather than assuming: RSS 1.0 items do not key on rdf:about -- feed-rs never reads it, so the guid is the item's <link>. The Field Mapping table said otherwise, and Task 20 copies that table into docs/rss.md, so the error had to be fixed at the source. The fixture now gives the two attributes different values so a regression cannot be masked by coincidence. An Atom 0.3 document reaches the Atom parser by root-element name but its namespace maps to NS::Unknown, so it parses as an empty feed: zero items, no error, two missing-required-field notes. Worth stating, since a user subscribing to an Atom 0.3 feed sees an empty table rather than an error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…iteria 1-4, 13, 15
19 tests driving real SQL against a registered `news` catalog whose feeds live
on `MockFeedServer`, one per acceptance criterion: zero-network registration,
full-scan vs pruned-scan fetch counts, the TTL and the conditional-GET/304
path, dead-feed isolation with a `stale-error`-stamped window, a `feeds` scan
staying request-free right after a failure (and the failure fuse keeping an
`items` scan off the dead feed), and reserved-range refusal both directly and
through a redirect -- with 169.254.169.254 named in its own test.
Plus the bounds: the decompressed-size cap against the committed 8 KiB /
8 MiB gzip bomb, a per-request timeout isolating a slow feed, `Retry-After`
honoured inside the scan, the LIMIT launch gate versus a Top-K, cancellation
stopping unlaunched fetches, `count(*)` row accuracy through the empty
projection, and the prescribed absence-check anti-join.
Two additions close gaps a review found in Task 13: a conjunction of two
prunable feed predicates intersecting at the SQL surface (both operand orders),
and a `feed_url` shared by two differently-named subscriptions visiting both.
In-crate, because registration's test seam is `#[cfg(test)] pub(crate)` -- it
takes the loopback-allowing egress policy the mock needs, and widening it to a
public entry point would be the production-reachable private-network switch the
design forbids.
The suite surfaced one real gap, recorded rather than fixed (no production
change in a test task): a short `IN` list does not prune. DataFusion's
`ShortenInListSimplifier` rewrites `col IN (...)` into an OR chain for lists of
three or fewer values, so `WHERE feed IN ('a','c')` reaches the provider as an
unprunable `BinaryExpr` and every subscription is fetched. Rows stay correct --
the filter is applied above the scan -- but the fetches are not pruned. Pinned
from both sides: `a_long_in_list_prunes_to_its_members` (4 values, prunes) and
`a_short_in_list_is_rewritten_and_prunes_nothing` (2 values, right rows, wrong
fetch count). See the task report.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… repair note The `encoding_latin1_mislabeled` corpus row asserts `sanitation: reencoded-to-utf8` and the row count, which say a rung changed bytes — not that it changed them correctly. A rung that rescued the document into mojibake (a different single-byte sniff) or into U+FFFD replacements would leave the row green. Adds a branch test alongside `control_chars_fixture_keeps_the_text_the_control_byte_split`, pinning the recovered `é` in three stored values: the channel title, the item title, and the item summary (two 0xE9 bytes in one field). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t archive, citability after window roll The provider is a pure protocol adapter; history, chunking and search are user-space composition. This suite proves that composition works with the primitives that already ship — a writable sqlite source, `INSERT … SELECT`, and the `chunk` UDF — and nothing RSS-specific beyond which catalog the rows come from. Five tests: * `federated_join_items_with_sqlite` — `items` joined against a sqlite `feed_meta` table on `feed`. * `archive_ingest_is_idempotent_and_survives_window_roll` — the two-INSERT archive: an anti-join into `news_items`, then chunk rows from `news_items`. Re-running both writes zero rows (asserted on the INSERTs' own reported counts, under `ttl_seconds = 0` so the second run is a live re-fetch rather than a cache hit). Shrinking the feed then shows the dropped entry's title, link and published still queryable from the archive. * `sync_closing_health_report_shape` — the closing SELECT over `feeds` in all three states: never-attempted, all-healthy (empty), one-degraded. * `subscription_add_is_config_only` — a second SessionContext over the same mock and the same archive file, three feeds instead of two; the added subscription costs exactly one new archive row. * `parameter_change_rebuild_from_retained_content` — delete `news_chunks`, re-chunk at 600/60 from retained content with the live window returning 500 and the request count unchanged. In-crate, for the reason `integration_tests` is: the registration seam is `#[cfg(test)] pub(crate)` because the mock binds loopback, and widening it would be the production-reachable private-network switch the design forbids. DataFusion has no `WITH ORDINALITY`, so `chunk_idx` comes from a window function over a plain `UNNEST(chunk(...))` subquery. Chunk-to-index assignment is unspecified without an ORDER BY, so the tests assert the indices are dense `0..n-1` per `(feed, guid)` and that the stored texts are exactly the set a fresh `chunk()` expansion produces — never which text got which index. The candle variant is `#[ignore]`d and additionally `#[cfg(feature = "candle")]`, following the repo's precedent that no model exists on disk in a default CI run; the default-run tests store a NULL embedding. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Configuration, freshness/caching, politeness, the Field Mapping table, conformance semantics, the tolerance floor, the egress policy, content handling, pipeline examples and troubleshooting. Every SQL block is lifted from a passing test: the archive DDL, both ingest INSERTs and the closing health report from composition_tests.rs; the pruning, absence-check and degradation queries from integration_tests.rs. Records the facts measured during implementation that a reader would otherwise get wrong: an Atom 0.3 document parses as an *empty* feed with no error, RSS 1.0 identity comes from <link> rather than rdf:about, the window cache is process-lifetime state so a restart costs a full 200 per feed, max_concurrent is per process, and `feed IN (…)` does not prune below four values (stated as a current limitation, not intent). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The overlay's column descriptions carry the Field Mapping provenance and
the window_status freshness semantics; the two table descriptions carry
the absence-check pattern and the bare-LIMIT caveat, so an agent
discovers both health signals from the schema alone. Loaded through the
standard <ctx_dir>/semantics/ mechanism.
The sample context registers offline against unresolvable .invalid
hosts, verified with:
cargo run -p skardi-cli --features rss -- query \
--ctx docs/sample_data/rss_context.yaml \
--sql "SELECT name, last_status FROM news.main.feeds"
which returns one `never` row per subscription and fetches nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`WHERE feed IN ('a','c')` fetched every subscription. DataFusion rewrites a
short `IN` list into an `OR` chain of equalities before the predicate reaches
the provider (datafusion-optimizer 52.5.0,
`src/simplify_expressions/inlist_simplifier.rs:38-56`, the non-negated fold at
`:82-90`, threshold `THRESHOLD_INLINE_INLIST` = 3 at
`src/simplify_expressions/expr_simplifier.rs:111`), so the classifier's
`InList` arm was unreachable from SQL below four values and the `OR` chain
landed `Unsupported`. The rows were right; the cost was HTTP requests to
upstream hosts.
`feed_filter` now recognises a disjunction whose every leaf is prunable over
one feed column, and prunes to the union of their literals. It recurses through
itself, so nested `Or` and an `InList` leaf fall out, and an `And` under an `Or`
— which has no arm — rejects the whole disjunction rather than pruning to the
half it understood. It remains the single classifier both `prune_feeds` and
`supports_filters_pushdown` consult.
Mixing the two feed columns is refused: `feed = 'a' OR feed_url = '…'` stays
`Unsupported`. The union across both would be sound, and is left out to keep
the rule "one feed column per disjunction" and `FeedFilter` a single key.
`a_disjunction_mixing_feed_and_feed_url_does_not_prune` pins the refusal by its
request count.
Tests assert both obligations per shape — the pruned feed list and the
classification — so a failure to classify can never read as "matches nothing".
`a_short_in_list_is_rewritten_and_prunes_nothing` asserted the old behaviour and
is rewritten and renamed, still at two values. Mutating
`supports_filters_pushdown` to over-claim `Exact` fails two tests on *wrong row
counts*, not merely on shape.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The TTL lives on a feed's observation and its rows live on its window, and `MemoryFeedCache` drops the second while keeping the first — window eviction is specified to preserve the observation (cache.rs:36-42) and is driven by *other* feeds' `record_success` calls against the byte or entry budget. Nothing about that eviction touches the evicted feed's timer, so "within TTL" and "has rows" are independent. The engine treated within-TTL as a cache hit unconditionally, so a feed in that state served zero rows with no HTTP request while `feeds` reported it `fresh` with a non-zero `item_count` and a NULL `last_error`, for the rest of the TTL. That is exactly the combination `record_not_modified`'s doc rejects on the 304-after-eviction path (cache.rs:216-223) — a healthy-looking feed whose `items` are empty with no column explaining why — and it is a silent capture gap for an archive pipeline, with `last_fetch` fresh so the prescribed diagnostic does not fire. `window_lost` now classifies it as a miss and the scan fetches. `FeedStatus::window_status_str` is the discriminator: `Some` for the three statuses that claim a servable window, `None` for `Never`/`Error`, so the negative cache still short-circuits and a dead feed is not re-poked. The resulting fetch has no validators to send and is an unconditional GET, which is the only request that can refill the window; commented at the call site. Two tests: one evicts a window under a byte budget measured off a real one and asserts the next within-TTL serve issues exactly one unconditional request and serves rows — the seam between `cache.rs`'s eviction test (which never runs the engine) and this module's TTL tests (which never evict) — and one pinning that a window-less `error` inside its fuse still refetches nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…riters that escaped the cap `engine.rs`'s module doc and docs/rss.md both stated the property absolutely — "no text taken from the feed body, with one exception" — and the test placed its sentinel only in character data. A reviewer placed sentinels in four other positions and all four reached the column: an Atom `<content type="X">`, a `</X>` mismatched end tag, and two JSON type mismatches. Probing wider adds a mismatch inside an `xhtml` body and every JSON member a string can mistype. The plan's error-redaction decision already keeps element names, MIME strings and version strings, so the defect is the wording, not the behaviour: what was chosen was "no body *prose*", and the property now says that, names the four channels measured at feed-rs 2.4.0 / quick-xml 0.41.0, and states plainly that the cap is the only bound on a quoted fragment's length. Measured and now documented: `serde_json` renders a type-mismatched string verbatim and unabbreviated, so a feed can put ~1 KB of arbitrary text of its own choosing in `tags`/`authors`/`attachments`/`size_in_bytes` and see MAX_ERROR_CHARS of it stored. No path was found by which prose sitting where prose belongs reaches the column — element text, a correctly typed JSON string, and an unparseable date all produce no error, and two new shapes pin that an entry `<summary>` and a JSON `title` stay out even when the document fails structurally elsewhere. `parse_failure_last_error_quotes_structure_not_prose` replaces the one-sided test with a ten-shape table where each shape declares whether its sentinel is expected to survive, asserted in both directions, so a leak fails and so does a kept fragment silently vanishing. Its counter guard now expects nine errors. MAX_ERROR_CHARS and truncate move to error.rs, which all three writers can depend on without inversion, and the two that escaped the cap are routed through it: parse.rs's `debug` line logged the dependency's untruncated reason (bounded only by max_response_bytes), and conformance.rs built `dialect_declared`'s `unknown:<root>` from a raw root element name — measured 4,104 characters from a 4 KB name, retained in a FeedObservation that MemoryFeedCache never byte-bounds. cache.rs's evicted-window literal is routed through it too, so the column has one bound and no writer outside it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
I1 — "no raw HTML" was false in two shapes, and convert.rs's own module doc already said so. An attribute value's `<` survives unescaped (`[t](# "<script>")`, pinned at convert.rs), and a plain-text-typed value passes through byte-exact, so `<content type="text"><script>…</content>` stores a literal `<script>…`. docs/rss.md documented that passthrough two sections earlier and contradicted itself. Narrowed to "no HTML *tag* survives as markup" in docs/rss.md and the semantics overlay, with both counterexamples spelled out, plus a corpus fixture (`plaintext_typed_markup.xml`) pinning the hostile `type="text"` shape as exact stored strings — so it is a reviewed contract rather than an accident. I2 — the prescribed absence check does fetch. Its `items` side carries no `feed` predicate, so it is a full scan and every subscription past its TTL is fetched; `absence_check_pattern_works` only passes because it warms the cache first, as its own comment says. Also noted: the two tables are scanned in one query with no ordering guarantee, so the check can report `never` for a feed it just fetched. I3 — the semantics overlay still claimed only `IN` lists of four or more values prune, which 4d93c59 falsified without updating the overlay. Since the overlay is what an agent reads when planning a query, it was steering agents away from the cheapest shape. Now states all three prunable shapes including the `OR` chains it omitted entirely, and the mixed-column limitation. I4 — RSS 2.0 `items.updated` is not NULL. feed-rs copies `<pubDate>` into it (pinned by corpus.rs with a source citation), so `WHERE updated IS NULL` never matches an RSS 2.0 feed while RSS 1.0 genuinely is NULL. Both docs said NULL for both dialects; they now say the two differ and give the workable predicate. I6 — `max_concurrent` is per source, not per process, and is not a per-host bound at all: the semaphore lives on RssEngine and one engine is built per registered source. Corrected in docs/rss.md's field table, Politeness section and rate-limiting entry, in both config samples, and in config.rs's own comments, with the worst case a host can see stated as a product of three terms. Implementation unchanged — narrowing it is a design decision. Folded in: the 64 MiB window budget and its eviction consequences (`:860` referred to "the bounded cache" the document never defined); the two warn-only clamps; `categories` is NULL not `[]`; what the `debug` log carries, including feed URLs; a third cause of `never` (a serve dropped at the scan deadline writes no health state, so it reads `never` with a NULL `last_error` forever); the `error` + non-NULL `item_count` row the diagnosis table lacked; `:410`'s incomplete `last_error` literal; `application/json`/`text/json` in the family-naming Content-Type list; that `unknown:<root>` is the XML fallback only and a JSON Feed declaring version 2 yields NULL; `<template>` content is lost; and 512 now cites MAX_ERROR_CHARS as its provenance rather than standing alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tion warning, and type last_status M8 — nothing tested `docs/rss/semantics.yaml` at all, yet it is the file an agent reads when planning a query, so drift between it and the surface is drift in what agents are told. `the_bundled_semantics_overlay_matches_the_two_schemas` parses it as a real `SemanticsFile`, checks `kind: semantics` (without which the loader skips it), and asserts its column set matches `feeds_schema()`/`items_schema()` in both directions plus a length check for duplicates. It would have caught both I3 and I4, which review found instead. Verified live by renaming one overlay column and watching it fail. I7 — AC4's "a tracing warning is emitted — nothing silent" clause was unpinned; deleting the `warn!` in `degrade` would have been invisible. `a_degraded_feed_emits_a_warning_naming_the_feed_and_reason` asserts exactly one WARN per degraded serve, its `source`/`feed` fields, and that its `error` field is byte-identical to the `feeds.last_error` the same serve recorded — so the log line and the column cannot drift apart. A healthy serve is asserted to add none. The `Interest` trap `exec.rs:470-482` documents from experience applies here and is handled: `tracing` caches a callsite's `Interest` globally on first use, so every other test in this module that reaches the `degrade` warn callsite now holds a `capture_events` guard — ten of them — or the assertion above could have been silently emptied by test ordering. Confirmed non-vacuous two ways: removing the `warn!` fails the test, and the full 1026-test lib binary was run three times with the assertion passing throughout. M1 — `FeedsRow.last_status` is now `FeedStatus` rather than `&'static str`, so `feeds.last_status` gets the same typed-domain protection `with_window_status` already gives `items.window_status`: `FeedStatus` is the only place those strings are spelled, and a typo cannot reach the column from a call site. One production caller, which was already going through `as_str()`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…other than themselves MAX_ERROR_CHARS, MAX_TTL and the engine's failure fuse were each asserted only against their own definition, so their values were free to drift. Mutation proved it: 512 -> 200, 512 -> 999, dropping `.min(MAX_TTL)`, and swapping `failure_fuse(self.ttl)` for a hardcoded 30s all left 252 tests green. - `MAX_ERROR_CHARS` is a published contract (docs/rss.md:974 and docs/rss/semantics.yaml:85 both state 512, and neither is Rust). Pinned at the definition site and, end to end, as the literal length of the stored `last_error` in `a_huge_json_version_is_still_capped`. - `MAX_TTL`'s clamp and its warning, at `ttl_seconds: u64::MAX` — written to the shape of `exec.rs`'s `an_absurd_scan_timeout_is_clamped_rather_than_overflowing` so the pair reads as a pair. Plus `arm`'s `checked_add` fallback directly, which is the only place that branch is reachable. - The engine's failure fuse, via a cache that records the `armed_until` it is handed: at `ttl_seconds: 900` the fuse is 225s, strictly between the 30s floor and the 300s ceiling, so the armed instant discriminates the real expression from either clamp arm. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
All three were checkable and wrong, and all three were verified against the code and by mutation rather than re-reasoned from the names. - `parse.rs`'s post-sanitation-guard comment said a lowercase `<!doctype` evades the raw-bytes guard. It does not: `refuse_internal_dtd` matches the keyword with `starts_with_ignore_ascii_case` (sanitize.rs:118, helper at :147), so the pre-sanitation pass already catches it. The re-run exists for the two cases a rung *reveals* — the split `<!DOCTY\x01PE` and a UTF-16 body. The corresponding test's docstring narrated an evasion its input never performs; it now says which guard the input reaches, and the three cases are relabelled so the two genuine evasions are the numbered ones. - Deleting the pre-sanitation guard left the suite green, because sanitized bytes that still carry a live subset reach the second pass by construction. Pinned from the other side: a body declaring `utf-16` while holding a byte that is not valid UTF-8 gets transcoded by rung 1 into text where `<!DOCTYPE r [` no longer exists, so only the raw-bytes pass can refuse it. The test asserts both halves, and says plainly that the mangled shape is not itself a live entity-expansion threat — what it pins is that the refusal happens before the rungs. - `limit_stops_launching_fetches` claimed the post-permit launch gate. Mutation (both gate halves defeated) leaves it passing while four `exec.rs` tests fail; what it actually pins is DataFusion dropping the un-polled partition streams. Docstring rewritten to claim that, pointing at exec.rs:531/:572 for the gate. - `never_fetched_failure_yields_zero_rows_and_error_status`'s negative-cache assertion ran against `feeds_row`, which is synchronous (engine.rs:615) and so cannot fetch whatever the TTL says. Moved to a second `serve_feed`, which makes it real: it now fails when the failure arms to `now`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…h ceiling's boundary Both constants were unpinned across a range that spans real behaviour changes. - `RETRYABLE_STATUSES`: only 429, 500 and 503 were ever exercised, so dropping 502 and 504 left the suite green even though the module doc names all five. `retries_exhaust_to_status_error` now loops the five literals (spelled out, not read from the array); 502/504 are what a CDN in front of a dead feed host returns. Same test, same name — parameterised rather than duplicated. - `MAX_HTML_DEPTH`: every degradation test nests ~1000 and every positive conversion is depth 1, so 100 could move anywhere in ~[20, 400] silently. A tightening to 20 would strip Markdown from ordinary nested feed content; a loosening to 400 walks toward the 500-600 htmd stack-overflow zone the constant exists to avoid. Pinned at exactly the boundary: open-tag depth 100 converts, 101 degrades. Note the marker element counts toward the depth — the fixture guard asserts the scanned depth so it cannot drift off the boundary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
abbccdda
self-requested a review
July 31, 2026 18:07
Contributor
|
This PR has grown really large, could you break it down into stacked PRs so that we can review smaller chunks more effectively? @gracexmatin |
…ess doc builds CI's Docs job runs `cargo doc --no-deps -p skardi` with default features and `-D warnings`. The `opml` module only exists under the `rss` feature, so the [`opml`] link on ResolvedSubscription — whose own sentence explains exactly that feature gap — was unresolvable there and failed the job. Plain code formatting says the same thing without asserting a link target that half the builds don't have. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
main's #170 rewrote the CLI as a thin HTTP client with no local engine, so this branch's CLI-side rss wiring (the `rss:` block on LocalDataSource, the `rss` arm of register_source, and the crate's `rss` feature) has nothing left to attach to and is dropped with it — rss sources are configured on the server, whose wiring merged cleanly. Union resolutions for the skardi crate's Cargo.toml (our rss feature + main's object_store) and the README source table (main's updated Open Connector / Documents rows + our RSS row). docs/rss.md's build-flag example updated to the thin-client invocation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ed, matching the live-resource convention CI runs every `--ignored` test under `--all-features` with no embedding model on disk, so the `.expect()` on SKARDI_TEST_EMBED_MODEL panicked the whole integration step. Live-resource tests in this repo skip with an eprintln when the resource is absent (documents' LibreOffice tests, sqlite-vec's SQLITE_VEC_PATH tests); this one now does the same. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
Superseded by a 4-phase stacked split of this same content (the final tree is byte-identical to this branch):
Merge in order; each next PR retargets to main after its base merges. Branch kept for reference. |
gracexmatin
added a commit
that referenced
this pull request
Aug 3, 2026
…own, parsing, Arrow schemas, TTL cache Content chain of the RSS/Atom provider (plan Tasks 5-10): - byte-level sanitation ladder: encoding repair (incl. a lying declaration over valid UTF-8), control-char stripping, internal-DTD refusal run on BOTH raw and sanitized bytes (case-insensitive) - HTML→Markdown via htmd behind a lexical MAX_HTML_DEPTH=100 pre-scan — htmd's DOM walk recurses unboundedly and a ~7KB feed with 600 nested divs aborts the process on a 2MiB Tokio stack; above the ceiling the content degrades to tag-stripped text - dialect conformance (RSS 0.9x/1.0/2.0, Atom, JSON Feed) + the feed-rs parse driver; field extraction per the spec's Field Mapping table - fixed Arrow schemas: items (17 cols), feeds (15 cols), surface_version metadata; batch builders routed through the FeedStatus domain - per-feed TTL cache: LRU-evicted windows, health observations never evicted, HTTP validators bundled with the window they validate - docs/rss/semantics.yaml ships here because a schema test pins the overlay against the two schemas Part 2 of 4 stacked PRs replacing #175 (base: rss-m1-phase1). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
gracexmatin
added a commit
that referenced
this pull request
Aug 3, 2026
…own, parsing, Arrow schemas, TTL cache Content chain of the RSS/Atom provider (plan Tasks 5-10): - byte-level sanitation ladder: encoding repair (incl. a lying declaration over valid UTF-8), control-char stripping, internal-DTD refusal run on BOTH raw and sanitized bytes (case-insensitive) - HTML→Markdown via htmd behind a lexical MAX_HTML_DEPTH=100 pre-scan — htmd's DOM walk recurses unboundedly and a ~7KB feed with 600 nested divs aborts the process on a 2MiB Tokio stack; above the ceiling the content degrades to tag-stripped text - dialect conformance (RSS 0.9x/1.0/2.0, Atom, JSON Feed) + the feed-rs parse driver; field extraction per the spec's Field Mapping table - fixed Arrow schemas: items (17 cols), feeds (15 cols), surface_version metadata; batch builders routed through the FeedStatus domain - per-feed TTL cache: LRU-evicted windows, health observations never evicted, HTTP validators bundled with the window they validate - docs/rss/semantics.yaml ships here because a schema test pins the overlay against the two schemas Part 2 of 4 stacked PRs replacing #175 (base: rss-m1-phase1). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
gracexmatin
added a commit
that referenced
this pull request
Aug 6, 2026
…own, parsing, Arrow schemas, TTL cache Content chain of the RSS/Atom provider (plan Tasks 5-10): - byte-level sanitation ladder: encoding repair (incl. a lying declaration over valid UTF-8), control-char stripping, internal-DTD refusal run on BOTH raw and sanitized bytes (case-insensitive) - HTML→Markdown via htmd behind a lexical MAX_HTML_DEPTH=100 pre-scan — htmd's DOM walk recurses unboundedly and a ~7KB feed with 600 nested divs aborts the process on a 2MiB Tokio stack; above the ceiling the content degrades to tag-stripped text - dialect conformance (RSS 0.9x/1.0/2.0, Atom, JSON Feed) + the feed-rs parse driver; field extraction per the spec's Field Mapping table - fixed Arrow schemas: items (17 cols), feeds (15 cols), surface_version metadata; batch builders routed through the FeedStatus domain - per-feed TTL cache: LRU-evicted windows, health observations never evicted, HTTP validators bundled with the window they validate - docs/rss/semantics.yaml ships here because a schema test pins the overlay against the two schemas Part 2 of 4 stacked PRs replacing #175 (base: rss-m1-phase1). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
gracexmatin
added a commit
that referenced
this pull request
Aug 7, 2026
…own, parsing, Arrow schemas, TTL cache Content chain of the RSS/Atom provider (plan Tasks 5-10): - byte-level sanitation ladder: encoding repair (incl. a lying declaration over valid UTF-8), control-char stripping, internal-DTD refusal run on BOTH raw and sanitized bytes (case-insensitive) - HTML→Markdown via htmd behind a lexical MAX_HTML_DEPTH=100 pre-scan — htmd's DOM walk recurses unboundedly and a ~7KB feed with 600 nested divs aborts the process on a 2MiB Tokio stack; above the ceiling the content degrades to tag-stripped text - dialect conformance (RSS 0.9x/1.0/2.0, Atom, JSON Feed) + the feed-rs parse driver; field extraction per the spec's Field Mapping table - fixed Arrow schemas: items (17 cols), feeds (15 cols), surface_version metadata; batch builders routed through the FeedStatus domain - per-feed TTL cache: LRU-evicted windows, health observations never evicted, HTTP validators bundled with the window they validate - docs/rss/semantics.yaml ships here because a schema test pins the overlay against the two schemas Part 2 of 4 stacked PRs replacing #175 (base: rss-m1-phase1). Co-Authored-By: Claude Fable 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
2 participants
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.
Implements first-class RSS/Atom support (
type: rss): one configured subscription list registers as a read-only catalog exposing<name>.main.feeds(per-subscription fetch health) and<name>.main.items(live union of current entries), fetched at scan time through a per-feed TTL cache with HTTP conditional requests, partition-per-feed execution, and in-bandwindow_statusfreshness stamps.Follow-up to the merged design PR #163. Implementation lands task-by-task on this branch, following the committed plan (
docs/superpowers/plans/2026-07-27-rss-feed-provider-m1-m2.md, 20 TDD tasks — M1 provider core + M2 docs). Each task is committed only after a two-stage review (spec compliance + code quality).In this PR so far
Design spec v2 refinements since #163 —
docs/superpowers/specs/2026-07-22-rss-feed-support-design.mdis the normative spec: Markdown item storage (converted once at extraction), default-deny egress (SSRF) policy, sanitation ladder + dialect conformance contract, LIMIT-pruning semantics, versioned engine↔skill surface.Implementation plan — 20 tasks with exact schemas, interfaces, and test lists; M3 (auto_news_base skill, pipeline statement-sequences, version handshake) deferred to a follow-up plan.
Task 1 — provider foundation (
crates/skardi/src/sources/providers/rss/):RssConfig/FeedSubscriptionwith spec defaults (ttl 900s, concurrency 6, request timeout 10s, scan deadline 60s, 5 MiB response cap, self-identifying UA) and a zero-I/Ovalidate()— mutually-exclusivefeeds/opml, URL scheme checks, effective-name uniqueness, non-zero bounds, unknown-field rejectionRssErrortaxonomy,RSS_SURFACE_VERSION = 1,rssCargo feature (config compiles unconditionally so feature-off builds still parse ctx.yaml and fail with a clear error)Task progress (1/20)
rssfeature, module skeleton, typedRssConfigdocs/rss.md, README row, semantics overlay, sample ctx)Test evidence
cargo test -p skardigreen at every commit (10 new rss tests);cargo check -p skardiwith and without--features rssboth clean;cargo fmt --all -- --checkclean.🤖 Generated with Claude Code