Repository navigation
refactor(native-chat): move the provider-exit proof out of structured-agent-session-adapter.ts so main's lint passes - #24323
Conversation
…-agent-session-adapter.ts so main's lint passes structured-agent-session-adapter.ts reached 301 lines once two changes on main met, one over the 300-line limit, so oxlint fails on main. The file is the adapter contract: its types, its interface and the typed failure verdicts. The only logic in it read what a cleanup or a stop proved about the provider child's exit: rethrowing a failed acquisition with the verdict its cleanup reached, and reading whether a stop left the provider root gone. That logic moves unchanged to structured-agent-session-provider-exit-proof.ts, which imports the verdicts from the adapter; nothing imports back. Its six importers now import from the new module. No behavior change.
|
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 (8)
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 pull request moves acquisition cleanup classification and provider-root stop handling from the adapter to the provider-exit-proof module. It updates call sites and test imports to use the new module. The adapter retains its types and acquisition error classes. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The helper extraction preserves acquisition cleanup and provider-stop behavior. No actionable merge-blocking risk remains beyond normal checks. Architecture SummaryArchitecture risk: 🟡 Medium · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
Reliability and maintainability
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 — a pure mechanical extraction that unblocks main's static analysis check.
- Provider-exit proofs extracted —
rethrowAfterAgentSessionAcquisitionCleanup, its privateprovenExitAcquisitionFailure, andstopAgentSessionProviderRootmove verbatim fromstructured-agent-session-adapter.tsinto the newstructured-agent-session-provider-exit-proof.ts. - Callers re-pointed — the six importers (
claude-structured-acquisition-launch.ts,structured-agent-session-acquisition.ts,structured-agent-session-attach-failure.ts,structured-agent-session-event-recovery.ts,structured-agent-session-eviction.ts, and the adapter test) now import from the new module. - Adapter trimmed — the now-unused runtime import of
isAgentSessionWireRefusalCodeis dropped;AgentSessionRefusalReasonstays type-only and is still referenced atstructured-agent-session-adapter.ts:58,129.
I confirmed the moved bodies are byte-identical to the removed originals, the six callers are the complete set of importers, the dependency direction is one-way (provider-exit-proof → adapter) so no cycle is introduced, npx oxlint over all eight changed files reports zero findings, and tsc --noEmit -p config/tsconfig.node.json is clean. No behavioral surface to review.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
ELI5
Main's lint check fails because one file grew past the 300-line limit when two recent PRs both added to it. This moves three helper functions out of that file, unchanged, so main's lint passes again. Nothing about how Orca behaves changes.
What Changed
The problem. The
static analysischeck fails on main and on every PR based on it:src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts:453 File has too many lines (301), against a maximum of 300. #24235 and #23935 each added to this file and passed on their own; together they crossed the limit. Because the lint check fails, theverifycheck fails too, so any PR that touches this area is blocked.User-facing change. None. This is a pure move.
Mechanism.
structured-agent-session-adapter.tsis the adapter contract: its types, its interface, and its typed failure errors. The only logic in it was three functions that read what a cleanup or a stop proved about whether the provider's process has exited:rethrowAfterAgentSessionAcquisitionCleanup;provenExitAcquisitionFailure;stopAgentSessionProviderRoot.They move verbatim into a new
structured-agent-session-provider-exit-proof.ts, and their 6 callers now import from it. The new module imports from the adapter, and the adapter doesn't import it back, so there's no import cycle.Why
Splitting by responsibility keeps the contract file as just the contract. The alternatives were worse:
Linked Issue
None: this fixes main's own red static-analysis check.
Visual Proof
N/A: this is a code move with no behavior or UI change.
Testing
No new tests, because nothing behaves differently. Ran:
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
Platform-, SSH- and mobile-neutral: no runtime change.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)