From 08a797cf764c3ae87aed61f7a3c27f702579a135 Mon Sep 17 00:00:00 2001
From: wenshao <shaojin.wensj@alibaba-inc.com>
Date: Tue, 7 Apr 2026 03:13:23 +0800
Subject: [PATCH] fix(review): address 4 Copilot comments

- Add model attribution to no-findings LGTM path
- Handle empty string from getModel() with .trim() || 'unknown'
- Add tests for {{model}} with args and empty model ID
- Fix doc contradiction: PR autofix pushes automatically from worktree

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 docs/users/features/code-review.md            |  2 +-
 .../src/services/BundledSkillLoader.test.ts   | 42 +++++++++++++++++++
 .../cli/src/services/BundledSkillLoader.ts    |  2 +-
 .../core/src/skills/bundled/review/SKILL.md   |  4 +-
 4 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/docs/users/features/code-review.md b/docs/users/features/code-review.md
index 00b7a2c29..08166018e 100644
--- a/docs/users/features/code-review.md
+++ b/docs/users/features/code-review.md
@@ -89,7 +89,7 @@ Found 3 issues with auto-fixable suggestions. Apply auto-fixes? (y/n)
 
 - Fixes are applied using the `edit` tool (targeted replacements, not full-file rewrites)
 - Per-file linter checks run after fixes to verify they don't introduce new issues
-- For PR reviews, fixes are committed on the PR branch — you need to `git push` to update the PR
+- For PR reviews, fixes are committed and pushed from the worktree automatically
 - Nice to have and low-confidence findings are never auto-fixed
 - For PR reviews, autofix operates in an isolated worktree — your working tree stays clean. Fixes are committed and pushed directly from the worktree.
 
diff --git a/packages/cli/src/services/BundledSkillLoader.test.ts b/packages/cli/src/services/BundledSkillLoader.test.ts
index a534eb3e2..364fdf1b0 100644
--- a/packages/cli/src/services/BundledSkillLoader.test.ts
+++ b/packages/cli/src/services/BundledSkillLoader.test.ts
@@ -170,6 +170,48 @@ describe('BundledSkillLoader', () => {
     });
   });
 
+  it('should resolve {{model}} when args are provided', async () => {
+    const skill = makeSkill({
+      body: 'Review by {{model}}',
+    });
+    mockSkillManager.listSkills.mockResolvedValue([skill]);
+    (mockConfig.getModel as ReturnType<typeof vi.fn>).mockReturnValue(
+      'qwen3-coder',
+    );
+
+    const loader = new BundledSkillLoader(mockConfig);
+    const commands = await loader.loadCommands(signal);
+    const result = await commands[0].action!(
+      { invocation: { raw: '/review 123', args: '123' } } as never,
+      '123',
+    );
+
+    expect(result).toEqual({
+      type: 'submit_prompt',
+      content: [{ text: 'Review by qwen3-coder\n\n/review 123' }],
+    });
+  });
+
+  it('should use "unknown" for {{model}} when getModel returns empty string', async () => {
+    const skill = makeSkill({
+      body: 'Review by {{model}}',
+    });
+    mockSkillManager.listSkills.mockResolvedValue([skill]);
+    (mockConfig.getModel as ReturnType<typeof vi.fn>).mockReturnValue('');
+
+    const loader = new BundledSkillLoader(mockConfig);
+    const commands = await loader.loadCommands(signal);
+    const result = await commands[0].action!(
+      { invocation: { raw: '/review', args: '' } } as never,
+      '',
+    );
+
+    expect(result).toEqual({
+      type: 'submit_prompt',
+      content: [{ text: 'Review by unknown' }],
+    });
+  });
+
   it('should not modify skill body without {{model}} template', async () => {
     const skill = makeSkill({ body: 'No template here' });
     mockSkillManager.listSkills.mockResolvedValue([skill]);
diff --git a/packages/cli/src/services/BundledSkillLoader.ts b/packages/cli/src/services/BundledSkillLoader.ts
index 61ebbab2a..29fc3c680 100644
--- a/packages/cli/src/services/BundledSkillLoader.ts
+++ b/packages/cli/src/services/BundledSkillLoader.ts
@@ -62,7 +62,7 @@ export class BundledSkillLoader implements ICommandLoader {
           // Resolve template variables in skill body (e.g., {{model}})
           let body = skill.body;
           if (body.includes('{{model}}')) {
-            const modelId = this.config?.getModel() ?? 'unknown';
+            const modelId = this.config?.getModel()?.trim() || 'unknown';
             body = body.replaceAll('{{model}}', modelId);
           }
 
diff --git a/packages/core/src/skills/bundled/review/SKILL.md b/packages/core/src/skills/bundled/review/SKILL.md
index c0267feba..f316810c4 100644
--- a/packages/core/src/skills/bundled/review/SKILL.md
+++ b/packages/core/src/skills/bundled/review/SKILL.md
@@ -434,7 +434,9 @@ gh pr review {pr_number} --comment --body-file /tmp/qwen-review-{target}-summary
 If there are **no confirmed findings**:
 
 ```bash
-gh pr review {pr_number} --approve --body "No issues found. LGTM! ✅"
+gh pr review {pr_number} --approve --body "No issues found. LGTM! ✅
+
+_Reviewed by {{model}} via Qwen Code /review_"
 ```
 
 ## Step 4.5: Save review report and cache