Nl ci workflow fixes

[dsarno]

Summary by CodeRabbit

• New Features

  • Additive Unity NL/T editing workflow and per-test reporting; opt-in sentinel flip; synchronous menu execution with structured success/error responses.

• Improvements

  • Quieter debug-gated logging; smarter anchor-based edit matching; robust JSON command routing and framed I/O; improved type/property resolution; safer material/shader handling and more resilient asset/script refresh behavior.

• Documentation

  • README overhaul; NL/T prompt files consolidated/updated.

• CI

  • Multi-path Unity licensing flow and updated NL/T workflow steps.

• Tests

  • Many new editor/unit tests added; one legacy test removed.

• Chores

  • Version bumps, .gitignore updates, metadata housekeeping.

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (2)

.claude/prompts/nl-unity-suite-full-additive.md (1)

125-133: Use standard validation level for consistency (replace “relaxed”).

Prior guidance asked to keep validation at "standard" across tests. T-B currently says to use relaxed.

-- Use `validate: "relaxed"` for interior-only edit
+- Use `validate_script(level:"standard")` for interior-only edit

.github/workflows/claude-nl-suite.yml (1)

117-158: Redact activation logs and tighten entitlement check path.

docker run emits Unity logs that may include secrets; also the entitlement check scans the whole config tree, risking false positives. Pipe logs through redaction and check specifically under Unity/licenses.

-            docker run --rm --network host \
+            docker run --rm --network host \
               -e HOME=/root \
               -e UNITY_EMAIL -e UNITY_PASSWORD -e UNITY_SERIAL \
               -v "$RUNNER_TEMP/unity-config:/root/.config/unity3d" \
               -v "$RUNNER_TEMP/unity-local:/root/.local/share/unity3d" \
-              "$UNITY_IMAGE" bash -lc '
+              "$UNITY_IMAGE" bash -lc '
                 set -euxo pipefail
                 if [[ -n "${UNITY_SERIAL:-}" ]]; then
                   /opt/unity/Editor/Unity -batchmode -nographics -logFile - \
                     -username "$UNITY_EMAIL" -password "$UNITY_PASSWORD" -serial "$UNITY_SERIAL" -quit || true
                 else
                   /opt/unity/Editor/Unity -batchmode -nographics -logFile - \
                     -username "$UNITY_EMAIL" -password "$UNITY_PASSWORD" -quit || true
                 fi
                 ls -la /root/.config/unity3d/Unity/licenses || true
-              '
+              ' 2>&1 | sed -E 's/((serial|license|password|token|email)[^[:space:]]*)/[REDACTED]/Ig'
 
-            # Verify entitlement written to host mount; allow ULF-only runs to proceed
-            if ! find "$RUNNER_TEMP/unity-config" -type f -iname "*.xml" | grep -q .; then
+            # Verify entitlement written to host mount; allow ULF-only runs to proceed
+            lic_dir="$RUNNER_TEMP/unity-config/Unity/licenses"
+            if ! find "$lic_dir" -maxdepth 1 -type f -iname "*.xml" | grep -q .; then
               if [[ "${{ steps.ulf.outputs.ok }}" == "true" ]]; then
                 echo "EBL entitlement not found; proceeding with ULF-only (ok=true)."
               else
                 echo "No entitlement produced and no valid ULF; cannot continue." >&2
                 exit 1
               fi
             fi

🧹 Nitpick comments (6)

.claude/prompts/nl-unity-suite-full-additive.md (2)

101-107: Specify language on fenced code block (markdownlint MD040).

Add a language to improve rendering and satisfy lint rules. C# fits the context.

-```
+```csharp
 // Tail test A
 // Tail test B  
 // Tail test C

---

`169-176`: **Clarify SHA usage in T-G step (make retry explicit).**

Recommend explicitly stating to call mcp__unity__get_sha after first edit before retrying, to avoid ambiguous stale_file handling.


```diff
-- Second should return `stale_file`, retry with updated SHA
+- Second should return `stale_file`; then call `mcp__unity__get_sha` and retry with the updated SHA

.github/workflows/claude-nl-suite.yml (4)

206-219: Minor: add --init to docker run and trim trailing spaces.

--init improves signal handling/zombie reaping; also remove trailing spaces flagged around Line 219.

-docker run -d --name unity-mcp --network host \
+docker run --init -d --name unity-mcp --network host \
@@
-              "$UNITY_IMAGE" /opt/unity/Editor/Unity -batchmode -nographics -logFile - \
+              "$UNITY_IMAGE" /opt/unity/Editor/Unity -batchmode -nographics -logFile - \
@@
-                -executeMethod MCPForUnity.Editor.MCPForUnityBridge.StartAutoConnect
+                -executeMethod MCPForUnity.Editor.MCPForUnityBridge.StartAutoConnect

---

232-251: Bound log growth: tail logs before pattern checks.

docker logs without a tail can grow large and slow the loop. Limit to recent lines.

-              logs="$(docker logs unity-mcp 2>&1 || true)"
+              logs="$(docker logs unity-mcp 2>&1 | tail -n 400 || true)"

---

381-393: Prompt/tools consistency: ensure prompt AllowedTools matches action’s allowed_tools.

Workflow allows Bash(scripts/nlt-revert.sh:*). Keep the prompt’s printed AllowedTools in sync (see prompt Line 6 fix).

---

398-446: Canon rules: tolerate en-dash and em-dash variants in separators.

Some agents may emit “NL-1 – …” (U+2013/2014). Expand the separator regex to accept unicode dashes.

-                  suffix = re.sub(rf"^\s*{re.escape(tid)}\s*[:.\-–—]?\s*", "", n, flags=re.I)
+                  suffix = re.sub(rf"^\s*{re.escape(tid)}\s*[:.\-–—]?\s*", "", n, flags=re.I)
+                  # (Already includes ASCII hyphen, en dash, em dash; keep as-is if you intended this.)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between eadfa7b and 0d43cfd.

📒 Files selected for processing (2)

• .claude/prompts/nl-unity-suite-full-additive.md (1 hunks)
• .github/workflows/claude-nl-suite.yml (7 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-08-29T21:31:02.469Z

Learnt from: dsarno
PR: dsarno/unity-mcp#56
File: .claude/prompts/nl-unity-suite-full.md:5-6
Timestamp: 2025-08-29T21:31:02.469Z
Learning: The Claude NL suite workflows (.github/workflows/claude-nl-suite.yml and .github/workflows/claude-nl-suite-mini.yml) allow "Bash" tools generally, including printf and echo operations, so prompts that reference Bash(printf:*) and Bash(echo:*) are consistent with the actual workflow permissions.

Applied to files:

• .github/workflows/claude-nl-suite.yml

🪛 YAMLlint (1.37.1)

.github/workflows/claude-nl-suite.yml

[warning] 75-75: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 75-75: too many spaces after colon

(colons)

---

[warning] 76-76: too many spaces after colon

(colons)

---

[warning] 78-78: too many spaces after colon

(colons)

---

[warning] 92-92: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 122-122: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 122-122: too many spaces after colon

(colons)

---

[warning] 123-123: too many spaces after colon

(colons)

---

[warning] 125-125: too many spaces after colon

(colons)

---

[warning] 166-166: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 196-196: wrong indentation: expected 14 but found 12

(indentation)

---

[error] 219-219: trailing spaces

(trailing-spaces)

---

[warning] 279-279: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 279-279: too many spaces after colon

(colons)

---

[warning] 281-281: too many spaces after colon

(colons)

🪛 LanguageTool

.claude/prompts/nl-unity-suite-full-additive.md

[grammar] ~5-~5: There might be a mistake here.
Context: ...this once, verbatim, early in the run:** AllowedTools: Write,mcp__unity__manage_e...

(QB_NEW_EN)

---

[grammar] ~13-~13: There might be a mistake here.
Context: ...contains evidence; include any key logs. - On failure or partial execution, still e...

(QB_NEW_EN)

---

[grammar] ~15-~15: There might be a mistake here.
Context: ...F, T-G, T-H, T-I, T-J. Use EXACT casing and dash. --- ## Mission 1) Pick target f...

(QB_NEW_EN)

---

[grammar] ~29-~29: There might be a mistake here.
Context: ...state. --- ## Environment & Paths (CI) - Always pass: `project_root: "TestProject...

(QB_NEW_EN)

---

[grammar] ~30-~30: There might be a mistake here.
Context: ...nd ctx: {} on list/read/edit/validate. - Canonical URIs only: - Primary: `uni...

(QB_NEW_EN)

---

[grammar] ~31-~31: There might be a mistake here.
Context: ...dit/validate. - Canonical URIs only: - Primary: unity://path/Assets/... (neve...

(QB_NEW_EN)

---

[grammar] ~32-~32: There might be a mistake here.
Context: ... (never embed project_root in the URI) - Relative (when supported): Assets/... ...

(QB_NEW_EN)

---

[grammar] ~41-~41: There might be a mistake here.
Context: ...esized from JUnit) --- ## Tool Mapping - Anchors/regex/structured: `mcp__unity_...

(QB_NEW_EN)

---

[grammar] ~42-~42: There might be a mistake here.
Context: ...Mapping - Anchors/regex/structured: mcp__unity__script_apply_edits - Allowed ops: anchor_insert, `replace_m...

(QB_NEW_EN)

---

[grammar] ~43-~43: There might be a mistake here.
Context: ...ethod, insert_method, delete_method, regex_replace - Foranchor_insert, always set "positi...

(QB_NEW_EN)

---

[grammar] ~44-~44: There might be a mistake here.
Context: ...set "position": "before" or "after". - Precise ranges / atomic batch: `mcp__u...

(QB_NEW_EN)

---

[grammar] ~45-~45: There might be a mistake here.
Context: ...ply_text_edits(non‑overlapping ranges) - **Hash-only**:mcp__unity__get_sha` — ret...

(QB_NEW_EN)

---

[grammar] ~46-~46: There might be a mistake here.
Context: ...ytes,lastModifiedUtc}without file body - **Validation**:mcp__unity__validate_scri...

(QB_NEW_EN)

---

[grammar] ~47-~47: There might be a mistake here.
Context: ...c}without file body - **Validation**:mcp__unity__validate_script(level:"standard")- **Dynamic targeting**: Usemcp__unity__fi...

(QB_NEW_EN)

---

[grammar] ~54-~54: There might be a mistake here.
Context: ...nciples Key Changes from Reset-Based: 1. Dynamic Targeting: Use find_in_file ...

(QB_NEW_EN)

---

[grammar] ~61-~61: There might be a mistake here.
Context: ...her in real workflows State Tracking: - Track file SHA after each test (`mcp__un...

(QB_NEW_EN)

---

[grammar] ~63-~63: There might be a mistake here.
Context: .../T‑I to exercise stale_file semantics. - Use content signatures (method names, co...

(QB_NEW_EN)

---

[grammar] ~71-~71: There might be a mistake here.
Context: ... Specs ### NL-0. Baseline State Capture Goal: Establish initial file state and...

(QB_NEW_EN)

---

[grammar] ~73-~73: There might be a mistake here.
Context: ...te and verify accessibility Actions: - Read file head and tail to confirm struc...

(QB_NEW_EN)

---

[grammar] ~79-~79: There might be a mistake here.
Context: ...ore Method Operations (Additive State A) Goal: Demonstrate method replacement o...

(QB_NEW_EN)

---

[grammar] ~81-~81: There might be a mistake here.
Context: ...thod replacement operations Actions: - Replace HasTarget() method body: `publ...

(QB_NEW_EN)

---

[grammar] ~88-~88: There might be a mistake here.
Context: ...hor Comment Insertion (Additive State B) Goal: Demonstrate anchor-based inserti...

(QB_NEW_EN)

---

[grammar] ~90-~90: There might be a mistake here.
Context: ...ed insertions above methods Actions: - Use find_in_file to locate current pos...

(QB_NEW_EN)

---

[grammar] ~91-~91: There might be a mistake here.
Context: ...ds Actions: - Use find_in_file to locate current position of Update() method -...

(QB_NEW_EN)

---

[grammar] ~96-~96: There might be a mistake here.
Context: ... End-of-Class Content (Additive State C) Goal: Demonstrate end-of-class inserti...

(QB_NEW_EN)

---

[grammar] ~98-~98: There might be a mistake here.
Context: ...s with smart brace matching Actions: - Match the final class-closing brace by s...

(QB_NEW_EN)

---

[grammar] ~100-~100: There might be a mistake here.
Context: ... + ranges; insert immediately before it. - Insert three comment lines before final ...

(QB_NEW_EN)

---

[grammar] ~101-~101: There might be a mistake here.
Context: ...before it. - Insert three comment lines before final class brace: ``` // Tail test...

(QB_NEW_EN)

---

[grammar] ~109-~109: There might be a mistake here.
Context: ...ole State Verification (No State Change) Goal: Verify Unity console integration...

(QB_NEW_EN)

---

[grammar] ~111-~111: There might be a mistake here.
Context: ...n without file modification Actions: - Read Unity console messages (INFO level)...

(QB_NEW_EN)

---

[grammar] ~116-~116: There might be a mistake here.
Context: ...ry Helper Lifecycle (Returns to State C) Goal: Test insert → verify → delete cy...

(QB_NEW_EN)

---

[grammar] ~118-~118: There might be a mistake here.
Context: ...te cycle for temporary code Actions: - Find current position of `GetCurrentTarg...

(QB_NEW_EN)

---

[grammar] ~125-~125: There might be a mistake here.
Context: ...od Body Interior Edit (Additive State D) Goal: Edit method interior without aff...

(QB_NEW_EN)

---

[grammar] ~127-~127: There might be a mistake here.
Context: ...structure, on modified file Actions: - Use find_in_file to locate current `Ha...

(QB_NEW_EN)

---

[grammar] ~129-~129: There might be a mistake here.
Context: ...dy interior: change return statement to return true; /* test modification */ - Use validate: "relaxed" for interior-o...

(QB_NEW_EN)

---

[grammar] ~134-~134: There might be a mistake here.
Context: ... Method Interior Edit (Additive State E) Goal: Edit a different method to show ...

(QB_NEW_EN)

---

[grammar] ~138-~138: There might be a mistake here.
Context: ... content search - Edit interior line to add null check: `if (animator == null) retu...

(QB_NEW_EN)

---

[grammar] ~142-~142: There might be a mistake here.
Context: .... End-of-Class Helper (Additive State F) Goal: Add permanent helper method at c...

(QB_NEW_EN)

---

[grammar] ~144-~144: There might be a mistake here.
Context: ... helper method at class end Actions: - Use smart anchor matching to find curren...

(QB_NEW_EN)

---

[grammar] ~149-~149: There might be a mistake here.
Context: ...d Evolution Lifecycle (Additive State G) Goal: Insert → modify → finalize a fie...

(QB_NEW_EN)

---

[grammar] ~150-~150: There might be a mistake here.
Context: ...fy → finalize a field + companion method Actions: - Insert field: `private in...

(QB_NEW_EN)

---

[grammar] ~151-~151: There might be a mistake here.
Context: ... a field + companion method Actions: - Insert field: private int Counter = 0;...

(QB_NEW_EN)

---

[grammar] ~152-~152: There might be a mistake here.
Context: ...ion method Actions: - Insert field: private int Counter = 0; - Update it: find and replace with `privat...

(QB_NEW_EN)

---

[grammar] ~153-~153: There might be a mistake here.
Context: ... 0;- Update it: find and replace withprivate int Counter = 42; // initialized- Add companion method:private void Incr...

(QB_NEW_EN)

---

[grammar] ~154-~154: There might be a mistake here.
Context: ...// initialized- Add companion method:private void IncrementCounter() { Counter++; }` - Expected final state: State F + Counte...

(QB_NEW_EN)

---

[grammar] ~157-~157: There might be a mistake here.
Context: ...-F. Atomic Multi-Edit (Additive State H) Goal: Multiple coordinated edits in si...

(QB_NEW_EN)

---

[grammar] ~158-~158: There might be a mistake here.
Context: ...H) Goal: Multiple coordinated edits in single atomic operation Actions: - ...

(QB_NEW_EN)

---

[grammar] ~158-~158: There might be a mistake here.
Context: ...dinated edits in single atomic operation Actions: - Read current file state t...

(QB_NEW_EN)

---

[grammar] ~159-~159: There might be a mistake here.
Context: ... in single atomic operation Actions: - Read current file state to compute preci...

(QB_NEW_EN)

---

[grammar] ~161-~161: There might be a mistake here.
Context: ... precise ranges - Atomic edit combining: 1. Add comment in HasTarget(): `// valida...

(QB_NEW_EN)

---

[grammar] ~162-~162: There might be a mistake here.
Context: ...ing: 1. Add comment in HasTarget(): // validated access 2. Add comment in ApplyBlend(): `// safe ...

(QB_NEW_EN)

---

[grammar] ~163-~163: There might be a mistake here.
Context: ... 2. Add comment inApplyBlend(): // safe animation 3. Add final class comment:// end of test...

(QB_NEW_EN)

---

[grammar] ~164-~164: There might be a mistake here.
Context: ...nimation 3. Add final class comment:// end of test modifications` - All edits computed from same file snapsh...

(QB_NEW_EN)

---

[grammar] ~165-~165: There might be a mistake here.
Context: ...est modifications` - All edits computed from same file snapshot, applied atomically ...

(QB_NEW_EN)

---

[grammar] ~168-~168: There might be a mistake here.
Context: ...ath Normalization Test (No State Change) Goal: Verify URI forms work equivalent...

(QB_NEW_EN)

---

[grammar] ~170-~170: There might be a mistake here.
Context: ...uivalently on modified file Actions: - Make identical edit using `unity://path/...

(QB_NEW_EN)

---

[grammar] ~171-~171: There might be a mistake here.
Context: ...Actions*: - Make identical edit using unity://path/Assets/Scripts/LongUnityScriptClaudeTest.cs - Then using `Assets/Scripts/LongUnityScri...

(QB_NEW_EN)

---

[grammar] ~172-~172: There might be a mistake here.
Context: ...gUnityScriptClaudeTest.cs- Then usingAssets/Scripts/LongUnityScriptClaudeTest.cs - Second should returnstale_file`, retry...

(QB_NEW_EN)

---

[grammar] ~174-~174: There might be a mistake here.
Context: ...ith updated SHA - Verify both URI forms target same file - Expected final state: S...

(QB_NEW_EN)

---

[grammar] ~177-~177: There might be a mistake here.
Context: ...ation on Modified File (No State Change) Goal: Ensure validation works correctl...

(QB_NEW_EN)

---

[grammar] ~179-~179: There might be a mistake here.
Context: ...ly on heavily modified file Actions: - Run validate_script(level:"standard") ...

(QB_NEW_EN)

---

[grammar] ~184-~184: There might be a mistake here.
Context: ...ailure Surface Testing (No State Change) Goal: Test error handling on real modi...

(QB_NEW_EN)

---

[grammar] ~186-~186: There might be a mistake here.
Context: ...dling on real modified file Actions: - Attempt overlapping edits (should fail c...

(QB_NEW_EN)

---

[grammar] ~192-~192: There might be a mistake here.
Context: ...ency on Modified File (Additive State I) Goal: Verify operations behave predict...

(QB_NEW_EN)

---

[grammar] ~194-~194: There might be a mistake here.
Context: ...e predictably when repeated Actions: - Insert (structured): `mcp__unity__scri...

(QB_NEW_EN)

---

