Improve MCP serve polling and attachment extraction reliability - #32556
Closed
aydnOktay wants to merge 3 commits into
Closed
Improve MCP serve polling and attachment extraction reliability#32556aydnOktay wants to merge 3 commits into
aydnOktay wants to merge 3 commits into
Conversation
Add UTC Z-aware timestamp parsing for EventBridge polling, deduplicate extracted attachments, and normalize MEDIA tag paths by stripping trailing punctuation, with regression tests covering each behavior. Co-authored-by: Cursor <cursoragent@cursor.com>
Run pull-request test jobs against changed and related test files while keeping full-suite runs on main pushes, and narrow supply-chain critical pattern scanning to reduce fixture-driven false positives. Co-authored-by: Cursor <cursoragent@cursor.com>
alt-glitch
added
type/bug
Collaborator
|
This PR bundles fixes from three existing open PRs by the same author into a single submission:
All three fixes target mcp_serve.py. Consider whether this superset PR supersedes the individual ones or should be split. |
teknium1
reviewed
Jul 13, 2026
teknium1
left a comment
teknium1
left a comment
Contributor
There was a problem hiding this comment.
Thanks for combining the attachment regressions and adding focused coverage. The attachment work remains relevant, but this branch needs selective salvage against current main.
Problems
- Current
mcp_serve.py:442-472now uses the state.db routing index introduced by747386ecf; the PR's sessions.json mtime/cache polling context is obsolete and should not be restored. mcp_serve.py:182in the PR strips!,),], quotes, and other legal filename suffixes unconditionally. That can turn a validMEDIA:path into a different path.- The Z-timestamp change is redundant: supported Python is
>=3.11(pyproject.toml:20), and current main already callsdatetime.fromisoformatfor EventBridge timestamps (mcp_serve.py:500).
Suggested changes
- Port only the attachment behavior onto the current state.db-based polling implementation.
- Preserve literal filenames ending in punctuation, with regression coverage for that case, before adding punctuation normalization.
Automated hermes-sweeper review.
| media_pattern = re.compile(r'MEDIA:\s*(\S+)') | ||
| for match in media_pattern.finditer(text): | ||
| path = match.group(1) | ||
| path = match.group(1).rstrip(".,;:!?)]}\"'") |
Contributor
There was a problem hiding this comment.
This unconditionally changes valid local filenames such as MEDIA:/tmp/report! or MEDIA:/tmp/a]. Please use a delimiter-aware MEDIA syntax, or otherwise preserve literal punctuation and add coverage for those valid path suffixes.
teknium1
added
sweeper:risk-message-delivery
Contributor
Author
|
Closing to concentrate review on a single linked product bug: #89979. |
Contributor
Author
|
Closing to concentrate review on a single linked product bug: #89979. |
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
3 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.
This PR strengthens mcp_serve.py by making EventBridge polling reliable for ISO-8601 UTC timestamps with a Z suffix, deduplicating extracted attachments while preserving order, and normalizing MEDIA: paths so trailing sentence punctuation is not captured as part of file paths; it adds focused regression coverage in tests/test_mcp_serve.py for each behavior and includes PR CI hardening so checks run targeted related tests instead of the full unrelated suite, while supply-chain scanning avoids recurring fixture-driven critical false positives.