test(mv3): prove real downloads compatibility - #43
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMV3 호환성 실행기가 ChangesMV3 호환성 검증
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🟡 Moderate · up to The PR adds real pinned-Chromium download coverage and fail-closed runner hardening, but an unhandled transport-error path could stop the compatibility run without bounded diagnostics. The required independent approval and security checks are also still outstanding, so the PR should not merge until these items are addressed or explicitly accepted. Sequence Diagram(s)sequenceDiagram
participant 호환성실행기
participant MV3서비스워커
participant LoopbackHTTP서버
participant 다운로드디렉터리
호환성실행기->>MV3서비스워커: downloads 표면 검사 요청
MV3서비스워커->>LoopbackHTTP서버: 로컬 payload 요청
LoopbackHTTP서버-->>MV3서비스워커: 다운로드 응답
MV3서비스워커->>다운로드디렉터리: 파일 저장
MV3서비스워커-->>호환성실행기: ready 상태와 진단 정보 반환
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
seonghobae
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head27ce89066ed1473dcd66eb26a2f91becf9df5424. -
Head SHA:
27ce89066ed1473dcd66eb26a2f91becf9df5424 -
Workflow run: 31641414172
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["CI script: run_mv3_compatibility.py"]
S1 --> I1["review and security gate shell path"]
I1 --> R1["Review risk: CI script: run_mv3_compatibility.py"]
R1 --> V1["bash -n plus Strix self-test"]
Evidence --> S2["Test (6 files)"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test (6 files)"]
R2 --> V2["targeted test run"]
OpenCode Review Overview
Pull request overviewOpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed. Findings1. HIGH Current-head GitHub Checks - Fix failed required checks before approval
Failed checks:
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file: CHANGELOG.md"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file: CHANGELOG.md"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (2 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (2 files)"]
R2 --> V2["docs review"]
Evidence --> S3["CI script: run_mv3_compatibility.py"]
S3 --> I3["review and security gate shell path"]
I3 --> R3["Review risk: CI script: run_mv3_compatibility.py"]
R3 --> V3["bash -n plus Strix self-test"]
Evidence --> S4["Test (13 files)"]
S4 --> I4["regression suite"]
I4 --> R4["Review risk: Test (13 files)"]
R4 --> V4["targeted test run"]
|
|
Scheduled review-feedback autofix for this PR head.
|
|
Scheduled review-feedback autofix for this PR head.
|
|
Scheduled review-feedback autofix for this PR head.
|
|
Scheduled review-feedback autofix for this PR head.
|
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head repair verification: the old #43 branch was 130 commits behind protected main and carried a product-writer-owned workflow delta. I preserved the valid downloads/fixture/test/doctoring/CHANGELOG tree through a non-force two-parent restack onto exact main@c789b802fc98a8d7fd8c09d9327f36828054d2a1. The merge tree carries protected-main .github/workflows/ci.yml and .github/workflows/mv3-compatibility.yml byte-for-byte rather than authoring workflow semantics. Fresh exact compare is ahead 106 / behind 0 with merge base exactly c789b802..., and the PR diff is now 19 product/docs/test files with no .github/** path. Previous head GREEN does not transfer: exact-head CI/MV3/SAST/Security/OSV/Scorecard are newly queued and the one-shot materializer is skipped. Keep Draft until current-head browser/security evidence is terminal.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head repair finding: the source-tree ownership reconstruction is correct, but it exposed an executable contract mismatch that must stay fail-closed. On exact head e5db34c57d7b1de61d613a196117ca2cce296bec, tests/test_mv3_compatibility_contract.py::test_workflow_installs_chromium_sandbox_helper requires the pinned Chrome for Testing chrome_sandbox helper to be root-owned, mode 4755, and exported through CHROME_DEVEL_SANDBOX. The reconstructed branch deliberately carries protected-main .github/workflows/mv3-compatibility.yml byte-for-byte; that live workflow currently contains none of those three requirements. At the same time the product runner correctly no longer passes --no-sandbox.
Do not repair this by deleting/weakening the sandbox contract or restoring --no-sandbox, and do not reintroduce a product-branch workflow mutation from this scheduled writer. Chromium's Linux SUID sandbox documentation for raw builds explicitly uses chown root:root chrome_sandbox, chmod 4755 chrome_sandbox, and CHROME_DEVEL_SANDBOX=$PWD/chrome_sandbox; the repository's own prior exact-head browser evidence also proved this pinned archive/helper combination can run successfully when configured.
Required causal owner repair: land the sandbox-helper setup through the authorized workflow-owner path on protected-main lineage (or a reviewed equivalent sandbox-capable mechanism), then non-destructively adopt that owner delta into #43 and require fresh exact-head repository contracts plus sandbox-enabled real Chromium MV3 execution. Until then #43 must remain Draft. Current queued runs are incomplete evidence, not GREEN.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Fresh hosted execution verifies the canonical workflow mismatch as a real RED, not a queued-runner hypothesis. CI 33740544401 / Rust contracts 100601170828 fails only at Check Python repository contracts: test_workflow_installs_chromium_sandbox_helper requires chrome_sandbox installation and CHROME_DEVEL_SANDBOX, but the protected-main/current-#43 workflow has neither. Rust formatting/check/test/Clippy/rustdoc are skipped after that repository-contract failure. Production coverage 100601171026 independently succeeds, so coverage is not causal. MV3 33740544442 / job 100601170874 then downloads exact Chrome/ChromeDriver 150.0.7871.129, executes the sandbox-preserving runner without the helper, and all 3 real-browser trials fail closed as webdriver_protocol_error / session not created; repeatability is 0/3. Failure artifact ID 9915138865 was retained. Issue #212 comment 5532890884 now carries the exact owner-path RCA. Do not restore --no-sandbox or weaken/delete the contract. Protected-main AGENTS forbids this scheduled writer from workflow changes; authorized workflow ownership must install root-owned mode-4755 chrome_sandbox, export CHROME_DEVEL_SANDBOX, retain harden-runner/egress/pins, then rerun exact-head contracts and pinned Chromium. COMMENT only; no approval or gate weakening.
Signed-off-by: Seongho Bae <me@seonghobae.me>
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head ownership finding on 6f3134d18d3118aab33d28048671dc71a5f47b77: the intervening commit adds exactly the sandbox-helper mechanics required by the hosted #43 RED (root:root, mode 4755, CHROME_DEVEL_SANDBOX) and should be preserved as a valid candidate delta, but it reintroduces .github/workflows/mv3-compatibility.yml into this leaf product PR. Protected-main AGENTS.md and issue #212 explicitly reserve workflow mutation for an authorized workflow-owner path, so this is a repair/retarget finding, not grounds to close or discard #43. Keep the PR Draft and keep the product/browser evidence; have the canonical workflow owner adopt the three-line workflow delta (or an equivalent reviewed repair), then non-destructively restack #43 onto that released/protected workflow generation and rerun repository contracts plus the sandbox-enabled three-trial Chromium fixture. Current exact-head runs 33866932365 (MV3), 33866932436 (CI), 33866932380 (SAST), and 33866932442 (Security) are queued, so no GREEN is claimed for this moved head.
|
Scheduled review-feedback autofix for this PR head.
|
|
Cross-lane adoption finding from #288: exact |
seonghobae
added
priority: medium
status: draft
type: maintenance
maintenance
labels
ChatGPT Codex Connector
3 participants
Buyer-visible gap
Partial implementation of #27. This Draft proves one bounded Manifest V3 downloads contract and hardens WebDriver cleanup, executable/status authority, startup/parser diagnostics, and evidence redaction without widening Agent authority. The current exact head now has successful sandbox-enabled real-Chromium evidence, but its sandbox-helper workflow delta is still in the wrong ownership location and therefore is not protected/canonical release evidence.
Current lineage and ownership
Current contributor head is exact
6f3134d18d3118aab33d28048671dc71a5f47b77, open, Draft, and mergeable. The PR's recorded main base isc789b802fc98a8d7fd8c09d9327f36828054d2a1; protectedmainhas advanced beyond that generation, so a later canonical-owner adoption/restack must use the then-live protected head rather than treating this stale base as current.The prior product/tree reconstruction at
e5db34c57d7b1de61d613a196117ca2cce296becpreserved the valid #43 source/test/docs delta while consuming then-protected workflow blobs. One intervening commit,6f3134d18d3118aab33d28048671dc71a5f47b77(fix(mv3): restore sandbox after restack), adds exactly the hosted helper mechanics to.github/workflows/mv3-compatibility.yml: root ownership for the pinned archive'schrome_sandbox, mode4755, andCHROME_DEVEL_SANDBOXfor the real fixture.Those lines are now executed and useful, but they remain in the wrong ownership location for this scheduled product lane. Protected
AGENTS.mdforbids scheduled agents from altering workflows, and issue #212 owns the canonical workflow repair. Review5112764360, issue comment5540222475, and the later exact-evidence handoff in #212 record this as repair/retarget work, not a Close condition. Preserve the delta until an authorized workflow owner adopts it or an equivalent reviewed implementation, then restack #43 non-destructively onto that canonical generation.#281 remains a dependent diagnostic-redaction lane and must follow the repaired #43 generation rather than compete as a direct-main writer.
Implemented product boundary
The branch:
chrome.downloads.downloadand boundedchrome.downloads.searchagainst one controlled loopback fixture;/statusidentity;--no-sandboxfrom the real compatibility runner, so an environment that cannot start the pinned browser securely fails rather than weakening the browser boundary.The download diagnostic distinction remains explicit: after an accepted integer download ID, bounded polling exhaustion is
download-timeout; a rejectedchrome.downloads.searchremainsdownload-not-evaluated.Hosted RED predecessor
On predecessor
e5db34c57d7b1de61d613a196117ca2cce296bec, CI33740544401failed because the repository contract required the pinnedchrome_sandboxto be root-owned, mode4755, and exposed throughCHROME_DEVEL_SANDBOX. Production coverage independently remained 100%.Manifest V3 Compatibility
33740544442, job100601170874, downloaded exact Chrome/ChromeDriver150.0.7871.129and failed all three sandbox-preserving browser trials withfailure_kind=webdriver_protocol_error,error_code=session not created; repeatability was 0/3. Failure artifact9915138865remains causal predecessor RED.Current exact-head GREEN — leaf implementation only
Exact
6f3134d18d3118aab33d28048671dc71a5f47b77now has terminal hosted evidence:33866932365: success; job101003808036successfully downloaded the exact Chrome/ChromeDriver pair and executed the real fixture;33866932436: success;33866932442: success;33866932380: success;33866932446: skipped and not product evidence.Downloaded artifact
9938906825(sha256:8644c2d16ce9d91ff7bb46075793b313507d3f37b590965a686f9b05a61ce519) independently reports Chrome150.0.7871.129, three successful real-browser trials out of three, and the expected bounded surfaces includingdownloads, real browser click, restart persistence, storage, DNR, tabs/windows, scripting, service worker, bookmarks and history.This closes the technical feasibility question for the three helper mechanics on this leaf generation. It does not legitimize leaf workflow ownership, transfer GREEN to #148/#288, satisfy an absent CodeQL/current-review/ruleset requirement, or make the stale base protected-current.
Causal repair boundary
Do not repair this lane by restoring
--no-sandbox, weakening/deleting the sandbox-helper contract, hidingsession not created, or treating the leaf workflow edit as final workflow authority. The required topology remains:Scope / non-claims
This PR does not prove full extension compatibility, extension-to-Agent authority isolation, native messaging, enterprise policy, Chrome Web Store behavior, Google services, codecs/DRM, remote-download policy, credentials, arbitrary filesystem authority, production browser-adapter authenticity, protected-main delivery, or release readiness. No qualifying independent current-head
APPROVEDreview is claimed. Keep Draft.The #43 PR-state/evidence writer is released after this update. No source, workflow, ruleset, secret, merge, tag, release or publication mutation was made in this coalescing step.