[grammar] ~198-~198: There might be a mistake here.
Context: ...o_op: true. - **Remove (structured)**: {"op":"regex_replace","pattern":"(?m)^\s*// idempotency test marker\r?\n?","text":""}- **Remove again** (sameregex_replace`) → ...

(QB_NEW_EN)

---

[grammar] ~233-~233: There might be a mistake here.
Context: ...erification Patterns After each test: 1. Verify expected content exists: `find_in...

(QB_NEW_EN)

---

[grammar] ~239-~239: There might be a mistake here.
Context: ...nges in test evidence Error Recovery: - If test fails, log current state but con...

(QB_NEW_EN)

---

[grammar] ~240-~240: There might be a mistake here.
Context: ...e Error Recovery: - If test fails, log current state but continue (don't resto...

(QB_NEW_EN)

---

[grammar] ~241-~241: There might be a mistake here.
Context: ...inue (don't restore) - Next test adapts to actual current state, not expected stat...

(QB_NEW_EN)

---

[grammar] ~241-~241: There might be a mistake here.
Context: ...xt test adapts to actual current state, not expected state - Demonstrates resilienc...

(QB_NEW_EN)

🪛 markdownlint-cli2 (0.17.2)

.claude/prompts/nl-unity-suite-full-additive.md

102-102: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🔇 Additional comments (2)

.github/workflows/claude-nl-suite.yml (2)

169-183: Good: manual license file passed via array is robust.

Switch to array-based manual_args fixes prior quoting pitfalls with paths containing spaces.

---

545-642: Nice: Markdown summary is resilient and capped.

Handles missing JUnit, truncation, and code fencing safely.

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (3)

.claude/prompts/nl-unity-suite-full-additive.md (2)

129-133: Use standard validation level consistently.

Doc prescribes standard validation elsewhere; “relaxed” here is inconsistent and was previously flagged.

-– Use `validate: "relaxed"` for interior-only edit
+– Validate with `mcp__unity__validate_script(level:"standard")` (keep a single, consistent level)

---

5-7: Align AllowedTools with workflow (add Bash revert helper).

Printed AllowedTools omits Bash(scripts/nlt-revert.sh:*), which is allowed in the workflow; agents will under-utilize the revert tool.

Apply:

-AllowedTools: Write,mcp__unity__manage_editor,mcp__unity__list_resources,mcp__unity__read_resource,mcp__unity__apply_text_edits,mcp__unity__script_apply_edits,mcp__unity__validate_script,mcp__unity__find_in_file,mcp__unity__read_console,mcp__unity__get_sha
+AllowedTools: Write,Bash(scripts/nlt-revert.sh:*),mcp__unity__manage_editor,mcp__unity__list_resources,mcp__unity__read_resource,mcp__unity__apply_text_edits,mcp__unity__script_apply_edits,mcp__unity__validate_script,mcp__unity__find_in_file,mcp__unity__read_console,mcp__unity__get_sha

.github/workflows/claude-nl-suite.yml (1)

131-157: EBL activation leaks secrets to logs and entitlement check is too broad.

docker run prints credentials/serial; also checking any *.xml under unity-config risks false positives.

Apply:

-            docker run --rm --network host \
+            # Redact sensitive tokens from activation logs
+            docker run --rm --network host \
               -e HOME=/root \
               -e UNITY_EMAIL -e UNITY_PASSWORD -e UNITY_SERIAL \
               -v "$RUNNER_TEMP/unity-config:/root/.config/unity3d" \
               -v "$RUNNER_TEMP/unity-local:/root/.local/share/unity3d" \
-              "$UNITY_IMAGE" bash -lc '
+              "$UNITY_IMAGE" bash -lc '
                 set -euxo pipefail
                 if [[ -n "${UNITY_SERIAL:-}" ]]; then
                   /opt/unity/Editor/Unity -batchmode -nographics -logFile - \
                     -username "$UNITY_EMAIL" -password "$UNITY_PASSWORD" -serial "$UNITY_SERIAL" -quit || true
                 else
                   /opt/unity/Editor/Unity -batchmode -nographics -logFile - \
                     -username "$UNITY_EMAIL" -password "$UNITY_PASSWORD" -quit || true
                 fi
                 ls -la /root/.config/unity3d/Unity/licenses || true
-              '
+              ' 2>&1 | sed -E 's/((serial|license|password|token|email)[^[:space:]]*)/[REDACTED]/Ig'
 
-            # Verify entitlement written to host mount; allow ULF-only runs to proceed
-            if ! find "$RUNNER_TEMP/unity-config" -type f -iname "*.xml" | grep -q .; then
+            # Verify entitlement written to host mount; allow ULF-only runs to proceed
+            lic_dir="$RUNNER_TEMP/unity-config/Unity/licenses"
+            if ! find "$lic_dir" -maxdepth 1 -type f -iname "*.xml" | grep -q .; then
               if [[ "${{ steps.ulf.outputs.ok }}" == "true" ]]; then
                 echo "EBL entitlement not found; proceeding with ULF-only (ok=true)."
               else
                 echo "No entitlement produced and no valid ULF; cannot continue." >&2
                 exit 1
               fi
             fi

🧹 Nitpick comments (3)

.claude/prompts/nl-unity-suite-full-additive.md (2)

103-107: Add language to fenced block (markdownlint MD040).

-```
+```csharp
 // Tail test A
 // Tail test B  
 // Tail test C

---

`208-229`: **Fix mislabeled “json” examples (not valid JSON).**

Use “text” to avoid misleading syntax highlighting and lint noise.

```diff
-```json
+```text
 {"startLine": 31, "startCol": 26, "endLine": 31, "endCol": 58}

-json +text

Find current method location

find_in_file(pattern: "public bool HasTarget\(\)")

Then compute edit ranges from found position

-```json
+```text
{"op": "replace_method", "className": "LongUnityScriptClaudeTest", "methodName": "HasTarget"}

-json +text
{"op": "anchor_insert", "anchor": "private void Update\(\)", "position": "before", "text": "// comment"}

.github/workflows/claude-nl-suite.yml (1)

219-219: Minor YAML lint nits (indent/colons/trailing space).

Not functional; clean up when convenient.

Also applies to: 75-78, 92-92, 122-125, 166-166, 196-196, 279-281

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0d43cfd and ebbed23.

📒 Files selected for processing (2)

• .claude/prompts/nl-unity-suite-full-additive.md (1 hunks)
• .github/workflows/claude-nl-suite.yml (11 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-08-29T21:31:02.469Z

Learnt from: dsarno
PR: dsarno/unity-mcp#56
File: .claude/prompts/nl-unity-suite-full.md:5-6
Timestamp: 2025-08-29T21:31:02.469Z
Learning: The Claude NL suite workflows (.github/workflows/claude-nl-suite.yml and .github/workflows/claude-nl-suite-mini.yml) allow "Bash" tools generally, including printf and echo operations, so prompts that reference Bash(printf:*) and Bash(echo:*) are consistent with the actual workflow permissions.

Applied to files:

• .claude/prompts/nl-unity-suite-full-additive.md
• .github/workflows/claude-nl-suite.yml

🪛 LanguageTool

.claude/prompts/nl-unity-suite-full-additive.md

[grammar] ~5-~5: There might be a mistake here.
Context: ...this once, verbatim, early in the run:** AllowedTools: Write,mcp__unity__manage_e...

(QB_NEW_EN)

---

[grammar] ~12-~12: There might be a mistake here.
Context: ...file must contain a SINGLE root element. - When writing a fragment, set `<testcase ...

(QB_NEW_EN)

---

[grammar] ~14-~14: There might be a mistake here.
Context: ...contains evidence; include any key logs. - On failure or partial execution, still e...

(QB_NEW_EN)

---

[grammar] ~16-~16: There might be a mistake here.
Context: ...F, T-G, T-H, T-I, T-J. Use EXACT casing and dash. --- ## Mission 1) Pick target f...

(QB_NEW_EN)

---

[grammar] ~30-~30: There might be a mistake here.
Context: ...state. --- ## Environment & Paths (CI) - Always pass: `project_root: "TestProject...

(QB_NEW_EN)

---

[grammar] ~31-~31: There might be a mistake here.
Context: ...nd ctx: {} on list/read/edit/validate. - Canonical URIs only: - Primary: `uni...

(QB_NEW_EN)

---

[grammar] ~32-~32: There might be a mistake here.
Context: ...dit/validate. - Canonical URIs only: - Primary: unity://path/Assets/... (neve...

(QB_NEW_EN)

---

[grammar] ~33-~33: There might be a mistake here.
Context: ... (never embed project_root in the URI) - Relative (when supported): Assets/... ...

(QB_NEW_EN)

---

[grammar] ~42-~42: There might be a mistake here.
Context: ...esized from JUnit) --- ## Tool Mapping - Anchors/regex/structured: `mcp__unity_...

(QB_NEW_EN)

---

[grammar] ~43-~43: There might be a mistake here.
Context: ...Mapping - Anchors/regex/structured: mcp__unity__script_apply_edits - Allowed ops: anchor_insert, `replace_m...

(QB_NEW_EN)

---

[grammar] ~44-~44: There might be a mistake here.
Context: ...ethod, insert_method, delete_method, regex_replace - Foranchor_insert, always set "positi...

(QB_NEW_EN)

---

[grammar] ~45-~45: There might be a mistake here.
Context: ...set "position": "before" or "after". - Precise ranges / atomic batch: `mcp__u...

(QB_NEW_EN)

---

[grammar] ~46-~46: There might be a mistake here.
Context: ...ply_text_edits(non‑overlapping ranges) - **Hash-only**:mcp__unity__get_sha` — ret...

(QB_NEW_EN)

---

[grammar] ~47-~47: There might be a mistake here.
Context: ...ytes,lastModifiedUtc}without file body - **Validation**:mcp__unity__validate_scri...

(QB_NEW_EN)

---

[grammar] ~48-~48: There might be a mistake here.
Context: ...c}without file body - **Validation**:mcp__unity__validate_script(level:"standard")- **Dynamic targeting**: Usemcp__unity__fi...

(QB_NEW_EN)

---

[grammar] ~55-~55: There might be a mistake here.
Context: ...nciples Key Changes from Reset-Based: 1. Dynamic Targeting: Use find_in_file ...

(QB_NEW_EN)

---

[grammar] ~62-~62: There might be a mistake here.
Context: ...her in real workflows State Tracking: - Track file SHA after each test (`mcp__un...

(QB_NEW_EN)

---

[grammar] ~64-~64: There might be a mistake here.
Context: .../T‑I to exercise stale_file semantics. - Use content signatures (method names, co...

(QB_NEW_EN)

---

[grammar] ~72-~72: There might be a mistake here.
Context: ... Specs ### NL-0. Baseline State Capture Goal: Establish initial file state and...

(QB_NEW_EN)

---

[grammar] ~74-~74: There might be a mistake here.
Context: ...te and verify accessibility Actions: - Read file head and tail to confirm struc...

(QB_NEW_EN)

---

[grammar] ~80-~80: There might be a mistake here.
Context: ...ore Method Operations (Additive State A) Goal: Demonstrate method replacement o...

(QB_NEW_EN)

---

[grammar] ~82-~82: There might be a mistake here.
Context: ...thod replacement operations Actions: - Replace HasTarget() method body: `publ...

(QB_NEW_EN)

---

[grammar] ~89-~89: There might be a mistake here.
Context: ...hor Comment Insertion (Additive State B) Goal: Demonstrate anchor-based inserti...

(QB_NEW_EN)

---

[grammar] ~91-~91: There might be a mistake here.
Context: ...ed insertions above methods Actions: - Use find_in_file to locate current pos...

(QB_NEW_EN)

---

[grammar] ~92-~92: There might be a mistake here.
Context: ...ds Actions: - Use find_in_file to locate current position of Update() method -...

(QB_NEW_EN)

---

[grammar] ~97-~97: There might be a mistake here.
Context: ... End-of-Class Content (Additive State C) Goal: Demonstrate end-of-class inserti...

(QB_NEW_EN)

---

[grammar] ~99-~99: There might be a mistake here.
Context: ...s with smart brace matching Actions: - Match the final class-closing brace by s...

(QB_NEW_EN)

---

[grammar] ~101-~101: There might be a mistake here.
Context: ... + ranges; insert immediately before it. - Insert three comment lines before final ...

(QB_NEW_EN)

---

[grammar] ~102-~102: There might be a mistake here.
Context: ...before it. - Insert three comment lines before final class brace: ``` // Tail test...

(QB_NEW_EN)

---

[grammar] ~110-~110: There might be a mistake here.
Context: ...ole State Verification (No State Change) Goal: Verify Unity console integration...

(QB_NEW_EN)

---

[grammar] ~112-~112: There might be a mistake here.
Context: ...n without file modification Actions: - Read Unity console messages (INFO level)...

(QB_NEW_EN)

---

[grammar] ~117-~117: There might be a mistake here.
Context: ...ry Helper Lifecycle (Returns to State C) Goal: Test insert → verify → delete cy...

(QB_NEW_EN)

---

[grammar] ~119-~119: There might be a mistake here.
Context: ...te cycle for temporary code Actions: - Find current position of `GetCurrentTarg...

(QB_NEW_EN)

---

[grammar] ~126-~126: There might be a mistake here.
Context: ...od Body Interior Edit (Additive State D) Goal: Edit method interior without aff...

(QB_NEW_EN)

---

[grammar] ~128-~128: There might be a mistake here.
Context: ...structure, on modified file Actions: - Use find_in_file to locate current `Ha...

(QB_NEW_EN)

---

[grammar] ~130-~130: There might be a mistake here.
Context: ...dy interior: change return statement to return true; /* test modification */ - Use validate: "relaxed" for interior-o...

(QB_NEW_EN)

---

[grammar] ~135-~135: There might be a mistake here.
Context: ... Method Interior Edit (Additive State E) Goal: Edit a different method to show ...

(QB_NEW_EN)

---

[grammar] ~139-~139: There might be a mistake here.
Context: ... content search - Edit interior line to add null check: `if (animator == null) retu...

(QB_NEW_EN)

---

[grammar] ~143-~143: There might be a mistake here.
Context: .... End-of-Class Helper (Additive State F) Goal: Add permanent helper method at c...

(QB_NEW_EN)

---

[grammar] ~145-~145: There might be a mistake here.
Context: ... helper method at class end Actions: - Use smart anchor matching to find curren...

(QB_NEW_EN)

---

[grammar] ~150-~150: There might be a mistake here.
Context: ...d Evolution Lifecycle (Additive State G) Goal: Insert → modify → finalize a fie...

(QB_NEW_EN)

---

[grammar] ~151-~151: There might be a mistake here.
Context: ...fy → finalize a field + companion method Actions: - Insert field: `private in...

(QB_NEW_EN)

---

[grammar] ~152-~152: There might be a mistake here.
Context: ... a field + companion method Actions: - Insert field: private int Counter = 0;...

(QB_NEW_EN)

---

[grammar] ~153-~153: There might be a mistake here.
Context: ...ion method Actions: - Insert field: private int Counter = 0; - Update it: find and replace with `privat...

(QB_NEW_EN)

---

[grammar] ~154-~154: There might be a mistake here.
Context: ... 0;- Update it: find and replace withprivate int Counter = 42; // initialized- Add companion method:private void Incr...

(QB_NEW_EN)

---

[grammar] ~155-~155: There might be a mistake here.
Context: ...// initialized- Add companion method:private void IncrementCounter() { Counter++; }` - Expected final state: State F + Counte...

(QB_NEW_EN)

---

[grammar] ~158-~158: There might be a mistake here.
Context: ...-F. Atomic Multi-Edit (Additive State H) Goal: Multiple coordinated edits in si...

(QB_NEW_EN)

---

[grammar] ~159-~159: There might be a mistake here.
Context: ...H) Goal: Multiple coordinated edits in single atomic operation Actions: - ...

(QB_NEW_EN)

---

[grammar] ~159-~159: There might be a mistake here.
Context: ...dinated edits in single atomic operation Actions: - Read current file state t...

(QB_NEW_EN)

---

[grammar] ~160-~160: There might be a mistake here.
Context: ... in single atomic operation Actions: - Read current file state to compute preci...

(QB_NEW_EN)

---

[grammar] ~162-~162: There might be a mistake here.
Context: ... precise ranges - Atomic edit combining: 1. Add comment in HasTarget(): `// valida...

(QB_NEW_EN)

---

[grammar] ~163-~163: There might be a mistake here.
Context: ...ing: 1. Add comment in HasTarget(): // validated access 2. Add comment in ApplyBlend(): `// safe ...

(QB_NEW_EN)

---

[grammar] ~164-~164: There might be a mistake here.
Context: ... 2. Add comment inApplyBlend(): // safe animation 3. Add final class comment:// end of test...

(QB_NEW_EN)

---

[grammar] ~165-~165: There might be a mistake here.
Context: ...nimation 3. Add final class comment:// end of test modifications` - All edits computed from same file snapsh...

(QB_NEW_EN)

---

[grammar] ~166-~166: There might be a mistake here.
Context: ...est modifications` - All edits computed from same file snapshot, applied atomically ...

(QB_NEW_EN)

---

[grammar] ~169-~169: There might be a mistake here.
Context: ...ath Normalization Test (No State Change) Goal: Verify URI forms work equivalent...

(QB_NEW_EN)

---

[grammar] ~171-~171: There might be a mistake here.
Context: ...uivalently on modified file Actions: - Make identical edit using `unity://path/...

(QB_NEW_EN)

---

[grammar] ~172-~172: There might be a mistake here.
Context: ...Actions*: - Make identical edit using unity://path/Assets/Scripts/LongUnityScriptClaudeTest.cs - Then using `Assets/Scripts/LongUnityScri...

(QB_NEW_EN)

---

[grammar] ~173-~173: There might be a mistake here.
Context: ...gUnityScriptClaudeTest.cs- Then usingAssets/Scripts/LongUnityScriptClaudeTest.cs - Second should returnstale_file`, retry...

(QB_NEW_EN)

---

[grammar] ~175-~175: There might be a mistake here.
Context: ...ith updated SHA - Verify both URI forms target same file - Expected final state: S...

(QB_NEW_EN)

---

[grammar] ~178-~178: There might be a mistake here.
Context: ...ation on Modified File (No State Change) Goal: Ensure validation works correctl...

(QB_NEW_EN)

---

[grammar] ~180-~180: There might be a mistake here.
Context: ...ly on heavily modified file Actions: - Run validate_script(level:"standard") ...

(QB_NEW_EN)

---

[grammar] ~185-~185: There might be a mistake here.
Context: ...ailure Surface Testing (No State Change) Goal: Test error handling on real modi...

(QB_NEW_EN)

---

[grammar] ~187-~187: There might be a mistake here.
Context: ...dling on real modified file Actions: - Attempt overlapping edits (should fail c...

(QB_NEW_EN)

---

[grammar] ~193-~193: There might be a mistake here.
Context: ...ency on Modified File (Additive State I) Goal: Verify operations behave predict...

(QB_NEW_EN)

---

[grammar] ~195-~195: There might be a mistake here.
Context: ...e predictably when repeated Actions: - Insert (structured): `mcp__unity__scri...

(QB_NEW_EN)

---

[grammar] ~199-~199: There might be a mistake here.
Context: ...o_op: true. - **Remove (structured)**: {"op":"regex_replace","pattern":"(?m)^\s*// idempotency test marker\r?\n?","text":""}- **Remove again** (sameregex_replace`) → ...

(QB_NEW_EN)

---

[grammar] ~234-~234: There might be a mistake here.
Context: ...erification Patterns After each test: 1. Verify expected content exists: `find_in...

(QB_NEW_EN)

---

[grammar] ~240-~240: There might be a mistake here.
Context: ...nges in test evidence Error Recovery: - If test fails, log current state but con...

(QB_NEW_EN)

---

[grammar] ~241-~241: There might be a mistake here.
Context: ...e Error Recovery: - If test fails, log current state but continue (don't resto...

(QB_NEW_EN)

---

[grammar] ~242-~242: There might be a mistake here.
Context: ...inue (don't restore) - Next test adapts to actual current state, not expected stat...

(QB_NEW_EN)

---

[grammar] ~242-~242: There might be a mistake here.
Context: ...xt test adapts to actual current state, not expected state - Demonstrates resilienc...

(QB_NEW_EN)

🪛 markdownlint-cli2 (0.17.2)

.claude/prompts/nl-unity-suite-full-additive.md

103-103: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🪛 YAMLlint (1.37.1)

.github/workflows/claude-nl-suite.yml

[warning] 75-75: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 75-75: too many spaces after colon

(colons)

---

[warning] 76-76: too many spaces after colon

(colons)

---

[warning] 78-78: too many spaces after colon

(colons)

---

[warning] 92-92: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 122-122: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 122-122: too many spaces after colon

(colons)

---

[warning] 123-123: too many spaces after colon

(colons)

---

[warning] 125-125: too many spaces after colon

(colons)

---

[warning] 166-166: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 196-196: wrong indentation: expected 14 but found 12

(indentation)

---

[error] 219-219: trailing spaces

(trailing-spaces)

---

[warning] 279-279: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 279-279: too many spaces after colon

(colons)

---

[warning] 281-281: too many spaces after colon

(colons)

🔇 Additional comments (2)

.github/workflows/claude-nl-suite.yml (2)

169-183: Good use of bash arrays for manual license args (fixes prior unquoted-expansion risk).

Also applies to: 199-203

---

225-273: Robust readiness gate looks good (deadline, fatal gating, log redaction, TCP probe).

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (4)

.claude/prompts/nl-unity-suite-full-additive.md (2)

138-139: Use standard validation level for consistency.

Use mcp__unity__validate_script(level:"standard") everywhere to avoid inconsistent guidance.

-- Use `validate: "relaxed"` for interior-only edit
+- Validate with `mcp__unity__validate_script(level:"standard")` even for interior-only edits

---

5-7: Add Bash revert helper to AllowedTools to match workflow.

The printed AllowedTools omits Bash(scripts/nlt-revert.sh:*), which the workflow allows. This can confuse the agent.

-AllowedTools: Write,mcp__unity__manage_editor,mcp__unity__list_resources,mcp__unity__read_resource,mcp__unity__apply_text_edits,mcp__unity__script_apply_edits,mcp__unity__validate_script,mcp__unity__find_in_file,mcp__unity__read_console,mcp__unity__get_sha
+AllowedTools: Write,Bash(scripts/nlt-revert.sh:*),mcp__unity__manage_editor,mcp__unity__list_resources,mcp__unity__read_resource,mcp__unity__apply_text_edits,mcp__unity__script_apply_edits,mcp__unity__validate_script,mcp__unity__find_in_file,mcp__unity__read_console,mcp__unity__get_sha

.github/workflows/claude-nl-suite.yml (2)

274-281: Move “Return Pro license” to teardown after Unity is stopped.

Returning the license here can invalidate Unity before tests complete. Relocate after the Stop Unity step.

-        - name: Return Pro license (if used)
-          if: always() && steps.lic.outputs.use_ebl == 'true' && steps.lic.outputs.has_serial == 'true'
-          uses: game-ci/unity-return-license@v2
-          continue-on-error: true
-          env:
-            UNITY_EMAIL:    ${{ secrets.UNITY_EMAIL }}
-            UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }}
-            UNITY_SERIAL:   ${{ secrets.UNITY_SERIAL }}
+        # (moved to teardown after "Stop Unity")

Add after the Stop Unity step:

        - name: Return Pro license (if used)
          if: always() && steps.lic.outputs.use_ebl == 'true' && steps.lic.outputs.has_serial == 'true'
          uses: game-ci/unity-return-license@v2
          continue-on-error: true
          env:
            UNITY_EMAIL:    ${{ secrets.UNITY_EMAIL }}
            UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }}
            UNITY_SERIAL:   ${{ secrets.UNITY_SERIAL }}

---

131-157: Redact secrets from activation logs and tighten entitlement check path.

docker run logs can leak email/serial/password; also, the entitlement check scans the entire config tree (false positives). Pipe logs through a redactor and check Unity/licenses specifically.

-            docker run --rm --network host \
+            docker run --rm --network host \
               -e HOME=/root \
               -e UNITY_EMAIL -e UNITY_PASSWORD -e UNITY_SERIAL \
               -v "$RUNNER_TEMP/unity-config:/root/.config/unity3d" \
               -v "$RUNNER_TEMP/unity-local:/root/.local/share/unity3d" \
-              "$UNITY_IMAGE" bash -lc '
+              "$UNITY_IMAGE" bash -lc '
                 set -euxo pipefail
                 if [[ -n "${UNITY_SERIAL:-}" ]]; then
                   /opt/unity/Editor/Unity -batchmode -nographics -logFile - \
                     -username "$UNITY_EMAIL" -password "$UNITY_PASSWORD" -serial "$UNITY_SERIAL" -quit || true
                 else
                   /opt/unity/Editor/Unity -batchmode -nographics -logFile - \
                     -username "$UNITY_EMAIL" -password "$UNITY_PASSWORD" -quit || true
                 fi
                 ls -la /root/.config/unity3d/Unity/licenses || true
-              '
+              ' 2>&1 | sed -E 's/((serial|license|password|token|email)[^[:space:]]*)/[REDACTED]/Ig'
@@
-            if ! find "$RUNNER_TEMP/unity-config" -type f -iname "*.xml" | grep -q .; then
+            lic_dir="$RUNNER_TEMP/unity-config/Unity/licenses"
+            if ! find "$lic_dir" -maxdepth 1 -type f -iname "*.xml" | grep -q .; then
               if [[ "${{ steps.ulf.outputs.ok }}" == "true" ]]; then
                 echo "EBL entitlement not found; proceeding with ULF-only (ok=true)."
               else
                 echo "No entitlement produced and no valid ULF; cannot continue." >&2
                 exit 1
               fi
             fi

Optional hardening: drop -x inside the container during credentialed calls to reduce sensitive echoing.

🧹 Nitpick comments (2)

.claude/prompts/nl-unity-suite-full-additive.md (2)

110-115: Specify a language on fenced code block (markdownlint MD040).

Add a language (e.g., text) to the fence to satisfy linters.

-  ```
+  ```text
   // Tail test A
   // Tail test B  
   // Tail test C

---

`223-229`: **Avoid labeling non-JSON examples as JSON.**

This block contains comments and is not valid JSON; label as text to prevent misleading syntax highlighting.


```diff
-```json
+```text
 # Find current method location
 find_in_file(pattern: "public bool HasTarget\\(\\)")
 # Then compute edit ranges from found position

</blockquote></details>

</blockquote></details>

<details>
<summary>📜 Review details</summary>

**Configuration used**: CodeRabbit UI

**Review profile**: CHILL

**Plan**: Pro

**💡 Knowledge Base configuration:**

- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between ebbed234c82ce9a34d2c73f65269b025872d217e and 3bc7bf5228c152fe679121a995e22dda18f0eb88.

</details>

<details>
<summary>📒 Files selected for processing (2)</summary>

* `.claude/prompts/nl-unity-suite-full-additive.md` (1 hunks)
* `.github/workflows/claude-nl-suite.yml` (11 hunks)

</details>

<details>
<summary>🧰 Additional context used</summary>

<details>
<summary>🧠 Learnings (1)</summary>

<details>
<summary>📚 Learning: 2025-08-29T21:31:02.469Z</summary>

Learnt from: dsarno
PR: #56
File: .claude/prompts/nl-unity-suite-full.md:5-6
Timestamp: 2025-08-29T21:31:02.469Z
Learning: The Claude NL suite workflows (.github/workflows/claude-nl-suite.yml and .github/workflows/claude-nl-suite-mini.yml) allow "Bash" tools generally, including printf and echo operations, so prompts that reference Bash(printf:) and Bash(echo:) are consistent with the actual workflow permissions.

**Applied to files:**
- `.claude/prompts/nl-unity-suite-full-additive.md`
- `.github/workflows/claude-nl-suite.yml`

</details>

</details><details>
<summary>🪛 LanguageTool</summary>

<details>
<summary>.claude/prompts/nl-unity-suite-full-additive.md</summary>

[grammar] ~5-~5: There might be a mistake here.
Context: ...this once, verbatim, early in the run:** AllowedTools: Write,mcp__unity__manage_e...

(QB_NEW_EN)

---

[grammar] ~17-~17: There might be a mistake here.
Context: ...T`.  **CRITICAL XML FORMAT REQUIREMENTS:** - Each file must contain EXACTLY one `<tes...

(QB_NEW_EN)

---

[grammar] ~32-~32: There might be a mistake here.
Context: ..., T-C, T-D, T-E, T-F, T-G, T-H, T-I, T-J 5) **NO RESTORATION** - tests build additivel...

(QB_NEW_EN)

---

[grammar] ~33-~33: There might be a mistake here.
Context: ... RESTORATION** - tests build additively on previous state.  ---  ## Environment & ...

(QB_NEW_EN)

---

[grammar] ~37-~37: There might be a mistake here.
Context: ...state.  ---  ## Environment & Paths (CI) - Always pass: `project_root: "TestProject...

(QB_NEW_EN)

---

[grammar] ~38-~38: There might be a mistake here.
Context: ...nd `ctx: {}` on list/read/edit/validate. - **Canonical URIs only**:   - Primary: `uni...

(QB_NEW_EN)

---

[grammar] ~39-~39: There might be a mistake here.
Context: ...dit/validate. - **Canonical URIs only**:   - Primary: `unity://path/Assets/...` (neve...

(QB_NEW_EN)

---

[grammar] ~40-~40: There might be a mistake here.
Context: ... (never embed `project_root` in the URI)   - Relative (when supported): `Assets/...` ...

(QB_NEW_EN)

---

[grammar] ~49-~49: There might be a mistake here.
Context: ...esized from JUnit)  ---  ## Tool Mapping - **Anchors/regex/structured**: `mcp__unity_...

(QB_NEW_EN)

---

[grammar] ~50-~50: There might be a mistake here.
Context: ...Mapping - **Anchors/regex/structured**: `mcp__unity__script_apply_edits`   - Allowed ops: `anchor_insert`, `replace_m...

(QB_NEW_EN)

---

[grammar] ~51-~51: There might be a mistake here.
Context: ...ethod`, `insert_method`, `delete_method`, `regex_replace`   - For `anchor_insert`, always set `"positi...

(QB_NEW_EN)

---

[grammar] ~52-~52: There might be a mistake here.
Context: ...set `"position": "before"` or `"after"`. - **Precise ranges / atomic batch**: `mcp__u...

(QB_NEW_EN)

---

[grammar] ~53-~53: There might be a mistake here.
Context: ...ply_text_edits` (non‑overlapping ranges) - **Hash-only**: `mcp__unity__get_sha` — ret...

(QB_NEW_EN)

---

[grammar] ~54-~54: There might be a mistake here.
Context: ...ytes,lastModifiedUtc}` without file body - **Validation**: `mcp__unity__validate_scri...

(QB_NEW_EN)

---

[grammar] ~55-~55: There might be a mistake here.
Context: ...c}` without file body - **Validation**: `mcp__unity__validate_script(level:"standard")` - **Dynamic targeting**: Use `mcp__unity__fi...

(QB_NEW_EN)

---

[grammar] ~62-~62: There might be a mistake here.
Context: ...nciples  **Key Changes from Reset-Based:** 1. **Dynamic Targeting**: Use `find_in_file` ...

(QB_NEW_EN)

---

[grammar] ~69-~69: There might be a mistake here.
Context: ...her in real workflows  **State Tracking:** - Track file SHA after each test (`mcp__un...

(QB_NEW_EN)

---

[grammar] ~71-~71: There might be a mistake here.
Context: .../T‑I to exercise `stale_file` semantics. - Use content signatures (method names, co...

(QB_NEW_EN)

---

[grammar] ~79-~79: There might be a mistake here.
Context: ... Specs  ### NL-0. Baseline State Capture **Goal**: Establish initial file state and...

(QB_NEW_EN)

---

[grammar] ~81-~81: There might be a mistake here.
Context: ...te and verify accessibility **Actions**: - Read file head and tail to confirm struc...

(QB_NEW_EN)

---

[grammar] ~87-~87: There might be a mistake here.
Context: ...ore Method Operations (Additive State A) **Goal**: Demonstrate method replacement o...

(QB_NEW_EN)

---

[grammar] ~89-~89: There might be a mistake here.
Context: ...thod replacement operations **Actions**:  - Replace `HasTarget()` method body: `publ...

(QB_NEW_EN)

---

[grammar] ~96-~96: There might be a mistake here.
Context: ...hor Comment Insertion (Additive State B)  **Goal**: Demonstrate anchor-based inserti...

(QB_NEW_EN)

---

[grammar] ~98-~98: There might be a mistake here.
Context: ...ed insertions above methods **Actions**: - Use `find_in_file` to locate current pos...

(QB_NEW_EN)

---

[grammar] ~99-~99: There might be a mistake here.
Context: ...ds **Actions**: - Use `find_in_file` to locate current position of `Update()` method -...

(QB_NEW_EN)

---

[grammar] ~104-~104: There might be a mistake here.
Context: ... End-of-Class Content (Additive State C) **Goal**: Demonstrate end-of-class inserti...

(QB_NEW_EN)

---

[grammar] ~106-~106: There might be a mistake here.
Context: ...s with smart brace matching **Actions**: - Match the final class-closing brace by s...

(QB_NEW_EN)

---

[grammar] ~108-~108: There might be a mistake here.
Context: ... + ranges; insert immediately before it. - Insert three comment lines before final ...

(QB_NEW_EN)

---

[grammar] ~109-~109: There might be a mistake here.
Context: ...before it. - Insert three comment lines before final class brace:   ```   // Tail test...

(QB_NEW_EN)

---

[grammar] ~117-~117: There might be a mistake here.
Context: ...ole State Verification (No State Change) **Goal**: Verify Unity console integration...

(QB_NEW_EN)

---

[grammar] ~119-~119: There might be a mistake here.
Context: ...n without file modification **Actions**: - Read Unity console messages (INFO level)...

(QB_NEW_EN)

---

[grammar] ~124-~124: There might be a mistake here.
Context: ...ry Helper Lifecycle (Returns to State C) **Goal**: Test insert → verify → delete cy...

(QB_NEW_EN)

---

[grammar] ~126-~126: There might be a mistake here.
Context: ...te cycle for temporary code **Actions**: - Find current position of `GetCurrentTarg...

(QB_NEW_EN)

---

[grammar] ~133-~133: There might be a mistake here.
Context: ...od Body Interior Edit (Additive State D) **Goal**: Edit method interior without aff...

(QB_NEW_EN)

---

[grammar] ~135-~135: There might be a mistake here.
Context: ...structure, on modified file **Actions**: - Use `find_in_file` to locate current `Ha...

(QB_NEW_EN)

---

[grammar] ~137-~137: There might be a mistake here.
Context: ...dy interior: change return statement to `return true; /* test modification */` - Use `validate: "relaxed"` for interior-o...

(QB_NEW_EN)

---

[grammar] ~142-~142: There might be a mistake here.
Context: ... Method Interior Edit (Additive State E) **Goal**: Edit a different method to show ...

(QB_NEW_EN)

---

[grammar] ~146-~146: There might be a mistake here.
Context: ... content search - Edit interior line to add null check: `if (animator == null) retu...

(QB_NEW_EN)

---

[grammar] ~150-~150: There might be a mistake here.
Context: .... End-of-Class Helper (Additive State F) **Goal**: Add permanent helper method at c...

(QB_NEW_EN)

---

[grammar] ~152-~152: There might be a mistake here.
Context: ... helper method at class end **Actions**: - Use smart anchor matching to find curren...

(QB_NEW_EN)

---

[grammar] ~159-~159: There might be a mistake here.
Context: ...d Evolution Lifecycle (Additive State G) **Goal**: Insert → modify → finalize a fie...

(QB_NEW_EN)

---

[grammar] ~160-~160: There might be a mistake here.
Context: ...fy → finalize a field + companion method **Actions**: - Insert field: `private in...

(QB_NEW_EN)

---

[grammar] ~161-~161: There might be a mistake here.
Context: ... a field + companion method **Actions**: - Insert field: `private int Counter = 0;`...

(QB_NEW_EN)

---

[grammar] ~162-~162: There might be a mistake here.
Context: ...ion method **Actions**: - Insert field: `private int Counter = 0;` - Update it: find and replace with `privat...

(QB_NEW_EN)

---

[grammar] ~163-~163: There might be a mistake here.
Context: ... 0;` - Update it: find and replace with `private int Counter = 42; // initialized` - Add companion method: `private void Incr...

(QB_NEW_EN)

---

[grammar] ~164-~164: There might be a mistake here.
Context: ...// initialized` - Add companion method: `private void IncrementCounter() { Counter++; }` - **Expected final state**: State F + Counte...

(QB_NEW_EN)

---

[grammar] ~167-~167: There might be a mistake here.
Context: ...-F. Atomic Multi-Edit (Additive State H) **Goal**: Multiple coordinated edits in si...

(QB_NEW_EN)

---

[grammar] ~168-~168: There might be a mistake here.
Context: ...H) **Goal**: Multiple coordinated edits in single atomic operation **Actions**: - ...

(QB_NEW_EN)

---

[grammar] ~168-~168: There might be a mistake here.
Context: ...dinated edits in single atomic operation **Actions**: - Read current file state t...

(QB_NEW_EN)

---

[grammar] ~169-~169: There might be a mistake here.
Context: ... in single atomic operation **Actions**: - Read current file state to compute preci...

(QB_NEW_EN)

---

[grammar] ~171-~171: There might be a mistake here.
Context: ... precise ranges - Atomic edit combining:   1. Add comment in `HasTarget()`: `// valida...

(QB_NEW_EN)

---

[grammar] ~172-~172: There might be a mistake here.
Context: ...ing:   1. Add comment in `HasTarget()`: `// validated access`     2. Add comment in `ApplyBlend()`: `// safe ...

(QB_NEW_EN)

---

[grammar] ~173-~173: There might be a mistake here.
Context: ...`     2. Add comment in `ApplyBlend()`: `// safe animation`   3. Add final class comment: `// end of test...

(QB_NEW_EN)

---

[grammar] ~174-~174: There might be a mistake here.
Context: ...nimation`   3. Add final class comment: `// end of test modifications` - All edits computed from same file snapsh...

(QB_NEW_EN)

---

[grammar] ~175-~175: There might be a mistake here.
Context: ...est modifications` - All edits computed from same file snapshot, applied atomically ...

(QB_NEW_EN)

---

[grammar] ~178-~178: There might be a mistake here.
Context: ...ath Normalization Test (No State Change) **Goal**: Verify URI forms work equivalent...

(QB_NEW_EN)

---

[grammar] ~180-~180: There might be a mistake here.
Context: ...uivalently on modified file **Actions**: - Make identical edit using `unity://path/...

(QB_NEW_EN)

---

[grammar] ~181-~181: There might be a mistake here.
Context: ...*Actions**: - Make identical edit using `unity://path/Assets/Scripts/LongUnityScriptClaudeTest.cs` - Then using `Assets/Scripts/LongUnityScri...

(QB_NEW_EN)

---

[grammar] ~182-~182: There might be a mistake here.
Context: ...gUnityScriptClaudeTest.cs` - Then using `Assets/Scripts/LongUnityScriptClaudeTest.cs`  - Second should return `stale_file`, retry...

(QB_NEW_EN)

---

[grammar] ~184-~184: There might be a mistake here.
Context: ...ith updated SHA - Verify both URI forms target same file - **Expected final state**: S...

(QB_NEW_EN)

---

[grammar] ~187-~187: There might be a mistake here.
Context: ...ation on Modified File (No State Change) **Goal**: Ensure validation works correctl...

(QB_NEW_EN)

---

[grammar] ~189-~189: There might be a mistake here.
Context: ...ly on heavily modified file **Actions**: - Run `validate_script(level:"standard")` ...

(QB_NEW_EN)

---

[grammar] ~194-~194: There might be a mistake here.
Context: ...ailure Surface Testing (No State Change) **Goal**: Test error handling on real modi...

(QB_NEW_EN)

---

[grammar] ~196-~196: There might be a mistake here.
Context: ...dling on real modified file **Actions**: - Attempt overlapping edits (should fail c...

(QB_NEW_EN)

---

[grammar] ~202-~202: There might be a mistake here.
Context: ...ency on Modified File (Additive State I) **Goal**: Verify operations behave predict...

(QB_NEW_EN)

---

[grammar] ~204-~204: There might be a mistake here.
Context: ...e predictably when repeated **Actions**: - **Insert (structured)**: `mcp__unity__scri...

(QB_NEW_EN)

---

[grammar] ~208-~208: There might be a mistake here.
Context: ...o_op: true`. - **Remove (structured)**: `{"op":"regex_replace","pattern":"(?m)^\\s*// idempotency test marker\\r?\\n?","text":""}` - **Remove again** (same `regex_replace`) → ...

(QB_NEW_EN)

---

[grammar] ~211-~211: There might be a mistake here.
Context: ...pt(level:"standard")` - **IMMEDIATELY** write clean XML fragment to `reports/T-J_resu...

(QB_NEW_EN)

---

[grammar] ~244-~244: There might be a mistake here.
Context: ...erification Patterns  **After each test:** 1. Verify expected content exists: `find_in...

(QB_NEW_EN)

---

[grammar] ~250-~250: There might be a mistake here.
Context: ...nges in test evidence  **Error Recovery:** - If test fails, log current state but con...

(QB_NEW_EN)

---

[grammar] ~251-~251: There might be a mistake here.
Context: ...e  **Error Recovery:** - If test fails, log current state but continue (don't resto...

(QB_NEW_EN)

---

[grammar] ~252-~252: There might be a mistake here.
Context: ...inue (don't restore) - Next test adapts to actual current state, not expected stat...

(QB_NEW_EN)

---

[grammar] ~252-~252: There might be a mistake here.
Context: ...xt test adapts to actual current state, not expected state - Demonstrates resilienc...

(QB_NEW_EN)

</details>

</details>
<details>
<summary>🪛 markdownlint-cli2 (0.17.2)</summary>

<details>
<summary>.claude/prompts/nl-unity-suite-full-additive.md</summary>

110-110: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</details>

</details>
<details>
<summary>🪛 YAMLlint (1.37.1)</summary>

<details>
<summary>.github/workflows/claude-nl-suite.yml</summary>

[warning] 75-75: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 75-75: too many spaces after colon

(colons)

---

[warning] 76-76: too many spaces after colon

(colons)

---

[warning] 78-78: too many spaces after colon

(colons)

---

[warning] 92-92: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 122-122: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 122-122: too many spaces after colon

(colons)

---

[warning] 123-123: too many spaces after colon

(colons)

---

[warning] 125-125: too many spaces after colon

(colons)

---

[warning] 166-166: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 196-196: wrong indentation: expected 14 but found 12

(indentation)

---

[error] 219-219: trailing spaces

(trailing-spaces)

---

[warning] 279-279: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 279-279: too many spaces after colon

(colons)

---

[warning] 281-281: too many spaces after colon

(colons)

</details>

</details>

</details>

<details>
<summary>🔇 Additional comments (1)</summary><blockquote>

<details>
<summary>.github/workflows/claude-nl-suite.yml (1)</summary><blockquote>

`169-183`: **Preserve array expansion safety (LGTM).**

manual_args array usage with "${manual_args[@]}" avoids word-splitting and handles spaces safely.

</blockquote></details>

</blockquote></details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (2)

.github/workflows/claude-nl-suite.yml (2)

117-158: Mask activation logs and tighten entitlement check path.

Docker activation logs can leak email/serial; and scanning the whole config tree risks false positives. Pipe through a redaction filter and check the licenses dir specifically.

-            docker run --rm --network host \
+            docker run --rm --network host \
               -e HOME=/root \
               -e UNITY_EMAIL -e UNITY_PASSWORD -e UNITY_SERIAL \
               -v "$RUNNER_TEMP/unity-config:/root/.config/unity3d" \
               -v "$RUNNER_TEMP/unity-local:/root/.local/share/unity3d" \
-              "$UNITY_IMAGE" bash -lc '
+              "$UNITY_IMAGE" bash -lc '
                 set -euxo pipefail
                 if [[ -n "${UNITY_SERIAL:-}" ]]; then
                   /opt/unity/Editor/Unity -batchmode -nographics -logFile - \
                     -username "$UNITY_EMAIL" -password "$UNITY_PASSWORD" -serial "$UNITY_SERIAL" -quit || true
                 else
                   /opt/unity/Editor/Unity -batchmode -nographics -logFile - \
                     -username "$UNITY_EMAIL" -password "$UNITY_PASSWORD" -quit || true
                 fi
                 ls -la /root/.config/unity3d/Unity/licenses || true
-              '
+              ' 2>&1 | sed -E 's/((serial|license|password|token|email)[^[:space:]]*)/[REDACTED]/Ig'
 
-            # Verify entitlement written to host mount; allow ULF-only runs to proceed
-            if ! find "$RUNNER_TEMP/unity-config" -type f -iname "*.xml" | grep -q .; then
+            # Verify entitlement written to host mount; allow ULF-only runs to proceed
+            lic_dir="$RUNNER_TEMP/unity-config/Unity/licenses"
+            if ! find "$lic_dir" -maxdepth 1 -type f -iname "*.xml" | grep -q .; then
               if [[ "${{ steps.ulf.outputs.ok }}" == "true" ]]; then
                 echo "EBL entitlement not found; proceeding with ULF-only (ok=true)."
               else
                 echo "No entitlement produced and no valid ULF; cannot continue." >&2
                 exit 1
               fi
             fi

---

274-282: Do not return Pro license before the suite runs. Move to teardown.

Returning the license here can invalidate Unity before tests, causing flakiness/failures. Relocate after “Stop Unity”.

-        - name: Return Pro license (if used)
-          if: always() && steps.lic.outputs.use_ebl == 'true' && steps.lic.outputs.has_serial == 'true'
-          uses: game-ci/unity-return-license@v2
-          continue-on-error: true
-          env:
-            UNITY_EMAIL:    ${{ secrets.UNITY_EMAIL }}
-            UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }}
-            UNITY_SERIAL:   ${{ secrets.UNITY_SERIAL }}

Add after the Stop Unity step at the end of the job:

        - name: Return Pro license (if used)
          if: always() && steps.lic.outputs.use_ebl == 'true' && steps.lic.outputs.has_serial == 'true'
          uses: game-ci/unity-return-license@v2
          continue-on-error: true
          env:
            UNITY_EMAIL:    ${{ secrets.UNITY_EMAIL }}
            UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }}
            UNITY_SERIAL:   ${{ secrets.UNITY_SERIAL }}

🧹 Nitpick comments (4)

.github/workflows/claude-nl-suite.yml (4)

70-87: Outputs logic is fine; simplify has_serial for readability.

Current inline command substitution is harder to read/debug.

-            echo "has_serial=$([[ -n \"${UNITY_SERIAL:-}\" ]] && echo true || echo false)" >> "$GITHUB_OUTPUT"
+            if [[ -n "${UNITY_SERIAL:-}" ]]; then
+              echo "has_serial=true" >> "$GITHUB_OUTPUT"
+            else
+              echo "has_serial=false" >> "$GITHUB_OUTPUT"
+            fi

---

221-273: Wait loop is robust; minor polish optional.

Current ok/error patterns and port probe are sane. Consider truncating logs before grep to reduce loop load, but not required.

-              logs="$(docker logs unity-mcp 2>&1 || true)"
+              logs="$(docker logs --tail 400 unity-mcp 2>&1 || true)"

---

214-219: Trim trailing whitespace to appease linters.

Static analysis flags trailing space around Line 219; strip it to keep CI green.

---

75-76: Normalize YAML indentation & colon spacing
Align these entries to the same indentation level as their siblings and use a single space after the colon. For example:

            UNITY_LICENSE: ${{ secrets.UNITY_LICENSE }}
            UNITY_EMAIL: ${{ secrets.UNITY_EMAIL }}

Repeat this adjustment at lines 92, 122–125, 166, 196, and 279–281.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3bc7bf5 and fd626ea.

📒 Files selected for processing (1)

• .github/workflows/claude-nl-suite.yml (11 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-08-29T21:31:02.469Z

Learnt from: dsarno
PR: dsarno/unity-mcp#56
File: .claude/prompts/nl-unity-suite-full.md:5-6
Timestamp: 2025-08-29T21:31:02.469Z
Learning: The Claude NL suite workflows (.github/workflows/claude-nl-suite.yml and .github/workflows/claude-nl-suite-mini.yml) allow "Bash" tools generally, including printf and echo operations, so prompts that reference Bash(printf:*) and Bash(echo:*) are consistent with the actual workflow permissions.

Applied to files:

• .github/workflows/claude-nl-suite.yml

🪛 YAMLlint (1.37.1)

.github/workflows/claude-nl-suite.yml

[warning] 75-75: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 75-75: too many spaces after colon

(colons)

---

[warning] 76-76: too many spaces after colon

(colons)

---

[warning] 78-78: too many spaces after colon

(colons)

---

[warning] 92-92: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 122-122: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 122-122: too many spaces after colon

(colons)

---

[warning] 123-123: too many spaces after colon

(colons)

---

[warning] 125-125: too many spaces after colon

(colons)

---

[warning] 166-166: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 196-196: wrong indentation: expected 14 but found 12

(indentation)

---

[error] 219-219: trailing spaces

(trailing-spaces)

---

[warning] 279-279: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 279-279: too many spaces after colon

(colons)

---

[warning] 281-281: too many spaces after colon

(colons)

🔇 Additional comments (10)

.github/workflows/claude-nl-suite.yml (10)

88-116: ULF staging: robust base64 handling and file perms — LGTM.

printf + explicit stdin to base64 and chmod 600 address newline/permissions issues; also covering mis-pasted XML is a nice touch.

---

161-183: Warm-up step uses array args correctly — LGTM.

Array-based manual license arg fixes earlier unquoted expansion pitfalls.

---

193-218: Bridge start: mounts and manual license handling look correct.

Read-only mounts for config/local-share are appropriate; array expansion is safe.

---

398-453: Canon step: solid normalization logic — LGTM.

Covers both filename- and system-out-derived IDs and preserves existing canonical names.

---

455-488: Backfill of missing NL/T fragments — LGTM.

Clear placeholders ensure visibility in reports without masking real failures.

---

506-591: JUnit assembly handles fragments and recomputes counts — LGTM.

Graceful fallback to regex parse covers malformed fragments; counts updated correctly.

---

594-711: Markdown summary builder is safe and concise — LGTM.

Good ID mapping, truncation, and unmapped-case handling.

---

750-766: Job summary emission — LGTM.

Truncation prevents step output overflow; aligns with artifact publication.

---

806-812: Stop Unity step — LGTM.

Redaction on logs is appropriate; unconditional cleanup is correct.

---

88-116: Use of printf over echo is aligned with allowed Bash tools in this workflow.

Matches the established “printf preferred” convention from prior NL suite workflows.

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (3)

.claude/prompts/nl-unity-suite-full-additive.md (1)

139-140: Use standard validation level for consistency.

Prior guidance requested validate_script(level:"standard") throughout; using “relaxed” here creates inconsistency with step 3 and other tests.

- - Use `validate: "relaxed"` for interior-only edit
+ - Validate with `mcp__unity__validate_script(level:"standard")`

.github/workflows/claude-nl-suite.yml (2)

274-281: Return license runs too early; move to teardown after Unity is stopped.

Returning the Pro license before/while Unity is active risks invalidation mid-suite.

-        - name: Return Pro license (if used)
-          if: always() && steps.lic.outputs.use_ebl == 'true' && steps.lic.outputs.has_serial == 'true'
-          uses: game-ci/unity-return-license@v2
-          continue-on-error: true
-          env:
-            UNITY_EMAIL:    ${{ secrets.UNITY_EMAIL }}
-            UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }}
-            UNITY_SERIAL:   ${{ secrets.UNITY_SERIAL }}

Re-add after the “Stop Unity” step:

+        - name: Return Pro license (if used)
+          if: always() && steps.lic.outputs.use_ebl == 'true' && steps.lic.outputs.has_serial == 'true'
+          uses: game-ci/unity-return-license@v2
+          continue-on-error: true
+          env:
+            UNITY_EMAIL:    ${{ secrets.UNITY_EMAIL }}
+            UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }}
+            UNITY_SERIAL:   ${{ secrets.UNITY_SERIAL }}

---

131-147: Activation logs may leak secrets (email/serial/password). Redact output.

Unity often echoes credentials/serial to logs. Pipe docker run output through a redaction filter.

-            docker run --rm --network host \
+            docker run --rm --network host \
               -e HOME=/root \
               -e UNITY_EMAIL -e UNITY_PASSWORD -e UNITY_SERIAL \
               -v "$RUNNER_TEMP/unity-config:/root/.config/unity3d" \
               -v "$RUNNER_TEMP/unity-local:/root/.local/share/unity3d" \
-              "$UNITY_IMAGE" bash -lc '
+              "$UNITY_IMAGE" bash -lc '
                 set -euxo pipefail
                 if [[ -n "${UNITY_SERIAL:-}" ]]; then
                   /opt/unity/Editor/Unity -batchmode -nographics -logFile - \
                     -username "$UNITY_EMAIL" -password "$UNITY_PASSWORD" -serial "$UNITY_SERIAL" -quit || true
                 else
                   /opt/unity/Editor/Unity -batchmode -nographics -logFile - \
                     -username "$UNITY_EMAIL" -password "$UNITY_PASSWORD" -quit || true
                 fi
                 ls -la /root/.config/unity3d/Unity/licenses || true
-              '
+              ' 2>&1 | sed -E 's/((email|serial|license|password|token)[^[:space:]]*)/[REDACTED]/ig'

🧹 Nitpick comments (5)

.claude/prompts/nl-unity-suite-full-additive.md (3)

31-35: Clarify which tests must emit fragments (list is incomplete).

Section 6 mentions only T-D and T-J, but specs below also require emissions for T-F, T-G, T-H, and T-I. Align the directive to avoid ambiguous agent behavior.

Apply:

-6) **STRICT FRAGMENT EMISSION** - After completing T-D and T-J, immediately emit a clean XML file under `reports/<TESTID>_results.xml` with exactly one `<testcase>` whose `name` begins with the exact test id. No prologue/epilogue or fences.
+6) **STRICT FRAGMENT EMISSION** - For tests that require emission (T-D, T-F, T-G, T-H, T-I, T-J), immediately write a clean XML file to `reports/<TESTID>_results.xml` with exactly one `<testcase>` whose `name` begins with the exact test id. No prologue/epilogue or fences.

---

111-116: Add language to fenced block (markdownlint MD040).

Specify a language for the tail comments block to quiet linters and improve readability.

-  ```
+  ```csharp
   // Tail test A
   // Tail test B  
   // Tail test C

---

`229-233`: **Fix misleading code fence language.**

This snippet is not JSON; prefer a neutral fence.

```diff
-```json
+```text
# Find current method location
find_in_file(pattern: "public bool HasTarget\\(\\)")
# Then compute edit ranges from found position

.github/workflows/claude-nl-suite.yml (2)

149-157: Tighten entitlement check path to avoid false positives.

Look specifically under Unity/licenses; keep ULF-only fallback intact.

-            if ! find "$RUNNER_TEMP/unity-config" -type f -iname "*.xml" | grep -q .; then
+            lic_dir="$RUNNER_TEMP/unity-config/Unity/licenses"
+            if ! find "$lic_dir" -maxdepth 1 -type f -iname "*.xml" | grep -q .; then
               if [[ "${{ steps.ulf.outputs.ok }}" == "true" ]]; then
                 echo "EBL entitlement not found; proceeding with ULF-only (ok=true)."
               else
                 echo "No entitlement produced and no valid ULF; cannot continue." >&2
                 exit 1
               fi
             fi

---

219-219: Trim trailing whitespace (YAML lint).

Line 219 has trailing spaces; remove to satisfy linters.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between fd626ea and 181f3ad.

📒 Files selected for processing (2)

• .claude/prompts/nl-unity-suite-full-additive.md (1 hunks)
• .github/workflows/claude-nl-suite.yml (11 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-08-29T21:31:02.469Z

Learnt from: dsarno
PR: dsarno/unity-mcp#56
File: .claude/prompts/nl-unity-suite-full.md:5-6
Timestamp: 2025-08-29T21:31:02.469Z
Learning: The Claude NL suite workflows (.github/workflows/claude-nl-suite.yml and .github/workflows/claude-nl-suite-mini.yml) allow "Bash" tools generally, including printf and echo operations, so prompts that reference Bash(printf:*) and Bash(echo:*) are consistent with the actual workflow permissions.

Applied to files:

• .claude/prompts/nl-unity-suite-full-additive.md
• .github/workflows/claude-nl-suite.yml

🪛 LanguageTool

.claude/prompts/nl-unity-suite-full-additive.md

[grammar] ~5-~5: There might be a mistake here.
Context: ...this once, verbatim, early in the run:** AllowedTools: Write,mcp__unity__manage_e...

(QB_NEW_EN)

---

[grammar] ~17-~17: There might be a mistake here.
Context: ...T. **CRITICAL XML FORMAT REQUIREMENTS:** - Each file must contain EXACTLY one <tes...

(QB_NEW_EN)

---

[grammar] ~32-~32: There might be a mistake here.
Context: ..., T-C, T-D, T-E, T-F, T-G, T-H, T-I, T-J 5) NO RESTORATION - tests build additivel...

(QB_NEW_EN)

---

[grammar] ~33-~33: There might be a mistake here.
Context: ... RESTORATION** - tests build additively on previous state. 6) **STRICT FRAGMENT EM...

(QB_NEW_EN)

---

[grammar] ~33-~33: There might be a mistake here.
Context: ...ests build additively on previous state. 6) STRICT FRAGMENT EMISSION - After compl...

