Skip to content

fix(runtime): bring terminal-send-method under the line limit - #34257

Closed
AmoabaKelvin wants to merge 1 commit into
mainfrom
fix/terminal-send-method-line-cap
Closed

AmoabaKelvin wants to merge 1 commit into
mainfrom
fix/terminal-send-method-line-cap

Conversation

@AmoabaKelvin

@AmoabaKelvin AmoabaKelvin commented Oct 27, 2026 •

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

#33957 and #33968 each added lines to terminal-send-method.ts and passed on their own. Merged together, they took it to 301 lines against the 300 cap, so lint is red on main.

The handler's first step, the size limits and the legacy AI Vault resume guard for the typed text and a resolved launch draft, moves into terminal-send-text-admission.ts unchanged and in the same order. No behaviour change.

@coderabbitai

coderabbitai Bot commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b1b84597-cc6f-4a58-85cf-63a635fccaa7
Commits

Reviewing files that changed from the base of the PR and between 887b781 and 63ce48d.

Files selected for processing: 2
  • src/main/runtime/rpc/methods/terminal/terminal-send-method.ts
  • src/main/runtime/rpc/methods/terminal/terminal-send-text-admission.ts

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

Walkthrough

The terminal send method now calls assertTerminalSendTextsAdmitted. The helper checks the primary text and optional resolved launch-draft text against the terminal text limit. For each nonempty text, it checks legacy AI Vault resume-command permission using the runtime’s structured-session-host callback.

Priority: Low

Merge Risk: Minimal · up to 63ce4

Terminal text admission behavior is unchanged by the extraction. No issue identified here prevents merging after normal checks.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Description check Warning The description explains the line-limit problem and the implementation change, but it omits most required template sections, including ELI5, explicit What Changed and Why sections, a linked issue in t… Complete the required template sections. Add ELI5, What Changed, Why, and a valid linked issue reference. Add Visual Proof with N/A and a reason if there is no visual or interaction change. Document testing and platform coverage, complete t…
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within survi… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly identifies the main change: reducing terminal-send-method.ts below the line limit. It is concise and specific.
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: Description check

Explanation

The description explains the line-limit problem and the implementation change, but it omits most required template sections, including ELI5, explicit What Changed and Why sections, a linked issue in the required format, visual proof or N/A, testing details and checkboxes, review, boundary status, notes, and the checklist.

Resolution

Complete the required template sections. Add ELI5, What Changed, Why, and a valid linked issue reference. Add Visual Proof with N/A and a reason if there is no visual or interaction change. Document testing and platform coverage, complete the testing checkboxes, state AI disclosure when applicable, complete Review and Agent skill upstream boundary, address the Notes items, and complete the Checklist.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • 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.

@AmoabaKelvin

Copy link
Copy Markdown
Collaborator Author

Superseded by #34256.

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