Skip to content

fix: Windows dynamic code snippets are easier to pass safely#1346

Merged
hatayama merged 2 commits into
v3-betafrom
fix/dynamic-code-windows
Jun 15, 2026
Merged

fix: Windows dynamic code snippets are easier to pass safely#1346
hatayama merged 2 commits into
v3-betafrom
fix/dynamic-code-windows

Conversation

@hatayama

@hatayama hatayama commented Jun 15, 2026

Copy link
Copy Markdown
Owner

Summary

  • Windows users can now discover the file-based fallback for dynamic code snippets directly from CLI help and completion.
  • PowerShell guidance now distinguishes PowerShell 7 from Windows PowerShell 5.1 so inline snippets with C# strings are not accidentally corrupted.

User Impact

  • Previously, Windows users could be told to pass inline C# strings in a way that Windows PowerShell 5.1 could alter before uloop received them.
  • The CLI now surfaces --code-file, and the skill guidance explains when inline --code is safe, when escaping is required, and when a legacy npm shim can still interfere.

Changes

  • Show --code-file in execute-dynamic-code --help and shell completion option lists.
  • Add regression coverage for preserving multiline inline --code arguments and for listing the CLI-side file option.
  • Update generated agent skill docs from the source skill guidance.
  • Improve Windows install diagnostics when legacy npm shims may shadow the native launcher.

Verification

  • bash -lc 'cd /mnt/c/Users/booql/oss/unity-cli-loop2 && scripts/check-go-cli.sh'
  • go test ./...
  • Windows PowerShell 5.1 argv smoke for escaped C# double quotes
  • go run ./cmd/uloop execute-dynamic-code --help
  • go run ./cmd/uloop --list-options execute-dynamic-code
  • codex-review v3-beta

Review in cubic

Preserve multiline --code coverage for execute-dynamic-code and make the CLI-side --code-file option discoverable in help and completion. Document the native Windows launcher expectation because legacy npm shims can alter multiline PowerShell arguments before the Go CLI receives them.
@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 838b6ab1-28de-4856-8efb-b33bf7ca6700

📥 Commits

Reviewing files that changed from the base of the PR and between 66e4a59 and 31cf3ec.

📒 Files selected for processing (2)
  • cli/internal/install/command.go
  • cli/internal/install/command_test.go

📝 Walkthrough

Walkthrough

Adds --code-file to the visible option names and help entries for execute-dynamic-code via new helper functions and wiring in tools.go and command_help.go. Adds npm shim multiline-argument warnings to the Windows install script. Expands PowerShell quoting and native launcher guidance across SKILL.md and reference docs in three skill definition locations.

Changes

CLI: --code-file Option Visibility and npm Shim Warnings

Layer / File(s) Summary
--code-file constants and append helpers
cli/internal/cli/dynamic_code_file.go
Defines --code-file option name, usage, and description constants, and adds appendDynamicCodeFileOptionName / appendDynamicCodeFileOptionHelpEntry helpers that conditionally insert the option for execute-dynamic-code without duplicates.
Wire --code-file into option name and help entry generation
cli/internal/cli/tools.go, cli/internal/cli/command_help.go
visibleOptionNamesForTool calls appendDynamicCodeFileOptionName and visibleOptionHelpEntriesForTool calls appendDynamicCodeFileOptionHelpEntry, so --code-file surfaces in both shell completion and --help output.
npm shim multiline-argument warnings in install script
cli/internal/install/command.go
Adds Write-Host warnings in both Write-LegacyNpmManualRemoval and Report-PathShadowing PowerShell functions, informing users that legacy npm shims can alter multiline arguments before the native CLI receives them.
Tests: option visibility, multiline --code, help output, install script
cli/internal/cli/tools_test.go, cli/internal/cli/help_test.go, cli/internal/cli/completion_test.go, cli/internal/install/command_test.go
Adds tests verifying multiline --code is preserved through buildToolParams, --code-file appears in visible option names and help entries while --compile-only stays hidden, execute-dynamic-code --help emits expected text, completion output includes --code-file, and the install script emits the npm shim warning with uloop.exe in the shim enumeration.

Documentation: PowerShell Quoting and SKILL.md Updates

Layer / File(s) Summary
SKILL.md workflow and Logs field updates
.agents/skills/uloop-execute-dynamic-code/SKILL.md, .claude/skills/uloop-execute-dynamic-code/SKILL.md, Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Skill/SKILL.md
Revises the inline C# execution workflow step, rewrites --code/--code-file parameter guidance (exclusivity, shell quoting for bash/zsh and PowerShell variants), and refines the Logs output field description to point to get-logs for Unity Debug.Log output. Applied to all three SKILL.md copies.
PowerShell reference doc expansions
.agents/skills/uloop-execute-dynamic-code/references/playmode-automation-powershell.md, .claude/skills/uloop-execute-dynamic-code/references/playmode-automation-powershell.md, Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Skill/references/playmode-automation-powershell.md
Adds a native launcher resolution check section, expands --code double-quote escaping guidance for PowerShell 7 vs Windows PowerShell 5.1, adds here-string instructions for multi-line snippets, and provides 5.1-specific here-string escaping examples. Applied to all three reference doc copies.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • hatayama/unity-cli-loop#1319: Both PRs modify the execute-dynamic-code CLI around --code-file support, including its mutual exclusion with --code and help/option visibility plumbing.
  • hatayama/unity-cli-loop#947: Overlaps with this PR's PowerShell PlayMode automation documentation updates for execute-dynamic-code, including quoting and here-string guidance.
  • hatayama/unity-cli-loop#1148: Both PRs adjust the execute-dynamic-code skill description text in SKILL.md to clarify parameter usage and wording.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 63.16% 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 accurately describes the main change: improving Windows dynamic code snippet handling by making them easier to pass safely through better discoverability and version-specific guidance.
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.
Description check ✅ Passed The PR description clearly relates to the changeset, describing the addition of --code-file discoverability and PowerShell-specific documentation for the execute-dynamic-code command.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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/dynamic-code-windows

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 and usage tips.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

2 issues found across 14 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread cli/internal/install/command.go Outdated
Comment thread cli/internal/install/command.go
Only show the multiline PowerShell warning when the shadowing command is detected as a legacy npm shim, and centralize the warning text so installer messages stay consistent.
@hatayama
hatayama merged commit eb91151 into v3-beta Jun 15, 2026
7 checks passed
@hatayama
hatayama deleted the fix/dynamic-code-windows branch June 15, 2026 15:51
@github-actions github-actions Bot mentioned this pull request Jun 15, 2026
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