(QB_NEW_EN)

---

[grammar] ~38-~38: There might be a mistake here.
Context: ...ences. --- ## Environment & Paths (CI) - Always pass: `project_root: "TestProject...

(QB_NEW_EN)

---

[grammar] ~39-~39: There might be a mistake here.
Context: ...nd ctx: {} on list/read/edit/validate. - Canonical URIs only: - Primary: `uni...

(QB_NEW_EN)

---

[grammar] ~40-~40: There might be a mistake here.
Context: ...dit/validate. - Canonical URIs only: - Primary: unity://path/Assets/... (neve...

(QB_NEW_EN)

---

[grammar] ~41-~41: There might be a mistake here.
Context: ... (never embed project_root in the URI) - Relative (when supported): Assets/... ...

(QB_NEW_EN)

---

[grammar] ~50-~50: There might be a mistake here.
Context: ...esized from JUnit) --- ## Tool Mapping - Anchors/regex/structured: `mcp__unity_...

(QB_NEW_EN)

---

[grammar] ~51-~51: There might be a mistake here.
Context: ...Mapping - Anchors/regex/structured: mcp__unity__script_apply_edits - Allowed ops: anchor_insert, `replace_m...

(QB_NEW_EN)

---

[grammar] ~52-~52: There might be a mistake here.
Context: ...ethod, insert_method, delete_method, regex_replace - Foranchor_insert, always set "positi...

(QB_NEW_EN)

---

[grammar] ~53-~53: There might be a mistake here.
Context: ...set "position": "before" or "after". - Precise ranges / atomic batch: `mcp__u...

