diff --git a/CHANGELOG.md b/CHANGELOG.md index c6c5ee6a8..ca5bdae8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ the archive; if it broke production, it belongs in `docs/incidents/`. See ## [Unreleased] +- **MCP** — the assistant can see where you are, and can record a chapter you finished somewhere else — backend · [details](docs/changelog-archive/2026-H2.md#2026-09-10-mcp-the-assistant-can-see-where-you-are) - **Mobile** — the assistant handoff and the conclusions it writes back reach the phone, and a percent that was silently a fraction — mobile, web, shared · [details](docs/changelog-archive/2026-H2.md#2026-09-08-mcp-the-book-opens-to-your-assistant-and-the-conclusions-come-back) - **MCP** — TextStack.Mcp 1.1.0 on NuGet: the tool ships the uploaded-library and write-back tools — backend · [details](docs/changelog-archive/2026-H2.md#2026-09-08-mcp-the-book-opens-to-your-assistant-and-the-conclusions-come-back) - **Library** — searching your own library answered 500 on every call and always had, behind an empty-looking result — backend · [details](docs/changelog-archive/2026-H2.md#2026-09-08-mcp-the-book-opens-to-your-assistant-and-the-conclusions-come-back) diff --git a/CLAUDE.md b/CLAUDE.md index 4f24879d5..33484a8a4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -514,7 +514,7 @@ Supported formats: EPUB, PDF. Processing order: Spelling → Hyphenation → Typ ## MCP Server (`backend/src/Ai/TextStack.Ai.Mcp/`) -Thin, stateless MCP↔HTTP bridge (Phase 8) — every tool call becomes an HTTP request to the public TextStack API (no DB/EF/OpenAI). 13 tools. Public catalog: `search_books`, `get_book`, `get_chapter`. The user's own uploads (Bearer): `search_my_library`, `get_my_book`, `get_my_chapter`, `save_my_highlight`, `list_my_book_highlights` — keyed by `bookId` (`UserBook.Id`), which is NOT an `editionId` and does not work in the edition-scoped tools. Write-back, either book type (Bearer): `save_insight`, `get_my_insights` — conclusions from an outside assistant, keyed by chapter **slug** (`BookInsight`, table `book_insight`), one per (user, book, chapter) so a re-run replaces rather than accumulates. Everything else (Bearer): `list_my_highlights`, `list_my_vocabulary`, `save_highlight`. +Thin, stateless MCP↔HTTP bridge (Phase 8) — every tool call becomes an HTTP request to the public TextStack API (no DB/EF/OpenAI). 16 tools. Public catalog: `search_books`, `get_book`, `get_chapter`. The user's own uploads (Bearer): `search_my_library`, `get_my_book`, `get_my_chapter`, `save_my_highlight`, `list_my_book_highlights` — keyed by `bookId` (`UserBook.Id`), which is NOT an `editionId` and does not work in the edition-scoped tools. Write-back, either book type (Bearer): `save_insight`, `get_my_insights` — conclusions from an outside assistant, keyed by chapter **slug** (`BookInsight`, table `book_insight`), one per (user, book, chapter) so a re-run replaces rather than accumulates. Everything else (Bearer): `list_my_highlights`, `list_my_vocabulary`, `save_highlight`. Reading state (Bearer): `get_my_reading` — the shelf, **no arguments**, the only way in when the model holds no id; `get_book_progress`; `set_book_progress` — records a chapter finished ANYWHERE (audiobook, paper) and resumes the reader at the next one. The tool count is asserted in four places (`McpManifestDriftTests`, `McpStdioSmokeTests`, `McpOverTheWireTests`, `McpManifestEndpointTests`) and the descriptions are mirrored verbatim into `Contracts/Mcp/McpManifest.cs`, which the drift test compares character for character. **There is no question-answering tool.** `ask_book` and the retrieval spine behind it were deleted 2026-09-10: 7 books of 1498 were ever indexed, and the vision PDF transcription that fed the index was 94% of the project's lifetime LLM spend. An assistant reads `get_chapter` as plain text and reasons over it better, on the reader's own subscription. diff --git a/backend/src/Ai/TextStack.Ai.Mcp/Http/TextStackApiClient.cs b/backend/src/Ai/TextStack.Ai.Mcp/Http/TextStackApiClient.cs index b6a0785ff..f2c70aca9 100644 --- a/backend/src/Ai/TextStack.Ai.Mcp/Http/TextStackApiClient.cs +++ b/backend/src/Ai/TextStack.Ai.Mcp/Http/TextStackApiClient.cs @@ -389,6 +389,109 @@ public async Task GetVocabularyAsync( return null; } + // ── reading state (Bearer) ─────────────────────────────────────────────────── + + /// + /// GET /me/library/shelves — the reader's shelf, both book kinds in one response with + /// titles and progress already joined. This is the only endpoint that answers "what am I + /// reading" without the caller stitching three others together. + /// + public async Task GetShelvesAsync(CancellationToken ct) + { + using var request = await AuthorizedRequestAsync(HttpMethod.Get, "/me/library/shelves", ct); + using var response = await _http.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, ct); + + if (response.StatusCode is HttpStatusCode.Unauthorized) throw new McpUnauthorizedException(); + if (response.StatusCode is HttpStatusCode.OK) + return await response.Content.ReadFromJsonAsync(JsonOptions, ct); + + return null; + } + + /// + /// GET /me/books — every upload, not paged. The shelves response is capped and filtered to + /// in-progress, so this is what answers "everything I have", including books never opened. + /// + public async Task?> GetMyBooksAsync(CancellationToken ct) + { + using var request = await AuthorizedRequestAsync(HttpMethod.Get, "/me/books", ct); + using var response = await _http.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, ct); + + if (response.StatusCode is HttpStatusCode.Unauthorized) throw new McpUnauthorizedException(); + if (response.StatusCode is HttpStatusCode.OK) + return await response.Content.ReadFromJsonAsync>(JsonOptions, ct); + + return null; + } + + /// GET /me/progress/{editionId}. 404 when the reader has never opened it. + public async Task GetEditionProgressAsync(Guid editionId, CancellationToken ct) + { + using var request = await AuthorizedRequestAsync(HttpMethod.Get, $"/me/progress/{editionId}", ct); + using var response = await _http.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, ct); + + if (response.StatusCode is HttpStatusCode.Unauthorized) throw new McpUnauthorizedException(); + if (response.StatusCode is HttpStatusCode.OK) + return await response.Content.ReadFromJsonAsync(JsonOptions, ct); + + return null; + } + + /// GET /me/books/{id}/progress. 404 when the reader has never opened it. + public async Task GetUserBookProgressAsync(Guid bookId, CancellationToken ct) + { + using var request = await AuthorizedRequestAsync(HttpMethod.Get, $"/me/books/{bookId}/progress", ct); + using var response = await _http.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, ct); + + if (response.StatusCode is HttpStatusCode.Unauthorized) throw new McpUnauthorizedException(); + if (response.StatusCode is HttpStatusCode.OK) + return await response.Content.ReadFromJsonAsync(JsonOptions, ct); + + return null; + } + + /// + /// PUT /me/progress/{editionId} — move a catalog book's position. + /// + /// percentUnit: "book" is not optional decoration: without it ProgressUnit.IsTrusted + /// is false and the server stores the position while silently discarding the number. + /// + /// + public async Task SetEditionProgressAsync( + Guid editionId, Guid chapterId, string locator, double? percent, CancellationToken ct) + { + using var request = await AuthorizedRequestAsync(HttpMethod.Put, $"/me/progress/{editionId}", ct); + request.Content = JsonContent.Create( + new SetEditionProgressJson(chapterId, locator, percent, "book"), options: JsonOptions); + + using var response = await _http.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, ct); + if (response.StatusCode is HttpStatusCode.Unauthorized) throw new McpUnauthorizedException(); + return response.IsSuccessStatusCode; + } + + /// + /// PUT /me/books/{id}/progress — move an upload's position. Slug-native, so no chapter-id + /// lookup is needed here; the server validates the slug against the book. + /// + /// is what makes the write land on a book last read as a PDF in + /// Original layout: the stored position is then page:<n>, and + /// LocatorSpace.MayReplace drops an undeclared write from another coordinate space + /// entirely — silently, from the caller's point of view, before this change reported it. + /// + /// + public async Task SetUserBookProgressAsync( + Guid bookId, string? chapterSlug, string locator, double? percent, string? locatorKind, + CancellationToken ct) + { + using var request = await AuthorizedRequestAsync(HttpMethod.Put, $"/me/books/{bookId}/progress", ct); + request.Content = JsonContent.Create( + new SetUserBookProgressJson(chapterSlug, locator, percent, "book", locatorKind), options: JsonOptions); + + using var response = await _http.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, ct); + if (response.StatusCode is HttpStatusCode.Unauthorized) throw new McpUnauthorizedException(); + return response.IsSuccessStatusCode; + } + // ── request builders ───────────────────────────────────────────────────────── // Public route: Host header only (site + EN default-language resolution). @@ -505,7 +608,10 @@ public sealed record BookAuthorJson(string Name); public sealed record BookGenreJson(string Name); -public sealed record ChapterSummaryJson(int ChapterNumber, string? Slug, string Title, int? WordCount); +// `Id` is deserialized because save_highlight needs a chapterId and its description tells the model +// to take it from get_book. That was false for as long as this record omitted the field: the server +// DTO (Contracts.Books.ChapterSummaryDto) has always carried it, the bridge just dropped it. +public sealed record ChapterSummaryJson(Guid Id, int ChapterNumber, string? Slug, string Title, int? WordCount); // GET /books/{slug}/chapters/{chapterSlug} → Contracts.Books.ChapterDto (subset). public sealed record ChapterJson( @@ -650,3 +756,58 @@ public sealed record SaveInsightJson( string? ChapterSlug, string Text, string? Question); + + +// GET /me/library/shelves → LibraryShelvesDto. Only the shelves an assistant needs to answer +// "what am I reading" and "what did I just finish". +public sealed record ShelvesJson( + IReadOnlyList? ContinueReading, + IReadOnlyList? RecentlyAdded, + IReadOnlyList? FinishedThisMonth); + +public sealed record ShelfItemJson( + Guid Id, + // "userbook" (an upload, addressed by bookId) or "savedbook" (a catalog edition, editionId). + // The two halves of the tool catalog split on exactly this. + string Type, + string Title, + string? Author, + string? Slug, + double ProgressPercent, + DateTimeOffset? LastOpenedAt, + string? ChapterSlug); + +// GET /me/books → UserBookListDto[] (the subset that answers "what is on my shelf"). +public sealed record MyBookJson( + Guid Id, + string Title, + string Slug, + string? Author, + string Status, + int ChapterCount, + double? ProgressPercent, + string? ProgressChapterSlug, + DateTimeOffset? CompletedAt); + +// GET /me/progress/{editionId} → ReadingProgressDto. +public sealed record EditionProgressJson( + Guid EditionId, + Guid ChapterId, + string? ChapterSlug, + string Locator, + double? Percent, + DateTimeOffset UpdatedAt, + DateTimeOffset? CompletedAt); + +// GET /me/books/{id}/progress → UserBookProgressDto. +public sealed record UserBookProgressJson( + string? ChapterSlug, + string? Locator, + double? Percent, + DateTimeOffset? UpdatedAt); + +public sealed record SetEditionProgressJson( + Guid ChapterId, string Locator, double? Percent, string PercentUnit); + +public sealed record SetUserBookProgressJson( + string? ChapterSlug, string? Locator, double? Percent, string PercentUnit, string? LocatorKind); diff --git a/backend/src/Ai/TextStack.Ai.Mcp/Tools/McpToolCatalog.cs b/backend/src/Ai/TextStack.Ai.Mcp/Tools/McpToolCatalog.cs index 2935337aa..acaa1ad43 100644 --- a/backend/src/Ai/TextStack.Ai.Mcp/Tools/McpToolCatalog.cs +++ b/backend/src/Ai/TextStack.Ai.Mcp/Tools/McpToolCatalog.cs @@ -43,6 +43,9 @@ public McpToolCatalog(TextStackApiClient api) BuildListMyBookHighlights(api), BuildSaveInsight(api), BuildGetMyInsights(api), + BuildGetMyReading(api), + BuildGetBookProgress(api), + BuildSetBookProgress(api), }; _byName = tools.ToDictionary(t => t.Name, StringComparer.Ordinal); } @@ -203,6 +206,9 @@ private static async Task InvokeAsync( genres = (book.Genres ?? []).Select(g => g.Name).ToArray(), chapters = (book.Chapters ?? []).Select(c => new { + // save_highlight is keyed by chapterId and its description says to get it + // from here. Omitting it made that instruction impossible to follow. + chapterId = c.Id, chapterNumber = c.ChapterNumber, slug = c.Slug, title = c.Title, @@ -942,6 +948,307 @@ private static bool TryReadSearchArgs( return true; } + // ── get_my_reading ────────────────────────────────────────────────────────── + + private static readonly JsonElement GetMyReadingSchema = JsonDocument.Parse( + """ + { + "type": "object", + "properties": {}, + "required": [], + "additionalProperties": false + } + """).RootElement; + + /// + /// The shelf, with no arguments. Everything else here needs an id the model does not have yet; + /// this is the tool that gives it one, and the only answer to "what have I been reading". + /// + private static McpToolDescriptor BuildGetMyReading(TextStackApiClient api) => new() + { + Name = "get_my_reading", + Description = + "List what the reader is reading right now and what they recently finished, with titles " + + "and how far in they are (requires authentication). Takes no arguments. Call this FIRST " + + "when you do not already have a bookId or editionId — nothing else here can find a book " + + "without one. `source` says which: \"userbook\" means a book they uploaded, addressed by " + + "`bookId` in the _my_ tools; \"savedbook\" is a catalog book, addressed by `slug` in " + + "get_book/get_chapter and by `editionId` in the insight tools. `chapterSlug` is where " + + "they stopped. `allBooks` lists every upload including ones never opened.", + InputSchema = GetMyReadingSchema, + Handler = (args, ct) => + { + if (!ArgReader.TryObject(args, out _, out var err)) + return Task.FromResult(Error(err)); + + return InvokeAsync("get_my_reading", ct, async () => + { + var shelves = await api.GetShelvesAsync(ct); + if (shelves is null) return Error("get_my_reading failed: the shelf is unavailable"); + + // The shelf is capped and filtered to in-progress, so it answers "right now" but not + // "everything I have". The upload list is neither, and is one more call. + var all = await api.GetMyBooksAsync(ct); + + var mapped = new + { + reading = (shelves.ContinueReading ?? []).Select(Shelf), + finishedRecently = (shelves.FinishedThisMonth ?? []).Select(Shelf), + allBooks = (all ?? []).Select(b => new + { + source = "userbook", + bookId = b.Id, + title = b.Title, + author = b.Author ?? "", + status = b.Status, + chapterCount = b.ChapterCount, + progressPercent = b.ProgressPercent, + chapterSlug = b.ProgressChapterSlug, + finishedAt = b.CompletedAt, + }), + }; + return Text(JsonSerializer.Serialize(mapped)); + }); + }, + }; + + private static object Shelf(ShelfItemJson i) => new + { + source = i.Type, + // Deliberately named for what the other tools take, so the model does not have to guess + // which id belongs where: an upload is a bookId, a catalog book is an editionId AND a slug. + bookId = i.Type == "userbook" ? (Guid?)i.Id : null, + editionId = i.Type == "savedbook" ? (Guid?)i.Id : null, + slug = i.Slug, + title = i.Title, + author = i.Author ?? "", + progressPercent = i.ProgressPercent, + chapterSlug = i.ChapterSlug, + lastOpenedAt = i.LastOpenedAt, + }; + + // ── get_book_progress ─────────────────────────────────────────────────────── + + private static readonly JsonElement GetBookProgressSchema = JsonDocument.Parse( + """ + { + "type": "object", + "properties": { + "bookId": { "type": "string", "format": "uuid" }, + "editionId": { "type": "string", "format": "uuid" } + }, + "required": [], + "additionalProperties": false + } + """).RootElement; + + /// + /// Where the reader is in one book. Nothing exposed this before: the position used to leak only + /// as ask_book's spoiler refusal, which named no chapter and is now deleted. + /// + private static McpToolDescriptor BuildGetBookProgress(TextStackApiClient api) => new() + { + Name = "get_book_progress", + Description = + "How far the reader has got in one book, and which chapter they stopped in (requires " + + "authentication). Give EITHER bookId (a book they uploaded) OR editionId (a catalog " + + "book). Ask this before discussing a book you have not just been told the position of " + + "— it is what lets you avoid spoiling what they have not reached yet. A book they have " + + "never opened has no progress and says so.", + InputSchema = GetBookProgressSchema, + Handler = (args, ct) => + { + if (!ArgReader.TryObject(args, out var obj, out var err, "bookId", "editionId") + || !TryReadInsightTarget(obj, out var editionId, out var bookId, out err)) + return Task.FromResult(Error(err)); + + return InvokeAsync("get_book_progress", ct, async () => + { + if (bookId is { } id) + { + var p = await api.GetUserBookProgressAsync(id, ct); + return Text(JsonSerializer.Serialize(p is null + ? (object)new { source = "userbook", bookId = id, opened = false } + : new + { + source = "userbook", + bookId = id, + opened = true, + chapterSlug = p.ChapterSlug, + progressPercent = p.Percent, + lastReadAt = p.UpdatedAt, + })); + } + + var e = await api.GetEditionProgressAsync(editionId!.Value, ct); + return Text(JsonSerializer.Serialize(e is null + ? (object)new { source = "edition", editionId, opened = false } + : new + { + source = "edition", + editionId, + opened = true, + chapterSlug = e.ChapterSlug, + progressPercent = e.Percent, + lastReadAt = e.UpdatedAt, + finishedAt = e.CompletedAt, + })); + }); + }, + }; + + // ── set_book_progress ─────────────────────────────────────────────────────── + + private static readonly JsonElement SetBookProgressSchema = JsonDocument.Parse( + """ + { + "type": "object", + "properties": { + "bookId": { "type": "string", "format": "uuid" }, + "slug": { "type": "string", "minLength": 1, "maxLength": 300 }, + "chapterSlug": { "type": "string", "minLength": 1, "maxLength": 300 } + }, + "required": ["chapterSlug"], + "additionalProperties": false + } + """).RootElement; + + /// + /// Record that the reader finished a chapter somewhere else — an audiobook, paper, another app. + /// The position lives here whether or not the reading did. + /// + private static McpToolDescriptor BuildSetBookProgress(TextStackApiClient api) => new() + { + Name = "set_book_progress", + Description = + "Record that the reader has FINISHED a chapter, including one they read or listened to " + + "somewhere else — an audiobook, paper, another app (requires authentication). Give " + + "EITHER bookId (a book they uploaded) OR slug (a catalog book), plus the chapterSlug " + + "they finished; get_my_reading and get_book list the slugs. The app then resumes them " + + "at the START of the next chapter and its progress becomes chapters-finished over " + + "chapters-total; finishing the last chapter marks the book complete. Only call this " + + "when the reader says they finished something — it overwrites the exact position their " + + "reader had stored, and it cannot be undone from here.", + InputSchema = SetBookProgressSchema, + Handler = (args, ct) => + { + if (!ArgReader.TryObject(args, out var obj, out var err, "bookId", "slug", "chapterSlug") + || !ArgReader.TryRequiredString(obj, "chapterSlug", 1, 300, out var chapterSlug, out err) + || !ArgReader.TryOptionalGuid(obj, "bookId", out var bookId, out err) + || !ArgReader.TryOptionalString(obj, "slug", 300, out var bookSlug, out err)) + return Task.FromResult(Error(err)); + + if ((bookId is null) == (bookSlug is null)) + return Task.FromResult(Error( + "Pass either 'bookId' (an uploaded book) or 'slug' (a catalog book), not both.")); + + return InvokeAsync("set_book_progress", ct, async () => + { + if (bookId is { } id) + { + var book = await api.GetMyBookAsync(id, ct); + if (book is null) return Error($"set_book_progress failed: no uploaded book '{id}'"); + + var chapters = book.Chapters ?? []; + var at = IndexOfChapter(chapters.Select(c => c.Slug), chapterSlug); + if (at < 0) + return Error($"set_book_progress failed: no chapter '{chapterSlug}' in '{book.Title}'"); + + var (resumeAt, percent, finished) = AfterFinishing(at, chapters.Count); + var resumeSlug = chapters[resumeAt].Slug; + + // Uploads live in scroll space (`scroll::` — progressPayload.ts), + // and the kind is declared because a book last read as an Original-layout PDF + // has a `page:` stored: without the declaration LocatorSpace.MayReplace drops + // the whole write. Declaring it is the documented way to say "this caller knows + // coordinate spaces exist" — which is also why it costs that reader their page. + var ok = await api.SetUserBookProgressAsync( + id, resumeSlug, $"scroll:{resumeSlug}:0", percent, LocatorSpaceScroll, ct); + + return ok + ? Text(JsonSerializer.Serialize(new + { + bookId = id, + finishedChapterSlug = chapterSlug, + resumeChapterSlug = resumeSlug, + progressPercent = percent, + bookFinished = finished, + saved = true, + })) + : Error("set_book_progress failed: the position was refused. The book's stored " + + "position may be in a coordinate space this write cannot replace."); + } + + // The catalog write is keyed by chapter GUID, so the slug has to be resolved first. + // get_book carries the ids for exactly this. + var edition = await api.GetBookAsync(bookSlug!, ct); + if (edition is null) return Error($"set_book_progress failed: no catalog book '{bookSlug}'"); + + var list = edition.Chapters ?? []; + var found = IndexOfChapter(list.Select(c => c.Slug), chapterSlug); + if (found < 0) + return Error($"set_book_progress failed: no chapter '{chapterSlug}' in '{bookSlug}'"); + + var (resume, pct, done) = AfterFinishing(found, list.Count); + // The app's own two sentinels, not a sixth locator format: end-of-book is what + // "mark as read" writes, and start-of-chapter is where the next chapter begins. + // An assistant knows the chapter; it never knows a scroll offset. + var saved = await api.SetEditionProgressAsync( + edition.Id, list[resume].Id, done ? EndOfBook : StartOfChapter, pct, ct); + + return saved + ? Text(JsonSerializer.Serialize(new + { + editionId = edition.Id, + finishedChapterSlug = chapterSlug, + resumeChapterSlug = list[resume].Slug, + progressPercent = pct, + bookFinished = done, + saved = true, + })) + : Error("set_book_progress failed: the position could not be saved"); + }); + }, + }; + + /// The app's own mark-as-read sentinel (`auth.ts markAsRead`), reused rather than reinvented. + private const string EndOfBook = """{"type":"end"}"""; + + /// The start of a chapter — the same sentinel `markAsUnread` writes. + private const string StartOfChapter = """{"type":"start"}"""; + + /// Mirrors `LocatorSpace.Scroll`, which lives in Application and is not referenced here. + private const string LocatorSpaceScroll = "scroll"; + + private static int IndexOfChapter(IEnumerable slugs, string wanted) + { + var i = 0; + foreach (var slug in slugs) + { + if (string.Equals(slug, wanted, StringComparison.Ordinal)) return i; + i++; + } + return -1; + } + + /// + /// Where the reader is once they have finished chapter : the START of the + /// next one, because "I finished chapter 2" means the app should open chapter 3 — not drop them + /// back into what they just finished. On the last chapter there is nowhere forward to go, so the + /// position stays and the book is simply complete. + /// + /// The percentage is chapters-done over chapters-total. It is a book-wide fraction, which + /// is the only unit the server stores (ProgressUnit), and it is sent rather than omitted + /// because the catalog path ASSIGNS the column — a write carrying no number blanks the one that + /// was there. + /// + private static (int ResumeIndex, double Percent, bool Finished) AfterFinishing(int at, int total) + { + var done = at + 1; + var finished = done >= total; + return (finished ? at : done, total > 0 ? (double)done / total : 0, finished); + } + // ── MCP result helpers ────────────────────────────────────────────────────── private static CallToolResult Text(string text) => new() diff --git a/backend/src/Application/Library/LibraryShelvesService.cs b/backend/src/Application/Library/LibraryShelvesService.cs index b74680b25..cc9cafaa1 100644 --- a/backend/src/Application/Library/LibraryShelvesService.cs +++ b/backend/src/Application/Library/LibraryShelvesService.cs @@ -105,6 +105,12 @@ orderby latestProgress.UpdatedAt descending e.Language, Progress = latestProgress.Percent ?? 0, CurrentChapterId = (Guid?)latestProgress.ChapterId, + // Resolved here rather than by the caller: the id is useless to a client, and a + // second round trip per book is what this shelf existed to avoid. + CurrentChapterSlug = db.Chapters + .Where(c => c.Id == latestProgress.ChapterId) + .Select(c => c.Slug) + .FirstOrDefault(), CurrentLocator = latestProgress.Locator, CurrentPositionJson = latestProgress.PositionJson, LastOpened = (DateTimeOffset?)latestProgress.UpdatedAt, @@ -130,7 +136,7 @@ orderby latestProgress.UpdatedAt descending u.Id, "userbook", u.Title, u.Author, u.CoverPath, u.Slug, u.Language, p, u.LastOpened, u.CreatedAt, EstimateRemaining(u.TotalWordCount, p, pace), - u.ProgressLocator, u.ProgressPositionJson); + u.ProgressLocator, u.ProgressPositionJson, u.ProgressChapterSlug); }) .Concat(saved.Select(s => { @@ -139,7 +145,7 @@ orderby latestProgress.UpdatedAt descending s.Id, "savedbook", s.Title, s.Author, s.CoverPath, s.Slug, s.Language, p, s.LastOpened, s.CreatedAt, EstimateRemaining(s.TotalWordCount, p, pace), - s.CurrentLocator, s.CurrentPositionJson); + s.CurrentLocator, s.CurrentPositionJson, s.CurrentChapterSlug); })) .OrderByDescending(i => i.LastOpenedAt ?? DateTimeOffset.MinValue) .Take(ShelfLimit) @@ -199,6 +205,9 @@ orderby ul.CreatedAt descending ul.CreatedAt, LatestProgress = latest != null ? latest.Percent : null, CurrentChapterId = latest != null ? (Guid?)latest.ChapterId : null, + CurrentChapterSlug = latest != null + ? db.Chapters.Where(c => c.Id == latest.ChapterId).Select(c => c.Slug).FirstOrDefault() + : null, CurrentLocator = latest != null ? latest.Locator : null, CurrentPositionJson = latest != null ? latest.PositionJson : null, LastOpened = latest != null ? (DateTimeOffset?)latest.UpdatedAt : null, @@ -215,7 +224,7 @@ orderby ul.CreatedAt descending u.Id, "userbook", u.Title, u.Author, u.CoverPath, u.Slug, u.Language, p, u.LastOpened, u.CreatedAt, EstimateRemaining(u.TotalWordCount, p, pace), - u.ProgressLocator, u.ProgressPositionJson); + u.ProgressLocator, u.ProgressPositionJson, u.ProgressChapterSlug); }) .Concat(saved.Select(s => { @@ -224,7 +233,7 @@ orderby ul.CreatedAt descending s.Id, "savedbook", s.Title, s.Author, s.CoverPath, s.Slug, s.Language, p, s.LastOpened, s.CreatedAt, EstimateRemaining(s.TotalWordCount, p, pace), - s.CurrentLocator, s.CurrentPositionJson); + s.CurrentLocator, s.CurrentPositionJson, s.CurrentChapterSlug); })) .OrderByDescending(i => i.CreatedAt) .Take(ShelfLimit) @@ -290,6 +299,9 @@ orderby totalWords e.Language, Progress = (latest != null ? latest.Percent : null) ?? 0, CurrentChapterId = latest != null ? (Guid?)latest.ChapterId : null, + CurrentChapterSlug = latest != null + ? db.Chapters.Where(c => c.Id == latest.ChapterId).Select(c => c.Slug).FirstOrDefault() + : null, CurrentLocator = latest != null ? latest.Locator : null, CurrentPositionJson = latest != null ? latest.PositionJson : null, LastOpened = latest != null ? (DateTimeOffset?)latest.UpdatedAt : null, diff --git a/backend/src/Application/UserBooks/UserBookService.cs b/backend/src/Application/UserBooks/UserBookService.cs index bd2158bcd..e83a4ddac 100644 --- a/backend/src/Application/UserBooks/UserBookService.cs +++ b/backend/src/Application/UserBooks/UserBookService.cs @@ -544,9 +544,25 @@ public async Task GetStorageQuotaAsync(Guid userId, Cancellatio // snapshot as the locator. if (!LocatorSpace.MayReplace(book.ProgressLocator, request.Locator, request.LocatorKind)) { - // Silent, like the stale-write branch below used to be: an old client - // cannot act on an error and would only retry into it. - return (true, null); + // Reported, not silent. This used to return (true, null) on the reasoning that an old + // client cannot act on an error and would only retry into it — true of a reader's app, + // and exactly wrong for an assistant writing progress over MCP, which will report + // "recorded" to a person on the strength of a 200 that recorded nothing. A refusal here + // means the write carried a position in a coordinate space the stored one is not in. + return (false, "This book's position is stored in a different coordinate space. " + + "Declare locatorKind to move it between them."); + } + + // Validated, not trusted. This was a raw assignment, so an assistant that invented a chapter + // slug had it stored verbatim — and every later read would resolve it to nothing. The + // bookmark path in this same file has always checked (AddBookmarkAsync); progress never did. + // Null stays legal: a chapterless PDF in Original layout has a page, not a chapter. + if (!string.IsNullOrWhiteSpace(request.ChapterSlug)) + { + var known = await db.UserChapters + .AnyAsync(c => c.UserBookId == bookId && c.Slug == request.ChapterSlug, ct); + if (!known) + return (false, $"No chapter '{request.ChapterSlug}' in this book"); } book.ProgressChapterSlug = request.ChapterSlug; diff --git a/backend/src/Contracts/Library/LibraryShelvesDto.cs b/backend/src/Contracts/Library/LibraryShelvesDto.cs index 1771f648a..1f3bc19e4 100644 --- a/backend/src/Contracts/Library/LibraryShelvesDto.cs +++ b/backend/src/Contracts/Library/LibraryShelvesDto.cs @@ -32,5 +32,20 @@ public record LibraryShelfItemDto( /// /// string? CurrentLocator = null, - string? PositionJson = null + string? PositionJson = null, + /// + /// The chapter the reader stopped in, by slug. + /// + /// Both queries have always SELECTED this — the upload one directly, the catalog one as a chapter + /// id — and then dropped it, for the same reason the locator was dropped: the DTO had nowhere to + /// put it. That is why continueReading.ts exists: its own doc comment says the shelves + /// payload "carries no chapterSlug, so a shelf tap structurally cannot resume at the right + /// chapter", and every client has been making a second request per book to find out. + /// + /// + /// Null for a chapterless PDF read in Original layout (ADR-012), whose position is a page in + /// rather than a chapter. + /// + /// + string? ChapterSlug = null ); diff --git a/backend/src/Contracts/Mcp/McpManifest.cs b/backend/src/Contracts/Mcp/McpManifest.cs index 8c3e7dae5..8b4401452 100644 --- a/backend/src/Contracts/Mcp/McpManifest.cs +++ b/backend/src/Contracts/Mcp/McpManifest.cs @@ -99,5 +99,31 @@ public static class McpManifestCatalog + "reading order (requires authentication). Give EITHER bookId (an uploaded book) OR " + "editionId (a catalog book). Call this FIRST when starting to work on a book the reader " + "has discussed before — it is what stops the next session repeating the last one."), + + new("get_my_reading", + "List what the reader is reading right now and what they recently finished, with titles " + + "and how far in they are (requires authentication). Takes no arguments. Call this FIRST " + + "when you do not already have a bookId or editionId — nothing else here can find a book " + + "without one. `source` says which: \"userbook\" means a book they uploaded, addressed by " + + "`bookId` in the _my_ tools; \"savedbook\" is a catalog book, addressed by `slug` in " + + "get_book/get_chapter and by `editionId` in the insight tools. `chapterSlug` is where " + + "they stopped. `allBooks` lists every upload including ones never opened."), + + new("get_book_progress", + "How far the reader has got in one book, and which chapter they stopped in (requires " + + "authentication). Give EITHER bookId (a book they uploaded) OR editionId (a catalog " + + "book). Ask this before discussing a book you have not just been told the position of " + + "— it is what lets you avoid spoiling what they have not reached yet. A book they have " + + "never opened has no progress and says so."), + + new("set_book_progress", + "Record that the reader has FINISHED a chapter, including one they read or listened to " + + "somewhere else — an audiobook, paper, another app (requires authentication). Give " + + "EITHER bookId (a book they uploaded) OR slug (a catalog book), plus the chapterSlug " + + "they finished; get_my_reading and get_book list the slugs. The app then resumes them " + + "at the START of the next chapter and its progress becomes chapters-finished over " + + "chapters-total; finishing the last chapter marks the book complete. Only call this " + + "when the reader says they finished something — it overwrites the exact position their " + + "reader had stored, and it cannot be undone from here."), ]; } diff --git a/docs/05-features/assistant-handoff.md b/docs/05-features/assistant-handoff.md index 3fc75b35d..1fee458fe 100644 --- a/docs/05-features/assistant-handoff.md +++ b/docs/05-features/assistant-handoff.md @@ -154,8 +154,10 @@ required", and `last_used_at` is stamped. That covers the bridge path depend on the answer. - **Mint a key on textstack.app and hold a real conversation about a real book**, then check the conclusion comes back. Every test so far has used an empty throwaway account. -- **Android developer verification, deadline 2026-09-30** — unregistered apps are removed from Play - globally. Unrelated to this feature and more urgent than all of it. +- ~~Android developer verification~~ — **already done**, verified in the console 2026-09-10: + `app.textstack.mobile` is Registered with both signing keys, last updated 2026-05-15, and Identity + is filled from the developer account. The September notification is informational; it was read as a + to-do here in error. ### Left behind by the cut — a follow-up, found 2026-09-10 after PR #596 opened @@ -198,13 +200,13 @@ residue this work exists to remove, so it goes in its own small PR rather than r | # | Item | Slice | |---|---|---| -| 1 | `get_my_reading` — the shelf, over the existing `GET /me/library/shelves` | 1 | -| 2 | `get_book_progress` — where am I in this book | 1 | -| 3 | `set_book_progress` — record progress made on another medium | 1 | -| 4 | `chapterSlug` on `LibraryShelfItemDto` — the service already selects it | 1 | -| 5 | Restore `chapterId` to the `get_book` projection (defect 2 below) | 1 | -| 6 | Validate the chapter slug on upload progress writes (defect 1) | 1 | -| 7 | Stop reporting success when `MayReplace` refused the write (defect 3) | 1 | +| 1 | ~~`get_my_reading` — the shelf, over the existing `GET /me/library/shelves`~~ — shipped 2026-09-10 | 1 | +| 2 | ~~`get_book_progress` — where am I in this book~~ — shipped 2026-09-10 | 1 | +| 3 | ~~`set_book_progress` — record progress made on another medium~~ — shipped 2026-09-10 | 1 | +| 4 | ~~`chapterSlug` on `LibraryShelfItemDto` — the service already selects it~~ — shipped 2026-09-10 | 1 | +| 5 | ~~Restore `chapterId` to the `get_book` projection (defect 2 below)~~ — shipped 2026-09-10 | 1 | +| 6 | ~~Validate the chapter slug on upload progress writes (defect 1)~~ — shipped 2026-09-10 | 1 | +| 7 | ~~Stop reporting success when `MayReplace` refused the write (defect 3)~~ — shipped 2026-09-10 | 1 | | 8 | Brief names highlights and vocabulary, within the 1200-char budget | 1 | | 9 | Catalog screens pass progress into the handoff | 1 | | 10 | Mobile handoff stops swallowing the open failure | 1 | @@ -313,15 +315,16 @@ retrieval vectors, not the type. These exist independently of this feature; they were found while tracing it. Also listed in [`STATUS.md`](../STATUS.md). -1. **No slug validation on upload progress writes.** `UserBookService.cs:556` assigns - `book.ProgressChapterSlug = request.ChapterSlug` raw. An invented slug is stored silently. The - bookmark path in the same file (`:640-646`) does validate. -2. **Catalog MCP tools drop `chapterId`.** `get_book` and `get_chapter` project chapters without the - Guid (`McpToolCatalog.cs:195-212`, `:253-261`) though the server DTOs carry it. Meanwhile - `save_highlight`'s description tells the model to take `chapterId` "from get_book" - (`McpToolCatalog.cs:637`) — which is not true today. -3. **A refused write reports success.** `LocatorSpace.MayReplace` refusal returns `(true, null)` - (`UserBookService.cs:551-554`). The caller gets 200 and believes it saved. +1. ~~**No slug validation on upload progress writes.**~~ Fixed 2026-09-10: the write is checked + against `UserChapters` for that book and an unknown slug is refused, the way `AddBookmarkAsync` + always has been. +2. ~~**Catalog MCP tools drop `chapterId`.**~~ Fixed 2026-09-10 for `get_book`, which is the one + `save_highlight`'s description names and the one `set_book_progress` needs to resolve a slug to + the GUID the catalog route requires. `get_chapter` still projects without it — it carries the + chapter the caller already asked for by slug, so nothing is unreachable through it. +3. ~~**A refused write reports success.**~~ Fixed 2026-09-10: the refusal returns `(false, …)` and + the endpoint answers 400 with the reason, so `set_book_progress` reports a failure instead of + telling a person their progress was recorded. 4. **Web and mobile write different locators for the same action.** Web `markAsRead` sends the `{"type":"end"}` sentinel (`apps/web/src/api/auth.ts:235-241`); mobile sends `scroll::0` (`apps/mobile/src/hooks/useBookActions.ts:38-66`). diff --git a/docs/05-features/mcp.md b/docs/05-features/mcp.md index e667b607d..da353df91 100644 --- a/docs/05-features/mcp.md +++ b/docs/05-features/mcp.md @@ -8,9 +8,9 @@ reading, and manage your own highlights and vocabulary — all from the chat. This is the canonical reference. The [package README](https://www.nuget.org/packages/TextStack.Mcp) and the [landing page](https://textstack.app/en/mcp) point here. -## The 13 tools +## The 16 tools -The server exposes 13 tools. The public ones need no auth; the user-scoped ones +The server exposes 16 tools. The public ones need no auth; the user-scoped ones require you to be signed in (see [Authentication](#authentication)). **Two halves, two identifiers.** The public catalog is made of `Edition`s and is @@ -36,11 +36,24 @@ uploads. | `list_my_highlights` | List your highlights for a given edition. | User | | `list_my_vocabulary` | List your saved vocabulary words, optionally filtered by SRS stage or search. | User | | `save_highlight` | Save a passage (text + optional color/note) to your highlights for a catalog book chapter. | User | +| `get_my_reading` | The shelf, with no arguments: what you are reading now, what you finished recently, every upload. The only tool that needs no id — it is how the assistant finds one. | User | +| `get_book_progress` | How far you have got in one book, and the chapter you stopped in. | User | +| `set_book_progress` | Record that you finished a chapter — including one you read or listened to somewhere else. | User | -All 13 tools are always listed regardless of whether you're signed in — only a +All 16 tools are always listed regardless of whether you're signed in — only a user-scoped *call* fails with a clean "authentication required" message when no token is available. +**Where the reader is.** `get_my_reading` takes no arguments and is the entry point: +it answers "what am I reading" with titles, the chapter you stopped in, and the id +each other tool takes — `bookId` for an upload, `editionId` *and* `slug` for a catalog +book. `get_book_progress` answers the same for one book, which is what lets an +assistant avoid spoiling what you have not reached. `set_book_progress` closes the +loop the other way: tell it you finished a chapter in an audiobook or on paper and +the app resumes you at the next one, with progress recorded as chapters-finished +over chapters-total. It is the only tool here that changes where your reader opens, +so it acts only when you say you finished something. + A typical catalog chain is `search_books → get_book` (to get the `editionId` / chapter ids) `→ get_chapter` / `save_highlight`. The chain for your own uploads is `search_my_library → get_my_book` (to get the chapter ids) `→ get_my_chapter`. @@ -230,7 +243,7 @@ Before wiring up a client, confirm the tool speaks MCP. This sends ``` Expect a response with `serverInfo` naming `textstack` and a `tools/list` -result containing all 13 tools. +result containing all 16 tools. ## Troubleshooting diff --git a/docs/STATUS.md b/docs/STATUS.md index 18ebfa054..f5b732d9d 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -38,11 +38,15 @@ answers "what happened" and nothing answered "what is half-finished right now". embedding workers, `ask_book`). The numbers that decided it — 26 chat messages lifetime, 7 books of 1498 ever indexed, $4.14 of $4.39 lifetime LLM spend on vision transcription — are in the doc. - **Not done, and it is what blocks the feature:** there is no UI to create a key, on either client, - so today one can only be minted with curl. The revoke-then-401 and `LastUsedAt` integration test is - also missing — the same gap that left `GuestActivityMiddleware` dead. Then: the read-side tools - (`get_my_reading`, `get_book_progress`, `set_book_progress`), and the catalog handoff brief, which - sends an `editionId` where the tools require a slug and therefore does not work at all. + **Since shipped:** key UI on both clients, the revoke-then-401 + `LastUsedAt` integration test, the + catalog handoff brief (which used to send an `editionId` where the tools require a slug), and the + three read-side tools — `get_my_reading`, `get_book_progress`, `set_book_progress` — with the four + progress-path defects they could not work around (unvalidated chapter slug, a refusal reporting + success, `chapterId` missing from `get_book`, `chapterSlug` selected by the shelf and discarded). + + **Not done:** insight categories (Conclusions · Watch for · Discussed · Questions) with tabs and a + DELETE, so a wrong conclusion can be removed; and the owner-only check that the mobile Claude and + ChatGPT apps accept a custom connector at all. **Not yet run:** CI, and both destructive migrations (`DropBookChat`, `DropRagSpine`) against production. Back up first. diff --git a/docs/changelog-archive/2026-H2.md b/docs/changelog-archive/2026-H2.md index f026d23fd..3a76d263b 100644 --- a/docs/changelog-archive/2026-H2.md +++ b/docs/changelog-archive/2026-H2.md @@ -3,6 +3,64 @@ Full write-ups, newest first. The one-line index lives in [`../../CHANGELOG.md`](../../CHANGELOG.md); the incidents worth reading on their own are in [`../incidents/`](../incidents/README.md). + + +## MCP — the assistant can see where you are, and record where you got to — backend — 2026-09-10 + +Thirteen tools could read any book in the library and write conclusions back into it. Not one of them +could answer *where the reader is*. The position existed on the server the whole time; the only way it +ever surfaced was as `ask_book`'s spoiler refusal, and that tool is gone. So a conversation about a +book began from nothing every time: which book, which chapter, how far — all of it re-typed by the +person who had just been reading it. + +Three tools, no migration. + +**`get_my_reading` takes no arguments.** That is the point of it: every other tool needs an id, and +nothing produced one. It answers over `GET /me/library/shelves`, which has always returned both book +kinds in one response with titles, authors and progress already joined, and which no MCP tool called. +Each row hands back the id the other tools actually take — `bookId` for an upload, `editionId` *and* +`slug` for a catalog book — because handing back the wrong one reads as "book not found" three calls +later, far from the cause. The shelf is capped and filtered to in-progress, so `GET /me/books` +supplies the rest, including books never opened. + +**`get_book_progress`** answers the same question for one book: the chapter you stopped in, how far +in, whether you finished it. A book never opened 404s upstream, which is an answer, not a failure — +reporting it as an error would have the model tell a reader their library is broken. + +**`set_book_progress` closes the loop the other way.** You listened to chapter two in Spotify; the app +should know. It takes a chapter *slug*, and what it does with it is the part worth stating: finishing +a chapter resumes you at the **start of the next one** — not back in what you just finished — using +the app's own two sentinels (`{"type":"start"}`, and `{"type":"end"}` on the last chapter, which is +exactly what "mark as read" writes). Progress becomes chapters-finished over chapters-total, declared +as a book fraction, because the catalog path *assigns* that column: a write carrying no number blanks +the one that was there. + +**Four defects had to be fixed for any of this to be true.** + +- **A chapter slug was stored unvalidated.** `UserBookService.UpsertProgressAsync` assigned + `request.ChapterSlug` raw. An assistant that invented a slug had it stored verbatim, and every later + read — resume, shelf card, reader — resolved it to nothing. The bookmark path in the same service + has always checked. Progress never did. +- **A refusal reported success.** `LocatorSpace.MayReplace` returning false dropped the whole write + and answered `(true, null)`. That was defensible while the only callers were readers' apps, which + cannot act on an error and would retry into it. It stopped being defensible the moment an assistant + became a caller: a 200 that stored nothing has it tell a person their progress was recorded. +- **`get_book`'s chapter projection dropped the chapter id** — while `save_highlight`'s own + description told the model to take `chapterId` "from `get_book`". Following that instruction was + impossible. The id is back, which is also what lets the catalog write resolve a slug to the GUID the + route requires. +- **The shelf selected `chapterSlug` and threw it away**, because `LibraryShelfItemDto` had nowhere to + put it. One optional field at the end of the record; the service already had the value in all three + projections. + +Proven against the running bridge rather than only the stubs: mint a key, `POST /mcp`, `get_my_reading` +returns the shelf **with titles**, `set_book_progress` moves the position to the next chapter, the next +`get_my_reading` shows it moved, and an invented slug comes back as an error instead of a silent save. + +**Tool count is asserted in four places** (`McpManifestDriftTests`, `McpStdioSmokeTests`, +`McpOverTheWireTests`, `McpManifestEndpointTests`) and every description is mirrored character for +character into `Contracts/Mcp/McpManifest.cs`, which the API serves and the drift test compares. + ## MCP — the book opens to your assistant, and the conclusions come back — backend, web — 2026-09-08 diff --git a/tests/TextStack.Ai.Mcp.Tests/McpManifestDriftTests.cs b/tests/TextStack.Ai.Mcp.Tests/McpManifestDriftTests.cs index 364d30a03..5098c8c2c 100644 --- a/tests/TextStack.Ai.Mcp.Tests/McpManifestDriftTests.cs +++ b/tests/TextStack.Ai.Mcp.Tests/McpManifestDriftTests.cs @@ -56,7 +56,7 @@ public void ManifestToolDescriptions_MatchRuntimeCatalog() [Fact] public void Manifest_AdvertisesTheWholeToolSurface() { - Assert.Equal(13, McpManifestCatalog.Tools.Count); + Assert.Equal(16, McpManifestCatalog.Tools.Count); } private sealed class NoTokenProvider : IMcpTokenProvider diff --git a/tests/TextStack.Ai.Mcp.Tests/McpOverTheWireTests.cs b/tests/TextStack.Ai.Mcp.Tests/McpOverTheWireTests.cs index 2ec6bbbb2..90bdf75be 100644 --- a/tests/TextStack.Ai.Mcp.Tests/McpOverTheWireTests.cs +++ b/tests/TextStack.Ai.Mcp.Tests/McpOverTheWireTests.cs @@ -43,6 +43,10 @@ public async ValueTask InitializeAsync() => private static JsonElement Json(CallToolResult result) => JsonDocument.Parse(TextOf(result)).RootElement; + /// Asserts the call succeeded, and puts the tool's own message in the failure. + private static void AssertOk(CallToolResult result) => + Assert.False(result.IsError == true, TextOf(result)); + private async Task CallAsync(McpClient client, string tool, Dictionary args) => await client.CallToolAsync(tool, args!, cancellationToken: Ct); @@ -211,7 +215,7 @@ public async Task ListTools_OverWire_ReturnsExactlyTheExpectedTools() var names = tools.Select(t => t.Name).OrderBy(n => n, StringComparer.Ordinal).ToArray(); Assert.Equal( - ["get_book", "get_chapter", "get_my_book", "get_my_chapter", "get_my_insights", "list_my_book_highlights", "list_my_highlights", "list_my_vocabulary", "save_highlight", "save_insight", "save_my_highlight", "search_books", "search_my_library"], + ["get_book", "get_book_progress", "get_chapter", "get_my_book", "get_my_chapter", "get_my_insights", "get_my_reading", "list_my_book_highlights", "list_my_highlights", "list_my_vocabulary", "save_highlight", "save_insight", "save_my_highlight", "search_books", "search_my_library", "set_book_progress"], names); } @@ -274,7 +278,7 @@ public async Task OneSession_OverWire_ListAndSave_AllSucceed() await using var client = await _harness.ConnectAsync(McpServerHarness.TestJwt, Ct); var tools = await client.ListToolsAsync(cancellationToken: Ct); - Assert.Equal(13, tools.Count); + Assert.Equal(16, tools.Count); var chapter = await CallAsync(client, "get_chapter", Args(("slug", "dracula"), ("chapterSlug", "ch-1"))); Assert.NotEqual(true, chapter.IsError); @@ -367,4 +371,193 @@ public async Task OneSession_OverWire_WriteBack_HighlightThenInsightThenReadBack Assert.Equal($"Bearer {McpServerHarness.TestJwt}", _harness.Stub.Last("save_insight")!.Authorization); } + + // ── 15. reading state: the shelf, the position, and moving it ───────────────── + + [Fact] + public async Task GetMyReading_OverWire_ReturnsBothBookKinds_WithTitlesAndTheIdsTheOtherToolsTake() + { + // The whole reason this tool exists: with no arguments it is the only way into everything + // else, so it has to hand back an id each other tool actually accepts — a bookId for an + // upload, an editionId AND a slug for a catalog book. Handing back the wrong one reads as + // "book not found" three calls later, far from the cause. + await using var client = await _harness.ConnectAsync(McpServerHarness.TestJwt, Ct); + + var result = await CallAsync(client, "get_my_reading", Args()); + + AssertOk(result); + var reading = Json(result).GetProperty("reading").EnumerateArray().ToArray(); + Assert.Equal(2, reading.Length); + + var upload = reading.Single(r => r.GetProperty("source").GetString() == "userbook"); + Assert.Equal("Designing Data-Intensive Applications", upload.GetProperty("title").GetString()); + Assert.Equal(StubBackend.UserBookId, upload.GetProperty("bookId").GetString()); + Assert.Equal(JsonValueKind.Null, upload.GetProperty("editionId").ValueKind); + Assert.Equal("replication", upload.GetProperty("chapterSlug").GetString()); + + var catalog = reading.Single(r => r.GetProperty("source").GetString() == "savedbook"); + Assert.Equal(StubBackend.GoodEdition, catalog.GetProperty("editionId").GetString()); + Assert.Equal("dracula", catalog.GetProperty("slug").GetString()); + Assert.Equal(JsonValueKind.Null, catalog.GetProperty("bookId").ValueKind); + + // The shelf is capped and filtered to in-progress, so a book never opened appears only here. + var all = Json(result).GetProperty("allBooks").EnumerateArray().ToArray(); + Assert.Equal(2, all.Length); + Assert.Contains(all, b => b.GetProperty("title").GetString() == "The Mom Test"); + } + + [Fact] + public async Task GetMyReading_NoBearer_OverWire_AuthRequired() + { + await using var client = await _harness.ConnectAsync(bearer: null, Ct); + + var result = await CallAsync(client, "get_my_reading", Args()); + + Assert.True(result.IsError); + Assert.Contains("authentication required", TextOf(result)); + } + + [Fact] + public async Task GetBookProgress_OverWire_UploadAndCatalog_ReportWhereTheReaderStopped() + { + await using var client = await _harness.ConnectAsync(McpServerHarness.TestJwt, Ct); + + var upload = await CallAsync(client, "get_book_progress", Args(("bookId", StubBackend.UserBookId))); + AssertOk(upload); + Assert.True(Json(upload).GetProperty("opened").GetBoolean()); + Assert.Equal("replication", Json(upload).GetProperty("chapterSlug").GetString()); + + var catalog = await CallAsync(client, "get_book_progress", Args(("editionId", StubBackend.GoodEdition))); + AssertOk(catalog); + Assert.Equal("ch-1", Json(catalog).GetProperty("chapterSlug").GetString()); + Assert.Equal($"/me/progress/{StubBackend.GoodEdition}", _harness.Stub.Last("get_edition_progress")!.PathAndQuery); + } + + [Fact] + public async Task GetBookProgress_NeverOpened_OverWire_SaysNotStarted_NotAnError() + { + // 404 here means "this reader has not opened this book", which is an answer. Reporting it as + // a failure would have the model tell a reader their library is broken. + await using var client = await _harness.ConnectAsync(McpServerHarness.TestJwt, Ct); + + var result = await CallAsync(client, "get_book_progress", Args(("editionId", StubBackend.UnopenedEdition))); + + AssertOk(result); + Assert.False(Json(result).GetProperty("opened").GetBoolean()); + } + + [Fact] + public async Task GetBookProgress_BothIds_OverWire_Refused_NoUpstreamCall() + { + await using var client = await _harness.ConnectAsync(McpServerHarness.TestJwt, Ct); + + var result = await CallAsync(client, "get_book_progress", + Args(("bookId", StubBackend.UserBookId), ("editionId", StubBackend.GoodEdition))); + + Assert.True(result.IsError); + Assert.Equal(0, _harness.Stub.TotalRequests); + } + + [Fact] + public async Task SetBookProgress_CatalogMidBook_OverWire_ResumesAtTheNextChapter_WithABookWidePercent() + { + // "I finished chapter 1" must not drop the reader back into chapter 1. The stored position + // becomes the START of chapter 2 — the app's own markAsUnread sentinel — and the percentage + // is chapters-done over chapters-total, declared as a BOOK fraction because a number without + // a declared unit is silently discarded (ProgressUnit). + await using var client = await _harness.ConnectAsync(McpServerHarness.TestJwt, Ct); + + var result = await CallAsync(client, "set_book_progress", Args(("slug", "dracula"), ("chapterSlug", "ch-1"))); + + AssertOk(result); + Assert.Equal("ch-2", Json(result).GetProperty("resumeChapterSlug").GetString()); + Assert.False(Json(result).GetProperty("bookFinished").GetBoolean()); + + var put = _harness.Stub.Last("set_edition_progress")!; + Assert.Equal("PUT", put.Method); + var body = JsonDocument.Parse(put.Body).RootElement; + Assert.Equal("66666666-6666-6666-6666-666666666666", body.GetProperty("chapterId").GetString()); + Assert.Equal("{\"type\":\"start\"}", body.GetProperty("locator").GetString()); + Assert.Equal(0.5, body.GetProperty("percent").GetDouble()); + Assert.Equal("book", body.GetProperty("percentUnit").GetString()); + } + + [Fact] + public async Task SetBookProgress_CatalogLastChapter_OverWire_MarksTheBookFinished() + { + await using var client = await _harness.ConnectAsync(McpServerHarness.TestJwt, Ct); + + var result = await CallAsync(client, "set_book_progress", Args(("slug", "dracula"), ("chapterSlug", "ch-2"))); + + AssertOk(result); + Assert.True(Json(result).GetProperty("bookFinished").GetBoolean()); + + var body = JsonDocument.Parse(_harness.Stub.Last("set_edition_progress")!.Body).RootElement; + // The app's own mark-as-read sentinel, and the 1.0 the server turns into CompletedAt. + Assert.Equal("{\"type\":\"end\"}", body.GetProperty("locator").GetString()); + Assert.Equal(1d, body.GetProperty("percent").GetDouble()); + } + + [Fact] + public async Task SetBookProgress_Upload_OverWire_WritesAScrollLocator_AndDeclaresTheSpace() + { + // Uploads are slug-native, and the locator has to stay in the coordinate space the reader's + // own app writes. The declared kind is what lets the write land on a book last read as an + // Original-layout PDF, whose stored position is `page:` — LocatorSpace.MayReplace drops + // an undeclared cross-space write whole. + await using var client = await _harness.ConnectAsync(McpServerHarness.TestJwt, Ct); + + var result = await CallAsync(client, "set_book_progress", + Args(("bookId", StubBackend.UserBookId), ("chapterSlug", "replication"))); + + AssertOk(result); + var body = JsonDocument.Parse(_harness.Stub.Last("set_my_book_progress")!.Body).RootElement; + Assert.Equal("replication", body.GetProperty("chapterSlug").GetString()); + Assert.Equal("scroll:replication:0", body.GetProperty("locator").GetString()); + Assert.Equal("scroll", body.GetProperty("locatorKind").GetString()); + Assert.Equal("book", body.GetProperty("percentUnit").GetString()); + } + + [Fact] + public async Task SetBookProgress_UnknownChapter_OverWire_Refused_WithoutWritingAnything() + { + // The defect this closes: an invented slug used to be stored verbatim, and every later read + // resolved it to nothing. The tool must refuse before the write, not after. + await using var client = await _harness.ConnectAsync(McpServerHarness.TestJwt, Ct); + + var result = await CallAsync(client, "set_book_progress", + Args(("slug", "dracula"), ("chapterSlug", "ch-99"))); + + Assert.True(result.IsError); + Assert.Contains("no chapter 'ch-99'", TextOf(result)); + Assert.Null(_harness.Stub.Last("set_edition_progress")); + } + + [Fact] + public async Task SetBookProgress_UpstreamRefusal_OverWire_ReportsFailure_NotSuccess() + { + // The other half of the same defect, on the server side: MayReplace used to answer (true, + // null), so the API returned 200 having stored nothing and an assistant told a person their + // progress was recorded. A non-2xx must surface as a tool error. + await using var client = await _harness.ConnectAsync(McpServerHarness.TestJwt, Ct); + + var result = await CallAsync(client, "set_book_progress", + Args(("bookId", StubBackend.RefusingBookId), ("chapterSlug", "replication"))); + + Assert.True(result.IsError); + Assert.Contains("refused", TextOf(result)); + } + + [Fact] + public async Task SetBookProgress_BothBookIdAndSlug_OverWire_Refused_NoUpstreamCall() + { + await using var client = await _harness.ConnectAsync(McpServerHarness.TestJwt, Ct); + + var result = await CallAsync(client, "set_book_progress", + Args(("bookId", StubBackend.UserBookId), ("slug", "dracula"), ("chapterSlug", "replication"))); + + Assert.True(result.IsError); + Assert.Equal(0, _harness.Stub.TotalRequests); + } + } diff --git a/tests/TextStack.Ai.Mcp.Tests/McpStdioSmokeTests.cs b/tests/TextStack.Ai.Mcp.Tests/McpStdioSmokeTests.cs index 2a8037493..aceb766f0 100644 --- a/tests/TextStack.Ai.Mcp.Tests/McpStdioSmokeTests.cs +++ b/tests/TextStack.Ai.Mcp.Tests/McpStdioSmokeTests.cs @@ -53,7 +53,7 @@ public async Task Stdio_SubprocessServer_InitializeAndListToolsReturnsWholeSurfa Assert.Equal("textstack", client.ServerInfo.Name); var tools = await client.ListToolsAsync(cancellationToken: ct); - Assert.Equal(13, tools.Count); + Assert.Equal(16, tools.Count); } // The MCP project is a ProjectReference, so its DLL is built into the test diff --git a/tests/TextStack.Ai.Mcp.Tests/StubBackend.cs b/tests/TextStack.Ai.Mcp.Tests/StubBackend.cs index 37924d5db..66d4833db 100644 --- a/tests/TextStack.Ai.Mcp.Tests/StubBackend.cs +++ b/tests/TextStack.Ai.Mcp.Tests/StubBackend.cs @@ -29,6 +29,15 @@ public sealed class StubBackend : IAsyncDisposable public const string UserBookId = "77777777-7777-7777-7777-777777777777"; public const string UserChapterId = "88888888-8888-8888-8888-888888888888"; public const string InsightId = "99999999-9999-9999-9999-999999999999"; + + // An upload whose progress write is REFUSED upstream (the shape LocatorSpace.MayReplace + // produces: a position in a coordinate space this write may not replace). It exists because a + // refusal that a tool reports as success is worse than no tool at all. + public const string RefusingBookId = "cccccccc-cccc-cccc-cccc-cccccccccccc"; + + // An edition the reader has never opened: GET progress 404s, which is not an error and must + // come back as "not started" rather than as a failure. + public const string UnopenedEdition = "dddddddd-dddd-dddd-dddd-dddddddddddd"; public const string NewHighlightId = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"; private readonly WebApplication _app; @@ -112,6 +121,9 @@ private void MapRoutes() { await RecordAsync("get_my_book", ctx); if (!HasBearer(ctx)) { ctx.Response.StatusCode = StatusCodes.Status401Unauthorized; return; } + var bookId = (string?)ctx.Request.RouteValues["id"]; + if (string.Equals(bookId, RefusingBookId, StringComparison.OrdinalIgnoreCase)) + { await WriteJsonAsync(ctx, MyBookBody.Replace(UserBookId, RefusingBookId)); return; } await WriteJsonAsync(ctx, MyBookBody); }); @@ -171,6 +183,58 @@ private void MapRoutes() await WriteJsonAsync(ctx, SavedInsightBody, StatusCodes.Status201Created); }); + // GET /me/library/shelves → the shelf, both book kinds. + _app.MapGet("/me/library/shelves", async ctx => + { + await RecordAsync("get_shelves", ctx); + if (!HasBearer(ctx)) { ctx.Response.StatusCode = StatusCodes.Status401Unauthorized; return; } + await WriteJsonAsync(ctx, ShelvesBody); + }); + + // GET /me/books → every upload, not paged. + _app.MapGet("/me/books", async ctx => + { + await RecordAsync("get_my_books", ctx); + if (!HasBearer(ctx)) { ctx.Response.StatusCode = StatusCodes.Status401Unauthorized; return; } + await WriteJsonAsync(ctx, MyBooksBody); + }); + + // GET /me/progress/{editionId} → 404 for the edition never opened. + _app.MapGet("/me/progress/{editionId}", async ctx => + { + await RecordAsync("get_edition_progress", ctx); + if (!HasBearer(ctx)) { ctx.Response.StatusCode = StatusCodes.Status401Unauthorized; return; } + if (string.Equals((string?)ctx.Request.RouteValues["editionId"], UnopenedEdition, StringComparison.OrdinalIgnoreCase)) + { ctx.Response.StatusCode = StatusCodes.Status404NotFound; return; } + await WriteJsonAsync(ctx, EditionProgressBody); + }); + + // GET /me/books/{id}/progress. + _app.MapGet("/me/books/{id}/progress", async ctx => + { + await RecordAsync("get_my_book_progress", ctx); + if (!HasBearer(ctx)) { ctx.Response.StatusCode = StatusCodes.Status401Unauthorized; return; } + await WriteJsonAsync(ctx, UserBookProgressBody); + }); + + // PUT /me/progress/{editionId} → 200 with the stored row. + _app.MapPut("/me/progress/{editionId}", async ctx => + { + await RecordAsync("set_edition_progress", ctx); + if (!HasBearer(ctx)) { ctx.Response.StatusCode = StatusCodes.Status401Unauthorized; return; } + await WriteJsonAsync(ctx, EditionProgressBody); + }); + + // PUT /me/books/{id}/progress → 400 for the refusing book, else 204. + _app.MapPut("/me/books/{id}/progress", async ctx => + { + await RecordAsync("set_my_book_progress", ctx); + if (!HasBearer(ctx)) { ctx.Response.StatusCode = StatusCodes.Status401Unauthorized; return; } + if (string.Equals((string?)ctx.Request.RouteValues["id"], RefusingBookId, StringComparison.OrdinalIgnoreCase)) + { ctx.Response.StatusCode = StatusCodes.Status400BadRequest; return; } + ctx.Response.StatusCode = StatusCodes.Status204NoContent; + }); + // POST /books/{editionId}/ask → 401 if no bearer; spoiler edition → Insufficient. _app.MapPost("/books/{editionId}/ask", async ctx => { @@ -458,6 +522,112 @@ public async ValueTask DisposeAsync() } """; + private const string ShelvesBody = + """ + { + "continueReading": [ + { + "id": "77777777-7777-7777-7777-777777777777", + "type": "userbook", + "title": "Designing Data-Intensive Applications", + "author": "Martin Kleppmann", + "coverPath": null, + "slug": "designing-data-intensive-applications", + "language": "en", + "progressPercent": 0.35, + "lastOpenedAt": "2026-09-01T10:00:00+00:00", + "createdAt": "2026-08-01T10:00:00+00:00", + "estimatedMinutesRemaining": 420, + "chapterSlug": "replication" + }, + { + "id": "33333333-3333-3333-3333-333333333333", + "type": "savedbook", + "title": "Dracula", + "author": "Bram Stoker", + "coverPath": null, + "slug": "dracula", + "language": "en", + "progressPercent": 0.5, + "lastOpenedAt": "2026-09-02T10:00:00+00:00", + "createdAt": "2026-08-01T10:00:00+00:00", + "estimatedMinutesRemaining": 120, + "chapterSlug": "ch-1" + } + ], + "recentlyAdded": [], + "quickReads": [], + "finishedThisMonth": [] + } + """; + + private const string MyBooksBody = + """ + [ + { + "id": "77777777-7777-7777-7777-777777777777", + "title": "Designing Data-Intensive Applications", + "slug": "designing-data-intensive-applications", + "language": "en", + "author": "Martin Kleppmann", + "description": null, "coverPath": null, "genre": "Computing", + "status": "Ready", "errorMessage": null, + "chapterCount": 12, "totalWordCount": 210000, + "createdAt": "2026-08-01T10:00:00+00:00", + "completedAt": null, + "progressPercent": 0.35, + "progressUpdatedAt": "2026-09-01T10:00:00+00:00", + "progressChapterSlug": "replication", + "tags": [], "suggestedTags": [], + "sourceUrl": null, "isClip": false, "isRead": false, "readAt": null, + "hasOriginalPdf": false + }, + { + "id": "eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee", + "title": "The Mom Test", + "slug": "the-mom-test", + "language": "en", + "author": "Rob Fitzpatrick", + "description": null, "coverPath": null, "genre": null, + "status": "Ready", "errorMessage": null, + "chapterCount": 9, "totalWordCount": 30000, + "createdAt": "2026-08-20T10:00:00+00:00", + "completedAt": null, + "progressPercent": null, + "progressUpdatedAt": null, + "progressChapterSlug": null, + "tags": [], "suggestedTags": [], + "sourceUrl": null, "isClip": false, "isRead": false, "readAt": null, + "hasOriginalPdf": false + } + ] + """; + + private const string EditionProgressBody = + """ + { + "editionId": "33333333-3333-3333-3333-333333333333", + "chapterId": "44444444-4444-4444-4444-444444444444", + "chapterSlug": "ch-1", + "locator": "scroll:ch-1:1200", + "percent": 0.5, + "updatedAt": "2026-09-02T10:00:00+00:00", + "completedAt": null, + "positionJson": null + } + """; + + private const string UserBookProgressBody = + """ + { + "chapterSlug": "replication", + "locator": "scroll:replication:900", + "percent": 0.35, + "updatedAt": "2026-09-01T10:00:00+00:00", + "positionJson": null + } + """; + private const string AskBody = """ { diff --git a/tests/TextStack.IntegrationTests/McpManifestEndpointTests.cs b/tests/TextStack.IntegrationTests/McpManifestEndpointTests.cs index 3c4478d57..fed5f1cab 100644 --- a/tests/TextStack.IntegrationTests/McpManifestEndpointTests.cs +++ b/tests/TextStack.IntegrationTests/McpManifestEndpointTests.cs @@ -30,6 +30,9 @@ public class McpManifestEndpointTests : IClassFixture "list_my_book_highlights", "save_insight", "get_my_insights", + "get_my_reading", + "get_book_progress", + "set_book_progress", ]; public McpManifestEndpointTests(LiveApiFixture fixture) @@ -79,7 +82,7 @@ public async Task GetManifest_ListsTheExpectedTools() cancellationToken: TestContext.Current.CancellationToken); Assert.NotNull(manifest); - Assert.Equal(13, manifest.Tools.Count); + Assert.Equal(16, manifest.Tools.Count); var names = manifest.Tools.Select(t => t.Name).ToHashSet(); Assert.Equal(ExpectedToolNames.ToHashSet(), names); diff --git a/tests/TextStack.UnitTests/McpReadToolsTests.cs b/tests/TextStack.UnitTests/McpReadToolsTests.cs index 80048fe59..c20fb5d95 100644 --- a/tests/TextStack.UnitTests/McpReadToolsTests.cs +++ b/tests/TextStack.UnitTests/McpReadToolsTests.cs @@ -84,7 +84,7 @@ public void ListTools_ExposesTheWholeSurface() var names = catalog.ListTools().Select(t => t.Name).OrderBy(n => n).ToArray(); Assert.Equal( - ["get_book", "get_chapter", "get_my_book", "get_my_chapter", "get_my_insights", "list_my_book_highlights", "list_my_highlights", "list_my_vocabulary", "save_highlight", "save_insight", "save_my_highlight", "search_books", "search_my_library"], + ["get_book", "get_book_progress", "get_chapter", "get_my_book", "get_my_chapter", "get_my_insights", "get_my_reading", "list_my_book_highlights", "list_my_highlights", "list_my_vocabulary", "save_highlight", "save_insight", "save_my_highlight", "search_books", "search_my_library", "set_book_progress"], names); } @@ -147,7 +147,7 @@ public async Task GetBook_IssuesPublicGet_NoBearer_MapsEditionIdAndMetadata() "description": "A tale.", "authors": [{ "id": "1", "slug": "lc", "name": "Lewis Carroll", "role": "author" }], "genres": [{ "id": "2", "slug": "fantasy", "name": "Fantasy" }], - "chapters": [{ "id": "9", "chapterNumber": 1, "slug": "ch-1", "title": "Down", "wordCount": 1200 }] + "chapters": [{ "id": "44444444-4444-4444-4444-444444444444", "chapterNumber": 1, "slug": "ch-1", "title": "Down", "wordCount": 1200 }] } """; var (catalog, handler) = BuildCatalog(Json(body)); @@ -169,6 +169,9 @@ public async Task GetBook_IssuesPublicGet_NoBearer_MapsEditionIdAndMetadata() Assert.Equal(1, ch.GetProperty("chapterNumber").GetInt32()); Assert.Equal("ch-1", ch.GetProperty("slug").GetString()); Assert.Equal(1200, ch.GetProperty("wordCount").GetInt32()); + // The chapter GUID save_highlight's own description tells the model to take "from get_book". + // The projection used to drop it, so following that instruction was impossible. + Assert.Equal("44444444-4444-4444-4444-444444444444", ch.GetProperty("chapterId").GetString()); } [Fact] diff --git a/tests/TextStack.UnitTests/UserBookProgressServiceTests.cs b/tests/TextStack.UnitTests/UserBookProgressServiceTests.cs index 421588496..7a19d576e 100644 --- a/tests/TextStack.UnitTests/UserBookProgressServiceTests.cs +++ b/tests/TextStack.UnitTests/UserBookProgressServiceTests.cs @@ -19,6 +19,12 @@ private sealed class Harness { public List Users { get; } = []; public List UserBooks { get; } = []; + + /// + /// The book's chapters. Progress writes are validated against these: a slug that names no + /// chapter is refused rather than stored, so a test that writes one must seed it. + /// + public List UserChapters { get; } = []; public UserBookService Service { get; } public Harness() @@ -26,12 +32,13 @@ public Harness() var db = new Mock(); db.Setup(x => x.Users).Returns(() => FakeSet(Users).Object); db.Setup(x => x.UserBooks).Returns(() => FakeSet(UserBooks).Object); + db.Setup(x => x.UserChapters).Returns(() => FakeSet(UserChapters).Object); db.Setup(x => x.SaveChangesAsync(It.IsAny())).ReturnsAsync(0); Service = new UserBookService(db.Object, new Mock().Object, TestEntitlements.Resolver); } - public UserBook SeedBook(Guid userId) + public UserBook SeedBook(Guid userId, params string[] chapterSlugs) { var b = new UserBook { @@ -45,6 +52,18 @@ public UserBook SeedBook(Guid userId) UpdatedAt = DateTimeOffset.UtcNow }; UserBooks.Add(b); + foreach (var slug in chapterSlugs) + UserChapters.Add(new UserChapter + { + Id = Guid.NewGuid(), + UserBookId = b.Id, + ChapterNumber = UserChapters.Count, + Slug = slug, + Title = slug, + Html = "

