feat(servo): adopt unified theorem v0.5 fork - #3
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8b393ff9b
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Turvo is the sole home of Theorem's Servo integration, exact engine pin, | ||
| migration lane, hosted engine validation, and desktop bundling path. Theorem |
There was a problem hiding this comment.
When crates.io publishes a Servo version newer than 0.5.0, the unchanged Resolve and prefetch the latest stable Servo section of .github/workflows/servo-next.yml still targets the registry release, while .github/prompts/servo-next.md permits changing only manifests, sources, examples, CI, and a migration note. It cannot update the now-authoritative patches/servo/integration.json, FORK.md, upstream-base, patch stack, or Rust channel. Consequently, any generated PR either retains an incompatible 0.5 fork override or switches to registry Servo, after which scripts/check_integration.py rejects the source family and the new integration-policy job rejects stale lineage, so the migration lane this record assigns to Turvo cannot produce a promotable update.
AGENTS.md reference: AGENTS.md:L77-L80
Useful? React with 👍 / 👎.
| "substitution_allowed": false, | ||
| "live_oracle_required": true, | ||
| "proof_commands": [ | ||
| "python3 scripts/check-servo-pin-unity.py", |
There was a problem hiding this comment.
When W10 reaches verification, V10 cannot execute its newly required first proof command because scripts/check-servo-pin-unity.py does not exist. A repository-wide rg finds only this generated node and the canonical JSON reference, while rg --files scripts confirms there is no implementation; moreover, W10's declared scope is limited to the isolated Theorem worktree and V10 is read-only, so neither node is authorized to create it. This leaves the sole-pin obligation without its declared executable oracle and blocks the graph at V10 unless the script is added to an implementation scope first.
AGENTS.md reference: AGENTS.md:L49-L55
Useful? React with 👍 / 👎.
| - Exactly one repository declares the product Servo pin: Turvo. | ||
| - Theorem must remove its duplicate pin and fork ledger when it consumes Turvo. |
There was a problem hiding this comment.
When Theorem adds Turvo as a dependency and removes its own Servo patch as required here, Cargo does not inherit Turvo's workspace-root [patch.crates-io]; the published crate manifest contains only servo = "=0.5.0", so the consumer resolves registry Servo rather than revision b70d4e64. A local cargo metadata reproduction with a patch declared in a non-root dependency resolved the original source, and README.md:56-58 explicitly acknowledges that these overrides are not inherited. Therefore the stated sole-pin design cannot supply the fork APIs to Theorem: either Theorem must retain a duplicate patch or Turvo needs a consumable dependency/artifact mechanism that carries the selected engine.
AGENTS.md reference: AGENTS.md:L77-L80
Useful? React with 👍 / 👎.
Turvo pinned b70d4e64c0, which is 36 commits behind the revision two
independent verifications accepted. The last green run pair, 34738526924
and 34738526952, is therefore green at the wrong revision.
The pin lives in seven places, not one, and all seven move together:
the five [patch.crates-io] revisions, Cargo.lock, patches/servo/integration.json,
patches/servo/README.md, patches/servo/FORK.md, AGENTS.md, and the patch
stack the Servo integration policy job reverse-applies.
Two new patch files, one per independently verified slice:
0008 b70d4e64c0..6ed6091e4e, 33 commits, the IndexedDB conformance
slice accepted at 6ed6091e4e against the web-platform suite.
0009 6ed6091e4e..e92cdaa790, 3 commits, Web Locks, shared-worker
teardown and named window proxies, accepted at e92cdaa790.
0005-storage-engine-factories.patch is deliberately NOT rewritten. The
36 commits rewrite components/shared/storage/indexeddb.rs by +232/-31,
so reverse-applying the old seven-patch stack at the new tip fails on
0005 across five files. Folding those commits into 0005 would have made
a patch whose subject says "storage engine factories" carry the whole
IndexedDB series, and integration.json pins that file by digest while
README.md describes it in prose. Adding one file per verified slice
keeps the stack a decomposition.
Verified locally before any push, all read-only against a clean clone of
the fork at /Volumes/servo-build/servo-fork:
* The nine patches, in the order read out of servo-integration.yml,
reverse-apply at the revision read out of integration.json and
produce tree aa41ca6aa70d446a791453c8ac3ce741d7acdcc8. Reversing the
existing seven at the old pin produces the same tree, so the two new
files add exactly the 36-commit delta and nothing else.
* python3 scripts/check_integration.py passes in both modes, and
cargo metadata --locked --no-deps returns 0.
* All 112 changed Cargo.lock lines name the servo fork. No unrelated
version moved.
* Lineage against upstream v0.5.0 (1d44e5dd6a): ahead_by=56,
behind_by=0, base is an ancestor, matching the new integration.json.
* rust-toolchain.toml is channel 1.95.0 at both the old and the new
Servo revision and in this repository, so rust_channel is unchanged.
NOT verified here: cargo test -p turvo --lib --locked needs a full Servo
link, and this machine has 12 GiB free against a build that takes 13.
That check belongs to CI.
BLOCKED ON ONE PUSH THAT IS NOT MINE TO MAKE. The policy job asserts
that integration.json's revision is the tip of integration.json's
branch. theorem/v0.5.0 is still at b5fead2675, which is a strict
ancestor of e92cdaa790, 35 behind and 0 ahead. FORK.md rule 2 says to
grow theorem/v0.5.0 by commits and not to retarget it silently, so this
change keeps the branch name and needs that branch fast-forwarded on
Travis-Gilbert/servo. The alternative, retargeting the pin to
theorem/v0.5.0-indexeddb, needs no servo push but does need rule 2
amended, and it is two string fields away in integration.json.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 participant
Summary
Local validation
This is the ADDENDUM-1 migration lane. Keep it draft until Linux and macOS hosted receipts are green.
Greptile Summary
The PR moves Turvo to the unified Servo theorem/v0.5.0 fork and makes the repository authoritative for its exact engine lineage.
e92cdaa790797479c1821c33470c64e0d166feb2with Rust 1.95.Confidence Score: 5/5
No actionable regression caused by the changes since the previous review was identified, so the PR appears safe to merge after its documented hosted Linux and macOS receipts pass.
The updated pin, lineage metadata, and reverse patch-stack order are internally consistent, while the newly imported Servo surfaces do not conflict with an established Turvo or available related-repository consumer contract.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR U[Servo upstream v0.5.0] --> B[Recorded base revision] B --> P1[Patches 0001–0007] P1 --> P8[0008 IndexedDB slice] P8 --> P9[0009 Web Locks and window proxies] P9 --> T[Pinned theorem/v0.5.0 tip] T --> C[Turvo Cargo graph] T --> V[Hosted lineage and patch-stack verification] C --> D[Theorem desktop consumer]Reviews (2): Last reviewed commit: "chore(servo): move the pin to the combin..." | Re-trigger Greptile