(QB_NEW_EN)

---

[grammar] ~54-~54: There might be a mistake here.
Context: ...ply_text_edits(non‑overlapping ranges) - **Hash-only**:mcp__unity__get_sha` — ret...

(QB_NEW_EN)

---

[grammar] ~55-~55: There might be a mistake here.
Context: ...ytes,lastModifiedUtc}without file body - **Validation**:mcp__unity__validate_scri...

(QB_NEW_EN)

---

[grammar] ~56-~56: There might be a mistake here.
Context: ...c}without file body - **Validation**:mcp__unity__validate_script(level:"standard")- **Dynamic targeting**: Usemcp__unity__fi...

(QB_NEW_EN)

---

[grammar] ~63-~63: There might be a mistake here.
Context: ...nciples Key Changes from Reset-Based: 1. Dynamic Targeting: Use find_in_file ...

(QB_NEW_EN)

---

[grammar] ~70-~70: There might be a mistake here.
Context: ...her in real workflows State Tracking: - Track file SHA after each test (`mcp__un...

(QB_NEW_EN)

---

[grammar] ~72-~72: There might be a mistake here.
Context: .../T‑I to exercise stale_file semantics. - Use content signatures (method names, co...

(QB_NEW_EN)

---

[grammar] ~80-~80: There might be a mistake here.
Context: ... Specs ### NL-0. Baseline State Capture Goal: Establish initial file state and...

(QB_NEW_EN)

---

[grammar] ~82-~82: There might be a mistake here.
Context: ...te and verify accessibility Actions: - Read file head and tail to confirm struc...

(QB_NEW_EN)

---

[grammar] ~88-~88: There might be a mistake here.
Context: ...ore Method Operations (Additive State A) Goal: Demonstrate method replacement o...

(QB_NEW_EN)

---

[grammar] ~90-~90: There might be a mistake here.
Context: ...thod replacement operations Actions: - Replace HasTarget() method body: `publ...

(QB_NEW_EN)

---

[grammar] ~97-~97: There might be a mistake here.
Context: ...hor Comment Insertion (Additive State B) Goal: Demonstrate anchor-based inserti...

(QB_NEW_EN)

---

[grammar] ~99-~99: There might be a mistake here.
Context: ...ed insertions above methods Actions: - Use find_in_file to locate current pos...

(QB_NEW_EN)

---

[grammar] ~100-~100: There might be a mistake here.
Context: ...ds Actions: - Use find_in_file to locate current position of Update() method -...

(QB_NEW_EN)

---

[grammar] ~105-~105: There might be a mistake here.
Context: ... End-of-Class Content (Additive State C) Goal: Demonstrate end-of-class inserti...

(QB_NEW_EN)

---

[grammar] ~107-~107: There might be a mistake here.
Context: ...s with smart brace matching Actions: - Match the final class-closing brace by s...

(QB_NEW_EN)

---

[grammar] ~109-~109: There might be a mistake here.
Context: ... + ranges; insert immediately before it. - Insert three comment lines before final ...

(QB_NEW_EN)

---

[grammar] ~110-~110: There might be a mistake here.
Context: ...before it. - Insert three comment lines before final class brace: ``` // Tail test...

(QB_NEW_EN)

---

[grammar] ~118-~118: There might be a mistake here.
Context: ...ole State Verification (No State Change) Goal: Verify Unity console integration...

(QB_NEW_EN)

---

[grammar] ~120-~120: There might be a mistake here.
Context: ...n without file modification Actions: - Read Unity console messages (INFO level)...

(QB_NEW_EN)

---

[grammar] ~125-~125: There might be a mistake here.
Context: ...ry Helper Lifecycle (Returns to State C) Goal: Test insert → verify → delete cy...

(QB_NEW_EN)

---

[grammar] ~127-~127: There might be a mistake here.
Context: ...te cycle for temporary code Actions: - Find current position of `GetCurrentTarg...

(QB_NEW_EN)

---

[grammar] ~134-~134: There might be a mistake here.
Context: ...od Body Interior Edit (Additive State D) Goal: Edit method interior without aff...

(QB_NEW_EN)

---

[grammar] ~136-~136: There might be a mistake here.
Context: ...structure, on modified file Actions: - Use find_in_file to locate current `Ha...

(QB_NEW_EN)

---

[grammar] ~138-~138: There might be a mistake here.
Context: ...dy interior: change return statement to return true; /* test modification */ - Use validate: "relaxed" for interior-o...

(QB_NEW_EN)

---

[grammar] ~143-~143: There might be a mistake here.
Context: ... Method Interior Edit (Additive State E) Goal: Edit a different method to show ...

(QB_NEW_EN)

---

[grammar] ~147-~147: There might be a mistake here.
Context: ... content search - Edit interior line to add null check: `if (animator == null) retu...

(QB_NEW_EN)

---

[grammar] ~151-~151: There might be a mistake here.
Context: .... End-of-Class Helper (Additive State F) Goal: Add permanent helper method at c...

(QB_NEW_EN)

---

[grammar] ~153-~153: There might be a mistake here.
Context: ... helper method at class end Actions: - Use smart anchor matching to find curren...

(QB_NEW_EN)

---

[grammar] ~160-~160: There might be a mistake here.
Context: ...d Evolution Lifecycle (Additive State G) Goal: Insert → modify → finalize a fie...

(QB_NEW_EN)

---

[grammar] ~161-~161: There might be a mistake here.
Context: ...fy → finalize a field + companion method Actions: - Insert field: `private in...

(QB_NEW_EN)

---

[grammar] ~162-~162: There might be a mistake here.
Context: ... a field + companion method Actions: - Insert field: private int Counter = 0;...

(QB_NEW_EN)

---

[grammar] ~163-~163: There might be a mistake here.
Context: ...ion method Actions: - Insert field: private int Counter = 0; - Update it: find and replace with `privat...

(QB_NEW_EN)

---

[grammar] ~164-~164: There might be a mistake here.
Context: ... 0;- Update it: find and replace withprivate int Counter = 42; // initialized- Add companion method:private void Incr...

(QB_NEW_EN)

---

[grammar] ~165-~165: There might be a mistake here.
Context: ...// initialized- Add companion method:private void IncrementCounter() { Counter++; }` - Expected final state: State F + Counte...

(QB_NEW_EN)

---

[grammar] ~168-~168: There might be a mistake here.
Context: ...-F. Atomic Multi-Edit (Additive State H) Goal: Multiple coordinated edits in si...

(QB_NEW_EN)

---

[grammar] ~169-~169: There might be a mistake here.
Context: ...H) Goal: Multiple coordinated edits in single atomic operation Actions: - ...

(QB_NEW_EN)

---

[grammar] ~169-~169: There might be a mistake here.
Context: ...dinated edits in single atomic operation Actions: - Read current file state t...

(QB_NEW_EN)

---

[grammar] ~170-~170: There might be a mistake here.
Context: ... in single atomic operation Actions: - Read current file state to compute preci...

(QB_NEW_EN)

---

[grammar] ~172-~172: There might be a mistake here.
Context: ... precise ranges - Atomic edit combining: 1. Add comment in HasTarget(): `// valida...

(QB_NEW_EN)

---

[grammar] ~173-~173: There might be a mistake here.
Context: ...ing: 1. Add comment in HasTarget(): // validated access 2. Add comment in ApplyBlend(): `// safe ...

(QB_NEW_EN)

---

[grammar] ~174-~174: There might be a mistake here.
Context: ... 2. Add comment inApplyBlend(): // safe animation 3. Add final class comment:// end of test...

(QB_NEW_EN)

---

[grammar] ~175-~175: There might be a mistake here.
Context: ...nimation 3. Add final class comment:// end of test modifications` - All edits computed from same file snapsh...

(QB_NEW_EN)

---

[grammar] ~176-~176: There might be a mistake here.
Context: ...est modifications` - All edits computed from same file snapshot, applied atomically ...

(QB_NEW_EN)

---

[grammar] ~177-~177: There might be a mistake here.
Context: ...**: State G + three coordinated comments - After applying the atomic edits, run `va...

(QB_NEW_EN)

---

[grammar] ~180-~180: There might be a mistake here.
Context: ...ath Normalization Test (No State Change) Goal: Verify URI forms work equivalent...

(QB_NEW_EN)

---

[grammar] ~182-~182: There might be a mistake here.
Context: ...uivalently on modified file Actions: - Make identical edit using `unity://path/...

(QB_NEW_EN)

---

[grammar] ~183-~183: There might be a mistake here.
Context: ...Actions*: - Make identical edit using unity://path/Assets/Scripts/LongUnityScriptClaudeTest.cs - Then using `Assets/Scripts/LongUnityScri...

(QB_NEW_EN)

---

[grammar] ~186-~186: There might be a mistake here.
Context: ...ith updated SHA - Verify both URI forms target same file - Expected final state: S...

(QB_NEW_EN)

---

[grammar] ~187-~187: There might be a mistake here.
Context: ...H (no content change, just path testing) - Emit reports/T-G_results.xml showing e...

(QB_NEW_EN)

---

[grammar] ~190-~190: There might be a mistake here.
Context: ...ation on Modified File (No State Change) Goal: Ensure validation works correctl...

(QB_NEW_EN)

---

[grammar] ~192-~192: There might be a mistake here.
Context: ...ly on heavily modified file Actions: - Run validate_script(level:"standard") ...

(QB_NEW_EN)

---

[grammar] ~195-~195: There might be a mistake here.
Context: ...e**: State H (validation only, no edits) - Emit reports/T-H_results.xml confirmin...

(QB_NEW_EN)

---

[grammar] ~198-~198: There might be a mistake here.
Context: ...ailure Surface Testing (No State Change) Goal: Test error handling on real modi...

(QB_NEW_EN)

---

[grammar] ~200-~200: There might be a mistake here.
Context: ...dling on real modified file Actions: - Attempt overlapping edits (should fail c...

(QB_NEW_EN)

---

[grammar] ~207-~207: There might be a mistake here.
Context: ...ency on Modified File (Additive State I) Goal: Verify operations behave predict...

(QB_NEW_EN)

---

[grammar] ~209-~209: There might be a mistake here.
Context: ...e predictably when repeated Actions: - Insert (structured): `mcp__unity__scri...

(QB_NEW_EN)

---

[grammar] ~213-~213: There might be a mistake here.
Context: ...o_op: true. - **Remove (structured)**: {"op":"regex_replace","pattern":"(?m)^\s*// idempotency test marker\r?\n?","text":""}- **Remove again** (sameregex_replace`) → ...

(QB_NEW_EN)

---

[grammar] ~216-~216: There might be a mistake here.
Context: ...pt(level:"standard")- **IMMEDIATELY** write clean XML fragment toreports/T-J_resu...

(QB_NEW_EN)

---