", + PlainText = "", + CreatedAt = DateTimeOffset.UtcNow, + }); return b; } } @@ -97,7 +116,7 @@ public async Task UpsertProgressAsync_ChapterBased_StillPersistsAndRoundTrips() { var h = new Harness(); var userId = Guid.NewGuid(); - var book = h.SeedBook(userId); + var book = h.SeedBook(userId, "chapter-3"); var req = new UpsertUserBookProgressRequest( ChapterSlug: "chapter-3", Locator: "word:42", Percent: 0.5, UpdatedAt: null, PercentUnit: ProgressUnit.Book); @@ -142,7 +161,7 @@ public async Task UpsertProgressAsync_NullPercent_KeepsStoredPercentAndStillMove // bottom of every chapter. var h = new Harness(); var userId = Guid.NewGuid(); - var book = h.SeedBook(userId); + var book = h.SeedBook(userId, "ch-3", "ch-4"); await h.Service.UpsertProgressAsync(userId, book.Id, new UpsertUserBookProgressRequest( ChapterSlug: "ch-3", Locator: "scroll:ch-3:1200", Percent: 0.42, UpdatedAt: null, PercentUnit: ProgressUnit.Book), @@ -162,7 +181,7 @@ public async Task UpsertProgressAsync_NullPercent_DoesNotCompleteTheBook() { var h = new Harness(); var userId = Guid.NewGuid(); - var book = h.SeedBook(userId); + var book = h.SeedBook(userId, "ch-1"); await h.Service.UpsertProgressAsync(userId, book.Id, new UpsertUserBookProgressRequest( ChapterSlug: "ch-1", Locator: "scroll:ch-1:10", Percent: null, UpdatedAt: null, PercentUnit: ProgressUnit.Book), @@ -192,7 +211,7 @@ public async Task UpsertProgressAsync_UndeclaredUnit_KeepsStoredPercentButMovesP // scale, so the position it reports is honoured and the number is not. var h = new Harness(); var userId = Guid.NewGuid(); - var book = h.SeedBook(userId); + var book = h.SeedBook(userId, "ch-1", "ch-9"); await h.Service.UpsertProgressAsync(userId, book.Id, new UpsertUserBookProgressRequest( ChapterSlug: "ch-1", Locator: "scroll:ch-1:10", Percent: 0.30, UpdatedAt: null, PercentUnit: ProgressUnit.Book), @@ -221,7 +240,7 @@ public async Task UpsertProgressAsync_ScrollWriteWithoutKind_DoesNotReplaceAPage // installed build still does. var h = new Harness(); var userId = Guid.NewGuid(); - var book = h.SeedBook(userId); + var book = h.SeedBook(userId, "2-the-mom-test"); await h.Service.UpsertProgressAsync(userId, book.Id, new UpsertUserBookProgressRequest( ChapterSlug: null, Locator: "page:16", Percent: 0.139, UpdatedAt: null, @@ -243,7 +262,7 @@ public async Task UpsertProgressAsync_DeclaredScrollWrite_ReplacesAPageLocator() // legitimately in scroll space, which is why the rule is not a ranking. var h = new Harness(); var userId = Guid.NewGuid(); - var book = h.SeedBook(userId); + var book = h.SeedBook(userId, "ch-2"); await h.Service.UpsertProgressAsync(userId, book.Id, new UpsertUserBookProgressRequest( ChapterSlug: null, Locator: "page:16", Percent: 0.139, UpdatedAt: null, @@ -265,7 +284,7 @@ public async Task UpsertProgressAsync_SameSpaceWithoutKind_IsStillStored() // of their reading positions. var h = new Harness(); var userId = Guid.NewGuid(); - var book = h.SeedBook(userId); + var book = h.SeedBook(userId, "ch-1", "ch-4"); await h.Service.UpsertProgressAsync(userId, book.Id, new UpsertUserBookProgressRequest( ChapterSlug: "ch-1", Locator: "scroll:ch-1:10", Percent: 0.1, UpdatedAt: null, @@ -310,7 +329,7 @@ public async Task UpsertProgressAsync_LaterWriteWithAnEarlierClientClock_IsStore // was silently dropped. One clock per column, and the gate is gone with it. var h = new Harness(); var userId = Guid.NewGuid(); - var book = h.SeedBook(userId); + var book = h.SeedBook(userId, "ch-1", "ch-2"); await h.Service.UpsertProgressAsync(userId, book.Id, new UpsertUserBookProgressRequest( ChapterSlug: "ch-1", Locator: "scroll:ch-1:10", Percent: 0.1, @@ -337,7 +356,7 @@ public async Task UpsertProgressAsync_ScrollWriteWithPosition_StoresAndRoundTrip { var h = new Harness(); var userId = Guid.NewGuid(); - var book = h.SeedBook(userId); + var book = h.SeedBook(userId, "2-act-i"); var req = new UpsertUserBookProgressRequest( ChapterSlug: "2-act-i", Locator: "scroll:2-act-i:4200", Percent: 0.31, UpdatedAt: null, @@ -361,7 +380,7 @@ public async Task UpsertProgressAsync_WriteWithoutPosition_ClearsTheStoredOne() // leave the row contradicting itself for as long as that device kept reading. var h = new Harness(); var userId = Guid.NewGuid(); - var book = h.SeedBook(userId); + var book = h.SeedBook(userId, "2-act-i"); book.ProgressLocator = "scroll:2-act-i:4200"; book.ProgressPositionJson = Anchor; @@ -404,7 +423,7 @@ public async Task UpsertProgressAsync_RefusedWrite_LeavesThePositionAlone() // the write, so it must not be the one field that leaks through. var h = new Harness(); var userId = Guid.NewGuid(); - var book = h.SeedBook(userId); + var book = h.SeedBook(userId, "1-intro"); book.ProgressLocator = "page:16"; book.ProgressPositionJson = null; @@ -412,13 +431,40 @@ public async Task UpsertProgressAsync_RefusedWrite_LeavesThePositionAlone() ChapterSlug: "1-intro", Locator: "scroll:1-intro:0", Percent: 0.04, UpdatedAt: null, PercentUnit: ProgressUnit.Book, PositionJson: Anchor); // undeclared cross-space - var (success, _) = await h.Service.UpsertProgressAsync(userId, book.Id, req, CancellationToken.None); + var (success, error) = await h.Service.UpsertProgressAsync(userId, book.Id, req, CancellationToken.None); - Assert.True(success); // refusals are silent + // Reported, not silent. Silence was defensible while the only callers were readers' apps, + // which cannot act on an error and would retry into it. It stopped being defensible when an + // assistant became a caller over MCP: a 200 that stored nothing has it tell a person their + // progress was recorded. + Assert.False(success); + Assert.Contains("coordinate space", error); Assert.Equal("page:16", book.ProgressLocator); Assert.Null(book.ProgressPositionJson); } + [Fact] + public async Task UpsertProgressAsync_UnknownChapterSlug_IsRefused_AndNothingIsStored() + { + // The slug used to be assigned raw. An assistant that invented one had it stored verbatim, + // and every later read — resume, shelf card, the reader itself — resolved it to nothing. + // The bookmark path in this same service has always checked; progress never did. + var h = new Harness(); + var userId = Guid.NewGuid(); + var book = h.SeedBook(userId, "1-intro"); + book.ProgressChapterSlug = "1-intro"; + + var req = new UpsertUserBookProgressRequest( + ChapterSlug: "chapter-the-model-made-up", Locator: "scroll:chapter-the-model-made-up:0", + Percent: 0.5, UpdatedAt: null, PercentUnit: ProgressUnit.Book); + + var (success, error) = await h.Service.UpsertProgressAsync(userId, book.Id, req, CancellationToken.None); + + Assert.False(success); + Assert.Contains("chapter-the-model-made-up", error); + Assert.Equal("1-intro", book.ProgressChapterSlug); + } + [Fact] public async Task UpsertProgressAsync_OversizedPosition_DropsOnlyThePosition() { @@ -427,7 +473,7 @@ public async Task UpsertProgressAsync_OversizedPosition_DropsOnlyThePosition() // percentage still land. var h = new Harness(); var userId = Guid.NewGuid(); - var book = h.SeedBook(userId); + var book = h.SeedBook(userId, "2-act-i"); var req = new UpsertUserBookProgressRequest( ChapterSlug: "2-act-i", Locator: "scroll:2-act-i:4200", Percent: 0.31, UpdatedAt: null,