feat(sources): RSS provider 4/4 — server wiring, acceptance suites, fixture corpus, M2 docs

[gracexmatin]

Phase 4 of 4 stacked PRs splitting #175 (plan Tasks 15-20). Base: rss-m1-phase3; retarget after its merge. The tree after this PR is byte-identical to branch add_RSS_Feed — all four phases together are exactly that branch's content.

Scope

• Server wiring: DataSourceType::Rss, the typed rss: block on data_sources[] (parses without the feature; registration is what names the missing build capability), catalog-hierarchy enforcement, rss sources rejected as job destinations (read-only). The CLI needs no wiring: after feat(cli): reframe skardi CLI as thin HTTP client for skardi-server #170 it is a thin HTTP client and rss sources are configured on the server.
• Fixture corpus (Task 17): committed feed documents across dialects + field-reviewed golden Markdown outputs — the regression ratchet that turns a feed-rs upgrade's behavior drift into a test failure instead of a silent SQL change
• Mock-HTTP integration suite (Task 18): acceptance criteria at the SQL surface — pruning, TTL, 304 revalidation, per-feed fault isolation, the short-IN-list disjunction case pinned from both sides. (No reserved-range SSRF tests here: egress is exercised via the injected-policy seam tests in phases 1 and 3, since OSS ships AllowAll.)
• E2E composition (Task 19): federated join across sources, idempotent two-INSERT archive pipeline, citability after the live window rolls, candle-embedding composition (skips gracefully without SKARDI_TEST_EMBED_MODEL)
• M2 docs (Task 20): docs/rss.md (states OSS does not sandbox fetch egress — operator/infra-layer or injected-policy control), sample context YAML, README source-table row. Plus the Skardi Cloud egress governance design doc.

Verification

cargo test -p skardi --features rss passes (1054 tests). git diff against add_RSS_Feed is empty.

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 89.83051% with 60 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
crates/skardi/src/sources/providers/rss/corpus.rs	86.56%	54 Missing ⚠️
crates/server/src/config.rs	98.26%	3 Missing ⚠️
crates/skardi/src/jobs/executor.rs	0.00%	3 Missing ⚠️

📢 Thoughts on this report? Let us know!

[BtXin]

Requesting changes on final server integration and contract reconciliation. The RSS catalog registers successfully, but the agent discovery endpoint cannot expose either inner table or the bundled qualified semantics. The final documentation also narrows the normative no-raw-HTML guarantee without reconciling the design acceptance criteria. At stack level, the observability section promises provider metrics for fetches, retries, bytes, rows, duration, and terminal categories, but the implementation currently emits tracing events only; either add the instruments or narrow that requirement before merge.

[BtXin]

[P1] Enumerate the RSS catalog tables in /data_source

Adding RSS as a pathless source is not enough for discovery: the code below still asks the default datafusion.public schema for one table named after the source (news). RSS actually registers news.main.feeds and news.main.items, so the endpoint returns a fake news table with an empty schema. The qualified news.main.* entries in the bundled semantics overlay therefore never reach agents or skardi schema, missing the design requirement that agents discover the health and absence semantics from the schema. Please make this endpoint enumerate catalog-mode inner tables, merge semantics by qualified name, and add an endpoint-level RSS test.

[BtXin]

[P2] Reconcile this narrowed claim with the normative design

This section correctly documents what the implementation does, but the normative RSS design and its acceptance criteria still promise that no raw HTML is stored. The converter intentionally preserves HTML-looking attribute text, and text/plain bodies pass through byte-exact, so consumers following only the design may enable raw HTML rendering under a guarantee the provider does not provide. Please choose one contract before landing: escape such text to uphold the design, or update the normative design, acceptance criteria, semantics, and security wording to state the narrower guarantee consistently.

[BtXin]

Sweep 2 (head 34515ef). Prior-thread status:

• [P1] pipeline_handlers.rs /data_source discovery — NOT FIXED. get_data_sources still builds tables: vec![TableInfo { name: data_source.name, … }] and get_table_schema still resolves only datafusion.public.<name>, so an RSS source lists a single table named after the source with an empty schema (the lookup fails into the warn + empty-Vec arm); feeds/items are never enumerated. The only in-diff change is the | DataSourceType::Rss => None pathless arm. There is also no server-level test of GET /data_source with an RSS source configured — such a test would have caught this.
• [P2] docs/rss.md:644 vs normative design — PARTIALLY reconciled. rss.md is now internally coherent and test-pinned ("no HTML tag survives as markup", not "no raw HTML stored", with plaintext_typed_content_is_stored_byte_exact_tags_included). But docs/superpowers/specs/2026-07-22-rss-feed-support-design.md is byte-identical to phase 3 and still asserts the stronger claim in six places (e.g. line 94 "stored content carries no executable markup", line 534 / AC18 "Markdown with no raw HTML", and line 558, which describes rss.md as saying the exact sentence rss.md now disclaims). The cross-document half of the ask is still due.

Other notes:

• [P3] cancellation_stops_further_fetches flake window. The only margin between "mock observed request 1" and query.abort() is the scripted 500ms response delay — a CI scheduler stall >~490ms lets feed a complete, releases the single permit, launches /b.xml, and fails the paths().len() == 1 assertion. Every other timing test in the suite has verified headroom; this one could use a longer scripted delay or an explicit abort-before-release barrier.
• Stack hygiene: this branch (via phase 3) is based on the pre-fix phase 1 — the proxy/OPML fix commits are not in its history; needs the stack rebase.

Verified clean this round: feature gating compiles both ways with the featureless registration error named and pinned; Rss correctly absent from writable/connection-string arms and present in catalog-supported; docs-vs-code spot checks all pass (retry table, redirect cap, Retry-After clamp, cache budget, MAX_ERROR_CHARS, deny_unknown_fields, categories NULL semantics); acceptance tests assert real wire behavior with zero real-network access and ephemeral ports; lock poisoning handled correctly; all new .unwrap()s are in test code.

[BtXin]

[P2] The "Try it" command cannot run at all

Three independent failures: (a) crates/cli/Cargo.toml has no [features] section, so cargo aborts with "Package skardi-cli does not have the feature rss" before anything executes; (b) the Query subcommand accepts only -e/--sql, -f/--file, --max-rows, --table — there is no --ctx flag; (c) the CLI is a thin HTTP client ("It holds no local query engine", main.rs header), so "this works offline and fetches nothing" is impossible — a running server loaded with this context is required. It also contradicts docs/rss.md's own header ("the CLI is a thin HTTP client; only the server needs the feature"). This is the doc's only runnable example — please rewrite it as: start the server with this context (--features rss on the server crate), then skardi-cli query -e "SELECT name, last_status FROM news.main.feeds".

[BtXin]

[P3] Convention + stale architecture claim

skardi::sources::providers::rss::register_rss_tables(…) is a full inline crate path — the project rule is use-imports at the top (a #[cfg(feature = "rss")] use … satisfies it; yes, the pre-existing documents arm has the same flaw, but new code shouldn't copy it). The comment above this arm also claims the enforcement point is "shared with the CLI" — the CLI has had no registration path since #170 made it a thin HTTP client; only the server front-end reaches register_with_policy.

[gracexmatin]

Cross-referencing a phase-1 review item (PR #180, round-3 review): this 512 literal is the other half of a cross-file contract. Phase 1 defines MAX_ERROR_CHARS = 512 and has a test (max_error_chars_is_the_number_the_docs_publish) that pins the constant to the literal 512 precisely so this doc and the code can't silently drift apart — neither the doc nor the semantics.yaml column description (also 512, semantics.yaml:85) is Rust, so nothing else would catch a change to one but not the other.

At the phase-1 merge point these docs didn't exist yet, so the phase-1 references were softened to "published later in this stack" rather than citing exact line numbers. Now that the docs land here: if anyone ever changes this 512 (here, in semantics.yaml, or the MAX_ERROR_CHARS constant), all three move together, and the phase-1 test is what enforces it. Flagging so the tie is visible from this side too.