[grammar] ~249-~249: There might be a mistake here.
Context: ...erification Patterns After each test: 1. Verify expected content exists: `find_in...

(QB_NEW_EN)

---

[grammar] ~255-~255: There might be a mistake here.
Context: ...nges in test evidence Error Recovery: - If test fails, log current state but con...

(QB_NEW_EN)

---

[grammar] ~256-~256: There might be a mistake here.
Context: ...e Error Recovery: - If test fails, log current state but continue (don't resto...

(QB_NEW_EN)

---

[grammar] ~257-~257: There might be a mistake here.
Context: ...inue (don't restore) - Next test adapts to actual current state, not expected stat...

(QB_NEW_EN)

---

[grammar] ~257-~257: There might be a mistake here.
Context: ...xt test adapts to actual current state, not expected state - Demonstrates resilienc...

(QB_NEW_EN)

🪛 markdownlint-cli2 (0.17.2)

.claude/prompts/nl-unity-suite-full-additive.md

111-111: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🪛 YAMLlint (1.37.1)

.github/workflows/claude-nl-suite.yml

[warning] 75-75: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 75-75: too many spaces after colon

(colons)

---

[warning] 76-76: too many spaces after colon

(colons)

---

[warning] 78-78: too many spaces after colon

(colons)

---

[warning] 92-92: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 122-122: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 122-122: too many spaces after colon

(colons)

---

[warning] 123-123: too many spaces after colon

(colons)

---

[warning] 125-125: too many spaces after colon

(colons)

---

[warning] 166-166: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 196-196: wrong indentation: expected 14 but found 12

(indentation)

---

[error] 219-219: trailing spaces

(trailing-spaces)

---

[warning] 279-279: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 279-279: too many spaces after colon

(colons)

---

[warning] 281-281: too many spaces after colon

(colons)

🔇 Additional comments (1)

.claude/prompts/nl-unity-suite-full-additive.md (1)

5-7: AllowedTools matches workflow (good).

The printed AllowedTools set aligns with the workflow’s allowed_tools/disallowed_tools (Bash is excluded). No action needed.

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (3)

.github/workflows/claude-nl-suite.yml (3)

230-231: Redact emails consistently in all log dumps.

Extend the sed alternation to include “email”.

-case "$st" in exited*|dead*) docker logs unity-mcp --tail 200 | sed -E 's/((serial|license|password|token)[^[:space:]]*)/[REDACTED]/Ig'; exit 1;; esac
+case "$st" in exited*|dead*) docker logs unity-mcp --tail 200 | sed -E 's/((email|serial|license|password|token)[^[:space:]]*)/[REDACTED]/Ig'; exit 1;; esac
@@
-echo "$logs" | tail -n 200 | sed -E 's/((serial|license|password|token)[^[:space:]]*)/[REDACTED]/Ig'
+echo "$logs" | tail -n 200 | sed -E 's/((email|serial|license|password|token)[^[:space:]]*)/[REDACTED]/Ig'
@@
-echo "::error::Unity container exited during wait"; docker logs unity-mcp --tail 200 | sed -E 's/((serial|license|password|token)[^[:space:]]*)/[REDACTED]/Ig'
+echo "::error::Unity container exited during wait"; docker logs unity-mcp --tail 200 | sed -E 's/((email|serial|license|password|token)[^[:space:]]*)/[REDACTED]/Ig'
@@
-docker logs unity-mcp --tail 200 | sed -E 's/((serial|license|password|token)[^[:space:]]*)/[REDACTED]/Ig'
+docker logs unity-mcp --tail 200 | sed -E 's/((email|serial|license|password|token)[^[:space:]]*)/[REDACTED]/Ig'
@@
-docker logs --tail 400 unity-mcp | sed -E 's/((serial|license|password|token)[^[:space:]]*)/[REDACTED]/ig' || true
+docker logs --tail 400 unity-mcp | sed -E 's/((email|serial|license|password|token)[^[:space:]]*)/[REDACTED]/ig' || true

Also applies to: 256-257, 263-264, 271-272, 854-854

---

274-282: Return Pro license after stopping Unity (teardown).

Returning before teardown risks invalidating Unity mid-log/aggregation. Move the step after Stop Unity.

-        - name: Return Pro license (if used)
-          if: always() && steps.lic.outputs.use_ebl == 'true' && steps.lic.outputs.has_serial == 'true'
-          uses: game-ci/unity-return-license@v2
-          continue-on-error: true
-          env:
-            UNITY_EMAIL:    ${{ secrets.UNITY_EMAIL }}
-            UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }}
-            UNITY_SERIAL:   ${{ secrets.UNITY_SERIAL }}
@@
         - name: Stop Unity
           if: always()
           run: |
             docker logs --tail 400 unity-mcp | sed -E 's/((serial|license|password|token)[^[:space:]]*)/[REDACTED]/ig' || true
             docker rm -f unity-mcp || true
+
+        - name: Return Pro license (if used)
+          if: always() && steps.lic.outputs.use_ebl == 'true' && steps.lic.outputs.has_serial == 'true'
+          uses: game-ci/unity-return-license@v2
+          continue-on-error: true
+          env:
+            UNITY_EMAIL:    ${{ secrets.UNITY_EMAIL }}
+            UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }}
+            UNITY_SERIAL:   ${{ secrets.UNITY_SERIAL }}

Also applies to: 850-856

---

131-157: Redact activation logs and tighten entitlement check to licenses dir.

docker run may emit email/serial; pipe through sed and check the specific Unity/licenses path to avoid false positives.

-            docker run --rm --network host \
+            docker run --rm --network host \
               -e HOME=/root \
               -e UNITY_EMAIL -e UNITY_PASSWORD -e UNITY_SERIAL \
               -v "$RUNNER_TEMP/unity-config:/root/.config/unity3d" \
               -v "$RUNNER_TEMP/unity-local:/root/.local/share/unity3d" \
-              "$UNITY_IMAGE" bash -lc '
+              "$UNITY_IMAGE" bash -lc '
                 set -euxo pipefail
                 if [[ -n "${UNITY_SERIAL:-}" ]]; then
                   /opt/unity/Editor/Unity -batchmode -nographics -logFile - \
                     -username "$UNITY_EMAIL" -password "$UNITY_PASSWORD" -serial "$UNITY_SERIAL" -quit || true
                 else
                   /opt/unity/Editor/Unity -batchmode -nographics -logFile - \
                     -username "$UNITY_EMAIL" -password "$UNITY_PASSWORD" -quit || true
                 fi
                 ls -la /root/.config/unity3d/Unity/licenses || true
-              '
+              ' 2>&1 | sed -E 's/((email|serial|license|password|token)[^[:space:]]*)/[REDACTED]/Ig'
 
-            # Verify entitlement written to host mount; allow ULF-only runs to proceed
-            if ! find "$RUNNER_TEMP/unity-config" -type f -iname "*.xml" | grep -q .; then
+            # Verify entitlement written to host mount; allow ULF-only runs to proceed
+            lic_dir="$RUNNER_TEMP/unity-config/Unity/licenses"
+            if ! find "$lic_dir" -maxdepth 1 -type f -iname "*.xml" | grep -q .; then
               if [[ "${{ steps.ulf.outputs.ok }}" == "true" ]]; then
                 echo "EBL entitlement not found; proceeding with ULF-only (ok=true)."
               else
                 echo "No entitlement produced and no valid ULF; cannot continue." >&2
                 exit 1
               fi
             fi

🧹 Nitpick comments (3)

.github/workflows/claude-nl-suite.yml (3)

393-393: Re-allow Bash tool for the NL/T action (prompt references Bash).

Your additive prompt and prior workflow allow Bash; disallowing Bash here may cause test/tool failures.

-            disallowed_tools: TodoWrite,Task,Bash
+            disallowed_tools: TodoWrite,Task

If intentional, confirm the prompt doesn’t require Bash(printf:) or Bash(echo:).

---

149-157: Entitlement check: handle ULF-only path explicitly.

When ULF-only, consider echoing the found ULF path for diagnostics.

-              if [[ "${{ steps.ulf.outputs.ok }}" == "true" ]]; then
-                echo "EBL entitlement not found; proceeding with ULF-only (ok=true)."
+              if [[ "${{ steps.ulf.outputs.ok }}" == "true" ]]; then
+                echo "EBL entitlement not found; proceeding with ULF-only (ok=true). Using: $RUNNER_TEMP/unity-local/Unity/Unity_lic.ulf"

---

219-219: Trim trailing spaces to appease YAML/action linters.

Static analysis flagged trailing whitespace here.

-                -executeMethod MCPForUnity.Editor.MCPForUnityBridge.StartAutoConnect 
+                -executeMethod MCPForUnity.Editor.MCPForUnityBridge.StartAutoConnect

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 181f3ad and 40463d1.

📒 Files selected for processing (1)

• .github/workflows/claude-nl-suite.yml (11 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-08-29T21:31:02.469Z

Learnt from: dsarno
PR: dsarno/unity-mcp#56
File: .claude/prompts/nl-unity-suite-full.md:5-6
Timestamp: 2025-08-29T21:31:02.469Z
Learning: The Claude NL suite workflows (.github/workflows/claude-nl-suite.yml and .github/workflows/claude-nl-suite-mini.yml) allow "Bash" tools generally, including printf and echo operations, so prompts that reference Bash(printf:*) and Bash(echo:*) are consistent with the actual workflow permissions.

Applied to files:

• .github/workflows/claude-nl-suite.yml

🪛 YAMLlint (1.37.1)

.github/workflows/claude-nl-suite.yml

[warning] 75-75: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 75-75: too many spaces after colon

(colons)

---

[warning] 76-76: too many spaces after colon

(colons)

---

[warning] 78-78: too many spaces after colon

(colons)

---

[warning] 92-92: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 122-122: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 122-122: too many spaces after colon

(colons)

---

[warning] 123-123: too many spaces after colon

(colons)

---

[warning] 125-125: too many spaces after colon

(colons)

---

[warning] 166-166: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 196-196: wrong indentation: expected 14 but found 12

(indentation)

---

[error] 219-219: trailing spaces

(trailing-spaces)

---

[warning] 279-279: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 279-279: too many spaces after colon

(colons)

---

[warning] 281-281: too many spaces after colon

(colons)

🔇 Additional comments (1)

.github/workflows/claude-nl-suite.yml (1)

170-173: Array-safe manual license args: good fix.

Switching to an array and expanding with "${manual_args[@]}" resolves prior quoting issues around spaces in paths.

Also applies to: 200-203

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (1)

.github/workflows/claude-nl-suite.yml (1)

131-157: Redact sensitive fields in activation logs and tighten entitlement check path.

docker run logs can leak email/serial; also scan licenses dir specifically to avoid false positives.

-            docker run --rm --network host \
+            docker run --rm --network host \
               -e HOME=/root \
               -e UNITY_EMAIL -e UNITY_PASSWORD -e UNITY_SERIAL \
               -v "$RUNNER_TEMP/unity-config:/root/.config/unity3d" \
               -v "$RUNNER_TEMP/unity-local:/root/.local/share/unity3d" \
-              "$UNITY_IMAGE" bash -lc '
+              "$UNITY_IMAGE" bash -lc '
                 set -euxo pipefail
                 if [[ -n "${UNITY_SERIAL:-}" ]]; then
                   /opt/unity/Editor/Unity -batchmode -nographics -logFile - \
                     -username "$UNITY_EMAIL" -password "$UNITY_PASSWORD" -serial "$UNITY_SERIAL" -quit || true
                 else
                   /opt/unity/Editor/Unity -batchmode -nographics -logFile - \
                     -username "$UNITY_EMAIL" -password "$UNITY_PASSWORD" -quit || true
                 fi
                 ls -la /root/.config/unity3d/Unity/licenses || true
-              '
+              ' 2>&1 | sed -E 's/((email|serial|license|password|token)[^[:space:]]*)/[REDACTED]/Ig'
 
-            # Verify entitlement written to host mount; allow ULF-only runs to proceed
-            if ! find "$RUNNER_TEMP/unity-config" -type f -iname "*.xml" | grep -q .; then
+            # Verify entitlement written to host mount; allow ULF-only runs to proceed
+            lic_dir="$RUNNER_TEMP/unity-config/Unity/licenses"
+            if ! find "$lic_dir" -maxdepth 1 -type f -iname "*.xml" | grep -q .; then
               if [[ "${{ steps.ulf.outputs.ok }}" == "true" ]]; then
                 echo "EBL entitlement not found; proceeding with ULF-only (ok=true)."
               else
                 echo "No entitlement produced and no valid ULF; cannot continue." >&2
                 exit 1
               fi
             fi

🧹 Nitpick comments (2)

.claude/prompts/nl-unity-suite-full-additive.md (1)

110-116: Add a language to this fenced block to satisfy markdownlint (MD040).

Use csharp since these are C# comment lines.

-  ```
+  ```csharp
   // Tail test A
   // Tail test B  
   // Tail test C

</blockquote></details>
<details>
<summary>.github/workflows/claude-nl-suite.yml (1)</summary><blockquote>

`219-219`: **Trim trailing whitespace.**

Minor cleanup to avoid lints.

```diff
-        # (moved) — return license after Unity is stopped
-        
+        # (moved) — return license after Unity is stopped

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 40463d1 and e1d8ac5.

📒 Files selected for processing (5)

• .claude/prompts/nl-unity-claude-tests-mini.md (0 hunks)
• .claude/prompts/nl-unity-suite-full-additive.md (1 hunks)
• .claude/prompts/nl-unity-suite-full.md (0 hunks)
• .github/workflows/claude-nl-suite-mini.yml (0 hunks)
• .github/workflows/claude-nl-suite.yml (14 hunks)

💤 Files with no reviewable changes (3)

• .claude/prompts/nl-unity-suite-full.md
• .claude/prompts/nl-unity-claude-tests-mini.md
• .github/workflows/claude-nl-suite-mini.yml

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-08-29T21:31:02.469Z

Learnt from: dsarno
PR: dsarno/unity-mcp#56
File: .claude/prompts/nl-unity-suite-full.md:5-6
Timestamp: 2025-08-29T21:31:02.469Z
Learning: The Claude NL suite workflows (.github/workflows/claude-nl-suite.yml and .github/workflows/claude-nl-suite-mini.yml) allow "Bash" tools generally, including printf and echo operations, so prompts that reference Bash(printf:*) and Bash(echo:*) are consistent with the actual workflow permissions.

Applied to files:

• .claude/prompts/nl-unity-suite-full-additive.md
• .github/workflows/claude-nl-suite.yml

🪛 LanguageTool

.claude/prompts/nl-unity-suite-full-additive.md

[grammar] ~5-~5: There might be a mistake here.
Context: ...this once, verbatim, early in the run:** AllowedTools: Write,mcp__unity__manage_e...

(QB_NEW_EN)

---

[grammar] ~17-~17: There might be a mistake here.
Context: ...T. **CRITICAL XML FORMAT REQUIREMENTS:** - Each file must contain EXACTLY one <tes...

(QB_NEW_EN)

---

[grammar] ~32-~32: There might be a mistake here.
Context: ..., T-C, T-D, T-E, T-F, T-G, T-H, T-I, T-J 5) NO RESTORATION - tests build additivel...

(QB_NEW_EN)

---

[grammar] ~33-~33: There might be a mistake here.
Context: ... RESTORATION** - tests build additively on previous state. 6) **STRICT FRAGMENT EM...

(QB_NEW_EN)

---

[grammar] ~33-~33: There might be a mistake here.
Context: ...ests build additively on previous state. 6) STRICT FRAGMENT EMISSION - After compl...

(QB_NEW_EN)

---

[grammar] ~38-~38: There might be a mistake here.
Context: ...ences. --- ## Environment & Paths (CI) - Always pass: `project_root: "TestProject...

(QB_NEW_EN)

---

[grammar] ~39-~39: There might be a mistake here.
Context: ...nd ctx: {} on list/read/edit/validate. - Canonical URIs only: - Primary: `uni...

(QB_NEW_EN)

---

[grammar] ~40-~40: There might be a mistake here.
Context: ...dit/validate. - Canonical URIs only: - Primary: unity://path/Assets/... (neve...

(QB_NEW_EN)

---

[grammar] ~41-~41: There might be a mistake here.
Context: ... (never embed project_root in the URI) - Relative (when supported): Assets/... ...

(QB_NEW_EN)

---

[grammar] ~50-~50: There might be a mistake here.
Context: ...esized from JUnit) --- ## Tool Mapping - Anchors/regex/structured: `mcp__unity_...

(QB_NEW_EN)

---

[grammar] ~51-~51: There might be a mistake here.
Context: ...Mapping - Anchors/regex/structured: mcp__unity__script_apply_edits - Allowed ops: anchor_insert, `replace_m...

(QB_NEW_EN)

---

[grammar] ~52-~52: There might be a mistake here.
Context: ...ethod, insert_method, delete_method, regex_replace - Foranchor_insert, always set "positi...

(QB_NEW_EN)

---

[grammar] ~53-~53: There might be a mistake here.
Context: ...set "position": "before" or "after". - Precise ranges / atomic batch: `mcp__u...

(QB_NEW_EN)

---

[grammar] ~54-~54: There might be a mistake here.
Context: ...ply_text_edits(non‑overlapping ranges) - **Hash-only**:mcp__unity__get_sha` — ret...

(QB_NEW_EN)

---

[grammar] ~55-~55: There might be a mistake here.
Context: ...ytes,lastModifiedUtc}without file body - **Validation**:mcp__unity__validate_scri...

(QB_NEW_EN)

---

[grammar] ~56-~56: There might be a mistake here.
Context: ...c}without file body - **Validation**:mcp__unity__validate_script(level:"standard")- **Dynamic targeting**: Usemcp__unity__fi...

(QB_NEW_EN)

---

[grammar] ~63-~63: There might be a mistake here.
Context: ...nciples Key Changes from Reset-Based: 1. Dynamic Targeting: Use find_in_file ...

(QB_NEW_EN)

---

[grammar] ~70-~70: There might be a mistake here.
Context: ...her in real workflows State Tracking: - Track file SHA after each test (`mcp__un...

(QB_NEW_EN)

---

[grammar] ~72-~72: There might be a mistake here.
Context: .../T‑I to exercise stale_file semantics. - Use content signatures (method names, co...

(QB_NEW_EN)

---

[grammar] ~80-~80: There might be a mistake here.
Context: ... Specs ### NL-0. Baseline State Capture Goal: Establish initial file state and...

(QB_NEW_EN)

---

[grammar] ~82-~82: There might be a mistake here.
Context: ...te and verify accessibility Actions: - Read file head and tail to confirm struc...

(QB_NEW_EN)

---

[grammar] ~88-~88: There might be a mistake here.
Context: ...ore Method Operations (Additive State A) Goal: Demonstrate method replacement o...

(QB_NEW_EN)

---

[grammar] ~90-~90: There might be a mistake here.
Context: ...thod replacement operations Actions: - Replace HasTarget() method body: `publ...

(QB_NEW_EN)

---

[grammar] ~97-~97: There might be a mistake here.
Context: ...hor Comment Insertion (Additive State B) Goal: Demonstrate anchor-based inserti...

(QB_NEW_EN)

---

[grammar] ~99-~99: There might be a mistake here.
Context: ...ed insertions above methods Actions: - Use find_in_file to locate current pos...

(QB_NEW_EN)

---

[grammar] ~100-~100: There might be a mistake here.
Context: ...ds Actions: - Use find_in_file to locate current position of Update() method -...

(QB_NEW_EN)

---

[grammar] ~105-~105: There might be a mistake here.
Context: ... End-of-Class Content (Additive State C) Goal: Demonstrate end-of-class inserti...

(QB_NEW_EN)

---

[grammar] ~107-~107: There might be a mistake here.
Context: ...s with smart brace matching Actions: - Match the final class-closing brace by s...

(QB_NEW_EN)

---

[grammar] ~109-~109: There might be a mistake here.
Context: ... + ranges; insert immediately before it. - Insert three comment lines before final ...

(QB_NEW_EN)

---

[grammar] ~110-~110: There might be a mistake here.
Context: ...before it. - Insert three comment lines before final class brace: ``` // Tail test...

(QB_NEW_EN)

---

[grammar] ~118-~118: There might be a mistake here.
Context: ...ole State Verification (No State Change) Goal: Verify Unity console integration...

(QB_NEW_EN)

---

[grammar] ~120-~120: There might be a mistake here.
Context: ...n without file modification Actions: - Read Unity console messages (INFO level)...

(QB_NEW_EN)

---

[grammar] ~125-~125: There might be a mistake here.
Context: ...ry Helper Lifecycle (Returns to State C) Goal: Test insert → verify → delete cy...

(QB_NEW_EN)

---

[grammar] ~127-~127: There might be a mistake here.
Context: ...te cycle for temporary code Actions: - Find current position of `GetCurrentTarg...

(QB_NEW_EN)

---

[grammar] ~134-~134: There might be a mistake here.
Context: ...od Body Interior Edit (Additive State D) Goal: Edit method interior without aff...

(QB_NEW_EN)

---

[grammar] ~136-~136: There might be a mistake here.
Context: ...structure, on modified file Actions: - Use find_in_file to locate current `Ha...

(QB_NEW_EN)

---

[grammar] ~138-~138: There might be a mistake here.
Context: ...dy interior: change return statement to return true; /* test modification */ - Validate with `mcp__unity__validate_scri...

(QB_NEW_EN)

---

[grammar] ~143-~143: There might be a mistake here.
Context: ... Method Interior Edit (Additive State E) Goal: Edit a different method to show ...

(QB_NEW_EN)

---

[grammar] ~147-~147: There might be a mistake here.
Context: ... content search - Edit interior line to add null check: `if (animator == null) retu...

(QB_NEW_EN)

---

[grammar] ~151-~151: There might be a mistake here.
Context: .... End-of-Class Helper (Additive State F) Goal: Add permanent helper method at c...

(QB_NEW_EN)

---

[grammar] ~153-~153: There might be a mistake here.
Context: ... helper method at class end Actions: - Use smart anchor matching to find curren...

(QB_NEW_EN)

---

[grammar] ~160-~160: There might be a mistake here.
Context: ...d Evolution Lifecycle (Additive State G) Goal: Insert → modify → finalize a fie...

(QB_NEW_EN)

---

[grammar] ~161-~161: There might be a mistake here.
Context: ...fy → finalize a field + companion method Actions: - Insert field: `private in...

(QB_NEW_EN)

---

[grammar] ~162-~162: There might be a mistake here.
Context: ... a field + companion method Actions: - Insert field: private int Counter = 0;...

(QB_NEW_EN)

---

[grammar] ~163-~163: There might be a mistake here.
Context: ...ion method Actions: - Insert field: private int Counter = 0; - Update it: find and replace with `privat...

(QB_NEW_EN)

---

[grammar] ~164-~164: There might be a mistake here.
Context: ... 0;- Update it: find and replace withprivate int Counter = 42; // initialized- Add companion method:private void Incr...

(QB_NEW_EN)

---

[grammar] ~165-~165: There might be a mistake here.
Context: ...// initialized- Add companion method:private void IncrementCounter() { Counter++; }` - Expected final state: State F + Counte...

(QB_NEW_EN)

---

[grammar] ~168-~168: There might be a mistake here.
Context: ...-F. Atomic Multi-Edit (Additive State H) Goal: Multiple coordinated edits in si...

(QB_NEW_EN)

---

[grammar] ~169-~169: There might be a mistake here.
Context: ...H) Goal: Multiple coordinated edits in single atomic operation Actions: - ...

(QB_NEW_EN)

---

[grammar] ~169-~169: There might be a mistake here.
Context: ...dinated edits in single atomic operation Actions: - Read current file state t...

(QB_NEW_EN)

---

[grammar] ~170-~170: There might be a mistake here.
Context: ... in single atomic operation Actions: - Read current file state to compute preci...

(QB_NEW_EN)

---

[grammar] ~172-~172: There might be a mistake here.
Context: ... precise ranges - Atomic edit combining: 1. Add comment in HasTarget(): `// valida...

(QB_NEW_EN)

---

[grammar] ~173-~173: There might be a mistake here.
Context: ...ing: 1. Add comment in HasTarget(): // validated access 2. Add comment in ApplyBlend(): `// safe ...

(QB_NEW_EN)

---

[grammar] ~174-~174: There might be a mistake here.
Context: ... 2. Add comment inApplyBlend(): // safe animation 3. Add final class comment:// end of test...

(QB_NEW_EN)

---

[grammar] ~175-~175: There might be a mistake here.
Context: ...nimation 3. Add final class comment:// end of test modifications` - All edits computed from same file snapsh...

(QB_NEW_EN)

---

[grammar] ~176-~176: There might be a mistake here.
Context: ...est modifications` - All edits computed from same file snapshot, applied atomically ...

(QB_NEW_EN)

---

[grammar] ~177-~177: There might be a mistake here.
Context: ...**: State G + three coordinated comments - After applying the atomic edits, run `va...

(QB_NEW_EN)

---

[grammar] ~180-~180: There might be a mistake here.
Context: ...ath Normalization Test (No State Change) Goal: Verify URI forms work equivalent...

(QB_NEW_EN)

---

[grammar] ~182-~182: There might be a mistake here.
Context: ...uivalently on modified file Actions: - Make identical edit using `unity://path/...

(QB_NEW_EN)

---

[grammar] ~183-~183: There might be a mistake here.
Context: ...Actions*: - Make identical edit using unity://path/Assets/Scripts/LongUnityScriptClaudeTest.cs - Then using `Assets/Scripts/LongUnityScri...

(QB_NEW_EN)

---

