Skip to content

Fix memory amplification in engagement rankings - #139

Closed
LIghtJUNction wants to merge 1 commit into
mainfrom
codex/propose-fix-for-memory-dos-vulnerability
Closed

LIghtJUNction wants to merge 1 commit into
mainfrom
codex/propose-fix-for-memory-dos-vulnerability

Conversation

@LIghtJUNction

Copy link
Copy Markdown
Contributor

Motivation

  • Engagement ranking could overfetch thousands of ranked IDs and materialize full post rows (including bodies) before slicing, enabling high-memory reads from unauthenticated requests.

Description

  • Add Store.post_boards_by_ids to perform a lightweight SELECT id, board FROM posts WHERE id IN (...) lookup that avoids loading post bodies (new function in src/msgd/store.py).
  • Update Handler._ranked_posts to resolve existence and per-board membership using the lightweight lookup, remove stale IDs from the ranking index as before, and only call posts_by_ids for the bounded set of selected IDs to be returned (changes in src/msgd/server.py).
  • Add a regression test RankingMemoryCase.test_ranking_only_loads_bodies_for_returned_posts to tests/test_engagement.py which simulates a 100-ID ranking and asserts only the returned six posts are materialized.
  • The change preserves existing stale-ID cleanup and board-filtering semantics while preventing large body materialization during ranking resolution.

Testing

  • Ran the focused engagement test: PYTHONPATH=src uv run pytest tests/test_engagement.py -q, which reported 1 passed, 1 skipped (skip when VALKEY_TEST_URL is not configured).
  • Ran full test suite with PYTHONPATH=src uv run pytest -q, which completed successfully with 145 passed, 1 skipped, 8 subtests passed.
  • Ran style checks using uv run ruff check src/msgd/server.py src/msgd/store.py tests/test_engagement.py and git diff --check, both of which passed.

Codex Task

Copilot AI lite review requested due to automatic review settings September 28, 2026 11:44

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
🔒 Security Review ✅ Completed 2026-09-28T11:48:11.824948Z 3487719 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@LIghtJUNction

Copy link
Copy Markdown
Contributor Author

关闭:现行 src/msg 中不存在 WebSub、持久 agent state、engagement ranking 或 mention 别名扇出,因此没有对应的无界路径可修。此关闭仅表示补丁不适用于现行代码,不代表生产已验收。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants