Skip to content

fix: resolve macOS E2E failures#1834

Merged
hatayama merged 6 commits into
v3-betafrom
fix/macos-e2e-integration
Jul 18, 2026
Merged

fix: resolve macOS E2E failures#1834
hatayama merged 6 commits into
v3-betafrom
fix/macos-e2e-integration

Conversation

@hatayama

@hatayama hatayama commented Jul 18, 2026

Copy link
Copy Markdown
Owner

Summary

  • Fix CLI recovery/readiness smoke setup by carrying the project runner pin into temporary projects.
  • Align replay verification with the CLI's PascalCase JSON response keys.
  • Capture Unity compilation settings on the main thread before asynchronous compilation continues.
  • Add the public set-game-view-size first-party tool and replace reflection-based Game View handling in the mouse E2E script.

Verification

  • sh scripts/run-posix-e2e.sh passed twice consecutively on fix/macos-e2e-integration.
  • Both runs reported RESULT: MATCH (11 events identical), RESULT: PASS, and All POSIX terminal-driven E2E checks passed.
  • The protocol version declarations were not changed.

Review in cubic

@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f4aaaf6c-0eb9-4f1d-8d7b-0aa11396db83

📥 Commits

Reviewing files that changed from the base of the PR and between b8ae2d6 and 8e7f9db.

📒 Files selected for processing (2)
  • Assets/Tests/Editor/DynamicCodeToolTests/RoslynCompilerOptionsTests.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/RoslynCompilerOptions.cs
💤 Files with no reviewable changes (1)
  • Assets/Tests/Editor/DynamicCodeToolTests/RoslynCompilerOptionsTests.cs

📝 Walkthrough

Walkthrough

The PR adds a set-game-view-size CLI tool, passes Unity compiler settings through dynamic Roslyn compilation, updates demo and replay scripts for current JSON fields, and prepares recovery smoke tests with the project runner pin.

Changes

Game View resolution tool