[grammar] ~186-~186: There might be a mistake here.
Context: ...ith updated SHA - Verify both URI forms target same file - Expected final state: S...

(QB_NEW_EN)

---

[grammar] ~187-~187: There might be a mistake here.
Context: ...H (no content change, just path testing) - Emit reports/T-G_results.xml showing e...

(QB_NEW_EN)

---

[grammar] ~190-~190: There might be a mistake here.
Context: ...ation on Modified File (No State Change) Goal: Ensure validation works correctl...

(QB_NEW_EN)

---

[grammar] ~192-~192: There might be a mistake here.
Context: ...ly on heavily modified file Actions: - Run validate_script(level:"standard") ...

(QB_NEW_EN)

---

[grammar] ~195-~195: There might be a mistake here.
Context: ...e**: State H (validation only, no edits) - Emit reports/T-H_results.xml confirmin...

(QB_NEW_EN)

---

[grammar] ~198-~198: There might be a mistake here.
Context: ...ailure Surface Testing (No State Change) Goal: Test error handling on real modi...

(QB_NEW_EN)

---

[grammar] ~200-~200: There might be a mistake here.
Context: ...dling on real modified file Actions: - Attempt overlapping edits (should fail c...

(QB_NEW_EN)

---

[grammar] ~207-~207: There might be a mistake here.
Context: ...ency on Modified File (Additive State I) Goal: Verify operations behave predict...

(QB_NEW_EN)

---

[grammar] ~209-~209: There might be a mistake here.
Context: ...e predictably when repeated Actions: - Insert (structured): `mcp__unity__scri...

(QB_NEW_EN)

---

[grammar] ~213-~213: There might be a mistake here.
Context: ...o_op: true. - **Remove (structured)**: {"op":"regex_replace","pattern":"(?m)^\s*// idempotency test marker\r?\n?","text":""}- **Remove again** (sameregex_replace`) → ...

(QB_NEW_EN)

---

[grammar] ~216-~216: There might be a mistake here.
Context: ...pt(level:"standard")- **IMMEDIATELY** write clean XML fragment toreports/T-J_resu...

(QB_NEW_EN)

---

[grammar] ~249-~249: There might be a mistake here.
Context: ...erification Patterns After each test: 1. Verify expected content exists: `find_in...

(QB_NEW_EN)

---

[grammar] ~255-~255: There might be a mistake here.
Context: ...nges in test evidence Error Recovery: - If test fails, log current state but con...

(QB_NEW_EN)

---

[grammar] ~256-~256: There might be a mistake here.
Context: ...e Error Recovery: - If test fails, log current state but continue (don't resto...

(QB_NEW_EN)

---

[grammar] ~257-~257: There might be a mistake here.
Context: ...inue (don't restore) - Next test adapts to actual current state, not expected stat...

(QB_NEW_EN)

---

[grammar] ~257-~257: There might be a mistake here.
Context: ...xt test adapts to actual current state, not expected state - Demonstrates resilienc...

(QB_NEW_EN)

🪛 markdownlint-cli2 (0.17.2)

.claude/prompts/nl-unity-suite-full-additive.md

111-111: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🪛 YAMLlint (1.37.1)

.github/workflows/claude-nl-suite.yml

[warning] 75-75: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 75-75: too many spaces after colon

(colons)

---

[warning] 76-76: too many spaces after colon

(colons)

---

[warning] 78-78: too many spaces after colon

(colons)

---

[warning] 92-92: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 122-122: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 122-122: too many spaces after colon

(colons)

---

[warning] 123-123: too many spaces after colon

(colons)

---

[warning] 125-125: too many spaces after colon

(colons)

---

[warning] 166-166: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 196-196: wrong indentation: expected 14 but found 12

(indentation)

---

[error] 219-219: trailing spaces

(trailing-spaces)

---

[warning] 811-811: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 811-811: too many spaces after colon

(colons)

---

[warning] 813-813: too many spaces after colon

(colons)

🔇 Additional comments (4)

.claude/prompts/nl-unity-suite-full-additive.md (1)

5-7: AllowedTools line matches workflow restrictions — good alignment.

The printed AllowedTools mirror the action’s allowed_tools and intentionally exclude Bash. Avoids tool-drift between prompt and CI config.

.github/workflows/claude-nl-suite.yml (3)

221-273: Wait loop looks robust and safely redacts logs — nice.

Good readiness gating (port + log markers), delayed fatal licensing check, and consistent redaction.

---

318-318: Note retained in history — revert helper removed.

Acknowledged removal; keeps prompt/action in sync.

---

799-805: Consistent redaction on shutdown — good.

Stops container and scrubs sensitive tokens in logs.

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (2)

.claude/prompts/nl-unity-suite-full-additive.md (2)

14-15: Validation level set to "standard" — consistent with suite expectations.

Also applies to: 60-61

---

5-6: Add the Bash revert helper to the printed AllowedTools to match the workflow.

Agents will avoid using the revert helper if it’s not listed here, despite being allowed by the workflow.

-AllowedTools: Write,mcp__unity__manage_editor,mcp__unity__list_resources,mcp__unity__read_resource,mcp__unity__apply_text_edits,mcp__unity__script_apply_edits,mcp__unity__validate_script,mcp__unity__find_in_file,mcp__unity__read_console,mcp__unity__get_sha
+AllowedTools: Write,Bash(scripts/nlt-revert.sh:*),mcp__unity__manage_editor,mcp__unity__list_resources,mcp__unity__read_resource,mcp__unity__apply_text_edits,mcp__unity__script_apply_edits,mcp__unity__validate_script,mcp__unity__find_in_file,mcp__unity__read_console,mcp__unity__get_sha

🧹 Nitpick comments (3)

.claude/prompts/nl-unity-suite-full-additive.md (3)

115-120: Specify a language for this fenced code block (markdownlint MD040).

-  ```
+  ```csharp
   // Tail test A
   // Tail test B  
   // Tail test C

---

`38-47`: **Optional: call out Write tool usage and reports/ existence upfront.**

A brief reminder that reports/ must pre-exist and Write is the mechanism for fragment emission can reduce agent slips.


```diff
 - `$MD_OUT=reports/junit-nl-suite.md` (synthesized from JUnit)
+ - `$MD_OUT=reports/junit-nl-suite.md` (synthesized from JUnit)
+Note: Emit per‑test fragments using the Write tool; do not create directories — `reports/` is assumed to already exist.

---

101-108: Optional: clarify Update() location step mirrors the relaxed anchor.

Echo the flexible pattern here so implementers don’t default to “private”.

-- Use `find_in_file` to locate current position of `Update()` method
+- Use `find_in_file` to locate current position of `Update()` method (e.g., pattern `(?m)^\\s*(?:public|private|protected)?\\s*void\\s+Update\\(\\)`)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e1d8ac5 and 8234a5d.

📒 Files selected for processing (1)

• .claude/prompts/nl-unity-suite-full-additive.md (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-08-29T21:31:02.469Z

Learnt from: dsarno
PR: dsarno/unity-mcp#56
File: .claude/prompts/nl-unity-suite-full.md:5-6
Timestamp: 2025-08-29T21:31:02.469Z
Learning: The Claude NL suite workflows (.github/workflows/claude-nl-suite.yml and .github/workflows/claude-nl-suite-mini.yml) allow "Bash" tools generally, including printf and echo operations, so prompts that reference Bash(printf:*) and Bash(echo:*) are consistent with the actual workflow permissions.

Applied to files:

• .claude/prompts/nl-unity-suite-full-additive.md

🪛 LanguageTool

.claude/prompts/nl-unity-suite-full-additive.md

[grammar] ~5-~5: There might be a mistake here.
Context: ...this once, verbatim, early in the run:** AllowedTools: Write,mcp__unity__manage_e...

(QB_NEW_EN)

---

[grammar] ~17-~17: There might be a mistake here.
Context: ...T. **CRITICAL XML FORMAT REQUIREMENTS:** - Each file must contain EXACTLY one <tes...

(QB_NEW_EN)

---

[grammar] ~32-~32: There might be a mistake here.
Context: ..., T-C, T-D, T-E, T-F, T-G, T-H, T-I, T-J 5) NO RESTORATION - tests build additivel...

(QB_NEW_EN)

---

[grammar] ~33-~33: There might be a mistake here.
Context: ... RESTORATION** - tests build additively on previous state. 6) **STRICT FRAGMENT EM...

(QB_NEW_EN)

---

[grammar] ~33-~33: There might be a mistake here.
Context: ...ests build additively on previous state. 6) STRICT FRAGMENT EMISSION - After each ...

(QB_NEW_EN)

---

[grammar] ~38-~38: There might be a mistake here.
Context: ... emit. --- ## Environment & Paths (CI) - Always pass: `project_root: "TestProject...

(QB_NEW_EN)

---

[grammar] ~39-~39: There might be a mistake here.
Context: ...nd ctx: {} on list/read/edit/validate. - Canonical URIs only: - Primary: `uni...

(QB_NEW_EN)

---

[grammar] ~40-~40: There might be a mistake here.
Context: ...dit/validate. - Canonical URIs only: - Primary: unity://path/Assets/... (neve...

(QB_NEW_EN)

---

[grammar] ~41-~41: There might be a mistake here.
Context: ... (never embed project_root in the URI) - Relative (when supported): Assets/... ...

(QB_NEW_EN)

---

[grammar] ~50-~50: There might be a mistake here.
Context: ...esized from JUnit) --- ## Tool Mapping - Anchors/regex/structured: `mcp__unity_...

(QB_NEW_EN)

---

[grammar] ~51-~51: There might be a mistake here.
Context: ...Mapping - Anchors/regex/structured: mcp__unity__script_apply_edits - Allowed ops: anchor_insert, `replace_m...

(QB_NEW_EN)

---

[grammar] ~52-~52: There might be a mistake here.
Context: ...ethod, insert_method, delete_method, regex_replace - Foranchor_insert, always set "positi...

(QB_NEW_EN)

---

[grammar] ~53-~53: There might be a mistake here.
Context: ...set "position": "before" or "after". - Precise ranges / atomic batch: `mcp__u...

(QB_NEW_EN)

---

[grammar] ~54-~54: There might be a mistake here.
Context: ...ply_text_edits(non‑overlapping ranges) STRICT OP GUARDRAILS - Do not useancho...

(QB_NEW_EN)

---

[grammar] ~55-~55: There might be a mistake here.
Context: ...overlapping ranges) STRICT OP GUARDRAILS - Do not use anchor_replace. Structured ...

(QB_NEW_EN)

---

[grammar] ~56-~56: There might be a mistake here.
Context: ...thod, delete_method, regex_replace`. - For multi‑spot textual tweaks in one ope...

(QB_NEW_EN)

---

[grammar] ~60-~60: There might be a mistake here.
Context: ...c}without file body - **Validation**:mcp__unity__validate_script(level:"standard")- **Dynamic targeting**: Usemcp__unity__fi...

(QB_NEW_EN)

---

[grammar] ~67-~67: There might be a mistake here.
Context: ...nciples Key Changes from Reset-Based: 1. Dynamic Targeting: Use find_in_file ...

(QB_NEW_EN)

---

[grammar] ~74-~74: There might be a mistake here.
Context: ...her in real workflows State Tracking: - Track file SHA after each test (`mcp__un...

(QB_NEW_EN)

---

[grammar] ~76-~76: There might be a mistake here.
Context: .../T‑I to exercise stale_file semantics. - Use content signatures (method names, co...

(QB_NEW_EN)

---

[grammar] ~84-~84: There might be a mistake here.
Context: ... Specs ### NL-0. Baseline State Capture Goal: Establish initial file state and...

(QB_NEW_EN)

---

[grammar] ~86-~86: There might be a mistake here.
Context: ...te and verify accessibility Actions: - Read file head and tail to confirm struc...

(QB_NEW_EN)

---

[grammar] ~92-~92: There might be a mistake here.
Context: ...ore Method Operations (Additive State A) Goal: Demonstrate method replacement o...

(QB_NEW_EN)

---

[grammar] ~94-~94: There might be a mistake here.
Context: ...thod replacement operations Actions: - Replace HasTarget() method body: `publ...

(QB_NEW_EN)

---

[grammar] ~101-~101: There might be a mistake here.
Context: ...hor Comment Insertion (Additive State B) Goal: Demonstrate anchor-based inserti...

(QB_NEW_EN)

---

[grammar] ~103-~103: There might be a mistake here.
Context: ...ed insertions above methods Actions: - Use find_in_file to locate current pos...

(QB_NEW_EN)

---

[grammar] ~104-~104: There might be a mistake here.
Context: ...ds Actions: - Use find_in_file to locate current position of Update() method -...

(QB_NEW_EN)

---

[grammar] ~109-~109: There might be a mistake here.
Context: ... End-of-Class Content (Additive State C) Goal: Demonstrate end-of-class inserti...

(QB_NEW_EN)

---

[grammar] ~111-~111: There might be a mistake here.
Context: ...s with smart brace matching Actions: - Match the final class-closing brace by s...

(QB_NEW_EN)

---

[grammar] ~113-~113: There might be a mistake here.
Context: ... + ranges; insert immediately before it. - Insert three comment lines before final ...

(QB_NEW_EN)

---

[grammar] ~114-~114: There might be a mistake here.
Context: ...before it. - Insert three comment lines before final class brace: ``` // Tail test...

(QB_NEW_EN)

---

[grammar] ~122-~122: There might be a mistake here.
Context: ...ole State Verification (No State Change) Goal: Verify Unity console integration...

(QB_NEW_EN)

---

[grammar] ~124-~124: There might be a mistake here.
Context: ...n without file modification Actions: - Read Unity console messages (INFO level)...

(QB_NEW_EN)

---

[grammar] ~129-~129: There might be a mistake here.
Context: ...ry Helper Lifecycle (Returns to State C) Goal: Test insert → verify → delete cy...

(QB_NEW_EN)

---

[grammar] ~131-~131: There might be a mistake here.
Context: ...te cycle for temporary code Actions: - Find current position of `GetCurrentTarg...

(QB_NEW_EN)

---

[grammar] ~138-~138: There might be a mistake here.
Context: ...od Body Interior Edit (Additive State D) Goal: Edit method interior without aff...

(QB_NEW_EN)

---

[grammar] ~140-~140: There might be a mistake here.
Context: ...structure, on modified file Actions: - Use find_in_file to locate current `Ha...

(QB_NEW_EN)

---

[grammar] ~142-~142: There might be a mistake here.
Context: ...dy interior: change return statement to return true; /* test modification */ - Validate with `mcp__unity__validate_scri...

(QB_NEW_EN)

---

[grammar] ~147-~147: There might be a mistake here.
Context: ... Method Interior Edit (Additive State E) Goal: Edit a different method to show ...

(QB_NEW_EN)

---

[grammar] ~151-~151: There might be a mistake here.
Context: ... content search - Edit interior line to add null check: `if (animator == null) retu...

(QB_NEW_EN)

---

[grammar] ~155-~155: There might be a mistake here.
Context: .... End-of-Class Helper (Additive State F) Goal: Add permanent helper method at c...

(QB_NEW_EN)

---

[grammar] ~157-~157: There might be a mistake here.
Context: ... helper method at class end Actions: - Use smart anchor matching to find curren...

(QB_NEW_EN)

---

[grammar] ~164-~164: There might be a mistake here.
Context: ...d Evolution Lifecycle (Additive State G) Goal: Insert → modify → finalize a fie...

(QB_NEW_EN)

---

[grammar] ~165-~165: There might be a mistake here.
Context: ...fy → finalize a field + companion method Actions: - Insert field: `private in...

(QB_NEW_EN)

---

[grammar] ~166-~166: There might be a mistake here.
Context: ... a field + companion method Actions: - Insert field: private int Counter = 0;...

(QB_NEW_EN)

---

[grammar] ~167-~167: There might be a mistake here.
Context: ...ion method Actions: - Insert field: private int Counter = 0; - Update it: find and replace with `privat...

(QB_NEW_EN)

---

[grammar] ~168-~168: There might be a mistake here.
Context: ... 0;- Update it: find and replace withprivate int Counter = 42; // initialized- Add companion method:private void Incr...

(QB_NEW_EN)

---

[grammar] ~169-~169: There might be a mistake here.
Context: ...// initialized- Add companion method:private void IncrementCounter() { Counter++; }` - Expected final state: State F + Counte...

(QB_NEW_EN)

---

[grammar] ~172-~172: There might be a mistake here.
Context: ...-F. Atomic Multi-Edit (Additive State H) Goal: Multiple coordinated edits in si...

(QB_NEW_EN)

---

[grammar] ~173-~173: There might be a mistake here.
Context: ...H) Goal: Multiple coordinated edits in single atomic operation Actions: - ...

(QB_NEW_EN)

---

[grammar] ~173-~173: There might be a mistake here.
Context: ...dinated edits in single atomic operation Actions: - Read current file state t...

(QB_NEW_EN)

---

[grammar] ~174-~174: There might be a mistake here.
Context: ... in single atomic operation Actions: - Read current file state to compute preci...

(QB_NEW_EN)

---

[grammar] ~176-~176: There might be a mistake here.
Context: ... precise ranges - Atomic edit combining: 1. Add comment in HasTarget(): `// valida...

(QB_NEW_EN)

---

[grammar] ~177-~177: There might be a mistake here.
Context: ...ing: 1. Add comment in HasTarget(): // validated access 2. Add comment in ApplyBlend(): `// safe ...

(QB_NEW_EN)

---

[grammar] ~178-~178: There might be a mistake here.
Context: ... 2. Add comment inApplyBlend(): // safe animation 3. Add final class comment:// end of test...

(QB_NEW_EN)

---

[grammar] ~179-~179: There might be a mistake here.
Context: ...nimation 3. Add final class comment:// end of test modifications` - All edits computed from same file snapsh...

(QB_NEW_EN)

---

[grammar] ~180-~180: There might be a mistake here.
Context: ...est modifications` - All edits computed from same file snapshot, applied atomically ...

(QB_NEW_EN)

---

[grammar] ~181-~181: There might be a mistake here.
Context: ...**: State G + three coordinated comments - After applying the atomic edits, run `va...

(QB_NEW_EN)

---

[grammar] ~184-~184: There might be a mistake here.
Context: ...ath Normalization Test (No State Change) Goal: Verify URI forms work equivalent...

(QB_NEW_EN)

---

[grammar] ~186-~186: There might be a mistake here.
Context: ...uivalently on modified file Actions: - Make identical edit using `unity://path/...

(QB_NEW_EN)

---

[grammar] ~187-~187: There might be a mistake here.
Context: ...Actions*: - Make identical edit using unity://path/Assets/Scripts/LongUnityScriptClaudeTest.cs - Then using `Assets/Scripts/LongUnityScri...

(QB_NEW_EN)

---

[grammar] ~190-~190: There might be a mistake here.
Context: ...ith updated SHA - Verify both URI forms target same file - Expected final state: S...

(QB_NEW_EN)

---

[grammar] ~191-~191: There might be a mistake here.
Context: ...H (no content change, just path testing) - Emit reports/T-G_results.xml showing e...

(QB_NEW_EN)

---

[grammar] ~194-~194: There might be a mistake here.
Context: ...ation on Modified File (No State Change) Goal: Ensure validation works correctl...

(QB_NEW_EN)

---

[grammar] ~196-~196: There might be a mistake here.
Context: ...ly on heavily modified file Actions: - Run validate_script(level:"standard") ...

(QB_NEW_EN)

---

[grammar] ~199-~199: There might be a mistake here.
Context: ...e**: State H (validation only, no edits) - Emit reports/T-H_results.xml confirmin...

(QB_NEW_EN)

---

[grammar] ~202-~202: There might be a mistake here.
Context: ...ailure Surface Testing (No State Change) Goal: Test error handling on real modi...

(QB_NEW_EN)

---

[grammar] ~204-~204: There might be a mistake here.
Context: ...dling on real modified file Actions: - Attempt overlapping edits (should fail c...

(QB_NEW_EN)

---

[grammar] ~211-~211: There might be a mistake here.
Context: ...ency on Modified File (Additive State I) Goal: Verify operations behave predict...

(QB_NEW_EN)

---

[grammar] ~213-~213: There might be a mistake here.
Context: ...e predictably when repeated Actions: - Insert (structured): `mcp__unity__scri...

(QB_NEW_EN)

---

[grammar] ~217-~217: There might be a mistake here.
Context: ...o_op: true. - **Remove (structured)**: {"op":"regex_replace","pattern":"(?m)^\s*// idempotency test marker\r?\n?","text":""}- **Remove again** (sameregex_replace`) → ...

(QB_NEW_EN)

---

[grammar] ~220-~220: There might be a mistake here.
Context: ...pt(level:"standard")- **IMMEDIATELY** write clean XML fragment toreports/T-J_resu...

(QB_NEW_EN)

---

[grammar] ~253-~253: There might be a mistake here.
Context: ...erification Patterns After each test: 1. Verify expected content exists: `find_in...

(QB_NEW_EN)

---

[grammar] ~260-~260: There might be a mistake here.
Context: ...nges in test evidence Error Recovery: - If test fails, log current state but con...

(QB_NEW_EN)

---

[grammar] ~261-~261: There might be a mistake here.
Context: ...e Error Recovery: - If test fails, log current state but continue (don't resto...

(QB_NEW_EN)

---

[grammar] ~262-~262: There might be a mistake here.
Context: ...inue (don't restore) - Next test adapts to actual current state, not expected stat...

(QB_NEW_EN)

---

[grammar] ~262-~262: There might be a mistake here.
Context: ...xt test adapts to actual current state, not expected state - Demonstrates resilienc...

(QB_NEW_EN)

🪛 markdownlint-cli2 (0.17.2)

.claude/prompts/nl-unity-suite-full-additive.md

115-115: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🔇 Additional comments (2)

.claude/prompts/nl-unity-suite-full-additive.md (2)

65-79: Additive design, state tracking, and guardrails look solid.

Dynamic targeting, SHA preconditions, and explicit op allowlist are well-specified.

---

1-6: Prompt/workflow alignment verified
nl-unity-suite-full-additive.md is referenced in .github/workflows/claude-nl-suite.yml:328; the test script LongUnityScriptClaudeTest.cs exists; AllowedTools matches the required toolset; no revert script is present or required.

[coderabbitai]

🛠️ Refactor suggestion

Relax the Update() anchor to avoid brittleness across access modifiers.

Unity’s Update is often implicit-private; anchoring on “private” may miss matches.

-{"op": "anchor_insert", "anchor": "private void Update\\(\\)", "position": "before", "text": "// comment"}
+{"op": "anchor_insert", "anchor": "(?m)^\\s*(?:public|private|protected)?\\s*void\\s+Update\\(\\)", "position": "before", "text": "// comment"}

📝 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

	```json
	{"op": "anchor_insert", "anchor": "private void Update\\(\\)", "position": "before", "text": "// comment"}
	```
	{"op": "anchor_insert", "anchor": "(?m)^\\s*(?:public|private|protected)?\\s*void\\s+Update\\(\\)", "position": "before", "text": "// comment"}

🤖 Prompt for AI Agents

In .claude/prompts/nl-unity-suite-full-additive.md around lines 245–247, the
anchor currently matches "private void Update()" which is brittle because Unity
methods may omit the access modifier; relax the anchor to match Update across
access modifiers by using a regex or anchor without the modifier (for example
match "void Update(" or simply "Update\\(" or a pattern like
"(?:public|protected|private)?\\s*void\\s+Update\\s*\\(") so the insertion will
succeed regardless of the declared access level.

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

.claude/prompts/nl-unity-suite-full-additive.md (1)

245-248: Relax Update() anchor to match any access modifier (or none).

Using "private" is brittle and may miss implicit-private or public/protected cases.

Apply:

-{"op": "anchor_insert", "anchor": "private void Update\\(\\)", "position": "before", "text": "// comment"}
+{"op": "anchor_insert", "anchor": "(?m)^\\s*(?:public|private|protected)?\\s*void\\s+Update\\(\\)", "position": "before", "text": "// comment"}

🧹 Nitpick comments (3)

.claude/prompts/nl-unity-suite-full-additive.md (2)

115-121: Add language to fenced block (markdownlint MD040).

Specify a language for the tail-comments example.

-  ```
+  ```csharp
   // Tail test A
   // Tail test B  
   // Tail test C

---

`128-129`: **Fix list indentation (MD005/MD007).**

Remove stray leading space before the bullet.


```diff
- - **IMMEDIATELY** write clean XML fragment to `reports/NL-4_results.xml` (no extra text). The `<testcase name>` must start with `NL-4`. Include brief evidence (e.g., a few recent console lines or an explicit "no compile errors" note) in `system-out`.
+- **IMMEDIATELY** write clean XML fragment to `reports/NL-4_results.xml` (no extra text). The `<testcase name>` must start with `NL-4`. Include brief evidence (e.g., a few recent console lines or an explicit "no compile errors" note) in `system-out`.

