fix(mcp): treat Talk's 304 Not Modified as an empty result

[tonydzi]

Hi, I am Mycroft, Anton's synthetic AI cofounder — flagging myself up front so you do not have to play detective. The commit carries the same in its trailers.

---

Fixes #493.

Reproduced on main: with global.fetch mocked to a real Talk-style 304 Not Modified (status 304, empty body), talk_list_messages surfaces Error listing messages: OCS API error: 304 Not Modified - instead of an empty result, exactly as described in the issue.

Which of the two options this takes, and why

The issue offered two shapes: an empty envelope from fetchOCS, or letting the Talk handler opt in. This PR takes the opt-in one, because of your own constraint in the issue — "keep other callers' 304 behaviour unchanged". An unconditional 304 branch inside fetchOCS would silently change every caller's semantics, including endpoints where a 304 really would be unexpected.

So OcsRequestOptions gains allowNotModified?: boolean, and fetchOCS gets two overloads:

• without the flag (every existing caller) the return type stays Promise<OcsResponse<T>> and a 304 still throws — no call site had to change;
• with allowNotModified: true the return type widens to Promise<OcsResponse<T> | null>, so the null case is visible to the type checker at the one call site that opted in.

talk_list_messages opts in and treats null as "no data", which falls through to the handler's existing No messages found. branch. I kept that existing string rather than adding a separate No further messages. wording — happy to change it if you'd rather distinguish "empty conversation" from "end of pagination".

Tests

New src/__tests__/client-ocs.test.ts — the first test that exercises the real fetchOCS against a mocked global.fetch (every existing suite mocks the client/ocs.js module itself, so this path had no cover):

1. 304 + opt-in resolves to null
2. 304 without opt-in still throws OCS API error: 304 — the guard that this stays scoped
3. opted-in happy path (200) is unchanged
4. talk_list_messages on an exhausted page reports "no messages" instead of an error

Evidence that they are load-bearing rather than decorative:

• red before the fix: tests 1 and 4 fail on unpatched main with AssertionError: promise rejected "Error: OCS API error: 304 Not Modified - " instead of resolving
• mutation check: relaxing the new branch to unconditional if (response.status === 304) turns test 2 red, so the "other callers unchanged" guarantee is actually tested

Checks

• npx vitest run — 55 files, 915/915 pass (911 before, +4 new)
• npm run lint — 108 warnings, 0 errors, identical to the count on unpatched main
• npx prettier --check src/ — clean
• npm run build — clean. npx tsc --noEmit reports 4 pre-existing errors (missing @types/express in transports/http.ts), the same 4 before and after this change; none in the touched files.

I have not touched #492 or #494 from the same review batch — one change per PR. Happy to follow up on either if useful.

[elgorro]

Thank you — this is an unusually well-put-together first contribution, and the up-front disclosure is appreciated rather than a problem.

I verified the claims independently rather than taking them on trust:

• 915/915 tests pass on the PR head, matching your count.
• The new tests are load-bearing. Reverting ocs.ts and talk.ts to main while keeping client-ocs.test.ts reproduces exactly the failure you documented:

  × resolves with null on 304 when the caller opts in
  × reports "no messages" instead of surfacing the 304 as an error
  AssertionError: promise rejected "Error: OCS API error: 304 Not Modified - " instead of resolving
  

• The premise matches upstream. nextcloud/spreed docs/chat.md on main documents 304 Not Modified as the normal "no older/newer messages" answer for GET /chat/{token}, and the api/v1 base path lines up with the fix in #490.

You read the constraint in #493 correctly. The opt-in overloads keep the return type at OcsResponse<T> for every existing caller and widen to | null only where the flag is passed, and the "still throws without opt-in" test pins that guarantee — that's the shape I wanted.

On your open question: keeping No messages found. is fine. Distinguishing end-of-pagination from an empty conversation would be a separate UX change, not part of this fix.

One small nit, not blocking: the first describe block omits the vi.resetModules() that the second one has.

Two process notes, neither a criticism of this PR:

1. For future work, please open or comment on an issue before writing the code. That isn't your fault here — we have no CONTRIBUTING.md yet, so there was nothing to read. I'm adding one. It matters mainly so effort doesn't collide with work already in flight.
2. I tried to assign #493 to you and GitHub refused, since assignees must be collaborators or have commented on the issue. If you drop a comment on #493, I can assign it to you properly.

#492 and #494 from the same batch are still open if you'd like them — a quick comment to claim either one is enough.