Repository navigation
fix(native-chat): move the chat-tab surface out of the session host so main passes lint - #24496
Conversation
…o main passes lint The host file reached 302 lines after #24203 and #24311, over the 300-line limit, so the static-analysis job failed on every commit to main. The five chat-tab members now come from createStructuredAgentSessionTabSurface in the existing structured-agent-session-host-tabs module; behaviour is unchanged.
 |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe host now delegates tab listing, persisted-index and tab-ID lookups, tab display, and visibility updates to a new tab surface. The surface uses the host’s store. After a successful hide, it calls the supplied Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to This is an internal refactor that moves tab handling out of the session host to satisfy the line-limit lint. No user-visible behavior change is expected, and no actionable merge-blocking risk remains. Security Architecture ReviewSecurity architecture risk: ⚪ Minimal · up to The change preserves existing tab operations, persistence behavior, and cleanup conditions. No new caller access, privileges, or materially weakened controls were identified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This is a mechanical extraction that gets main under the max-lines cap again, with no behavior change.
- Chat-tab surface moved into the tabs module — the host's five tab members are now built by
createStructuredAgentSessionTabSurfaceinstructured-agent-session-host-tabs.ts; the host aliases the returned functions under the same public names/signatures. - Late-bound reads preserved — the factory takes
host(this) and readshost.deps/host.restartResumeinside its closures, so the wrapped logger and the constructor-builtrestartResumeare still resolved per call.sessionsandforgetStatusare captured once, butthis.sessionsisreadonlyand never reassigned, so the close check is equivalent. max-linesreduction —structured-agent-session-host.tsis back under 300 counted lines; the ratchet baseline (config/max-lines-baseline.txt) tracks suppression entries only, so it needs no update.
Verified locally: oxlint on both files is clean; typecheck:node shows only the 4 pre-existing #24494 errors, none in the changed files; the host/close/forget-status tests pass (50/50).
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
1 participant
ELI5
Main has failed its lint check on every commit since #24203 and #24311 landed. One file grew two lines past the 300-line limit. This PR moves a small, self-contained part of that file into the module that already holds its logic, so the check passes again. Nothing the user sees changes.
What Changed
structured-agent-session-host.tswas 302 counted lines (limit 300). The static-analysis job failed on main and on every PR based on it:File has too many lines (302).createStructuredAgentSessionTabSurfaceinstructured-agent-session-host-tabs.ts, the module that already held the tab listing and visibility logic. The host assigns them under the same public names and signatures. The new function readshost.deps,host.restartResume, and the status forget at call time, just as the old inline code did, so the host's wrapped logger and its late-built restart-resume helper are still the ones used.Why
Of the options, I chose the one that groups related code. The tab members form one unit, and their logic already lived in the tabs module, so moving them there sets up no new dependency. Squeezing lines (merging statements, dropping comments) would only put off the next overflow, and raising the limit or adding a lint disable is not allowed.
This pairs with #24494, which fixes the typecheck half of the same breakage (the renamed remote-runtime capability list). This PR does not touch those files.
Linked Issue
N/A (maintainer fix for a red main)
Visual Proof
N/A. This is an internal refactor with no visual or interaction change.
Testing
pnpm test src/main/native-chat/agent-session-wire: 160 files, 1369 tests passoxlinton the touched files and on the files changed by fix(native-chat): a paired server admits structured chat by client capability, not its own chat setting #24203, fix(native-chat): a Stop still reads as yours after Orca restarts, because the turn's end reads the Stop's event #24311, and feat(runtime): SSH access links for paired servers in a downgrade-safe sidecar (#16741 T5-1+T5-2) #24420: clean. A full-repooxlintrun is also clean. As a control, the original main file still fails withmax-lines (302).pnpm run check:code-quality:changed: 0 new findingspnpm tc:node: the only errors are main's 4 known ones, all in the files fix(build): import the Electron remote capability list from its own module #24494 fixes. This PR adds no new errors.I manually tested these changes locally
Automated tests added/updated, or explained why not below (pure move; the existing host tests cover the tab surface)
AI Disclosure
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
No platform, SSH, mobile, or wire impact: this is a main-process refactor inside one host class, and its public members keep the same names and signatures.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred). Typecheck stays red until fix(build): import the Electron remote capability list from its own module #24494 lands.