.github/workflows/claude-nl-suite.yml (1)

321-341: Strip line numbers from rg output in the CI assertion script. Change both rg -n calls to omit the -n flag—or add --no-heading --no-filename—so only the raw tool names (no 331-, 332-, etc.) are captured before normalizing and comparing.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8234a5d and c92f605.

📒 Files selected for processing (2)

• .claude/prompts/nl-unity-suite-full-additive.md (1 hunks)
• .github/workflows/claude-nl-suite.yml (14 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-08-29T21:31:02.469Z

Learnt from: dsarno
PR: dsarno/unity-mcp#56
File: .claude/prompts/nl-unity-suite-full.md:5-6
Timestamp: 2025-08-29T21:31:02.469Z
Learning: The Claude NL suite workflows (.github/workflows/claude-nl-suite.yml and .github/workflows/claude-nl-suite-mini.yml) allow "Bash" tools generally, including printf and echo operations, so prompts that reference Bash(printf:*) and Bash(echo:*) are consistent with the actual workflow permissions.

Applied to files:

• .claude/prompts/nl-unity-suite-full-additive.md
• .github/workflows/claude-nl-suite.yml

🪛 LanguageTool

.claude/prompts/nl-unity-suite-full-additive.md

[grammar] ~5-~5: There might be a mistake here.
Context: ...this once, verbatim, early in the run:** AllowedTools: Write,mcp__unity__manage_e...

(QB_NEW_EN)

---

[grammar] ~17-~17: There might be a mistake here.
Context: ...T. **CRITICAL XML FORMAT REQUIREMENTS:** - Each file must contain EXACTLY one <tes...

(QB_NEW_EN)

---

[grammar] ~32-~32: There might be a mistake here.
Context: ..., T-C, T-D, T-E, T-F, T-G, T-H, T-I, T-J 5) NO RESTORATION - tests build additivel...

(QB_NEW_EN)

---

[grammar] ~33-~33: There might be a mistake here.
Context: ... RESTORATION** - tests build additively on previous state. 6) **STRICT FRAGMENT EM...

(QB_NEW_EN)

---

[grammar] ~33-~33: There might be a mistake here.
Context: ...ests build additively on previous state. 6) STRICT FRAGMENT EMISSION - After each ...

(QB_NEW_EN)

---

[grammar] ~38-~38: There might be a mistake here.
Context: ... emit. --- ## Environment & Paths (CI) - Always pass: `project_root: "TestProject...

(QB_NEW_EN)

---

[grammar] ~39-~39: There might be a mistake here.
Context: ...nd ctx: {} on list/read/edit/validate. - Canonical URIs only: - Primary: `uni...

(QB_NEW_EN)

---

[grammar] ~40-~40: There might be a mistake here.
Context: ...dit/validate. - Canonical URIs only: - Primary: unity://path/Assets/... (neve...

(QB_NEW_EN)

---

[grammar] ~41-~41: There might be a mistake here.
Context: ... (never embed project_root in the URI) - Relative (when supported): Assets/... ...

(QB_NEW_EN)

---

[grammar] ~50-~50: There might be a mistake here.
Context: ...esized from JUnit) --- ## Tool Mapping - Anchors/regex/structured: `mcp__unity_...

(QB_NEW_EN)

---

[grammar] ~51-~51: There might be a mistake here.
Context: ...Mapping - Anchors/regex/structured: mcp__unity__script_apply_edits - Allowed ops: anchor_insert, `replace_m...

(QB_NEW_EN)

---

[grammar] ~52-~52: There might be a mistake here.
Context: ...ethod, insert_method, delete_method, regex_replace - Foranchor_insert, always set "positi...

(QB_NEW_EN)

---

[grammar] ~53-~53: There might be a mistake here.
Context: ...set "position": "before" or "after". - Precise ranges / atomic batch: `mcp__u...

(QB_NEW_EN)

---

[grammar] ~54-~54: There might be a mistake here.
Context: ...ply_text_edits(non‑overlapping ranges) STRICT OP GUARDRAILS - Do not useancho...

(QB_NEW_EN)

---

[grammar] ~55-~55: There might be a mistake here.
Context: ...overlapping ranges) STRICT OP GUARDRAILS - Do not use anchor_replace. Structured ...

(QB_NEW_EN)

---

[grammar] ~56-~56: There might be a mistake here.
Context: ...thod, delete_method, regex_replace`. - For multi‑spot textual tweaks in one ope...

(QB_NEW_EN)

---

[grammar] ~60-~60: There might be a mistake here.
Context: ...c}without file body - **Validation**:mcp__unity__validate_script(level:"standard")- **Dynamic targeting**: Usemcp__unity__fi...

(QB_NEW_EN)

---

[grammar] ~67-~67: There might be a mistake here.
Context: ...nciples Key Changes from Reset-Based: 1. Dynamic Targeting: Use find_in_file ...

(QB_NEW_EN)

---

[grammar] ~74-~74: There might be a mistake here.
Context: ...her in real workflows State Tracking: - Track file SHA after each test (`mcp__un...

(QB_NEW_EN)

---

[grammar] ~76-~76: There might be a mistake here.
Context: .../T‑I to exercise stale_file semantics. - Use content signatures (method names, co...

(QB_NEW_EN)

---

[grammar] ~84-~84: There might be a mistake here.
Context: ... Specs ### NL-0. Baseline State Capture Goal: Establish initial file state and...

(QB_NEW_EN)

---

[grammar] ~86-~86: There might be a mistake here.
Context: ...te and verify accessibility Actions: - Read file head and tail to confirm struc...

(QB_NEW_EN)

---

[grammar] ~92-~92: There might be a mistake here.
Context: ...ore Method Operations (Additive State A) Goal: Demonstrate method replacement o...

(QB_NEW_EN)

---

[grammar] ~94-~94: There might be a mistake here.
Context: ...thod replacement operations Actions: - Replace HasTarget() method body: `publ...

(QB_NEW_EN)

---

[grammar] ~101-~101: There might be a mistake here.
Context: ...hor Comment Insertion (Additive State B) Goal: Demonstrate anchor-based inserti...

(QB_NEW_EN)

---

[grammar] ~103-~103: There might be a mistake here.
Context: ...ed insertions above methods Actions: - Use find_in_file to locate current pos...

(QB_NEW_EN)

---

[grammar] ~104-~104: There might be a mistake here.
Context: ...ds Actions: - Use find_in_file to locate current position of Update() method -...

(QB_NEW_EN)

---

[grammar] ~109-~109: There might be a mistake here.
Context: ... End-of-Class Content (Additive State C) Goal: Demonstrate end-of-class inserti...

(QB_NEW_EN)

---

[grammar] ~111-~111: There might be a mistake here.
Context: ...s with smart brace matching Actions: - Match the final class-closing brace by s...

(QB_NEW_EN)

---

[grammar] ~113-~113: There might be a mistake here.
Context: ... + ranges; insert immediately before it. - Insert three comment lines before final ...

(QB_NEW_EN)

---

[grammar] ~114-~114: There might be a mistake here.
Context: ...before it. - Insert three comment lines before final class brace: ``` // Tail test...

(QB_NEW_EN)

---

[grammar] ~122-~122: There might be a mistake here.
Context: ...ole State Verification (No State Change) Goal: Verify Unity console integration...

(QB_NEW_EN)

---

[grammar] ~124-~124: There might be a mistake here.
Context: ...n without file modification Actions: - Read Unity console messages (INFO level)...

(QB_NEW_EN)

---

[grammar] ~128-~128: There might be a mistake here.
Context: ... State C (unchanged) - IMMEDIATELY write clean XML fragment to `reports/NL-4_res...

(QB_NEW_EN)

---

[grammar] ~130-~130: There might be a mistake here.
Context: ...ry Helper Lifecycle (Returns to State C) Goal: Test insert → verify → delete cy...

(QB_NEW_EN)

---

[grammar] ~132-~132: There might be a mistake here.
Context: ...te cycle for temporary code Actions: - Find current position of `GetCurrentTarg...

(QB_NEW_EN)

---

[grammar] ~139-~139: There might be a mistake here.
Context: ...od Body Interior Edit (Additive State D) Goal: Edit method interior without aff...

(QB_NEW_EN)

---

[grammar] ~141-~141: There might be a mistake here.
Context: ...structure, on modified file Actions: - Use find_in_file to locate current `Ha...

(QB_NEW_EN)

---

[grammar] ~143-~143: There might be a mistake here.
Context: ...dy interior: change return statement to return true; /* test modification */ - Validate with `mcp__unity__validate_scri...

(QB_NEW_EN)

---

[grammar] ~148-~148: There might be a mistake here.
Context: ... Method Interior Edit (Additive State E) Goal: Edit a different method to show ...

(QB_NEW_EN)

---

[grammar] ~152-~152: There might be a mistake here.
Context: ... content search - Edit interior line to add null check: `if (animator == null) retu...

(QB_NEW_EN)

---

[grammar] ~156-~156: There might be a mistake here.
Context: .... End-of-Class Helper (Additive State F) Goal: Add permanent helper method at c...

(QB_NEW_EN)

---

[grammar] ~158-~158: There might be a mistake here.
Context: ... helper method at class end Actions: - Use smart anchor matching to find curren...

(QB_NEW_EN)

---

[grammar] ~165-~165: There might be a mistake here.
Context: ...d Evolution Lifecycle (Additive State G) Goal: Insert → modify → finalize a fie...

(QB_NEW_EN)

---

[grammar] ~166-~166: There might be a mistake here.
Context: ...fy → finalize a field + companion method Actions: - Insert field: `private in...

(QB_NEW_EN)

---

[grammar] ~167-~167: There might be a mistake here.
Context: ... a field + companion method Actions: - Insert field: private int Counter = 0;...

(QB_NEW_EN)

---

[grammar] ~168-~168: There might be a mistake here.
Context: ...ion method Actions: - Insert field: private int Counter = 0; - Update it: find and replace with `privat...

(QB_NEW_EN)

---

[grammar] ~169-~169: There might be a mistake here.
Context: ... 0;- Update it: find and replace withprivate int Counter = 42; // initialized- Add companion method:private void Incr...

(QB_NEW_EN)

---

[grammar] ~170-~170: There might be a mistake here.
Context: ...// initialized- Add companion method:private void IncrementCounter() { Counter++; }` - Expected final state: State F + Counte...

(QB_NEW_EN)

---

[grammar] ~173-~173: There might be a mistake here.
Context: ...-F. Atomic Multi-Edit (Additive State H) Goal: Multiple coordinated edits in si...

(QB_NEW_EN)

---

[grammar] ~174-~174: There might be a mistake here.
Context: ...H) Goal: Multiple coordinated edits in single atomic operation Actions: - ...

(QB_NEW_EN)

---

[grammar] ~174-~174: There might be a mistake here.
Context: ...dinated edits in single atomic operation Actions: - Read current file state t...

(QB_NEW_EN)

---

[grammar] ~175-~175: There might be a mistake here.
Context: ... in single atomic operation Actions: - Read current file state to compute preci...

(QB_NEW_EN)

---

[grammar] ~177-~177: There might be a mistake here.
Context: ... precise ranges - Atomic edit combining: 1. Add comment in HasTarget(): `// valida...

(QB_NEW_EN)

---

[grammar] ~178-~178: There might be a mistake here.
Context: ...ing: 1. Add comment in HasTarget(): // validated access 2. Add comment in ApplyBlend(): `// safe ...

(QB_NEW_EN)

---

[grammar] ~179-~179: There might be a mistake here.
Context: ... 2. Add comment inApplyBlend(): // safe animation 3. Add final class comment:// end of test...

(QB_NEW_EN)

---

[grammar] ~180-~180: There might be a mistake here.
Context: ...nimation 3. Add final class comment:// end of test modifications` - All edits computed from same file snapsh...

(QB_NEW_EN)

---

[grammar] ~181-~181: There might be a mistake here.
Context: ...est modifications` - All edits computed from same file snapshot, applied atomically ...

(QB_NEW_EN)

---

[grammar] ~182-~182: There might be a mistake here.
Context: ...**: State G + three coordinated comments - After applying the atomic edits, run `va...

(QB_NEW_EN)

---

[grammar] ~185-~185: There might be a mistake here.
Context: ...ath Normalization Test (No State Change) Goal: Verify URI forms work equivalent...

(QB_NEW_EN)

---

[grammar] ~187-~187: There might be a mistake here.
Context: ...uivalently on modified file Actions: - Make identical edit using `unity://path/...

(QB_NEW_EN)

---

[grammar] ~188-~188: There might be a mistake here.
Context: ...Actions*: - Make identical edit using unity://path/Assets/Scripts/LongUnityScriptClaudeTest.cs - Then using `Assets/Scripts/LongUnityScri...

(QB_NEW_EN)

---

[grammar] ~191-~191: There might be a mistake here.
Context: ...ith updated SHA - Verify both URI forms target same file - Expected final state: S...

(QB_NEW_EN)

---

[grammar] ~192-~192: There might be a mistake here.
Context: ...H (no content change, just path testing) - Emit reports/T-G_results.xml showing e...

(QB_NEW_EN)

---

[grammar] ~195-~195: There might be a mistake here.
Context: ...ation on Modified File (No State Change) Goal: Ensure validation works correctl...

(QB_NEW_EN)

---

[grammar] ~197-~197: There might be a mistake here.
Context: ...ly on heavily modified file Actions: - Run validate_script(level:"standard") ...

(QB_NEW_EN)

---

[grammar] ~200-~200: There might be a mistake here.
Context: ...e**: State H (validation only, no edits) - Emit reports/T-H_results.xml confirmin...

(QB_NEW_EN)

---

[grammar] ~203-~203: There might be a mistake here.
Context: ...ailure Surface Testing (No State Change) Goal: Test error handling on real modi...

(QB_NEW_EN)

---

[grammar] ~205-~205: There might be a mistake here.
Context: ...dling on real modified file Actions: - Attempt overlapping edits (should fail c...

(QB_NEW_EN)

---

[grammar] ~212-~212: There might be a mistake here.
Context: ...ency on Modified File (Additive State I) Goal: Verify operations behave predict...

(QB_NEW_EN)

---

[grammar] ~214-~214: There might be a mistake here.
Context: ...e predictably when repeated Actions: - Insert (structured): `mcp__unity__scri...

(QB_NEW_EN)

---

[grammar] ~218-~218: There might be a mistake here.
Context: ...o_op: true. - **Remove (structured)**: {"op":"regex_replace","pattern":"(?m)^\s*// idempotency test marker\r?\n?","text":""}- **Remove again** (sameregex_replace`) → ...

(QB_NEW_EN)

---

[grammar] ~221-~221: There might be a mistake here.
Context: ...pt(level:"standard")- **IMMEDIATELY** write clean XML fragment toreports/T-J_resu...

(QB_NEW_EN)

---

[grammar] ~254-~254: There might be a mistake here.
Context: ...erification Patterns After each test: 1. Verify expected content exists: `find_in...

(QB_NEW_EN)

---

[grammar] ~261-~261: There might be a mistake here.
Context: ...nges in test evidence Error Recovery: - If test fails, log current state but con...

(QB_NEW_EN)

---

[grammar] ~262-~262: There might be a mistake here.
Context: ...e Error Recovery: - If test fails, log current state but continue (don't resto...

(QB_NEW_EN)

---

[grammar] ~263-~263: There might be a mistake here.
Context: ...inue (don't restore) - Next test adapts to actual current state, not expected stat...

(QB_NEW_EN)

---

[grammar] ~263-~263: There might be a mistake here.
Context: ...xt test adapts to actual current state, not expected state - Demonstrates resilienc...

(QB_NEW_EN)

🪛 markdownlint-cli2 (0.17.2)

.claude/prompts/nl-unity-suite-full-additive.md

115-115: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

---

128-128: Inconsistent indentation for list items at the same level
Expected: 0; Actual: 1

(MD005, list-indent)

---

128-128: Unordered list indentation
Expected: 0; Actual: 1

(MD007, ul-indent)

🪛 YAMLlint (1.37.1)

.github/workflows/claude-nl-suite.yml

[warning] 75-75: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 75-75: too many spaces after colon

(colons)

---

[warning] 76-76: too many spaces after colon

(colons)

---

[warning] 78-78: too many spaces after colon

(colons)

---

[warning] 92-92: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 122-122: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 122-122: too many spaces after colon

(colons)

---

[warning] 123-123: too many spaces after colon

(colons)

---

[warning] 125-125: too many spaces after colon

(colons)

---

[warning] 166-166: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 196-196: wrong indentation: expected 14 but found 12

(indentation)

---

[error] 219-219: trailing spaces

(trailing-spaces)

---

[warning] 815-815: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 815-815: too many spaces after colon

(colons)

---

[warning] 817-817: too many spaces after colon

(colons)

🔇 Additional comments (2)

.claude/prompts/nl-unity-suite-full-additive.md (1)

5-7: AllowedTools line matches workflow configuration; no drift detected.

.github/workflows/claude-nl-suite.yml (1)

803-809: Good teardown ordering and redaction.

Stopping Unity before license return and masking sensitive tokens looks correct.

[coderabbitai]

⚠️ Potential issue

Redact activation logs and tighten entitlement check path.

Unity may echo credentials; also check the specific licenses dir to avoid false positives.

             docker run --rm --network host \
               -e HOME=/root \
               -e UNITY_EMAIL -e UNITY_PASSWORD -e UNITY_SERIAL \
               -v "$RUNNER_TEMP/unity-config:/root/.config/unity3d" \
               -v "$RUNNER_TEMP/unity-local:/root/.local/share/unity3d" \
-              "$UNITY_IMAGE" bash -lc '
+              "$UNITY_IMAGE" bash -lc '
                 set -euxo pipefail
                 if [[ -n "${UNITY_SERIAL:-}" ]]; then
                   /opt/unity/Editor/Unity -batchmode -nographics -logFile - \
                     -username "$UNITY_EMAIL" -password "$UNITY_PASSWORD" -serial "$UNITY_SERIAL" -quit || true
                 else
                   /opt/unity/Editor/Unity -batchmode -nographics -logFile - \
                     -username "$UNITY_EMAIL" -password "$UNITY_PASSWORD" -quit || true
                 fi
                 ls -la /root/.config/unity3d/Unity/licenses || true
-              '
+              ' 2>&1 | sed -E 's/((email|serial|license|password|token)[^[:space:]]*)/[REDACTED]/Ig'
 
-            # Verify entitlement written to host mount; allow ULF-only runs to proceed
-            if ! find "$RUNNER_TEMP/unity-config" -type f -iname "*.xml" | grep -q .; then
+            # Verify entitlement written to host mount; allow ULF-only runs to proceed
+            lic_dir="$RUNNER_TEMP/unity-config/Unity/licenses"
+            if ! find "$lic_dir" -maxdepth 1 -type f -iname "*.xml" | grep -q .; then
               if [[ "${{ steps.ulf.outputs.ok }}" == "true" ]]; then
                 echo "EBL entitlement not found; proceeding with ULF-only (ok=true)."
               else
                 echo "No entitlement produced and no valid ULF; cannot continue." >&2
                 exit 1
               fi
             fi

Also applies to: 149-157

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (2)

.claude/prompts/nl-unity-suite-full-additive.md (2)

260-262: Relax Update() anchor to handle any access modifier and whitespace.

Prevents brittleness when Update is implicit-private or formatted differently.

-{"op": "anchor_insert", "anchor": "private void Update\\(\\)", "position": "before", "text": "// comment"}
+{"op": "anchor_insert", "anchor": "(?m)^\\s*(?:public|private|protected)?\\s*void\\s+Update\\s*\\(", "position": "before", "text": "// comment"}

---

5-7: Add Bash tools (revert, echo, printf) to the printed AllowedTools to match workflow.

Agents won’t “see” the revert helper or lightweight Bash prints otherwise; this diverges from workflow permissions.

Apply:

-AllowedTools: Write,mcp__unity__manage_editor,mcp__unity__list_resources,mcp__unity__read_resource,mcp__unity__apply_text_edits,mcp__unity__script_apply_edits,mcp__unity__validate_script,mcp__unity__find_in_file,mcp__unity__read_console,mcp__unity__get_sha
+AllowedTools: Write,Bash(scripts/nlt-revert.sh:*),Bash(printf:*),Bash(echo:*),mcp__unity__manage_editor,mcp__unity__list_resources,mcp__unity__read_resource,mcp__unity__apply_text_edits,mcp__unity__script_apply_edits,mcp__unity__validate_script,mcp__unity__find_in_file,mcp__unity__read_console,mcp__unity__get_sha

To confirm allowed tools in the repo, run:

#!/bin/bash
# Verify workflow allows these Bash tools
rg -nC2 -g '.github/workflows/*.yml' -e 'Bash\(scripts/nlt-revert\.sh:\*\)|Bash\(printf:\*\)|Bash\(echo:\*\)'

🧹 Nitpick comments (2)

.claude/prompts/nl-unity-suite-full-additive.md (2)

71-74: Call out baseSha usage explicitly for stale_file semantics.

Make the precondition explicit so agents consistently supply it on text edits.

 - **Hash-only**: `mcp__unity__get_sha` — returns `{sha256,lengthBytes,lastModifiedUtc}` without file body
 - **Validation**: `mcp__unity__validate_script(level:"standard")`
 - **Dynamic targeting**: Use `mcp__unity__find_in_file` to locate current positions of methods/markers
+ - For `mcp__unity__apply_text_edits`, include the current `baseSha` to enforce `stale_file` detection and safe retries.

---

127-131: Specify a language on this fenced block (markdownlint MD040).

Minor lint fix; “csharp” is appropriate for Unity comments.

-  ```
+  ```csharp
   // Tail test A
   // Tail test B  
   // Tail test C

</blockquote></details>

</blockquote></details>

<details>
<summary>📜 Review details</summary>

**Configuration used**: CodeRabbit UI

**Review profile**: CHILL

**Plan**: Pro

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between c92f6051e82d6c1853c2ca8267d92d87df59e094 and 25985160a60758a010c45fa865c408ad3fbf04a1.

</details>

<details>
<summary>📒 Files selected for processing (1)</summary>

* `.claude/prompts/nl-unity-suite-full-additive.md` (1 hunks)

</details>

<details>
<summary>🧰 Additional context used</summary>

<details>
<summary>🧠 Learnings (1)</summary>

<details>
<summary>📚 Learning: 2025-08-29T21:31:02.469Z</summary>

Learnt from: dsarno
PR: #56
File: .claude/prompts/nl-unity-suite-full.md:5-6
Timestamp: 2025-08-29T21:31:02.469Z
Learning: The Claude NL suite workflows (.github/workflows/claude-nl-suite.yml and .github/workflows/claude-nl-suite-mini.yml) allow "Bash" tools generally, including printf and echo operations, so prompts that reference Bash(printf:) and Bash(echo:) are consistent with the actual workflow permissions.

**Applied to files:**
- `.claude/prompts/nl-unity-suite-full-additive.md`

</details>

</details><details>
<summary>🪛 LanguageTool</summary>

<details>
<summary>.claude/prompts/nl-unity-suite-full-additive.md</summary>

[grammar] ~5-~5: There might be a mistake here.
Context: ...this once, verbatim, early in the run:** AllowedTools: Write,mcp__unity__manage_e...

(QB_NEW_EN)

---

[grammar] ~17-~17: There might be a mistake here.
Context: ...T`.  **CRITICAL XML FORMAT REQUIREMENTS:** - Each file must contain EXACTLY one `<tes...

(QB_NEW_EN)

---

[grammar] ~32-~32: There might be a mistake here.
Context: ..., T-C, T-D, T-E, T-F, T-G, T-H, T-I, T-J 5) **NO RESTORATION** - tests build additivel...

(QB_NEW_EN)

---

[grammar] ~33-~33: There might be a mistake here.
Context: ... RESTORATION** - tests build additively on previous state. 6) **STRICT FRAGMENT EM...

(QB_NEW_EN)

---

[grammar] ~33-~33: There might be a mistake here.
Context: ...ests build additively on previous state. 6) **STRICT FRAGMENT EMISSION** - After each ...

(QB_NEW_EN)

---

[grammar] ~38-~38: There might be a mistake here.
Context: ... emit.  ---  ## Environment & Paths (CI) - Always pass: `project_root: "TestProject...

