Add a scheduled fresh-resolve job to catch dependency drift - #318
Add a scheduled fresh-resolve job to catch dependency drift#318alex-clickhouse wants to merge 2 commits into
Conversation
|
Verified the job does what it claims rather than just going green — So it resolved with the cache off and index metadata refreshed, and the suite One thing a reviewer will notice: only the fresh-resolve check appears on this Worth being explicit about what this run does and doesn't prove. It proves the |
A fresh install has been failing at startup with
ImportError: cannot import name 'request_ctx'
from 'mcp.server.lowlevel.server'
mcp was never declared in pyproject.toml — it arrived transitively via
claude-agent-sdk, whose constraint is `mcp<3.0.0,>=1.23.0`. Nothing else in
the tree capped it, so once mcp 2.0.0 landed on PyPI every fresh resolve
picked it up. mcp 2.0 is an API redesign of the lowlevel server, so this ports
the endpoint and declares `mcp>=2,<3` explicitly. Declaring it is correct
regardless: three modules under nerve/mcp_server/ import mcp directly.
What changed upstream, and what it forced here:
- Handler registration moved from `@server.list_tools()` / `@server.call_tool()`
decorators to `Server(..., on_list_tools=, on_call_tool=)` constructor
callbacks. `Server.call_tool` no longer exists.
- The request context is now an argument, not a contextvar. `request_ctx` is
gone with no replacement, so `_resolve_client_info` and
`_bound_identity_from_request` take a `ServerRequestContext` explicitly and
`build_ctx_resolver`'s closure threads it down from the call handler.
- `on_list_tools` returns `ListToolsResult` rather than `list[Tool]`, and
`on_call_tool` receives `CallToolRequestParams` rather than positional
`(name, arguments)`.
- Model attributes are snake_case in Python now (the camelCase wire aliases are
unchanged). Constructing with `isError=` still works via `populate_by_name`,
but reads had to move: `client_params.clientInfo` -> `.client_info`.
Two things worth a reviewer's attention:
mcp 1.x validated tool arguments against `inputSchema` inside its `call_tool`
decorator (`validate_input=True` by default). The 2.x callback does not, so
porting mechanically would have silently dropped argument validation on an
endpoint external clients can reach. `build_mcp_server` now validates
explicitly with jsonschema — declared as a direct dependency, since we import
it — and returns the same isError shape as the other failure paths. Tests pin
the behaviour, including that a rejected call never reaches the handler.
The `clientInfo` -> `client_info` rename was invisible to the entire existing
suite: every unit test builds a context with `session=None`, so that line never
executed, and all 3303 tests passed with it broken. Over real HTTP it raised
AttributeError inside the resolver, which `_call_tool` turns into "Context
error" — every external MCP tool call would have failed. The two new
`test_mcp_http_integration` cases drive `initialize` -> `tools/call` through the
real transport and assert satellite attribution, which is what caught it. The
module docstring had claimed a tools/call flow that was never actually there.
Verified: 3310 tests pass under mcp 2.0.0 on a fresh `--refresh` resolve;
`import nerve.gateway.server` clean; the SDK's in-process MCP bridge (which
serves every agent tool, not just /mcp/v1) round-trips a real tool call on its
mcp-2.x branch; and forcing mcp 1.29.0 fails at import, confirming the `>=2`
floor is load-bearing.
Fixes #316
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nerve has no Python lockfile, so what `uv pip install -e .` resolves changes silently as upstream publishes releases. ci.yml structurally cannot see that: it keys its uv cache on pyproject.toml, and a restored cache carries stale index metadata, so an unchanged pyproject means an unchanged resolution indefinitely. That let mcp 2.0.0 break every fresh install on 2026-07-28 while CI stayed green for three weeks still resolving mcp 1.29.0 (run 32061993270, main, 2026-08-17). The break surfaced only when a user filed #316. This adds a separate daily workflow that installs with the cache off and index metadata refreshed, then runs the suite — so a green CI run stops being mistaken for evidence that a fresh install works. It publishes the full resolution to the run summary, so drift is reviewable even when the run is green, and prints an explicit "an upstream release broke this, not your PR" note on failure. Kept out of ci.yml deliberately: PR runs should stay cached and fast, and this needs different triggers. The frontend is out of scope because web/package-lock.json plus `npm ci` already pin it reproducibly. The pull_request trigger is scoped to this workflow's own path — schedule and workflow_dispatch only fire for the copy on the default branch, so without it a change here could not be exercised until after it merged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ed25ccc to
d23d438
Compare
There was a problem hiding this comment.
Pull request overview
Adds a daily uncached Python dependency resolution job to detect upstream drift independently of regular CI.
Changes:
- Runs daily, manually, and when its workflow changes.
- Records resolved versions and runs backend tests.
- Adds failure guidance.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # Nerve has no Python lockfile — pyproject.toml carries lower bounds only — so | ||
| # what `uv pip install -e .` actually resolves changes silently as upstream |
| - name: Explain a failure | ||
| if: failure() | ||
| run: | | ||
| { | ||
| echo "## A fresh install of Nerve is broken" | ||
| echo | ||
| echo "This job installs with **no cache** and refreshed index" | ||
| echo "metadata, so it reflects what a user gets running" | ||
| echo '`uv pip install -e .` today.' | ||
| echo | ||
| echo "The repository tree did not change — an upstream release did." | ||
| echo "Check the resolved versions above against the last green run" | ||
| echo "to find which dependency moved, then add an upper bound in" | ||
| echo '`pyproject.toml` (or fix the incompatibility).' | ||
| echo | ||
| echo "Note that \`ci.yml\` can stay green while this fails: it" | ||
| echo "resolves from a cache keyed on \`pyproject.toml\`." | ||
| } >> "$GITHUB_STEP_SUMMARY" |
|
Closing as a separate PR — folded into #320, which already carries this commit Splitting these stopped making sense once #320 started editing this workflow. The One thing I kept on purpose, in case it's what you meant to drop: the daily
Say the word if you did want the schedule gone and I'll strip it to PR-only. |
Stacked on #319 — merge second. The diff shows two commits; read
d23d438alone (a single new workflow file). Based on #319 rather than plain
mainbecausethis job installs unpinned: on a
mainthat still lacks the mcp bound it would gored, correctly, since fresh installs are genuinely broken there.
The gap
Nerve has no Python lockfile —
pyproject.tomlcarries lower bounds only — so whatuv pip install -e .resolves changes silently as upstream publishes releases.ci.ymlstructurally cannot see that drift: it keys its uv cache onpyproject.toml, so a restored cache carries stale index metadata and an unchangedpyproject means an unchanged resolution, indefinitely.
Not hypothetical. mcp 2.0.0 published 2026-07-28 and broke every fresh install at
startup. CI stayed green for three weeks —
run 32061993270
(main, 2026-08-17) installed
mcp==1.29.0. It surfaced only when a user filed #316,and the delayed cost landed on whoever next touched
pyproject.toml.The uncomfortable framing: a green CI run has never been evidence that a fresh
install of Nerve works. With ~15 lower-bound-only direct dependencies, every one of
them can do what mcp did.
What this adds
A separate daily workflow that resolves with the cache off and index metadata
refreshed, then runs the suite. It answers exactly one question: does a brand-new
install of Nerve still work today?
workflow_dispatch. Detects drift within a day for ~2 min of runner time.uv pip freeze) to the run summary, so drift is reviewable on green runs and diffable between runs when something breaks.ci.ymlcan legitimately still be green.Deliberate scoping
ci.yml— different triggers, and PR runs should stay cached and fast. Aschedulejob insideci.ymlwould needif: github.event_name == 'schedule'guards on every job.web/package-lock.json+npm cialready pin it reproducibly. The gap is Python-only.pull_requesttrigger scoped to this file's own path —scheduleandworkflow_dispatchonly fire for the copy on the default branch, so without it a change to this workflow couldn't be exercised until after merging. It also means future edits here get validated, rather than this being a one-off testing hack.Relationship to #320
#320 pins
ci.ymlthrough a lockfile. That's the other half of the same idea, and thesplit is the point:
ci.ymlpinned so a PR is never broken by an unrelatedupstream release, this job unpinned so drift is still caught somewhere that isn't
someone's PR. Either one alone leaves a gap.
Testing
enable-cache: false, and step list all verified programmaticallypull_requesttrigger — and verified from the log that it did what it claims rather than merely passing:enable-cache: false, no cache restore attempted,--refreshused,Resolved 120 packages,+ mcp==1.29.0(on the earlier pre-port base)pyproject.tomlthey resolvemcp==2.0.0and the suite dies with 8 collection errors — exactly the signal this job exists to raiseNot covered
This detects drift; it doesn't prevent it — that's #320.
🤖 Generated with Claude Code