CI Test Coverage job fails: unpinned fastmcp resolves to 4.x / mcp 2.x on a fresh install #37

Closed
opened 2026-09-15 08:54:38 +00:00 by ric · 3 comments
Owner

Symptom

The Test Coverage job in security.yml fails on the PR #36 merge into main. The last green run (and the last coverage badge update) was the v6.5.1 merge on 2026-07-24.

The job has no --cov-fail-under, so coverage isn't what fails it: pytest exits non-zero on one test.

FAILED tests/test_oauth.py::TestClientRegistration::test_register_and_get
pydantic_core._pydantic_core.ValidationError: 1 validation error for OAuthClientInformationFull
client_id
  Field required [type=missing, ...]
1 failed, 1913 passed, 1 skipped

Reproduced locally by running the job's exact steps on Python 3.12 against a clean worktree of origin/main.

Cause

mcp_server/requirements.txt pins fastmcp>=2.13.0 with no upper bound. A fresh install now resolves:

package tested for 6.7.1 fresh install today
fastmcp 3.4.7 4.0.3
mcp 1.29.0 2.2.0

In mcp 2.x OAuthClientInformationFull.client_id is a required field, so the test's _make_client(client_id=None) no longer constructs. Nothing in the OmniMem code changed; the dependency moved two majors underneath it.

Wider impact

The Docker images build from the same unpinned requirements, so the images the v6.7.1 tag just triggered will have picked up fastmcp 4 / mcp 2, a combination nothing has been tested against. The OAuth provider, the middleware and server.py all import from both packages. Those images should be rebuilt once the pin lands.

Also found

tools/briefing.py lines 46 and 54-58 (the empty-row skip and the exception fallback in _skill_source_keys, added for #34) are uncovered, so tools/ sits at 99% rather than the 100% the changelog states. It doesn't fail the job, but it's worth closing in the same fix.

Plan

  1. Put an upper bound on fastmcp (and mcp, if fastmcp 3.x allows mcp 2) matching the versions 6.7.1 was tested on
  2. Add tests for the two uncovered briefing branches
  3. Re-run the job's steps on a clean 3.12 install to confirm green
  4. Moving to fastmcp 4 is a separate piece of work with its own testing, not a CI fix
## Symptom The `Test Coverage` job in `security.yml` fails on the PR #36 merge into main. The last green run (and the last coverage badge update) was the v6.5.1 merge on 2026-07-24. The job has no `--cov-fail-under`, so coverage isn't what fails it: pytest exits non-zero on one test. ``` FAILED tests/test_oauth.py::TestClientRegistration::test_register_and_get pydantic_core._pydantic_core.ValidationError: 1 validation error for OAuthClientInformationFull client_id Field required [type=missing, ...] 1 failed, 1913 passed, 1 skipped ``` Reproduced locally by running the job's exact steps on Python 3.12 against a clean worktree of `origin/main`. ## Cause `mcp_server/requirements.txt` pins `fastmcp>=2.13.0` with no upper bound. A fresh install now resolves: | package | tested for 6.7.1 | fresh install today | |---|---|---| | fastmcp | 3.4.7 | **4.0.3** | | mcp | 1.29.0 | **2.2.0** | In mcp 2.x `OAuthClientInformationFull.client_id` is a required field, so the test's `_make_client(client_id=None)` no longer constructs. Nothing in the OmniMem code changed; the dependency moved two majors underneath it. ## Wider impact The Docker images build from the same unpinned requirements, so the images the v6.7.1 tag just triggered will have picked up fastmcp 4 / mcp 2, a combination nothing has been tested against. The OAuth provider, the middleware and `server.py` all import from both packages. Those images should be rebuilt once the pin lands. ## Also found `tools/briefing.py` lines 46 and 54-58 (the empty-row skip and the exception fallback in `_skill_source_keys`, added for #34) are uncovered, so `tools/` sits at 99% rather than the 100% the changelog states. It doesn't fail the job, but it's worth closing in the same fix. ## Plan 1. Put an upper bound on fastmcp (and mcp, if fastmcp 3.x allows mcp 2) matching the versions 6.7.1 was tested on 2. Add tests for the two uncovered briefing branches 3. Re-run the job's steps on a clean 3.12 install to confirm green 4. Moving to fastmcp 4 is a separate piece of work with its own testing, not a CI fix
Author
Owner

Fixed on v6.7.x in 332fab3, with a PR open into main.

  • fastmcp>=2.13.0,<4 and mcp>=1.24.0,<2 in mcp_server/requirements.txt. mcp gets its own bound because fastmcp's package metadata doesn't declare it
  • Tests for the two uncovered _skill_source_keys branches in tools/briefing.py
  • Changelog entry under [6.7.2] - Unreleased

Verified by re-running the job's exact steps on a clean Python 3.12 install with no cache: resolved fastmcp 3.4.7 / mcp 1.30.0, 1916 passed, 1 skipped, every memory/ and tools/ module at 100%, badge generation succeeds.

Still to do once it's on main: rebuild the images, since the v6.7.1 tag build resolved fastmcp 4 / mcp 2.

Fixed on `v6.7.x` in 332fab3, with a PR open into main. - `fastmcp>=2.13.0,<4` and `mcp>=1.24.0,<2` in `mcp_server/requirements.txt`. mcp gets its own bound because fastmcp's package metadata doesn't declare it - Tests for the two uncovered `_skill_source_keys` branches in `tools/briefing.py` - Changelog entry under `[6.7.2] - Unreleased` Verified by re-running the job's exact steps on a clean Python 3.12 install with no cache: resolved fastmcp 3.4.7 / mcp 1.30.0, 1916 passed, 1 skipped, every `memory/` and `tools/` module at 100%, badge generation succeeds. Still to do once it's on main: rebuild the images, since the v6.7.1 tag build resolved fastmcp 4 / mcp 2.
Author
Owner

Merged into main via #38 (9de0fc2). The next security workflow run on main should install fastmcp 3.4.x / mcp 1.x and go green.

Open follow-up, not part of this issue: the images built from the v6.7.1 tag resolved fastmcp 4 / mcp 2 and should be replaced, most simply by tagging v6.7.2 from v6.7.x.

Merged into main via #38 (9de0fc2). The next security workflow run on main should install fastmcp 3.4.x / mcp 1.x and go green. Open follow-up, not part of this issue: the images built from the v6.7.1 tag resolved fastmcp 4 / mcp 2 and should be replaced, most simply by tagging v6.7.2 from `v6.7.x`.
ric closed this issue 2026-09-15 09:00:22 +00:00
Author
Owner

Correction to the diagnosis above. There's nothing wrong with fastmcp 4. Production has been running OmniMem 6.7.1 on fastmcp 4.0.3 / mcp 2.2.0 without trouble, so the <4 / <2 bound from #38 was the wrong fix, and the v6.7.1 images don't need rebuilding.

What broke was test-side only. mcp 2 requires client_id on OAuthClientInformationFull and strips null/"" at parse, while its registration handler assigns a uuid before calling the provider, so real registrations always carry an id. Only the test built a client without one.

Fixed properly in 66272a2 on v6.7.x (PR into main): fastmcp>=4.0.3,<5, mcp>=2.2.0,<3, the test registers a client with its id assigned, and a new test reaches the generate-if-missing fallback via model_copy. CI's steps on a clean 3.12 install: fastmcp 4.0.3 / mcp 2.2.0, 1917 passed, 100% on memory/ and tools/.

Correction to the diagnosis above. There's nothing wrong with fastmcp 4. Production has been running OmniMem 6.7.1 on fastmcp 4.0.3 / mcp 2.2.0 without trouble, so the `<4` / `<2` bound from #38 was the wrong fix, and the v6.7.1 images don't need rebuilding. What broke was test-side only. mcp 2 requires `client_id` on `OAuthClientInformationFull` and strips `null`/`""` at parse, while its registration handler assigns a uuid before calling the provider, so real registrations always carry an id. Only the test built a client without one. Fixed properly in 66272a2 on `v6.7.x` (PR into main): `fastmcp>=4.0.3,<5`, `mcp>=2.2.0,<3`, the test registers a client with its id assigned, and a new test reaches the generate-if-missing fallback via `model_copy`. CI's steps on a clean 3.12 install: fastmcp 4.0.3 / mcp 2.2.0, 1917 passed, 100% on `memory/` and `tools/`.
Sign in to join this conversation.
No labels
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
ric/omnimem#37
No description provided.