Repository navigation
fix(linear): enforce list-issues pagination backstops - #16128
brennanb2025 wants to merge 5 commits into
Conversation
Stop non-progressing issue pages before the page ceiling, abort in-flight reads at the shared deadline, and keep partial empty results visibly truncated. Tighten the SSH result guard without requiring legacy row-count metadata, and document mixed-version fallbacks.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
🚧 Files skipped from review as they are similar to previous changes (13)
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughLinear issue listing now creates request-scoped clients with abort signals and shared read deadlines. Pagination stops on repeated cursors, empty pages, or expired requests, and deduplicates issues. Truncation metadata remains available after bounded reads. Remote result guards validate issue-list metadata and workspace errors. CLI formatting reports truncation for empty results. Documentation describes uncapped reads, pagination, truncation metadata, and host-specific Merge Risk: ⚪ Minimal · up to The change adds bounded pagination and timeout safeguards while preserving existing output and mixed-version compatibility; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the audit finding, implementation, compatibility, testing, and visual-proof status. However, it omits several required template sections, including ELI5, Why, Linked Issue with an issue number, AI Disclosure, Review, Agent skill upstream boundary, Notes, and the Checklist. It also does not report the known CI failures described in the objectives. Resolution Add the missing required template sections. Include the linked issue number after "Fixes #", complete the checklist, document AI disclosure and upstream-boundary status, and report the three known CI failures with their resolution or current status. Full details: Docstring CoverageExplanation Docstring coverage is 6.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 11 files. (2 skipped: 2 unsupported.)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2e2f8c6d-f6c7-4f0a-aaad-b98213a47f23
📒 Files selected for processing (14)
skill-guides/linear-tickets.mdskill-guides/orca-linear.mdsrc/cli/bundled-skill-guides.tssrc/cli/linear-format.test.tssrc/cli/linear-format.tssrc/cli/specs/linear-mcp.tssrc/main/linear/client.test.tssrc/main/linear/client.tssrc/main/linear/mcp-issue-list-pagination.test.tssrc/main/linear/mcp-issue-list.test.tssrc/main/linear/mcp-issue-list.tssrc/main/ssh/ssh-remote-cli-format.test.tssrc/main/ssh/ssh-remote-linear-result-guards.tssrc/shared/linear/mcp-issue-list.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.
|
Exact-head review update (53eea01): malformed Linear MCP issue rows are fully validated (including required fields and optional nested metadata), with safe JSON fallback for SSH formatting; the fix is covered by the focused suite (22 files, 197 tests). Local typecheck, changed-code quality, React Doctor, bundled skill-guide check, oxfmt, and diff checks pass; re-review finds no additional in-scope issues, and the sole CodeRabbit thread is resolved and current (1 resolved, 0 unresolved, 0 outdated). Final GitHub CI reports 50 success, 3 failure (Node 24/26 shard 10 runtime export-parity mismatch plus dependent verify), and 4 skipped; merge is blocked by failed checks, and this CLI/SSH-only change has no Electron/mobile validation surface. |
|
Exact-head QA evidence (53eea01): |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Greptile SummaryThe PR adds pagination progress backstops, issue deduplication, request cancellation, truthful empty-result truncation formatting, and stricter SSH result validation for Linear
Confidence Score: 4/5The deadline must cover concurrency-slot acquisition before merging, otherwise list-issues can still exceed its advertised read budget. The in-flight request is cancellable, but the operation can wait without a bound before reaching that cancellation logic because shared limiter acquisition has no timeout or abort path. Files Needing Attention: src/main/linear/mcp-issue-list.ts
|
| Filename | Overview |
|---|---|
| src/main/linear/mcp-issue-list.ts | Adds pagination and deadline safeguards, but the deadline does not cover waiting for the shared request semaphore. |
| src/main/linear/client.ts | Adds a request-scoped Linear client that forwards the supplied AbortSignal. |
| src/main/ssh/ssh-remote-linear-result-guards.ts | Strengthens issue-row and metadata validation while preserving optional mixed-version fields. |
| src/cli/linear-format.ts | Correctly retains the truncation marker when a partial list contains zero issues. |
| src/shared/linear/mcp-issue-list.ts | Clarifies that a null limit removes only the caller cap and does not imply a complete read. |
Sequence Diagram
sequenceDiagram
participant CLI as list-issues
participant Pool as Shared Linear limiter
participant Page as Page read
participant SDK as Linear SDK
CLI->>Pool: acquire()
Note over CLI,Pool: Queue wait is currently unbounded
Pool-->>Page: slot granted
Page->>Page: check remaining 20s budget
alt budget remains
Page->>SDK: rawRequest(signal)
SDK-->>Page: page or abort
else deadline already passed
Page-->>CLI: truncated partial result
end
Comments Outside Diff (1)
-
src/main/linear/mcp-issue-list.ts, line 2019-2023 (link)Deadline excludes semaphore wait
If all four shared Linear request slots are occupied by slow or stalled operations,
withLinearReadwaits on the unboundedacquire()before this callback checks the remaining deadline. Thelist-issuesrequest therefore remains pending beyond its 20-second budget—and indefinitely when no slot is released—instead of returning a truncated partial result.
Reviews (1): Last reviewed commit: "Merge branch 'main' into brennanb2025/au..." | Re-trigger Greptile
Audit finding
The merged pagination safeguards were incomplete: a provider that repeated a cursor could trigger all 200 allowed requests, a stalled provider request could outlive the advertised 20-second budget, and an empty truncated human result hid the truncation marker. These are defensive correctness defects; no production incident from repeated or stalled Linear responses is known.
What changed
truncated: showing 0for empty partial human outputmeta.returned--limitremoves the caller cap but does not remove safety backstops, and that older hosts may omitpriorityLabelCompatibility
No request or result wire shape changes. Cancellation stays on the execution host,
truncatedandpriorityLabelremain optional for mixed-version peers, and SSH formatting still falls back tometa.hasMoreand rendered row counts.Non-vacuity
Temporarily neutering the production hunks produced five expected failures: the repeat and empty-page cases made 200 requests, the deadline promise remained unsettled, empty human output omitted
truncated: showing 0, and malformed SSH metadata formatted as valid. Omitting the signal from the request-scoped Linear client also failed its constructor contract test. Restoring the fixes returned every affected suite to green.Testing
pnpm vitest run --config config/vitest.config.ts src/main/linear src/cli/linear-format.test.ts src/cli/index.test.ts src/main/ssh/ssh-remote-cli-format.test.ts src/main/ssh/ssh-remote-linear-truncation-output.test.ts src/main/ssh/ssh-remote-linear-cli.test.ts(22 files, 195 tests)pnpm typechecknode config/scripts/check-changed-code-quality.mjsnode config/scripts/check-react-doctor-changed.mjsnode config/scripts/generate-bundled-skill-guides.mjs --checkgit diff --checkVisual proof: N/A — CLI JSON/text behavior only.
Merge note
PR #16119 concurrently touches the Linear guides and nearby formatter/SSH guard code. Whichever PR lands second should rebase and retain the combined contract wording.
Follow-up to #15824.