Skip to content

Fix main lint: move native-chat adapter error classes into their own file - #33046

Merged
brennanb2025 merged 1 commit into
mainfrom
brennanb2025/main-fix-adapter-max-lines
Oct 24, 2026
Merged

brennanb2025 merged 1 commit into
mainfrom
brennanb2025/main-fix-adapter-max-lines

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 24, 2026 •

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

Problem

Since #28051 merged, main's static lint check fails for every PR: src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts counts 303 lines against the 300-line max-lines limit. #28051 passed on its own branch; together with main's changes since, the file went over.

Change

The adapter's error classes (AgentSessionAcquisitionRefusal, AgentSessionPromptUnavailableError, AgentSessionPromptAnswerRejectedError, AgentSessionPreSpawnError, isAgentSessionPreSpawnError and the AgentSessionPreSpawnReason type) move unchanged into structured-agent-session-adapter-errors.ts. The adapter file re-exports them, so no importer changes and nothing behaves differently. No lint rule is disabled.

No user-facing change. CI is the check: static analysis, typecheck and the unit tests that import these classes.

Author: @BrennanKB5

Plan

Move the existing error declarations into one dependency-leaf module and re-export the same values and type from the adapter contract. Verify the declarations are byte-for-byte unchanged, every import still resolves, the split creates no runtime cycle, and static analysis and tests pass. No lifecycle, refusal, persistence, wire, or user-facing behavior changes.

Light-path classification (explicitly requested for this pure move):

  • Size: two production files, 77 added and 66 deleted lines (143 total).
  • Risk: no ownership, lifecycle, recovery, saved-data, ordering, process, auth, or cross-version mechanism changes; only declaration location and re-exports change.
  • History: refactor(native-chat): move the provider-exit proof out of structured-agent-session-adapter.ts so main's lint passes #24323 previously extracted exit-proof errors from this file for the same line limit. This change continues that separation without changing either mechanism; the requested light path is an explicit exception to the usual no-repeat criterion.
  • Local effect: unchanged error declarations and unchanged public import paths. QA is CI and tests only (qa=tests); there is no changed user flow requiring a real-app run.

…ct file

Main's static lint failed max-lines (303 of 300) on structured-agent-session-adapter.ts after #28051 merged. The adapter's error classes move to structured-agent-session-adapter-errors.ts and are re-exported, so every importer is unchanged.
@coderabbitai

coderabbitai Bot commented Oct 24, 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: eeae5a8c-f32d-4506-abd1-21f6cc7e5a7e
Commits

Reviewing files that changed from the base of the PR and between a75cdc8 and 2fb14c4.

Files selected for processing: 2
  • src/main/native-chat/agent-session-wire/structured-agent-session-adapter-errors.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-adapter.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

The changes add adapter error classes, a pre-spawn reason type, and a type guard in a dedicated module. The adapter module re-exports these definitions and removes their previous local declarations and related imports.

Priority: Low

Merge Risk: Minimal · up to 2fb14

The reported lint limit does not block this change, and no actionable merge risk remains after normal checks.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
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. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check Warning The description is mostly a complete account of a pure code move, but it departs from the repository template in several places. It has a Problem, Change, and Plan section, and it names the author and… Add the missing template sections. (1) ELI5: a short plain-language explanation of the move. (2) What Changed and Why: keep the existing Problem and Change text, and state why this split was chosen over alternatives. (3) Linked Issue: add a…
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: moving native-chat adapter error classes into a separate file to fix the lint issue.
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 is mostly a complete account of a pure code move, but it departs from the repository template in several places. It has a Problem, Change, and Plan section, and it names the author and the reason for the move (the 300-line max-lines limit that main's lint check fails on). It states that no importer changes and no user-facing behavior changes. However, the required template sections ELI5, Linked Issue, Visual Proof, Testing, AI Disclosure, Review, Agent skill upstream boundary, Notes, and Checklist are absent. The Linked Issue section is required for outside contributors and has no 'Fixes #' reference. The Testing checkboxes are not ticked, and no local manual test or automated test update is described beyond CI. The Visual Proof section is also absent, though the author's statement of no user-facing change would support 'N/A' with a reason.

Resolution

Add the missing template sections. (1) ELI5: a short plain-language explanation of the move. (2) What Changed and Why: keep the existing Problem and Change text, and state why this split was chosen over alternatives. (3) Linked Issue: add a 'Fixes #<issue>' reference to an existing issue, because the template requires one for outside contributors. (4) Visual Proof: write 'N/A' and the reason, since the author says there is no user-facing change. (5) Testing: state which platforms were tested and tick the checkboxes, or state that only CI was run. (6) AI Disclosure: complete it if an AI model was used, or leave it blank if the author is a StablyAI team member. (7) Review and Agent skill upstream boundary: confirm the boundary checkbox applies. (8) Notes: confirm security, cross-platform, SSH, mobile, backwards compatibility, and performance were considered. (9) Checklist: tick the items that apply, and confirm that pnpm lint, typecheck, test, and build pass.

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.

@brennanb2025

brennanb2025 commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor Author

Main's static check was failing because the native-chat adapter contract exceeded the 300-line limit after #28051. This PR restores the check by moving unchanged error declarations to a separate module and retaining their existing import paths. Chat behavior is unchanged.

One adversarial light-path review completed on 2fb14c4e375e00a344b44de4037ce5bb9412919b: no findings, fixes, or deferrals. The moved class, type, and type-guard bodies are byte-for-byte identical to origin/main; reconstructing the original file from the new files confirms all other changes are imports and re-exports. All six moved names were searched across src, mobile, and tests; every importer resolves. Values use runtime re-exports, the reason alias uses a type-only re-export, and the new module's two imports are type-only, so there is no new runtime cycle. Counted lines drop from 303 to 257 (43 lines of margin); the new file has 54 counted lines.

Verification: 55 tests passed in four named files through tc-gate, covering refusal details, processless acquisition, prompt cancellation, and Codex prompt answers. Exact-head CI is green: 23 successful checks, 17 skipped, including static analysis/typecheck, all five unit-test shards, cross-version compatibility, and packaging. QA is tests: this is a pure declaration move with no changed lifecycle, UI, or agent flow to exercise in a running app. No real-app run or screenshots were needed.

Reference comparison: no mechanism changes—value sources, refusal conditions, ownership, and all error consumers are unchanged. The move introduces no behavioral difference from the common pattern; refs=done records this no-mechanism-change result. The PR body documents the requested light path and the earlier same-file extraction in #24323.

For Brennan's review: unchanged declarations and preserved public exports.

Readiness check: brennan-ready --force --pr 33046 currently returns NOT READY: qa=tests claimed, but the PR changes no test files. The completed review and existing tests found no code issue; the readiness tool requires modified test files even for this pure move. This tooling mismatch is reported for correction; the PR is not yet reported ready.

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