Elijah hash7/feature/mutation api observer#206
Conversation
Feature/mutation api observer
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…ment-200 test: add LoopDetectorShim._detectApiShortcut and unit tests for all shortcut paths
|
Reviewed this branch and pushed a follow-up fix: What changed:
Validation:
|
There was a problem hiding this comment.
Pull request overview
This PR implements a "mutation API observer" (issue #189) across both the Chrome and Firefox builds. A background webRequest.onBeforeRequest listener records the XHR/fetch requests each tab fires into a per-tab, in-memory buffer exposed via globalThis.__webbrainApiRequests. When the agent's loop detector flags a repeated click/click_ax, the new Agent._detectApiShortcut checks whether each click triggered the same background request and, if so, augments the [LOOP DETECTED] nudge with a suggestion to call fetch_url directly instead of clicking again.
Changes:
- Added a per-tab API request observer (capped at 40 entries, cleaned up on tab removal) in both background scripts, plus the
webRequestpermission in both manifests. - Added
Agent._detectApiShortcutand wired it into_checkLoop's repeat-loop warning in both builds. - Added
_detectApiShortcutunit tests (and a mirroredLoopDetectorShimmethod) totest/run.js, including a chrome/firefox parity assertion.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/chrome/src/background.js | Adds the per-tab webRequest observer populating globalThis.__webbrainApiRequests. |
| src/firefox/src/background.js | Mirror of the Chrome observer using browser.webRequest. |
| src/chrome/src/agent/agent.js | Adds _detectApiShortcut and the API-shortcut loop warning branch. |
| src/firefox/src/agent/agent.js | Mirror of the agent change. |
| src/chrome/manifest.json | Adds the webRequest permission. |
| src/firefox/manifest.json | Adds the webRequest permission. |
| test/run.js | Adds shim method + tests for _detectApiShortcut, including build parity. |
The main concern is a logic flaw in _detectApiShortcut: a single recorded request can be attributed to multiple overlapping click windows, so matches can reach >= 2 from one request, producing a false-positive shortcut that contradicts the documented "strict matching only" intent (the new test passes despite counting one request three times). The same defect exists in all three mirrored copies.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let candidate = null; | ||
| let matches = 0; | ||
| for (const clickTs of clickTimes) { | ||
| const hit = apiRequests.find(r => | ||
| r.ts >= clickTs && r.ts <= clickTs + WINDOW_MS && | ||
| (!candidate || (r.url === candidate.url && r.method === candidate.method)) | ||
| ); | ||
| if (!hit) continue; | ||
| if (!candidate) candidate = { url: hit.url, method: hit.method }; | ||
| matches++; | ||
| } |
| let candidate = null; | ||
| let matches = 0; | ||
| for (const clickTs of clickTimes) { | ||
| const hit = apiRequests.find(r => | ||
| r.ts >= clickTs && r.ts <= clickTs + WINDOW_MS && | ||
| (!candidate || (r.url === candidate.url && r.method === candidate.method)) | ||
| ); | ||
| if (!hit) continue; | ||
| if (!candidate) candidate = { url: hit.url, method: hit.method }; | ||
| matches++; | ||
| } |
| let candidate = null; | ||
| let matches = 0; | ||
| for (const clickTs of clickTimes) { | ||
| const hit = apiRequests.find(r => | ||
| r.ts >= clickTs && r.ts <= clickTs + WINDOW_MS && | ||
| (!candidate || (r.url === candidate.url && r.method === candidate.method)) | ||
| ); | ||
| if (!hit) continue; | ||
| if (!candidate) candidate = { url: hit.url, method: hit.method }; | ||
| matches++; | ||
| } |
|
Addressed Copilot review What changed:
Validation:
|
|
Correction to my earlier GET-only follow-up: I pushed Rationale:
What changed:
Validation:
|
No description provided.