Layer / File(s) Summary
Tool contract and execution
Packages/src/Editor/FirstPartyTools/SetGameViewSize/*
Adds the schema, response, CLI tool, validation, resolution updates, and structured result handling.
Registration and documentation
cli/common/tools/default-tools.json, .agents/skills/..., .claude/skills/..., Packages/src/Editor/FirstPartyTools/SetGameViewSize/Skill/SKILL.md
Registers and documents the command, parameters, output, examples, and API usage.
Demo resolution integration
scripts/test-simulate-mouse-demo.sh
Replaces dropdown-index reflection logic with explicit resolution setting and restoration commands.

Roslyn compiler options

Layer / File(s) Summary
Options snapshot and tests
Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/RoslynCompilerOptions.cs, Assets/Tests/Editor/DynamicCodeToolTests/RoslynCompilerOptionsTests.cs
Adds immutable compiler options with define-symbol filtering and unsafe-code support, plus constructor behavior tests.
Compilation pipeline propagation
Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/{Compilation,DynamicCompilation}/*, Assets/Tests/Editor/DynamicCodeToolTests/ExternalCompilerPathResolverTests.cs
Reads Unity compiler settings and passes them through assembly building, backend compilation, and Roslyn request generation.

Smoke-test updates

Layer / File(s) Summary
CLI response parsing
Assets/Tests/Demo/scripts/verify-replay-via-cli.sh
Updates replay verification to parse capitalized JSON response fields.
Recovery runner pin setup
scripts/smoke-cli-recovery-readiness.go, scripts/test-smoke-cli-recovery-readiness.sh
Copies project-runner-pin.json into temporary projects used by stale recovery smoke tests.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 17.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: fixing macOS E2E failures.
Description check ✅ Passed The description matches the changeset and summarizes the main fixes and verification.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/macos-e2e-integration

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/RoslynCompilerOptions.cs`:
- Around line 22-25: Replace the null-check exception in the
RoslynCompilerOptions constructor with a Debug.Assert using the specified
defineSymbols message, and add the UnityEngine import required for it. In
Assets/Tests/Editor/DynamicCodeToolTests/RoslynCompilerOptionsTests.cs lines
50-59, delete Constructor_WithNullDefineSymbols_ThrowsArgumentNullException
because this precondition is now enforced by assertion rather than a runtime
exception.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 93774b10-48b8-4a23-a552-2ed8eec5ac14

📥 Commits

Reviewing files that changed from the base of the PR and between 00501a4 and b8ae2d6.

⛔ Files ignored due to path filters (11)
  • Assets/Tests/Editor/DynamicCodeToolTests/RoslynCompilerOptionsTests.cs.meta is excluded by none and included by none
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/RoslynCompilerOptions.cs.meta is excluded by none and included by none
  • Packages/src/Editor/FirstPartyTools/SetGameViewSize.meta is excluded by none and included by none
  • Packages/src/Editor/FirstPartyTools/SetGameViewSize/SetGameViewSizeResponse.cs.meta is excluded by none and included by none
  • Packages/src/Editor/FirstPartyTools/SetGameViewSize/SetGameViewSizeSchema.cs.meta is excluded by none and included by none
  • Packages/src/Editor/FirstPartyTools/SetGameViewSize/SetGameViewSizeTool.cs.meta is excluded by none and included by none
  • Packages/src/Editor/FirstPartyTools/SetGameViewSize/SetGameViewSizeUseCase.cs.meta is excluded by none and included by none
  • Packages/src/Editor/FirstPartyTools/SetGameViewSize/Skill.meta is excluded by none and included by none
  • Packages/src/Editor/FirstPartyTools/SetGameViewSize/Skill/SKILL.md.meta is excluded by none and included by none
  • Packages/src/Editor/FirstPartyTools/SetGameViewSize/UnityCLILoop.FirstPartyTools.SetGameViewSize.Editor.asmdef is excluded by none and included by none
  • Packages/src/Editor/FirstPartyTools/SetGameViewSize/UnityCLILoop.FirstPartyTools.SetGameViewSize.Editor.asmdef.meta is excluded by none and included by none
📒 Files selected for processing (20)
  • .agents/skills/uloop-set-game-view-size/SKILL.md
  • .claude/skills/uloop-set-game-view-size/SKILL.md
  • Assets/Tests/Demo/scripts/verify-replay-via-cli.sh
  • Assets/Tests/Editor/DynamicCodeToolTests/ExternalCompilerPathResolverTests.cs
  • Assets/Tests/Editor/DynamicCodeToolTests/RoslynCompilerOptionsTests.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Compilation/ICompiledAssemblyBuilder.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/CompiledAssemblyBuilder.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeCompiler.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCompilationBackend.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/RoslynCompilerBackend.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/RoslynCompilerOptions.cs
  • Packages/src/Editor/FirstPartyTools/SetGameViewSize/SetGameViewSizeResponse.cs
  • Packages/src/Editor/FirstPartyTools/SetGameViewSize/SetGameViewSizeSchema.cs
  • Packages/src/Editor/FirstPartyTools/SetGameViewSize/SetGameViewSizeTool.cs
  • Packages/src/Editor/FirstPartyTools/SetGameViewSize/SetGameViewSizeUseCase.cs
  • Packages/src/Editor/FirstPartyTools/SetGameViewSize/Skill/SKILL.md
  • cli/common/tools/default-tools.json
  • scripts/smoke-cli-recovery-readiness.go
  • scripts/test-simulate-mouse-demo.sh
  • scripts/test-smoke-cli-recovery-readiness.sh

Treat the compiler options snapshot as an internal contract: the only producer normalizes null define symbols before construction. Use the project's assertion convention for that programmer error and keep the remaining tests focused on snapshot behavior.
@hatayama
hatayama merged commit 6d498b4 into v3-beta Jul 18, 2026
10 checks passed
@hatayama
hatayama deleted the fix/macos-e2e-integration branch July 18, 2026 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant