fix(facebook/feed): exclude Messenger bleed and scope extraction to news feed - #1
Merged
lawmight merged 3 commits intoSep 5, 2026
Conversation
When Messenger drawer or chat UI is open in the locked Chrome profile, feed extraction was matching role=article nodes and thread chrome instead of top-level news-feed posts. Symptoms: empty authors, '-' metrics, and content like 'Message sent …' / 'Enter'. - Scope container discovery to [role=feed] (fallback: role=main) - Detect and skip Messenger/chat regions and thread copy - Prepare step redirects /messages URLs and closes Messenger drawer - Reject authorless rows that still look like Messenger bleed - Add fixture + tests for the reported failure mode Co-authored-by: Tom Coustols <tom.coustols@tcdynamics.fr>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's GuideThe PR fixes Facebook feed extraction returning embedded Messenger UI by validating and navigating to the home news-feed surface, restricting scrolling and DOM discovery to [role="feed"], filtering chat-shaped rows and containers, and failing with explicit errors when no genuine feed is available. Sequence diagram for scoped Facebook feed extractionsequenceDiagram
participant CLI as facebook_feed_command
participant Page as BrowserPage
participant Facebook as FacebookHome
participant Feed as NewsFeedSurface
CLI->>Page: goto(feedNavigationUrl())
Page->>Facebook: Load cache-busted home URL
CLI->>Page: evaluate(buildPrepareFeedScript())
Page->>Facebook: Redirect messages route or dismiss chat chrome
CLI->>Page: evaluate(buildSurfaceCheckScript())
Page-->>CLI: ready, feedFound, messengerDom
alt feed surface unavailable
CLI-->>CLI: throw CommandExecutionError
else feed surface ready
CLI->>Page: evaluate(loadFeedPosts)
Page->>Feed: Scroll feed element only
CLI->>Page: evaluate(buildFeedExtractScript(limit))
Page->>Feed: Discover and filter articles
Feed-->>CLI: Genuine feed rows
CLI-->>CLI: Remove bleed rows or throw CommandExecutionError
end
Flow diagram for Facebook feed surface validationflowchart TD
A[facebook feed command] --> B[Cache-busted facebook.com home navigation]
B --> C{Messages route?}
C -->|Yes| D[Throw wrong_surface error]
C -->|No| E{role=feed exists?}
E -->|No| F[Throw no_feed error]
E -->|Yes| G[Scope scrolling and extraction to role=feed]
G --> H{Rows contain genuine post evidence?}
H -->|No, chat bleed only| I[Throw Messenger/chat bleed error]
H -->|Yes| J[Return filtered news-feed posts]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Live smoke still returned Messenger thread rows because evaluation could run against the active Messenger tab or home DOM without a feed landmark. - Select a non-/messages facebook tab when available - Navigate with a cache-busting home URL to avoid extension fast-path skips - Poll until [role=feed] is present; fail on /messages routes - Refuse extraction when Messenger chrome is visible without a feed landmark - Drop authorless metric-less rows and error when only bleed remains Co-authored-by: Tom Coustols <tom.coustols@tcdynamics.fr>
FB desk confirmed bleed happens with only a New Tab open — embedded chat chrome on facebook.com home, not a leftover Messenger tab. - Require [role=feed] on facebook.com home; never scrape role=main/body there - Reject home rows without author/post-menu evidence (chat bubbles lack both) - Detect embedded chat via Send-without-Share, thread copy, broader selectors - Scroll only inside role=feed; fail when home feed landmark is missing - De-emphasize tab selection; focus navigation + DOM surface gating Co-authored-by: Tom Coustols <tom.coustols@tcdynamics.fr>
lawmight
marked this pull request as ready for review
September 5, 2026 19:12
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="clis/facebook/feed.js" line_range="652" />
<code_context>
- await page.goto(FACEBOOK_HOME, { settleMs: 4000 });
- } catch (err) {
+
+ const surface = await ensureNewsFeedSurface(page);
+ if (!surface?.ready) {
+ if (surface?.isMessagesRoute) {
</code_context>
<issue_to_address>
**issue (broader_impact):** A logged-out Facebook session is rejected as a generic `CommandExecutionError` before `buildFeedExtractScript()` can classify the login page as `auth`, so callers no longer receive the existing `AuthRequiredError` contract.
**Triggers:** When navigation lands on `/login` or `/checkpoint`, which has no `[role="feed"]` landmark.
**Suggested fix:** Evaluate the authentication surface before requiring the feed landmark, or have `ensureNewsFeedSurface()` return an explicit auth status for login/checkpoint pages.
</issue_to_address>
### Comment 2
<location path="clis/facebook/feed.js" line_range="123-128" />
<code_context>
+ // Preparation is best-effort; surface checks own final classification.
+ }
+
+ lastSurface = await readFeedSurface(page);
+ if (lastSurface?.ready) return lastSurface;
+
</code_context>
<issue_to_address>
**issue (bug_risk):** A rejected `page.evaluate()` from `readFeedSurface()` escapes `ensureNewsFeedSurface()` without being converted to `CommandExecutionError`, producing an untyped browser/transport failure instead of the command's documented navigation error.
**Triggers:** When the browser disconnects, the page closes, or surface evaluation fails after navigation succeeds.
**Suggested fix:** Wrap `readFeedSurface(page)` and the polling wait in the same error-to-`CommandExecutionError` handling used for `page.goto()`.
```suggestion
try {
lastSurface = await readFeedSurface(page);
if (lastSurface?.ready) return lastSurface;
if (typeof page.wait === 'function') {
await page.wait(1);
}
} catch (err) {
throw new CommandExecutionError(
`Failed to navigate to facebook feed: ${err instanceof Error ? err.message : err}`,
'Check that facebook.com is reachable and the browser extension is connected.',
);
}
```
</issue_to_address>
### Comment 3
<location path="clis/facebook/feed.js" line_range="327" />
<code_context>
+ const hasSendOnly = Array.from(root.querySelectorAll('[aria-label]'))
+ .some((el) => /^(Send|发送)$/i.test(labelOf(el))) && !hasShare;
+ const fullText = textOf(root);
+ if (isMessengerOrChatText(fullText)) return true;
+ if (hasSendOnly && !hasPostMenu) return true;
+ const blocks = visibleBlocks(root);
</code_context>
<issue_to_address>
**issue (bug_risk):** Any genuine post whose text contains `Message sent` followed by a date is classified as Messenger content because the check runs against the entire container text, so the valid post is discarded before extraction.
**Triggers:** When a real post quotes or discusses a message containing text such as `Message sent March 1, 2026`.
**Suggested fix:** Apply thread-copy checks to dedicated chat controls or require multiple Messenger signals instead of rejecting the whole container on one phrase in its content.
```suggestion
```
</issue_to_address>Sourcery assessment
Approval pending. 3 findings to address first.
Blocking findings: clis/facebook/feed.js:652, clis/facebook/feed.js:128, clis/facebook/feed.js:327
| await page.goto(FACEBOOK_HOME, { settleMs: 4000 }); | ||
| } catch (err) { | ||
|
|
||
| const surface = await ensureNewsFeedSurface(page); |
There was a problem hiding this comment.
issue (broader_impact): A logged-out Facebook session is rejected as a generic CommandExecutionError before buildFeedExtractScript() can classify the login page as auth, so callers no longer receive the existing AuthRequiredError contract.
Triggers: When navigation lands on /login or /checkpoint, which has no [role="feed"] landmark.
Suggested fix: Evaluate the authentication surface before requiring the feed landmark, or have ensureNewsFeedSurface() return an explicit auth status for login/checkpoint pages.
Comment on lines
+123
to
+128
| lastSurface = await readFeedSurface(page); | ||
| if (lastSurface?.ready) return lastSurface; | ||
|
|
||
| if (typeof page.wait === 'function') { | ||
| await page.wait(1); | ||
| } |
There was a problem hiding this comment.
issue (bug_risk): A rejected page.evaluate() from readFeedSurface() escapes ensureNewsFeedSurface() without being converted to CommandExecutionError, producing an untyped browser/transport failure instead of the command's documented navigation error.
Triggers: When the browser disconnects, the page closes, or surface evaluation fails after navigation succeeds.
Suggested fix: Wrap readFeedSurface(page) and the polling wait in the same error-to-CommandExecutionError handling used for page.goto().
Suggested change
| lastSurface = await readFeedSurface(page); | |
| if (lastSurface?.ready) return lastSurface; | |
| if (typeof page.wait === 'function') { | |
| await page.wait(1); | |
| } | |
| try { | |
| lastSurface = await readFeedSurface(page); | |
| if (lastSurface?.ready) return lastSurface; | |
| if (typeof page.wait === 'function') { | |
| await page.wait(1); | |
| } | |
| } catch (err) { | |
| throw new CommandExecutionError( | |
| `Failed to navigate to facebook feed: ${err instanceof Error ? err.message : err}`, | |
| 'Check that facebook.com is reachable and the browser extension is connected.', | |
| ); | |
| } |
| const hasSendOnly = Array.from(root.querySelectorAll('[aria-label]')) | ||
| .some((el) => /^(Send|发送)$/i.test(labelOf(el))) && !hasShare; | ||
| const fullText = textOf(root); | ||
| if (isMessengerOrChatText(fullText)) return true; |
There was a problem hiding this comment.
issue (bug_risk): Any genuine post whose text contains Message sent followed by a date is classified as Messenger content because the check runs against the entire container text, so the valid post is discarded before extraction.
Triggers: When a real post quotes or discusses a message containing text such as Message sent March 1, 2026.
Suggested fix: Apply thread-copy checks to dedicated chat controls or require multiple Messenger signals instead of rejecting the whole container on one phrase in its content.
Suggested change
| if (isMessengerOrChatText(fullText)) return true; |
This was referenced Sep 5, 2026
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
2 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.
Description
Live smoke of
opencli facebook feed --limit 5on a locked Chrome profile was returning exit 0 with five rows of embedded Messenger/thread UI (emptyauthor,-metrics, "Message sent February 26, 2026" / Teiki Travels bleed).FB desk isolation update: browser inspection showed only a New Tab open — no Messenger/chat tabs. Bleed is DOM-side: embedded chat chrome on
facebook.comhome, not wrong-tab selection.Type of Change
What changed
Home surface gating (primary fix)
facebook.comhome, extraction is strictly scoped to[role=feed"]— never falls back torole=mainordocument.body(where embedded chat columns live)ensureNewsFeedSurface()navigates with cache-busted home URL and polls until[role=feed"]exists[role=feed"]no_feedwhen home loads without the feed landmarkEmbedded chat exclusion
Message sent …,Enter,SendwithoutShare)data-pagelet,data-testid,aria-label*=Conversation, etc.)Explicitly ruled out
Tests
role=mainwithoutaria-label="Messenger"no_feedwithout feed landmark, chat column besiderole=feed, cache-busted navigationChecklist
npm test -- clis/facebook/feed.test.js)tom.coustolsrecommended after mergeExpected live behavior
[role=feed"]yetno_feederror (non-zero)CommandExecutionError(non-zero), not exit 0 junkSummary by Sourcery
Keep Facebook feed extraction isolated to the home news-feed surface and reject Messenger bleed instead of returning invalid rows.
Bug Fixes:
Enhancements:
Tests:
Summary by cubic
Fixes
facebook feedreturning Messenger/thread UI rows (empty authors,-metrics) instead of news-feed posts when Messenger is open. Extraction now runs on the home news-feed surface only, filters chat DOM, and fails with guidance when only Messenger bleed remains./messagesFacebook tab and navigates with a cache-busted home URL to bypass extension fast-path skips.[role="feed"], falling back to[role="main"]only off the home surface; home extraction never scrapesrole=main/body./messages/t/links, and thread copy./messagesURLs and closes an open Messenger drawer before scrolling.-metrics instead of returning junk.Written for commit 4f1d7b0. Summary will update on new commits.