(QB_NEW_EN)

---

[grammar] ~39-~39: There might be a mistake here.
Context: ...nd `ctx: {}` on list/read/edit/validate. - **Canonical URIs only**:   - Primary: `uni...

(QB_NEW_EN)

---

[grammar] ~40-~40: There might be a mistake here.
Context: ...dit/validate. - **Canonical URIs only**:   - Primary: `unity://path/Assets/...` (neve...

(QB_NEW_EN)

---

[grammar] ~41-~41: There might be a mistake here.
Context: ... (never embed `project_root` in the URI)   - Relative (when supported): `Assets/...` ...

(QB_NEW_EN)

---

[grammar] ~54-~54: There might be a mistake here.
Context: ...0 chars: brief status + latest SHA only. - Console evidence: fetch the last 10 line...

(QB_NEW_EN)

---

[grammar] ~56-~56: There might be a mistake here.
Context: ...i‑line diffs; reference markers instead. — Console scans: perform two reads — las...

(QB_NEW_EN)

---

[grammar] ~62-~62: There might be a mistake here.
Context: ... the T‑J fragment.  ---  ## Tool Mapping - **Anchors/regex/structured**: `mcp__unity_...

(QB_NEW_EN)

---

[grammar] ~63-~63: There might be a mistake here.
Context: ...Mapping - **Anchors/regex/structured**: `mcp__unity__script_apply_edits`   - Allowed ops: `anchor_insert`, `replace_m...

(QB_NEW_EN)

---

[grammar] ~64-~64: There might be a mistake here.
Context: ...ethod`, `insert_method`, `delete_method`, `regex_replace`   - For `anchor_insert`, always set `"positi...

(QB_NEW_EN)

---

[grammar] ~65-~65: There might be a mistake here.
Context: ...set `"position": "before"` or `"after"`. - **Precise ranges / atomic batch**: `mcp__u...

(QB_NEW_EN)

---

[grammar] ~66-~66: There might be a mistake here.
Context: ...ply_text_edits` (non‑overlapping ranges) STRICT OP GUARDRAILS - Do not use `ancho...

(QB_NEW_EN)

---

[grammar] ~67-~67: There might be a mistake here.
Context: ...overlapping ranges) STRICT OP GUARDRAILS - Do not use `anchor_replace`. Structured ...

(QB_NEW_EN)

---

[grammar] ~68-~68: There might be a mistake here.
Context: ...thod`, `delete_method`, `regex_replace`. - For multi‑spot textual tweaks in one ope...

(QB_NEW_EN)

---

[grammar] ~72-~72: There might be a mistake here.
Context: ...c}` without file body - **Validation**: `mcp__unity__validate_script(level:"standard")` - **Dynamic targeting**: Use `mcp__unity__fi...

(QB_NEW_EN)

---

[grammar] ~79-~79: There might be a mistake here.
Context: ...nciples  **Key Changes from Reset-Based:** 1. **Dynamic Targeting**: Use `find_in_file` ...

(QB_NEW_EN)

---

[grammar] ~86-~86: There might be a mistake here.
Context: ...her in real workflows  **State Tracking:** - Track file SHA after each test (`mcp__un...

(QB_NEW_EN)

---

[grammar] ~88-~88: There might be a mistake here.
Context: .../T‑I to exercise `stale_file` semantics. - Use content signatures (method names, co...

(QB_NEW_EN)

---

[grammar] ~96-~96: There might be a mistake here.
Context: ... Specs  ### NL-0. Baseline State Capture **Goal**: Establish initial file state and...

(QB_NEW_EN)

---

[grammar] ~98-~98: There might be a mistake here.
Context: ...te and verify accessibility **Actions**: - Read file head and tail to confirm struc...

(QB_NEW_EN)

---

[grammar] ~104-~104: There might be a mistake here.
Context: ...ore Method Operations (Additive State A) **Goal**: Demonstrate method replacement o...

(QB_NEW_EN)

---

[grammar] ~106-~106: There might be a mistake here.
Context: ...thod replacement operations **Actions**:  - Replace `HasTarget()` method body: `publ...

(QB_NEW_EN)

---

[grammar] ~113-~113: There might be a mistake here.
Context: ...hor Comment Insertion (Additive State B)  **Goal**: Demonstrate anchor-based inserti...

(QB_NEW_EN)

---

[grammar] ~115-~115: There might be a mistake here.
Context: ...ed insertions above methods **Actions**: - Use `find_in_file` to locate current pos...

(QB_NEW_EN)

---

[grammar] ~116-~116: There might be a mistake here.
Context: ...ds **Actions**: - Use `find_in_file` to locate current position of `Update()` method -...

(QB_NEW_EN)

---

[grammar] ~121-~121: There might be a mistake here.
Context: ... End-of-Class Content (Additive State C) **Goal**: Demonstrate end-of-class inserti...

(QB_NEW_EN)

---

[grammar] ~123-~123: There might be a mistake here.
Context: ...s with smart brace matching **Actions**: - Match the final class-closing brace by s...

(QB_NEW_EN)

---

[grammar] ~125-~125: There might be a mistake here.
Context: ... + ranges; insert immediately before it. - Insert three comment lines before final ...

(QB_NEW_EN)

---

[grammar] ~126-~126: There might be a mistake here.
Context: ...before it. - Insert three comment lines before final class brace:   ```   // Tail test...

(QB_NEW_EN)

---

[grammar] ~134-~134: There might be a mistake here.
Context: ...ole State Verification (No State Change) **Goal**: Verify Unity console integration...

(QB_NEW_EN)

---

[grammar] ~136-~136: There might be a mistake here.
Context: ...n without file modification **Actions**: - Read last 10 Unity console lines (log/in...

(QB_NEW_EN)

---

[grammar] ~141-~141: There might be a mistake here.
Context: ...: State C (unchanged) - **IMMEDIATELY** write clean XML fragment to `reports/NL-4_res...

(QB_NEW_EN)

---

[grammar] ~143-~143: There might be a mistake here.
Context: ...ry Helper Lifecycle (Returns to State C) **Goal**: Test insert → verify → delete cy...

(QB_NEW_EN)

---

[grammar] ~145-~145: There might be a mistake here.
Context: ...te cycle for temporary code **Actions**: - Find current position of `GetCurrentTarg...

(QB_NEW_EN)

---

[grammar] ~152-~152: There might be a mistake here.
Context: ...od Body Interior Edit (Additive State D) **Goal**: Edit method interior without aff...

(QB_NEW_EN)

---

[grammar] ~154-~154: There might be a mistake here.
Context: ...structure, on modified file **Actions**: - Use `find_in_file` to locate current `Ha...

(QB_NEW_EN)

---

[grammar] ~156-~156: There might be a mistake here.
Context: ...dy interior: change return statement to `return true; /* test modification */` - Validate with `mcp__unity__validate_scri...

(QB_NEW_EN)

---

[grammar] ~161-~161: There might be a mistake here.
Context: ... Method Interior Edit (Additive State E) **Goal**: Edit a different method to show ...

(QB_NEW_EN)

---

[grammar] ~165-~165: There might be a mistake here.
Context: ... content search - Edit interior line to add null check: `if (animator == null) retu...

(QB_NEW_EN)

---

[grammar] ~169-~169: There might be a mistake here.
Context: .... End-of-Class Helper (Additive State F) **Goal**: Add permanent helper method at c...

(QB_NEW_EN)

---

[grammar] ~171-~171: There might be a mistake here.
Context: ... helper method at class end **Actions**: - Use smart anchor matching to find curren...

(QB_NEW_EN)

---

[grammar] ~178-~178: There might be a mistake here.
Context: ...d Evolution Lifecycle (Additive State G) **Goal**: Insert → modify → finalize a fie...

(QB_NEW_EN)

---

[grammar] ~179-~179: There might be a mistake here.
Context: ...fy → finalize a field + companion method **Actions**: - Insert field: `private in...

(QB_NEW_EN)

---

[grammar] ~180-~180: There might be a mistake here.
Context: ... a field + companion method **Actions**: - Insert field: `private int Counter = 0;`...

(QB_NEW_EN)

---

[grammar] ~181-~181: There might be a mistake here.
Context: ...ion method **Actions**: - Insert field: `private int Counter = 0;` - Update it: find and replace with `privat...

(QB_NEW_EN)

---

[grammar] ~182-~182: There might be a mistake here.
Context: ... 0;` - Update it: find and replace with `private int Counter = 42; // initialized` - Add companion method: `private void Incr...

(QB_NEW_EN)

---

[grammar] ~183-~183: There might be a mistake here.
Context: ...// initialized` - Add companion method: `private void IncrementCounter() { Counter++; }` - **Expected final state**: State F + Counte...

(QB_NEW_EN)

---

[grammar] ~186-~186: There might be a mistake here.
Context: ...-F. Atomic Multi-Edit (Additive State H) **Goal**: Multiple coordinated edits in si...

(QB_NEW_EN)

---

[grammar] ~187-~187: There might be a mistake here.
Context: ...H) **Goal**: Multiple coordinated edits in single atomic operation **Actions**: - ...

(QB_NEW_EN)

---

[grammar] ~187-~187: There might be a mistake here.
Context: ...dinated edits in single atomic operation **Actions**: - Read current file state t...

(QB_NEW_EN)

---

[grammar] ~188-~188: There might be a mistake here.
Context: ... in single atomic operation **Actions**: - Read current file state to compute preci...

(QB_NEW_EN)

---

[grammar] ~190-~190: There might be a mistake here.
Context: ... precise ranges - Atomic edit combining:   1. Add comment in `HasTarget()`: `// valida...

(QB_NEW_EN)

---

[grammar] ~191-~191: There might be a mistake here.
Context: ...ing:   1. Add comment in `HasTarget()`: `// validated access`     2. Add comment in `ApplyBlend()`: `// safe ...

(QB_NEW_EN)

---

[grammar] ~192-~192: There might be a mistake here.
Context: ...`     2. Add comment in `ApplyBlend()`: `// safe animation`   3. Add final class comment: `// end of test...

(QB_NEW_EN)

---

[grammar] ~193-~193: There might be a mistake here.
Context: ...nimation`   3. Add final class comment: `// end of test modifications` - All edits computed from same file snapsh...

(QB_NEW_EN)

---

[grammar] ~194-~194: There might be a mistake here.
Context: ...est modifications` - All edits computed from same file snapshot, applied atomically ...

(QB_NEW_EN)

---

[grammar] ~195-~195: There might be a mistake here.
Context: ...**: State G + three coordinated comments - After applying the atomic edits, run `va...

(QB_NEW_EN)

---

[grammar] ~198-~198: There might be a mistake here.
Context: ...ath Normalization Test (No State Change) **Goal**: Verify URI forms work equivalent...

(QB_NEW_EN)

---

[grammar] ~200-~200: There might be a mistake here.
Context: ...uivalently on modified file **Actions**: - Make identical edit using `unity://path/...

(QB_NEW_EN)

---

[grammar] ~201-~201: There might be a mistake here.
Context: ...*Actions**: - Make identical edit using `unity://path/Assets/Scripts/LongUnityScriptClaudeTest.cs` - Then using `Assets/Scripts/LongUnityScri...

(QB_NEW_EN)

---

[grammar] ~204-~204: There might be a mistake here.
Context: ...ith updated SHA - Verify both URI forms target same file - **Expected final state**: S...

(QB_NEW_EN)

---

[grammar] ~205-~205: There might be a mistake here.
Context: ...H (no content change, just path testing) - Emit `reports/T-G_results.xml` showing e...

(QB_NEW_EN)

---

[grammar] ~208-~208: There might be a mistake here.
Context: ...ation on Modified File (No State Change) **Goal**: Ensure validation works correctl...

(QB_NEW_EN)

---

[grammar] ~210-~210: There might be a mistake here.
Context: ...ly on heavily modified file **Actions**: - Run `validate_script(level:"standard")` ...

(QB_NEW_EN)

---

[grammar] ~213-~213: There might be a mistake here.
Context: ...e**: State H (validation only, no edits) - Emit `reports/T-H_results.xml` confirmin...

(QB_NEW_EN)

---

[grammar] ~216-~216: There might be a mistake here.
Context: ...ailure Surface Testing (No State Change) **Goal**: Test error handling on real modi...

(QB_NEW_EN)

---

[grammar] ~218-~218: There might be a mistake here.
Context: ...dling on real modified file **Actions**: - Attempt overlapping edits (should fail c...

(QB_NEW_EN)

---

[grammar] ~225-~225: There might be a mistake here.
Context: ...ency on Modified File (Additive State I) **Goal**: Verify operations behave predict...

(QB_NEW_EN)

---

[grammar] ~227-~227: There might be a mistake here.
Context: ...e predictably when repeated **Actions**: - **Insert (structured)**: `mcp__unity__scri...

(QB_NEW_EN)

---

[grammar] ~231-~231: There might be a mistake here.
Context: ...o_op: true`. - **Remove (structured)**: `{"op":"regex_replace","pattern":"(?m)^\\s*// idempotency test marker\\r?\\n?","text":""}` - **Remove again** (same `regex_replace`) → ...

(QB_NEW_EN)

---

[grammar] ~235-~235: There might be a mistake here.
Context: ...e "no errors" if none - **IMMEDIATELY** write clean XML fragment to `reports/T-J_resu...

(QB_NEW_EN)

---

[grammar] ~268-~268: There might be a mistake here.
Context: ...erification Patterns  **After each test:** 1. Verify expected content exists: `find_in...

(QB_NEW_EN)

---

[grammar] ~275-~275: There might be a mistake here.
Context: ... paste raw tool JSON)  **Error Recovery:** - If test fails, log current state but con...

(QB_NEW_EN)

---

[grammar] ~276-~276: There might be a mistake here.
Context: ...)  **Error Recovery:** - If test fails, log current state but continue (don't resto...

(QB_NEW_EN)

---

[grammar] ~277-~277: There might be a mistake here.
Context: ...inue (don't restore) - Next test adapts to actual current state, not expected stat...

(QB_NEW_EN)

---

[grammar] ~277-~277: There might be a mistake here.
Context: ...xt test adapts to actual current state, not expected state - Demonstrates resilienc...

(QB_NEW_EN)

</details>

</details>
<details>
<summary>🪛 markdownlint-cli2 (0.17.2)</summary>

<details>
<summary>.claude/prompts/nl-unity-suite-full-additive.md</summary>

127-127: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</details>

</details>

</details>

<details>
<summary>🔇 Additional comments (1)</summary><blockquote>

<details>
<summary>.claude/prompts/nl-unity-suite-full-additive.md (1)</summary><blockquote>

`1-367`: **Solid additive NL/T design; aligns with CI and improves realism.**

The structure, guardrails, and per-test emission rules look coherent and CI-friendly. Nice work.

</blockquote></details>

</blockquote></details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (2)

.github/workflows/claude-nl-suite.yml (2)

174-183: Good fix: safe argv injection for -manualLicenseFile.

Using an array with "${manual_args[@]}" resolves earlier quoting/word-splitting concerns around MANUAL_ARG. LGTM.

Also applies to: 199-203

---

131-157: Redact activation logs, tighten entitlement check path, and add a timeout.

• docker run may emit secrets; pipe through a redactor.
• Look for entitlement XML specifically under Unity/licenses to avoid false positives.
• Guard against hangs with a timeout.

-            docker run --rm --network host \
+            # Redacted, timed activation
+            timeout 10m docker run --rm --network host \
               -e HOME=/root \
               -e UNITY_EMAIL -e UNITY_PASSWORD -e UNITY_SERIAL \
               -v "$RUNNER_TEMP/unity-config:/root/.config/unity3d" \
               -v "$RUNNER_TEMP/unity-local:/root/.local/share/unity3d" \
-              "$UNITY_IMAGE" bash -lc '
+              "$UNITY_IMAGE" bash -lc '
                 set -euxo pipefail
                 if [[ -n "${UNITY_SERIAL:-}" ]]; then
                   /opt/unity/Editor/Unity -batchmode -nographics -logFile - \
                     -username "$UNITY_EMAIL" -password "$UNITY_PASSWORD" -serial "$UNITY_SERIAL" -quit || true
                 else
                   /opt/unity/Editor/Unity -batchmode -nographics -logFile - \
                     -username "$UNITY_EMAIL" -password "$UNITY_PASSWORD" -quit || true
                 fi
                 ls -la /root/.config/unity3d/Unity/licenses || true
-              '
+              ' 2>&1 | sed -E 's/((email|serial|license|password|token)[^[:space:]]*)/[REDACTED]/Ig'
 
-            # Verify entitlement written to host mount; allow ULF-only runs to proceed
-            if ! find "$RUNNER_TEMP/unity-config" -type f -iname "*.xml" | grep -q .; then
+            # Verify entitlement written to host mount; allow ULF-only runs to proceed
+            lic_dir="$RUNNER_TEMP/unity-config/Unity/licenses"
+            if ! find "$lic_dir" -maxdepth 1 -type f -iname '*.xml' | grep -q .; then
               if [[ "${{ steps.ulf.outputs.ok }}" == "true" ]]; then
                 echo "EBL entitlement not found; proceeding with ULF-only (ok=true)."
               else
                 echo "No entitlement produced and no valid ULF; cannot continue." >&2
                 exit 1
               fi
             fi

🧹 Nitpick comments (2)

.github/workflows/claude-nl-suite.yml (2)

237-245: Trim log polling to fixed tail to avoid quadratic growth and speed the wait loop.

Reading full logs each iteration is O(n). Tail a bounded window.

-              logs="$(docker logs unity-mcp 2>&1 || true)"
+              logs="$(docker logs unity-mcp 2>&1 | tail -n 400 || true)"

Also applies to: 248-257, 260-264

---

75-78: YAML lint nits: indentation and a trailing space.

yamllint flagged minor indentation issues and a trailing space (Line 219). Clean up to keep CI lint-noisy steps quiet.

Examples:

• Fix mapping indent to expected depth under env/with/uses blocks.
• Strip trailing whitespace on Line 219.

Also applies to: 92-92, 122-125, 166-166, 196-196, 219-219, 855-857

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2598516 and 7a73d98.

📒 Files selected for processing (1)

• .github/workflows/claude-nl-suite.yml (14 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-08-29T21:31:02.469Z

Learnt from: dsarno
PR: dsarno/unity-mcp#56
File: .claude/prompts/nl-unity-suite-full.md:5-6
Timestamp: 2025-08-29T21:31:02.469Z
Learning: The Claude NL suite workflows (.github/workflows/claude-nl-suite.yml and .github/workflows/claude-nl-suite-mini.yml) allow "Bash" tools generally, including printf and echo operations, so prompts that reference Bash(printf:*) and Bash(echo:*) are consistent with the actual workflow permissions.

Applied to files:

• .github/workflows/claude-nl-suite.yml

🪛 YAMLlint (1.37.1)

.github/workflows/claude-nl-suite.yml

[warning] 75-75: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 75-75: too many spaces after colon

(colons)

---

[warning] 76-76: too many spaces after colon

(colons)

---

[warning] 78-78: too many spaces after colon

(colons)

---

[warning] 92-92: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 122-122: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 122-122: too many spaces after colon

(colons)

---

[warning] 123-123: too many spaces after colon

(colons)

---

[warning] 125-125: too many spaces after colon

(colons)

---

[warning] 166-166: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 196-196: wrong indentation: expected 14 but found 12

(indentation)

---

[error] 219-219: trailing spaces

(trailing-spaces)

---

[warning] 855-855: wrong indentation: expected 14 but found 12

(indentation)

---

[warning] 855-855: too many spaces after colon

(colons)

---

[warning] 857-857: too many spaces after colon

(colons)

🔇 Additional comments (1)

.github/workflows/claude-nl-suite.yml (1)

341-342: Confirm that disallowing the Bash tool is intentional for this suite.

Prompts/tests previously referenced Bash tools; disallowing Bash here will block them inside the agent. If intentional, ignore. Otherwise, drop Bash from disallowed_tools.

Would you like me to scan the prompt for Bash tool usage and adjust accordingly?