Skip to content

test(native-chat): use an agent without chat modes where the tests say so - #29887

Closed
Jinwoo-H wants to merge 1 commit into
mainfrom
fix/chat-permission-tests-no-mode-agent
Closed

Jinwoo-H wants to merge 1 commit into
mainfrom
fix/chat-permission-tests-no-mode-agent

Conversation

@Jinwoo-H

@Jinwoo-H Jinwoo-H commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​3 $\color{#cf222e}{\Huge{\mathbf{−}}}$​3 0
Prod 0 0 0 0

What

Two tests on main fail: agent-chat-permission-host-support.test.ts and structured-chat-host-permission.test.ts.

Why

#29442 used Grok as its example of an agent with no chat permission modes. #29335, merged around the same time, gave Grok (with OMP and OpenCode) its own modes, so those tests now get a mode back and fail. Each PR was green on its own.

Change

The three test cases that mean "an agent without chat modes" now use Aider, which has none. Test-only; no product code changes.

Testing

Both files pass locally (20 tests).

…y so

Grok gained chat permission modes (#29335) after these tests (#29442)
picked it as the agent without any, turning main red. Aider has none.
@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e7d297cd-fbc4-4c8b-bb46-9000e89e5ad8
📥 Commits

Reviewing files that changed from the base of the PR and between d963f2a and 19b998f.

📒 Files selected for processing: 2
  • src/renderer/src/runtime/structured-chat-host-permission.test.ts
  • src/shared/agent-chat-permission-host-support.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Three chat-permission test cases now use aider instead of grok for agents without chat permission modes. The expected verdicts and RPC assertions remain unchanged.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 19b99

The test-agent substitutions preserve the intended behavior, with no actionable merge risk identified.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description explains the failure, cause, scope, and local testing, but it omits several required template sections, including ELI5, Linked Issue, Visual Proof or N/A, AI Disclosure, Review, Agent … Complete the required template sections. Add the linked issue, mark Visual Proof as N/A with a reason, complete the applicable testing and checklist items, address AI Disclosure and Agent skill upstream boundary, and document cross-platform…
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies the test-only change: replacing an agent that now has chat modes with an agent without chat modes.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files.

Full details: Description check

Explanation

The description explains the failure, cause, scope, and local testing, but it omits several required template sections, including ELI5, Linked Issue, Visual Proof or N/A, AI Disclosure, Review, Agent skill upstream boundary, Notes, and the Checklist.

Resolution

Complete the required template sections. Add the linked issue, mark Visual Proof as N/A with a reason, complete the applicable testing and checklist items, address AI Disclosure and Agent skill upstream boundary, and document cross-platform, SSH, compatibility, security, and performance considerations or mark them not applicable.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@Jinwoo-H

Copy link
Copy Markdown
Contributor Author

Closing: #29891 already landed the same test fix on main.

@Jinwoo-H Jinwoo-H closed this Oct 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant