Add producer source freshness evidence#52
Conversation
Co-Authored-By: Codex <noreply@openai.com>
🤖 Codex PR Review🚫 Merge blocked: 2 serious issue(s) found in high-risk files ⚖️ Codex Review Arbitration🚫 block: Both current findings remain valid against the cumulative diff. First, 🚫 Blocking IssuesThese issues must be fixed before this PR can be merged: 1. 🟠 [HIGH] Bug in
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72af71565c
ℹ️ 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".
| headers = { | ||
| name: value | ||
| for name in ("Date", "Last-Modified") | ||
| if (value := response_headers.get(name)) is not None | ||
| } |
There was a problem hiding this comment.
When an HTTP server returns duplicate Date or Last-Modified headers, urllib exposes them via HTTPMessage.get_all(), but this code calls get() and keeps only the first value. That bypasses the downstream duplicate-header guard in _headers(), so a response with a fresh first Last-Modified and a stale/invalid second one can still be marked eligible instead of failing closed; the fetch boundary should preserve or reject duplicates before building evidence.
Useful? React with 👍 / 👎.
| if len(channels) != 1: | ||
| _fail("source_freshness_invalid") | ||
| if kind == "rss_channel_last_build_date": | ||
| values = [child.text.strip() for child in channels[0] if child.tag == "lastBuildDate" and child.text] |
There was a problem hiding this comment.
When an RSS feed contains an empty <lastBuildDate> element and also has a fresh HTTP Last-Modified, this filter drops the element because child.text is false, records the higher-priority RSS signal as absent, and allows fallback to HTTP evidence. The contract says present-but-invalid higher-priority signals fail closed, so a malformed feed can be marked eligible instead of rejected; detect the element's presence before parsing its text.
Useful? React with 👍 / 👎.
| parsed = dt.datetime.fromisoformat(value.replace("Z", "+00:00")) | ||
| if parsed.tzinfo is None: | ||
| parsed = parsed.replace(tzinfo=dt.UTC) |
There was a problem hiding this comment.
If a freshness signal supplies an ISO timestamp with no timezone, such as Atom <updated>2026-07-13T11:00:00</updated> or a Last-Modified value in that form, this fallback treats it as UTC and can mark the source eligible. These provider freshness signals need an explicit timezone to be interpreted unambiguously; otherwise a malformed/local-time higher-priority signal is accepted instead of failing closed as invalid.
Useful? React with 👍 / 👎.
Co-Authored-By: Codex <noreply@openai.com>
|
PERMANENT_FREEZE: the absolute-final closure is exhausted and both current HIGH findings are valid. Preserve the legacy body-only fetch API and require strict per-signal timestamp grammars in any fresh successor. No second closure, rerun, thread bypass, or merge on this PR. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8464a9ebe1
ℹ️ 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".
| if type(value) is not str or not value or len(value) > MAX_HEADER_VALUE_BYTES: | ||
| return False, None | ||
| try: | ||
| parsed = email.utils.parsedate_to_datetime(value) |
There was a problem hiding this comment.
When a provider sends a Last-Modified or RSS lastBuildDate value that starts with a valid date but includes trailing garbage, such as Sun, 12 Jul 2026 11:00:00 GMT junk, email.utils.parsedate_to_datetime() still returns the leading timestamp. That lets a present-but-invalid freshness signal be canonicalized and selected, so the source can be marked eligible instead of failing closed; validate that the whole signal matches an accepted date format before using it.
Useful? React with 👍 / 👎.
|
|
||
|
|
||
| def fetch_url(url: str) -> bytes: | ||
| return fetch_url_with_metadata(url)[0] |
There was a problem hiding this comment.
In the existing RSS ingestion path, fetch_rss_sources() still uses fetch_url as its default fetcher, so this delegation now makes a bytes-only fetch fail on duplicate Date or Last-Modified headers before parsing a feed that otherwise could be consumed. For feeds or proxies that duplicate these freshness headers, the legacy source pipeline becomes a failed fetch even though it does not use the freshness evidence; keep the duplicate-header rejection on the metadata/freshness path without changing fetch_url semantics.
Useful? React with 👍 / 👎.
1 participant
Scope
Fresh producer-boundary source freshness evidence only.
Validation
Review mode: preflight then one concentrated review; no merge/rerun/poll requested.