Repository navigation
Say which failing tests block a push, and name one skip variable per browser engine - #395
Merged
Merged
Conversation
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: af3f95ca7e
ℹ️ 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".
ahernsean
commented
Oct 7, 2026
ahernsean
changed the title
"Commits with failing tests must not be pushed" sat beside a paragraph saying a code commit needs "the suite", which reads as the whole suite and contradicts the focused-test rule above it. Name the bar as the focused tests on every engine they cover, browser tests included, and leave the rest to CI. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PpacbZRqT46b4Duhxq5QNQ
No test reads REQUIRE_WEBKIT_CONTAINER_TESTS; WebKit runs by default and only the SKIP_* variables turn it off. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PpacbZRqT46b4Duhxq5QNQ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PpacbZRqT46b4Duhxq5QNQ
SKIP_WEBKIT_CONTAINER_TESTS named how WebKit is launched, which is native first and a container only as a fallback, and SKIP_BROWSER_TESTS was the only way to skip Chromium, and skipped WebKit with it. The engines are peers, so each now has one opt-out: SKIP_CHROMIUM_TESTS and SKIP_WEBKIT_TESTS. A run covering neither sets both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PpacbZRqT46b4Duhxq5QNQ
ahernsean
force-pushed
the
claude/agents-test-scope
branch
from
October 7, 2026 02:40
2573e4c to
b5f1756
Compare
ahernsean
enabled auto-merge
October 7, 2026 02:41
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
1 participant
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.
Summary
AGENTS.md disagreed with itself about what has to pass before a push. "Your tests should be specific, then push" says a local full-suite pass is strictly worse than pushing once the focused tests pass. The paragraphs after "Commits with failing tests must not be pushed" said a markdown commit doesn't require "the suite", and that for any code change "the rule above applies in full". Read together, that means a code commit needs the whole suite. On PR #394 an agent held a push for the full 600-test browser suite because of that reading.
This PR changes those paragraphs to say:
report_client.htmlchange runs the browser tests that cover it (selected with-k) on both engines, plus their mutation checks. The rest of the module is CI's. "Both engines" says which browsers a focused run covers, not how many tests it runs.REQUIRE_WEBKIT_CONTAINER_TESTS.SKIP_CHROMIUM_TESTSandSKIP_WEBKIT_TESTS. They replaceSKIP_WEBKIT_CONTAINER_TESTSandSKIP_BROWSER_TESTS. The old WebKit name described how WebKit is launched, and it is launched natively first, as on CI, falling back to a container only when that fails. The only way to skip Chromium wasSKIP_BROWSER_TESTS, which skipped WebKit as well. The engines are peers in the test classes, so their opt-outs are now peers too, and a run covering neither engine sets both. The test module, the web session hook, and AGENTS.md are updated. A machine still exporting an old name runs the engine, and if the engine cannot start, the failure message names the new variable.🤖 Generated with Claude Code
https://claude.ai/code/session_01PpacbZRqT46b4Duhxq5QNQ