test(mv3): prove bounded bookmark mutation compatibility - #56
test(mv3): prove bounded bookmark mutation compatibility#56seonghobae wants to merge 16 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks 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 |
There was a problem hiding this comment.
Stale comment
Review — PR #56 bookmark mutation
Verdict: the create → get → remove lifecycle is the right compatibility slice, and the pinned-Chromium MV3 fixture job on
82403ecalready succeeded. This draft is still not merge-ready as an operator-actionable close.What is sound
- Mutation is bound to
http://127.0.0.1/page.htmlwithout userinfo.finallyalways attemptschrome.bookmarks.remove.- Raw
_error.messageis not retained.- Chrome
bookmarkspermission is not treated as an OriginWeave Agent grant.- Stacked draft on #43 /
test/mv3-downloadsis the correct dependency order.What still blocks a buyer-visible close
Bookmark failures collapse to
bookmarks: missingwith no classified stage. Downloads already emit allow-listed tokens (download-source-rejected,download-complete-ready, …). Without the same contract here, an operator cannot tell source rejection from create failure, get mismatch, or leftover residue afterremovefails.The unique commit also omits the current
chrome.bookmarksAPA 7th primary citation that the downloads lane already requires.Next action
Do not merge this draft while #43 is still the prerequisite and while bookmark failures stay unclassified. The diagnostic close is on
cursor/bc-62f8754b-f579-4115-9089-cace87f78155-206a(054708a): allow-listed bookmark stage tokens, runner sanitization, and the current Bookmarks API citation. Keep #59/#60/#61 stacked behind that close. Compatibility success remains evidence only — not Agent bookmark authority.Cannot approve: this identity authored the head. Formal approval stays with an eligible non-author reviewer if current rules require one; solo-maintainer independent-review remains on hold.
Sent by Cursor Automation: Fix Issues
| bookmarkMutationReady = false; | ||
| } | ||
| } | ||
| return bookmarkMutationReady; |
There was a problem hiding this comment.
This boolean return is the buyer-visible gap. A failed create, get mismatch, or failed remove all become bookmarks: missing with no classified stage. Mirror the downloads lane: return { ready, diagnostic } with allow-listed tokens such as bookmark-source-rejected, bookmark-create-rejected, bookmark-get-missing, bookmark-id-mismatch, bookmark-title-mismatch, bookmark-url-mismatch, bookmark-remove-rejected, and bookmark-complete-ready. Keep cleanup in finally, and if remove fails prefer bookmark-remove-rejected because leftover residue is the worse operator outcome. Do not copy _error.message, created.id, or node URLs into evidence.
| ): | ||
| with self.subTest(expected=expected): | ||
| self.assertIn(expected, worker) | ||
|
|
There was a problem hiding this comment.
The contract currently proves string presence of create/get/remove only. Add the same runner sanitization the downloads lane already has: require bookmarksDiagnostic in SURFACE_EVIDENCE_KEYS, allow-list the stage tokens, and prove that a raw Chrome error or the fixture bookmark title is reduced to unexpected before it can enter trial evidence.
There was a problem hiding this comment.
Review — exact head cea76b9e129d02a093ae237804e47ee3a4a08a6c
The new commit only realigns this draft with the downloads transport-protocol classification. The buyer-visible diagnostic gap is unchanged: exerciseBookmarkMutation still returns a boolean, so create failure, get mismatch, and leftover residue after remove all collapse to bookmarks: missing.
Prior threads remain not addressed on this head:
- allow-listed bookmark stage tokens (
bookmark-source-rejected…bookmark-complete-ready) - runner sanitization of
bookmarksDiagnostic(raw Chrome errors / fixture titles →unexpected) - current
chrome.bookmarksAPA 7th primary citation
Pinned Chrome for Testing MV3 fixture already succeeded on this head. Rust contracts and Production coverage were still in progress at review time and are not treated as passing.
Next action
Do not merge this draft. Keep #43 as the downloads prerequisite. The diagnostic close is #178 44cbfc2 (test(mv3): require bounded bookmark mutation diagnostics). Do not open a third bookmark-diagnostic pull request. Keep #59/#60/#61 stacked behind that close. Compatibility success is not OriginWeave Agent bookmark authority.
Cannot approve: this identity authored the head. Formal approval stays with an eligible non-author reviewer if current rules require one; solo-maintainer independent-review remains on hold.
Sent by Cursor Automation: Fix Issues
| bookmarkMutationReady = false; | ||
| } | ||
| } | ||
| return bookmarkMutationReady; |
There was a problem hiding this comment.
Unchanged on cea76b9. A failed create, get mismatch, and failed remove still all become bookmarks: missing. Keep the close on #178: return { ready, diagnostic } with allow-listed tokens, prefer bookmark-remove-rejected when cleanup fails, and never copy _error.message, created.id, or node URLs into evidence.
| ): | ||
| with self.subTest(expected=expected): | ||
| self.assertIn(expected, worker) | ||
|
|
There was a problem hiding this comment.
Still string-presence only. #178 is the landing for bookmarksDiagnostic in SURFACE_EVIDENCE_KEYS, allow-listed stage tokens, and proof that a raw Chrome error or the fixture bookmark title is reduced to unexpected.
Preserve the current bookmark diagnostic tree while adding exact ancestry to PR #56 head 71420d0. The diagnostic branch already contains the bookmark mutation lifecycle plus the same hardened downloads-root changes; this non-destructive merge repairs stack topology without transferring predecessor checks or reviews.
Rebuild the diagnostic lane on the exact current PR #56 tree while preserving the bounded bookmark diagnostic source/test delta. Keep the prerequisite ChromeDriver cleanup resilience and extension-authority hardening, retain the dedicated MV3 doctoring evidence, and avoid reintroducing duplicate/stale generic doctoring/TRD claims. No predecessor check or review evidence transfers.


