Repository navigation
Conversation
http.Server.WriteTimeout is the ordinary slow-write guard and applies to the whole response. A stream is a response the transport intends to hold open indefinitely, so the deadline passes while the stream sits idle and healthy: nothing notices, because nothing is being written, and the first real notification fails instead and takes the stream with it. Only the transport knows a response became a stream, so only the transport can move the deadline. Every write to a stream that runs a keep-alive now pushes it out by twice the interval. Extended rather than cleared, so WriteTimeout keeps meaning what it was set for and a peer that stopped reading still fails within a bounded time; twice rather than once, so a keep-alive landing a little late does not race the deadline it exists to defer. Scoped to streams with a keep-alive, which today means subscriptions/listen, because the interval is what the extension is measured in and a stream with no periodic write has nothing to measure. Fixes modelcontextprotocol#1262.
jmrplens
added a commit
to jmrplens/gitlab-mcp-server
that referenced
this pull request
Sep 25, 2026
The top of the stack: what the whole-stack review still held open after two fix rounds, one CI defect the stack itself ran into, the register brought up to date, and every generated artifact the layers below moved, regenerated once here. **The review's last two findings.** - The test that holds the individual tool Descriptions assembled at run time (the switches in deploykeys and deploytokens, the map in pages) checked only the tool names they spell. The rule `cmd/audit_action_ids` applies to a constant Description also refuses a dotted ID that resolves nowhere or only as an alias, so a misspelt ID in one of those switches passed every gate; the review reproduced it by changing `access.deploy_key_list_project` to `list_projekt`. The test now offers a dotted token as an ID on the same condition as the rule, one of its halves being one a canonical ID uses, leaves a `.git` tail alone, and reports an alias with the canonical ID it stands for. The planted misspelling fails it on both catalog classes. The limits paragraph in the cmd utilities page said "those are held by a test" of two holes when the test holds one, and says which now. - The comment on the temporary-file refusal of a download said the file it names is `output_path` unless the path was relative or its leaf a link. `canonicalDownloadOutputPath` resolves every existing component through symlinks, so a linked directory above the leaf renames it too, the macOS temp root among them. The comment and a test row say so. **Lint from the tree, not from a restored analysis.** The golangci-lint job restored `~/.cache/golangci-lint` with a prefix key, and a pull request reads its own ref's cache first. After a rebase that cache holds the analysis of the branch's previous tree, and on 2026-09-24 it failed a cascaded layer of this stack twice on an unused `//nolint:contextcheck` in a file the stack never touched, until that ref's caches were deleted. A reused result can hide a finding as easily as it invents one. The analysis cache is no longer restored, so a lint result depends on the tree alone; setup-go's build and module caches stay, since their keys are content hashes. On a 4-CPU host a cold analysis of main took 101 s against 29 s warm on the identical tree; in CI the restored cache never matched the tree anyway, and over twenty runs on 2026-09-24 the make step took 110 to 300 s while the cross-platform matrix that sets the wall clock took 800 to 1150 s. The static analysis page says the cache is not kept, and why. **The register.** `modelcontextprotocol/go-sdk#1232` merged on 2026-09-21, so the write-deadline fix issue 1262 asked for was opened as [modelcontextprotocol/go-sdk#1293](modelcontextprotocol/go-sdk#1293); row 8, its section and the re-verification paragraph name it. Row 12's section says what `modelcontextprotocol/go-sdk#1267` answers a POST whose every call was cancelled (204, since the 20th) and that it was rebased after `modelcontextprotocol/go-sdk#1232` conflicted in `deliverLocked`. The client-go paragraph named two gitlab.com reviewers with pronouns nobody here was told, and names them by handle and role instead. **Regenerated.** The tool snapshots (the served-prose rewrite of issue 910: tool names outside a See also clause replaced by canonical IDs), the token footprint, the llms files, the testing reference and the README stats. Over this tree every `check-*` target the generated-artifacts job runs passes, as do the whole unit suite with `-shuffle=on` and snapshot parity checked, and golangci-lint over every build tag. **Recorded.** The request inventory, which CI records during the coverage job and found 47 rows short: the layers below added tests that drive group access tokens, SAML group links, group security settings and the other refusals the permission hints describe. Recorded again over this tree it holds 1604 rows over 956 distinct paths in 177 packages, every one of the 1082 catalog actions owned by a package that issued a request, and R-PATH passes over it. And the e2e coverage, from both Docker runs against this tree on truenas: the CE suite ran 1024 tests with 9 skipped and none failed, the licensed EE suite 1282 with 8 skipped and none failed, and action coverage holds where it stood (CE L1 821 of 869, L2 721, L3 705; EE L1 1016 of 1089, L2 917, L3 901). Closes #945.
jmrplens
added a commit
to jmrplens/gitlab-mcp-server
that referenced
this pull request
Sep 29, 2026
I read every go-sdk issue and pull request `docs/development/upstream-bugs.md` links, and this records what moved. Nothing there asks anything of us, and go-sdk has cut no release since v1.8.0, so every merge recorded is still unreleased; the maintainer's estimate of the next one, about two months after v1.8.0, is now cited. - **Row 11, [modelcontextprotocol/go-sdk#1266](modelcontextprotocol/go-sdk#1266 A maintainer updated the branch from `main` on 2026-09-29 and all nine checks passed; `main` gained two commits after it, and I updated it again the same day with GitHub's Update branch, which kept the maintainer's merge commit, and the checks passed again. It has had no review since 2026-09-15. - **Rows 8 and 12.** Both were six commits behind `main`; I updated them from `main` on 2026-09-29 and their checks passed. [modelcontextprotocol/go-sdk#1293](modelcontextprotocol/go-sdk#1293) has had no review, and the one review thread on [modelcontextprotocol/go-sdk#1267](modelcontextprotocol/go-sdk#1267) still waits on the reviewer. - **Row 68.** [modelcontextprotocol/go-sdk#1299](modelcontextprotocol/go-sdk#1299) was triaged P2 on 2026-09-28. [modelcontextprotocol/go-sdk#1300](modelcontextprotocol/go-sdk#1300), which another contributor opened, now conflicts with `main` and its checks have never run. The dates of my comment and review, and of the author's follow-up, are corrected to UTC. - **Row 48.** The case is named under "Known limitations" of [modelcontextprotocol/go-sdk#1275](modelcontextprotocol/go-sdk#1275), which another contributor opened and a maintainer merged on 2026-09-18, so the summary row and the entry say so, with the maintainer's position on that pull request. Its discussion also showed the entry's transport claim too wide: v1.8.0's stateful HTTP handler refuses every 2026-07-28 request but `server/discover`, so only stdio reaches the case. ## knowledge-graph - **Rows 75 and 76, [gitlab-org/orbit/knowledge-graph!2650](https://gitlab.com/gitlab-org/orbit/knowledge-graph/-/merge_requests/2650) and [gitlab-org/orbit/knowledge-graph!2651](https://gitlab.com/gitlab-org/orbit/knowledge-graph/-/merge_requests/2651).** Both descriptions now carry `[skip pinned-version-check]`, which the project's check asks for when a change does not affect the shape, so the merged-results pipeline a maintainer runs will not refuse either for want of a `query_dsl` bump. The marker sits at the top of each description's collapsed details block, where the project's description lint does not count it and the 2700 characters a pipeline reads still hold it. Each merge request also offers the bump instead: row 75's a patch bump, so clients that cache the schema by version fetch the new text, and row 76's should the maintainers read the declared default that branch changes as part of the contract. Both Files lists now name the test pins. ## Checks - `npx markdownlint-cli2 docs/development/upstream-bugs.md` passes. - `go run ./cmd/format_md_tables/ --check` passes. - No em dashes in the added lines.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1262.
http.Server.WriteTimeoutis the ordinary slow-write guard, and it applies to the whole response. Asubscriptions/listenstream is a response the transport means to hold open indefinitely, so the deadline passes while the stream sits idle and healthy. Nothing notices, because nothing is being written, and the first real notification after it fails and takes the stream with it.Only the transport knows a response became a stream, so only the transport can move the deadline. As agreed in #1262, every write to a stream that runs a keep-alive (#1232) now pushes the deadline out by twice the keep-alive interval, through
http.ResponseController.SetWriteDeadline:WriteTimeoutkeeps meaning what it was set for, and a peer that stopped reading still fails within a bounded time.subscriptions/listen, because the interval is what the extension is measured in. A stream with no periodic write has nothing to measure it by.ErrNotSupported, and a server with noWriteTimeouthas no deadline to move, so the result is ignored.TestStreamWriteDeadline_SurvivesServerWriteTimeoutholds a listen stream open behind anhttptestserver whoseWriteTimeoutis three keep-alive intervals, for five times that timeout. Againstmainit fails every time: no keep-alive arrives after the 75 ms deadline, because the first write past it fails and ends the stream.