Skip to content

W0B: gate RSS publication on canonical feed status#49

Closed
Pigbibi wants to merge 2 commits into
mainfrom
feat/pert-weekly-workflow-w0b
Closed

W0B: gate RSS publication on canonical feed status#49
Pigbibi wants to merge 2 commits into
mainfrom
feat/pert-weekly-workflow-w0b

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Scope

Fresh W0B from merged main/H2C; no reuse of frozen PRs.

  • Integrate H0 hardened XML parsing and H2C canonical status into the existing RSS producer.
  • Build each feed outcome from its single parsed row snapshot.
  • Persist canonical status bytes before any producer failure is raised.
  • Upload the debug/source artifact before status validation.
  • Allow live commit/push only after canonical status readback reports success and eligibility.
  • Use explicit HEAD:refs/heads/main push with temporary credential cleanup.
  • Empty config, failed fetch, quarantine/partial status, tamper, noncanonical bytes, and count/digest mismatch fail closed.

Non-scope: weekly artifact, QAR, Pages, publisher, new sources, permissions, and generic contract abstractions.

Validation

  • focused: 28 passed
  • full: 136 passed
  • uv sync --locked --extra test: passed
  • compileall: passed
  • actionlint: passed
  • diff-check: passed
  • ruff: unavailable

Review mode: preflight then one concentrated closure; no merge/rerun/poll by worker.

Co-Authored-By: Codex <noreply@openai.com>
@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

🤖 Codex PR Review

🚫 Merge blocked: 1 serious issue(s) found in high-risk files

🚫 Blocking Issues

These issues must be fixed before this PR can be merged:

1. 🟠 [HIGH] Logic in .github/workflows/rss_source_pipeline.yml

max_items_per_feed is still taken directly from workflow_dispatch input even though the new boundary job only validates the three path inputs. A maintainer can dispatch the workflow on main with commit_outputs=true and max_items_per_feed=1 (or any other value), get an eligible canonical status for that truncated snapshot, and push materially different live data to main without tripping the new gate. (line 78)

Suggestion: Treat max_items_per_feed as a canonical input for publishable runs: either validate it equals the expected production value (currently 50) in validate-workflow-boundary, or ignore manual overrides whenever COMMIT_OUTPUTS=true.

ℹ️ Other Findings

1. 🟡 [MEDIUM] Reliability in scripts/validate_fetch_status.py

The publish gate only validates source_fetch_status.json and fetch_exit.txt; it never re-reads source_items.csv and compares its row count/digest to the canonical status being approved. Because of that, a later mutation or stale overwrite of source_items.csv after fetch_rss_sources() wrote the status would still be copied into data/live/ as long as the status file remains canonical and fetch_exit is 0. This leaves the claimed tamper/count-digest fail-closed behavior unenforced at the actual publication boundary. (line 16)

Suggestion: Pass source_items.csv into the validator and recompute the canonical row count/digest immediately before publish, failing if it does not match accepted_row_count and aggregate_row_digest (and, if needed, the per-feed digests) from the canonical status.


Review by Codex PR Review bot • PR

Co-Authored-By: Codex <noreply@openai.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a5199f82d9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +53 to +56
env:
FEEDS_PATH: ${{ github.event.inputs.feeds_path || 'config/free_rss_feeds.csv' }}
ALIASES_PATH: ${{ github.event.inputs.aliases_path || 'config/core_us_equity_aliases.csv' }}
WATCHLIST_PATH: ${{ github.event.inputs.watchlist_path || 'data/live/political_watchlist.csv' }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Validate max_items_per_feed before publishing

This boundary check pins the path inputs, but the fetch step still uses github.event.inputs.max_items_per_feed. In a manual main run with commit_outputs=true, setting that input to 1 (or another smaller/negative value) can still produce a canonical all-accepted status and then publish truncated live CSVs, so this publication-affecting input should also be pinned or live commits should ignore it.

Useful? React with 👍 / 👎.

Comment on lines +283 to +286
decision_kind = None
if status_output:
write_fetch_status(status_output, statuses, item_count=len(rows))
decision_kind = write_fetch_status(status_output, statuses)
if decision_kind is DecisionKind.HARD_FAIL:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve all-failed errors without status output

When callers use --continue-on-feed-error without --status-output, decision_kind stays None, so an all-feeds-failed run now writes an empty CSV and exits successfully. The previous behavior raised on all configured feeds failing regardless of status output; keep that failure detection independent of the optional status file so downstream jobs don't treat an empty scrape as a successful fetch.

Useful? React with 👍 / 👎.

Comment on lines +118 to +119
- name: Validate canonical feed status readback
run: python scripts/validate_fetch_status.py --status data/output/rss_source_pipeline/source_fetch_status.json --fetch-exit data/output/rss_source_pipeline/fetch_exit.txt

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Verify published rows against status digests

This readback gate only receives source_fetch_status.json and fetch_exit.txt, so it cannot catch a mismatch between the canonical status and the source_items.csv that is copied in the publish step. If source_items.csv is truncated or altered after write_fetch_status but before publish, an otherwise eligible status still passes here; pass the CSV into validation and recompute the accepted row count and aggregate digest before allowing the live copy.

Useful? React with 👍 / 👎.

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