Partial implementation of #27, stacked on live PR #43.
Buyer-visible gap
The pinned real-Chromium MV3 lane previously proved bookmark API visibility without proving a controlled mutation lifecycle. A defensible compatibility claim also needs one synthetic create/read/delete bookmark cycle without touching an ambient human profile.
Repaired live stack
Current exact prerequisite PR #43 head:
c5e33b47b7a2ade47ecf962a41220b62c96a88db.Current exact child head:
22bf82924f9bd254caf3bfa1db1891868c18a765.The prior child
2e5984a405bdfe1e41647e8e75c7197559e7e8f6had diverged after #43 advanced. The replacement tree was rebuilt from the exact current #43 tree and overlaid only the verified child semantic delta, then advanced non-destructively with the previous child as first parent and current #43 as second parent. Fresh comparison reports behind 0, merge base exactly the current prerequisite, and exactly three child-owned changed files:tests/fixtures/mv3_basic/service_worker.js;tests/test_mv3_bookmark_mutation_contract.py; andtests/test_mv3_bookmarks_history_contract.py.No prerequisite/predecessor check or review transfers.
Implemented boundary
The controlled fixture:
http://127.0.0.1/.../page.htmlsender with no embedded credentials;finallycleanup and fails the evidence if cleanup fails;Exact-current evidence
On unchanged exact head
22bf82924f9bd254caf3bfa1db1891868c18a765against exact prerequisitec5e33b47b7a2ade47ecf962a41220b62c96a88db:Manifest V3 Compatibilityrun31979956308: success;CIrun31979956373: success;Rust contractsjob95245034398: repository contracts, formatting, workspace checks, full tests, strict Clippy and rustdoc success;Production coveragejob95245034347: exact production coverage enforcement success;Feature-base organization security/review workflows that did not execute are absent evidence and are not represented as passing.
Scope / governance
This is compatibility evidence only. It does not grant OriginWeave Agent bookmark authority, expose a general bookmark tool, mutate a human profile, add model calls, create persistence ownership, or complete the full issue #27 extension matrix. Chromium permission and browser compatibility remain separate from OriginWeave Agent capability/policy authority.
Keep Draft while #43 remains active. Protected-main
AGENTS.mdforbids this scheduled actor from merging, self-approving, altering workflows, adding secrets, weakening checks, tagging, or publishing.