Sync claude-nl-suite.yml from proven main run#759
Merged
dsarno merged 2 commits intoCoplayDev:betafrom Feb 16, 2026
Merged
Conversation
Contributor
Reviewer's Guide.github/workflows/claude-nl-suite.yml is updated to match a validated upstream workflow, tightening Unity MCP readiness checks, switching to the new claude-code-action interface with explicit prompts and claude_args for NL/T/GO passes (including retry flows), trimming and restructuring markdown summaries, and reducing JUnit noise in GitHub summaries and annotations. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughGitHub Actions workflow updated: added id-token permission, replaced MCP preflight with a Python read_console retry loop, added readiness diagnostics and extensive port/registry debugging, expanded NL/T/GO prompt orchestration and Claude action invocation, and improved output trimming, CI gating, and artifact/log handling. Changes
Sequence DiagramsequenceDiagram
participant WF as Workflow<br/>Orchestrator
participant Preflight as Preflight<br/>(Python read_console)
participant Diagnostics as Readiness<br/>Diagnostics
participant Prompts as Prompt<br/>Generator
participant Claude as Claude<br/>Code Action
participant MCP as MCP/Unity<br/>Container
participant Output as Results &<br/>Reporting
WF->>Preflight: Run MCP preflight (retry up to 8)
alt Preflight succeeds
WF->>Prompts: Generate NL/T/GO prompts (pass 1)
Prompts->>Claude: Provide prompts & args
Claude->>MCP: Execute code actions
MCP->>Claude: Return execution results/logs
Claude->>Output: Aggregate and trim results
WF->>Prompts: Run retry passes (NL/T/GO) as needed
else Preflight fails
WF->>Diagnostics: Trigger diagnostics
Diagnostics->>MCP: Container status, logs, PortDiscovery
Diagnostics->>MCP: Raw port probing, registry queries
Diagnostics->>Output: Emit diagnostics artifacts
end
Output->>WF: Produce job summary, CI gate decisions
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `.github/workflows/claude-nl-suite.yml:520-524` </location>
<code_context>
while true; do
attempt=$((attempt+1))
- if uv run --active --directory Server mcp-for-unity --transport stdio --help > /tmp/mcp-preflight.log 2>&1; then
+ if uv run --active --directory Server python <<'PY' > /tmp/mcp-preflight.log 2>&1
+ import json
+ import os
+ import sys
+ sys.path.insert(0, "Server/src")
+ from transport.legacy.unity_connection import send_command_with_retry
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The preflight Python sys.path modification likely points to the wrong directory when run from `--directory Server`.
Because `uv run` is already using `--directory Server`, the process CWD is `Server`, so `sys.path.insert(0, "Server/src")` resolves to `<repo>/Server/Server/src`. That’s likely wrong and inconsistent with other snippets that prepend just `"src"`. This can lead to `ImportError` for `transport.legacy.unity_connection` in CI. Consider changing this to `sys.path.insert(0, "src")` (or removing it if the package is available via installation).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/workflows/claude-nl-suite.yml:
- Around line 810-827: The retry steps for T and GO are missing
continue-on-error, so they can hard-fail the job when the Claude action exits
nonzero; update the step named "Retry T pass (Sonnet) if incomplete" (the step
that uses anthropics/claude-code-action@cc5ef44546fda0649ddde3c5ab0cd3db7b7c5035
and contains claude_args with --model claude-sonnet-4-5-20250929) to include
continue-on-error: true, and likewise add continue-on-error: true to the
corresponding GO retry step referenced in the review (the GO retry step using
the anthropics action around the GO retry location). Ensure the
continue-on-error key is at the same indentation level as name/uses/env/with for
each step.
- Line 8: Remove the unused permission entry "id-token: write" from the workflow
permissions block (it isn't used by anthropics/claude-code-action or anywhere
else); either delete that single "id-token: write" line or remove the entire
permissions block if it becomes empty to follow least-privilege practice.
- Around line 818-821: Remove the invalid and redundant
MultiEdit(/!(reports/**)) entry from the claude_args --disallowedTools string:
locate the claude_args block and the --disallowedTools option and delete the
MultiEdit(/!(reports/**)) token (or replace it with correct .gitignore-style
negation like !reports/** if you intend a negation and apply it consistently);
note that --allowedTools already restricts MultiEdit to reports/** so simply
removing the erroneous MultiEdit(/!(reports/**)) token (referencing the
claude_args and --disallowedTools settings) fixes the syntax error and
redundancy.
| permissions: | ||
| contents: read | ||
| checks: write | ||
| id-token: write |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if claude-code-action references id-token or OIDC in its action metadata
echo "=== Searching for action.yml files ==="
fd "action.yml" --exec cat {} \; 2>/dev/null | grep -i "id-token\|oidc" && echo "Found OIDC references in action.yml" || echo "No OIDC in action.yml"
# Check if any step references the ACTIONS_ID_TOKEN_REQUEST_URL or related env
echo -e "\n=== Searching for OIDC references in workflow ==="
rg -n 'ACTIONS_ID_TOKEN_REQUEST|id.token|oidc|OIDC' .github/workflows/claude-nl-suite.yml || echo "No OIDC references in workflow"
# Also look for any AWS, GCP, Azure auth patterns that would use OIDC
echo -e "\n=== Searching for cloud provider auth patterns ==="
rg -n 'aws-actions|google-github-actions|azure' .github/workflows/claude-nl-suite.yml || echo "No cloud auth actions"
# Check the actual action being used and its metadata
echo -e "\n=== Checking claude-code-action usage and metadata ==="
rg -B2 -A10 'anthropics/claude-code-action' .github/workflows/claude-nl-suite.yml | head -50Repository: CoplayDev/unity-mcp
Length of output: 2470
Remove the unused id-token: write permission on line 8.
The permission is not referenced anywhere in the workflow — no OIDC token requests, no cloud provider authentication, and the anthropics/claude-code-action does not require it. Removing unnecessary permissions follows the principle of least privilege.
🤖 Prompt for AI Agents
In @.github/workflows/claude-nl-suite.yml at line 8, Remove the unused
permission entry "id-token: write" from the workflow permissions block (it isn't
used by anthropics/claude-code-action or anywhere else); either delete that
single "id-token: write" line or remove the entire permissions block if it
becomes empty to follow least-privilege practice.
Comment on lines
810
to
827
| - name: Retry T pass (Sonnet) if incomplete | ||
| if: steps.t_cov.outputs.missing != '0' | ||
| uses: anthropics/claude-code-base-action@beta | ||
| uses: anthropics/claude-code-action@cc5ef44546fda0649ddde3c5ab0cd3db7b7c5035 | ||
| env: | ||
| GITHUB_STEP_SUMMARY: /dev/null | ||
| with: | ||
| use_node_cache: false | ||
| prompt_file: .claude/prompts/nl-unity-suite-t.md | ||
| mcp_config: .claude/mcp.json | ||
| prompt: ${{ steps.t_prompt.outputs.retry_prompt }} | ||
| settings: .claude/settings.json | ||
| allowed_tools: "mcp__unity,Edit(reports/**),MultiEdit(reports/**)" | ||
| disallowed_tools: "Bash,MultiEdit(/!(reports/**)),WebFetch,WebSearch,Task,TodoWrite,NotebookEdit,NotebookRead" | ||
| model: claude-sonnet-4-5-20250929 | ||
| fallback_model: claude-haiku-4-5-20251001 | ||
| append_system_prompt: | | ||
| You are running the T pass only. | ||
| Output requirements: | ||
| - Emit exactly 10 test fragments: T-A, T-B, T-C, T-D, T-E, T-F, T-G, T-H, T-I, T-J. | ||
| - Write each fragment to reports/${ID}_results.xml (e.g., T-A_results.xml). | ||
| - Prefer a single MultiEdit(reports/**) call that writes all ten files in one batch. | ||
| - If MultiEdit is not used, emit individual writes for any missing IDs until all ten exist. | ||
| - Do not emit any NL-* fragments. | ||
| Stop condition: | ||
| - After T-J_results.xml is written, stop. | ||
| timeout_minutes: "30" | ||
| claude_args: | | ||
| --mcp-config .claude/mcp.json | ||
| --allowedTools mcp__unity,Edit(reports/**),MultiEdit(reports/**) | ||
| --disallowedTools Bash,MultiEdit(/!(reports/**)),WebFetch,WebSearch,Task,TodoWrite,NotebookEdit,NotebookRead | ||
| --model claude-sonnet-4-5-20250929 | ||
| --fallback-model claude-haiku-4-5-20251001 | ||
| track_progress: false | ||
| show_full_output: true | ||
| display_report: false | ||
| anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} |
Contributor
There was a problem hiding this comment.
Missing continue-on-error: true on T retry (and GO retry at line 922).
The initial NL pass (line 649), NL retry (line 710), T pass (line 773), and GO pass (line 887) all have continue-on-error: true. The T retry and GO retry steps are missing it.
If the Claude action returns a non-zero exit code (API timeout, partial completion, etc.), these retry steps will hard-fail the job. Subsequent if: always() assertion steps will still run, but the job conclusion is prematurely tainted — the assertion gates should be the single source of truth for pass/fail rather than the action exit code.
Proposed fix
- name: Retry T pass (Sonnet) if incomplete
if: steps.t_cov.outputs.missing != '0'
uses: anthropics/claude-code-action@cc5ef44546fda0649ddde3c5ab0cd3db7b7c5035
+ continue-on-error: true
env:
GITHUB_STEP_SUMMARY: /dev/nullAnd for the GO retry around line 922:
- name: Retry GO pass (Sonnet) if incomplete
if: steps.go_cov.outputs.missing != '0'
uses: anthropics/claude-code-action@cc5ef44546fda0649ddde3c5ab0cd3db7b7c5035
+ continue-on-error: true
env:
GITHUB_STEP_SUMMARY: /dev/null📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Retry T pass (Sonnet) if incomplete | |
| if: steps.t_cov.outputs.missing != '0' | |
| uses: anthropics/claude-code-base-action@beta | |
| uses: anthropics/claude-code-action@cc5ef44546fda0649ddde3c5ab0cd3db7b7c5035 | |
| env: | |
| GITHUB_STEP_SUMMARY: /dev/null | |
| with: | |
| use_node_cache: false | |
| prompt_file: .claude/prompts/nl-unity-suite-t.md | |
| mcp_config: .claude/mcp.json | |
| prompt: ${{ steps.t_prompt.outputs.retry_prompt }} | |
| settings: .claude/settings.json | |
| allowed_tools: "mcp__unity,Edit(reports/**),MultiEdit(reports/**)" | |
| disallowed_tools: "Bash,MultiEdit(/!(reports/**)),WebFetch,WebSearch,Task,TodoWrite,NotebookEdit,NotebookRead" | |
| model: claude-sonnet-4-5-20250929 | |
| fallback_model: claude-haiku-4-5-20251001 | |
| append_system_prompt: | | |
| You are running the T pass only. | |
| Output requirements: | |
| - Emit exactly 10 test fragments: T-A, T-B, T-C, T-D, T-E, T-F, T-G, T-H, T-I, T-J. | |
| - Write each fragment to reports/${ID}_results.xml (e.g., T-A_results.xml). | |
| - Prefer a single MultiEdit(reports/**) call that writes all ten files in one batch. | |
| - If MultiEdit is not used, emit individual writes for any missing IDs until all ten exist. | |
| - Do not emit any NL-* fragments. | |
| Stop condition: | |
| - After T-J_results.xml is written, stop. | |
| timeout_minutes: "30" | |
| claude_args: | | |
| --mcp-config .claude/mcp.json | |
| --allowedTools mcp__unity,Edit(reports/**),MultiEdit(reports/**) | |
| --disallowedTools Bash,MultiEdit(/!(reports/**)),WebFetch,WebSearch,Task,TodoWrite,NotebookEdit,NotebookRead | |
| --model claude-sonnet-4-5-20250929 | |
| --fallback-model claude-haiku-4-5-20251001 | |
| track_progress: false | |
| show_full_output: true | |
| display_report: false | |
| anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} | |
| - name: Retry T pass (Sonnet) if incomplete | |
| if: steps.t_cov.outputs.missing != '0' | |
| uses: anthropics/claude-code-action@cc5ef44546fda0649ddde3c5ab0cd3db7b7c5035 | |
| continue-on-error: true | |
| env: | |
| GITHUB_STEP_SUMMARY: /dev/null | |
| with: | |
| prompt: ${{ steps.t_prompt.outputs.retry_prompt }} | |
| settings: .claude/settings.json | |
| claude_args: | | |
| --mcp-config .claude/mcp.json | |
| --allowedTools mcp__unity,Edit(reports/**),MultiEdit(reports/**) | |
| --disallowedTools Bash,MultiEdit(/!(reports/**)),WebFetch,WebSearch,Task,TodoWrite,NotebookEdit,NotebookRead | |
| --model claude-sonnet-4-5-20250929 | |
| --fallback-model claude-haiku-4-5-20251001 | |
| track_progress: false | |
| show_full_output: true | |
| display_report: false | |
| anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} |
🤖 Prompt for AI Agents
In @.github/workflows/claude-nl-suite.yml around lines 810 - 827, The retry
steps for T and GO are missing continue-on-error, so they can hard-fail the job
when the Claude action exits nonzero; update the step named "Retry T pass
(Sonnet) if incomplete" (the step that uses
anthropics/claude-code-action@cc5ef44546fda0649ddde3c5ab0cd3db7b7c5035 and
contains claude_args with --model claude-sonnet-4-5-20250929) to include
continue-on-error: true, and likewise add continue-on-error: true to the
corresponding GO retry step referenced in the review (the GO retry step using
the anthropics action around the GO retry location). Ensure the
continue-on-error key is at the same indentation level as name/uses/env/with for
each step.
Comment on lines
+818
to
+821
| claude_args: | | ||
| --mcp-config .claude/mcp.json | ||
| --allowedTools mcp__unity,Edit(reports/**),MultiEdit(reports/**) | ||
| --disallowedTools Bash,MultiEdit(/!(reports/**)),WebFetch,WebSearch,Task,TodoWrite,NotebookEdit,NotebookRead |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file and find all Claude action steps
cd .github/workflows && wc -l claude-nl-suite.ymlRepository: CoplayDev/unity-mcp
Length of output: 87
🏁 Script executed:
# Now let's examine the section around lines 818-821
sed -n '810,830p' .github/workflows/claude-nl-suite.ymlRepository: CoplayDev/unity-mcp
Length of output: 1011
🏁 Script executed:
# Find all claude-code-action steps in the file to compare configurations
rg -n "claude_args:|--disallowedTools|--allowedTools" .github/workflows/claude-nl-suite.yml -A 1 -B 1Repository: CoplayDev/unity-mcp
Length of output: 2212
🌐 Web query:
claude-code-action disallowedTools glob negation patterns syntax documentation
💡 Result:
In anthropics/claude-code-action, you pass disallowed tools via claude_args using the Claude Code CLI flag --disallowedTools (comma-separated, or repeat the flag). Example: --disallowedTools "TaskOutput,KillTask". [1]
Syntax (including glob + negation)
--disallowedTools uses Claude Code permission rule syntax: Tool or Tool(specifier). Wildcards depend on the tool type. [2]
Bash patterns
Bash rules support * wildcards inside the command string, e.g. Bash(git push *). [2]
Read/Edit/Write (filesystem) patterns
For file-path specifiers like Read(...) / Edit(...), Claude Code follows the .gitignore pattern specification. [2]
That means gitignore-style negation is done with a leading ! (it “re-includes” something excluded by an earlier pattern), e.g. !/foo/bar. [3]
Important gotcha for --disallowedTools
Claude Code evaluates rules in this order: deny → ask → allow, and any matching deny wins. [2]
So, if you put a broad deny like Read(secrets/**) into --disallowedTools, you generally cannot “undo” it with an allow rule elsewhere—the deny still takes precedence if it matches. [2]
If you need “allow everything except X”, prefer an allowlist-first approach (tight allow rules and minimal/no broad denies), rather than relying on negation inside denies. [2]
Sources:
[1] claude-code-action configuration docs (examples showing --allowedTools / --disallowedTools)
[2] Claude Code “Configure permissions” (rule syntax, wildcard behavior, deny/ask/allow precedence, Read/Edit follow gitignore spec)
[3] Git gitignore docs (leading ! negation semantics)
Remove MultiEdit(/!(reports/**)) from line 821 — uses invalid syntax and is redundant.
Line 821 (T retry step) uniquely includes MultiEdit(/!(reports/**)) in --disallowedTools, but no other Claude step has this entry. More critically, the !() pattern uses bash extended glob syntax, which Claude Code does not support. File-path specifiers in --disallowedTools follow .gitignore patterns, where negation uses a leading ! (e.g., !reports/**), not bash negation.
Since --allowedTools already restricts MultiEdit to reports/**, this disallowed-tools entry is redundant. Either apply it consistently across all steps with correct syntax, or remove it.
🤖 Prompt for AI Agents
In @.github/workflows/claude-nl-suite.yml around lines 818 - 821, Remove the
invalid and redundant MultiEdit(/!(reports/**)) entry from the claude_args
--disallowedTools string: locate the claude_args block and the --disallowedTools
option and delete the MultiEdit(/!(reports/**)) token (or replace it with
correct .gitignore-style negation like !reports/** if you intend a negation and
apply it consistently); note that --allowedTools already restricts MultiEdit to
reports/** so simply removing the erroneous MultiEdit(/!(reports/**)) token
(referencing the claude_args and --disallowedTools settings) fixes the syntax
error and redundancy.
msanatan
pushed a commit
to msanatan/unity-mcp
that referenced
this pull request
Feb 25, 2026
* Sync claude-nl-suite workflow with proven main version * Fix preflight sys.path under Server working directory
msanatan
pushed a commit
to msanatan/unity-mcp
that referenced
this pull request
Feb 25, 2026
* Sync claude-nl-suite workflow with proven main version * Fix preflight sys.path under Server working directory
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
Why
Notes
Summary by CodeRabbit
Chores
Tests
Bug Fixes