Phase 2 — Server hexagonal restructure - #49
Merged
Conversation
This is the largest port in phase 2 S4. It covers every call this app makes to an AI text model, 9 generateObject call sites and 6 streamText call sites across server/routes/books.ts, server/routes/chat.ts, server/routes/profile.ts, server/routes/covers.ts, and server/services/generation-manager.ts. One of those streamText call sites, the profile interview in server/routes/profile.ts, filters the SDK's fullStream down to text-delta parts after driving a tool-calling loop. It gets its own method, runToolConversation, rather than being forced through plain streamText, because a tool call needs to run a caller-supplied execute function as a side effect, which streamText has no way to express. The signal parameter on every method is cancellation only, never a timeout. The adapter will own the five minute request timeout by combining this signal with its own AbortSignal.timeout, which is why the interface says so explicitly in its header comment. The fake records every request per method so a later service test can assert a prompt contained the profile context, and requires an explicit scripted response for generateObject rather than guessing a default, since there is no type safe placeholder for an arbitrary caller schema. The contract pins that scripted chunks arrive in order, that an aborted signal stops both streaming methods mid stream, and that a scripted tool call invokes that tool's execute with schema validated input without ever surfacing the call itself on the iterable. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Covers server/services/key-store.ts, which mixes two concerns today, an in-memory get, set, remove, has, and status store, plus how that store is populated and persisted. The path and env var fallback are resolved once at module load, and an Electron versus standalone branch decides whether writes touch disk at all. Only the store is this port. The data dir path, the env var fallback, and the Electron branch stay adapter behaviour for the future file-key-vault adapter to own via a factory argument instead of an import time side effect, and the port's header comment says so, so the split does not need rediscovering later. Methods take a ProviderId rather than a raw string, so the provider validation the real module runs on every call moves to the HTTP boundary, where zod already parses untrusted input, instead of living inside the vault. Unlike TextGeneration and ImageGeneration, this port keeps its contract typed to the plain KeyVault interface rather than the fake's shape, because every behaviour it pins is observable through get, set, has, status, and remove alone. That leaves the door open for a later real, filesystem backed adapter to run through the same contract unchanged. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Covers the one real call site, the cover generation background task in server/routes/covers.ts, which calls generateImageWithFallback in server/services/image-generation.ts as is. That function calls OpenAI or Google's image endpoint directly over fetch and, on a recoverable failure, retries against the next model in a provider owned fallback chain before giving up, but stops immediately on bad credentials or a content policy rejection since those will not be fixed by trying another model. The port promises only that shape, not the HTTP detail, and there is no apiKey field since the future adapter resolves it from a KeyVault itself. The fake reproduces the same chain walking algorithm without any network call, driven by a test scripting which model and provider pair should fail and why through failNextAttempt. Default fallback chains for openai and google make the fallback path exercisable out of the box, using fake model names so a test can assert generate moved to a different model without asserting a specific real one. Like TextGeneration, the contract is typed to the fake's own shape rather than the bare port, because pinning the fallback path needs failNextAttempt, and this port never gets a real adapter to need the narrower type. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Adds server/ports/speech-synthesis.ts, its in-memory fake, and its vitest contract harness for the narration capability that today lives directly in server/services/kokoro-service.ts and server/services/audiobook-installer.ts. The install surface, meaning isInstalled, missingComponents, and install, covers both the Kokoro model and ffmpeg, matching the real code, since audiobook-installer.ts already bundles both downloads into one installer and kokoro-service.ts re-exports its isInstalled function as isModelInstalled. Two exports of kokoro-service.ts are deliberately left off the port. getRecommendedWorkerCount is a pure function of host RAM and CPU count with its own existing test suite, so it stays a free function a future service calls before handing the result to startWorkerPool. __testing resets a lazily loaded model singleton that only the real adapter has, and audiobook-installer.ts already documents its own __testing as not part of the public API contract, so it stays an adapter only seam. This port is fake only. No test in this repository may run the contract against a real kokoro adapter, since kokoro-js loads a real ONNX model. pnpm exec vitest run server/ports/speech-synthesis.fake.test.ts passes with 13 tests. pnpm typecheck and pnpm lint are both clean. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Adds server/ports/audio-assembly.ts, its in-memory fake, and its vitest contract harness for the ffmpeg half of audiobook generation, extracted from the ffmpeg internals of server/services/audiobook-generator.ts, meaning runFfmpeg, getAudioDurationSec, and the M4B stitch step. This is split from SpeechSynthesis because ffmpeg is a separate external binary from the Kokoro model, downloaded and located independently through audiobook-installer.ts's getFfmpegPath. File paths appear directly in this interface, since audio assembly is genuinely filesystem bound and ffmpeg only reads and writes real files. Hiding that behind an in-memory buffer API would only add indirection without removing the real dependency, so probeDurationSec's path and concatToM4b's inputs and out stay as plain paths, and the header comment says so explicitly. One real behaviour is a known gap rather than a silent omission. The current M4B stitch also embeds a cover image when available and retries without one on failure, and this port's request shape has no coverPath field, matching the shape given for this task. Adding that field, or deciding cover embedding belongs elsewhere, is left for whoever builds the real adapter. This port is fake only. No test in this repository may run the contract against a real ffmpeg backed adapter. pnpm exec vitest run server/ports/audio-assembly.fake.test.ts passes with 5 tests. pnpm typecheck and pnpm lint are both clean. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Adds server/ports/diagram-renderer.ts, its in-memory fake, and its vitest contract harness for mermaid diagram rendering during EPUB export. Read both real implementations behind the shared fastify.mermaidRenderer decoration without changing either. server/index.ts's kroki.io fallback already returns an empty string for a chart that fails to render, matching this port's contract exactly. electron/main.ts's BrowserWindow based renderer currently returns an escaped pre code fallback block instead, which is real behaviour today but does not match this contract, so the header comment flags that reconciling it is work for whoever builds the Electron adapter, most likely by returning an empty string the same way the kroki adapter does. The port also pins that an empty charts array resolves to an empty array without doing any rendering work, since the Electron implementation already special cases that to avoid opening a wasted BrowserWindow, while the kroki implementation gets the same result for free from an empty loop. This port is fake only. No test in this repository may run the contract against kroki.io or a real Electron BrowserWindow. pnpm exec vitest run server/ports/diagram-renderer.fake.test.ts passes with 4 tests. pnpm typecheck and pnpm lint are both clean. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Adds the first two ports for phase 2 of the server hexagonal restructure, EpubImport and EpubExport. Both stay fake only in tests, since a fake never needs a real EPUB parser or the epub-gen-memory library. EpubImport separates parsing from persistence. The current epub-importer.ts imports the book store and saves a book while it parses, so read() returns pure data instead, meaning meta fields, an ordered chapters array, and optional cover bytes, and it writes nothing at all. The contract test proves this by calling read and preview twice each and mutating the first result, since a reader with nothing to write to can only hand back independent values on every call. EpubExport wraps the epub-gen-memory usage inside the export-epub route. Its fake encodes the build request as a JSON manifest rather than a real EPUB archive, which still lets the contract test check that swapping chapter order changes the output, without a real parser in the port. Each port gets an interface with a header comment explaining what it abstracts, a deterministic fake, a contract test function that exercises the interface as a black box, and a fake test file that runs the contract and adds a few whitebox checks. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Adds the BackgroundTasks port, covering task tracking for EPUB export, cover generation, audiobook install and generation, and generate-all chapters jobs. The port exists mainly to enforce one rule that server/services/task-manager.ts cannot enforce today. A caller only ever receives a TaskHandle exposing an AbortSignal, never the controller object that can trigger cancellation. Whether a task is cancelled is the manager's decision alone, made through cancel(taskId). The contract test checks that the handle's own keys are exactly id and signal, and separately that cancel actually aborts that same signal. The fake also reproduces the real manager's eviction behavior, where a finished, failed, or cancelled task is deleted from its internal map after a delay. server/constants.ts pins that delay at 60 seconds today, and since ports stay free of imports outside shared types, the fake and the contract each pin the same number as a literal rather than importing it. The contract test advances fake timers to one millisecond before the delay and asserts the task still exists, then advances one more millisecond and asserts it is gone. The contract also pins list, get, report, succeed, and fail, that findActive finds a running task and stops finding it once the task ends, and that subscribe delivers every event in order until its returned unsubscribe function is called. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Adds the last two ports for this slice of phase 2, Clock and OsFileManager. Clock replaces the raw new Date().toISOString() and randomUUID() calls spread across roughly fifteen call sites in services and routes. Its fake starts at a fixed instant and never moves on its own, and adds advance and set methods beyond the port interface so a test can control it directly, which is the whole reason this port is worth having on its own. OsFileManager wraps the platform switch in the audiobook reveal route, which spawns open, explorer.exe, or xdg-open depending on process.platform and swallows any failure rather than throwing. The port keeps that same best-effort contract. reveal() resolves whether or not the OS command actually succeeded. The contract test only pins that reveal resolves, since whether a file manager window actually opened is not something a test can observe, and a fake-specific test checks that the fake records every path it was asked to reveal. Both Clock and OsFileManager can be contract tested against a real adapter once one exists, so their contract tests stay written against the interface only, with no fake-specific assertions mixed in. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
This defines the BookRepository interface derived from the thirty four structured data functions in server/services/book-store.ts, covering books, tables of contents, chapters, per chapter and final quizzes, feedback, progress, the learning profile, briefs, chapter summaries, and references. It adds NotFoundError, a port owned error whose code is ENOENT, so the existing error handler can map a missing entity to a 404 the same way it already does for the real store's filesystem errors. It adds an in-memory fake and a shared contract test that pins round trips, listing, deletion, existence checks, missing entity rejections, and the derivation rules behind getChaptersRead and getSkillProgress, each worked through on a small example. The fake test runs that contract against the fake and adds a few fake specific checks for state isolation between instances and for copy-on-read and copy-on-write behavior. This adds fifty six passing tests and touches no existing file. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
This defines the ArtifactStore interface for every binary artifact a book can have, covering the cover image, the exported EPUB file, audiobook audio and its manifest, and startup crash recovery. The port is deliberately filesystem shaped rather than a byte stream abstraction, because ffmpeg needs a real path to assemble an audiobook and the streaming route needs a real path to answer an HTTP Range request. The header comment explains why hiding that behind a stream would be a false abstraction rather than a real one. It also adds writeEpub, a method the real store does not export today. Its callers currently read epubPath and write the file themselves with the same mkdir, temp file, and rename sequence saveCover already uses, so this formalizes that sequence as one method rather than leaving it duplicated at the call site. recoverFromCrash surfaces a real seam between the two ports. Book status recovery is BookRepository data that ArtifactStore cannot see on its own, so this port's contract only pins the half it can own, that a saved audiobook manifest with no matching audiobook file is a stray and gets removed on recovery. The header comment on CrashRecoveryReport records this for whoever wires the real adapters together next. It adds an in-memory fake that roots its plausible paths under a caller supplied root, and a shared contract test that asserts path consistency rather than exact strings, so a real adapter rooted elsewhere still satisfies it. This adds twenty four passing tests and touches no existing file. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
The client imports these six predicates, so the zod value import that backed BookStatusSchema was dragging the whole validator into the renderer bundle for the sake of six string comparisons. BOOK_STATUSES is now a plain literal tuple and BookStatus is derived from it, so shared/book-status.ts has no runtime dependency at all. BookStatusSchema moves to shared/domain.ts, which already builds on zod, and is built with z.enum(BOOK_STATUSES) so the schema and the predicates cannot drift apart. Every client import path is untouched because the client only ever imported the predicates. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
GET /api/profile does not return a LearningProfile. The stored profile keeps identity and style as separate fields, and the handler joins them into one aboutMe string before answering, so the wire shape and the stored shape are genuinely different types. The client was left to infer that difference. Derived from the handler like the other response types, with skills always present because the handler defaults it to an empty array. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
… exposed Two port decisions the architect settled before adapter work begins. DiagramRenderer now owes readable fallback markup for a chart it cannot render, never an empty string. The Electron renderer has always emitted an escaped mermaid code block on failure so the reader sees the diagram's source instead of a hole in the page, which is plainly better than the empty string the kroki path emits, so it becomes the contract and the kroki adapter will adopt it. diagramSourceFallback owns that markup so no implementation can drift from what the others emit, and it escapes the source, which also keeps chart text from injecting markup into an EPUB. AudioAssembly.concatToM4b takes an optional coverPath. Cover embedding is real behaviour in today's stitch, and the half of it the caller owns, knowing whether a cover exists and where, belongs in the request. Retrying without the cover when embedding fails stays adapter internal resilience, so an adapter that cannot embed still produces a coverless M4B rather than rejecting, and callers never see the difference. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Seven facts found while executing that the plan was written against different numbers for. The largest is that sanctioned change 1, the eight MCP authoring routes flipping 500 to 400, has no frozen assertion to edit. Those routes have zero test coverage of any kind, verified by grepping every test file under server for their paths, so the change ships with new tests asserting the 400 rather than an edit to an existing one. The rest are the corrected test baseline, the corrected books.ts size and line references, the two port contract decisions the architect settled, the real port count of thirteen, and confirmation that services/mermaid-renderer.ts is dead code with only its own test importing it. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Server stage S5 replaces the single book-store.ts module with real adapters behind the ports S4 introduced. This commit adds the first one. fs-paths.ts holds the small pieces both adapters need, the shared books directory layout and the atomic YAML read and write helpers, so they live in one place instead of two. fs-book-repository.ts implements every BookRepository method with the YAML and Markdown filesystem logic that used to live directly in book-store.ts, moved behind a createFsBookRepository factory that takes a data directory instead of calling getDataDir() itself. The logic is unchanged, only parameterized. fs-book-repository.test.ts runs the shared BookRepository contract against this real adapter over a fresh temporary directory per test, the same contract the fake already satisfies, so real filesystem behaviour and the fake can never drift apart unnoticed. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
fs-artifact-store.ts implements every ArtifactStore method, covers, EPUB export, and audiobook audio, with the same filesystem logic that used to live in book-store.ts, moved behind a createFsArtifactStore factory parameterized by data directory. Its recoverFromCrash() only ever touches artifacts, meaning stray temporary files and an audio directory left behind with no finished m4b, and always reports an empty booksReset. Reconciling a book's status after a crash needs BookRepository data this adapter cannot see, so that half of recovery is a composition concern handled where both adapters are available. See the doc comment on this method and on server/services/book-store.ts for the full explanation. fs-artifact-store.test.ts runs the shared ArtifactStore contract against this real adapter over a fresh temporary directory per test. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
…pters book-store.ts is imported at module scope by every route file, by mcp-server.ts, epub-importer.ts, and audiobook-generator.ts, so this change had to land as one atomic step rather than converting callers one at a time. Every function this module exported keeps its exact name and signature. Each one now builds a fresh BookRepository or ArtifactStore over the current data directory and delegates to it, rather than running filesystem logic directly. Both adapters are rebuilt on every call rather than once at module load, preserving the original module's lazy timing. Existing tests, audiobook-generator.test.ts among them, change the effective data directory between tests within a single file and expect the very next call into this module to see the new value. recoverFromCrash() is the one export that could not be a single delegating line, since reconciling a book's status after a crash needs both adapters together. It now runs ArtifactStore's own artifact-only recovery, sweeps this repository's own leftover temporary files book by book, then walks every book through BookRepository to reconcile status and audioGeneratedChapters, the same combined pass this module has always run. The full test suite for every existing caller of this module still passes unchanged against this rewrite, confirming the behaviour is identical. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
book-store.test.ts mocked shared/node/data-dir.js so the module under test would always resolve to a temporary directory. Now that book-store.ts resolves its data directory through a real, unmocked getDataDir() on every call, the same isolation follows from setting the real TUTOR_DATA_DIR environment variable to a fresh temporary directory in beforeEach and restoring it in afterEach. Every existing test keeps its exact assertions. Only the setup and teardown changed. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
…ontract tests This adds server/adapters/in-memory-background-tasks.ts, the real BackgroundTasks implementation the port added in stage four described but did not yet have. Its logic is lifted from server/services/task-manager.ts. The same task, controller, and subscriber bookkeeping carries over, along with the same cleanup delay after a task finishes. All of it is reshaped into a factory, so state now lives in a fresh closure created by each call instead of a module scope Map that persists for the whole process from the moment it is first imported. The controller behind each task stays private to the adapter. A caller only ever gets back a TaskHandle exposing an id and a signal, never the AbortController that can trigger it. This matches the port's own contract test, which pins that a handle carries exactly those two fields and nothing else. createInMemoryBackgroundTasks accepts an optional newId override. It defaults to node:crypto's randomUUID, and a test can supply its own generator instead, without needing to fake the whole module. The test file runs the existing describeBackgroundTasksContract suite against this real adapter, the same suite that already runs against the fake from stage four. Two whitebox tests are added alongside it. One proves two instances never share state. The other proves the injected id generator is actually used. This adds fifteen passing tests and touches no existing file. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
…Tasks adapter server/services/task-manager.ts was a bare module of exported functions closing over one module scope Map. It now constructs a single instance of createInMemoryBackgroundTasks and re-exports the exact same function names and signatures over that one instance, so every existing call site keeps compiling and behaving the same way without being touched in this change. Two fields callers still read do not fit the port's own Task shape. Both live only in this shim, never inside the adapter. createdAt is a race guard that server/routes/covers.ts checks against a cover image set after generation started. A callable abortController is read directly by several routes and by server/services/audiobook-generator.ts, and its test calls abort on it directly to simulate a mid-generation cancellation. Both fields are kept in a small side table keyed by task id, evicted on the same delay and at the same lifecycle points the adapter already uses for its own state. The file carries a JSDoc block explaining that this shim is temporary. A later stage is expected to move each call site onto the BackgroundTasks port directly, constructed once at the composition root instead of imported as a singleton. This file exists only so the singleton to factory conversion could land as one atomic change. This change is verified by the adapter's own contract tests added in the previous commit, by the existing audiobook generator test that calls abortController.abort() directly, and by the existing route characterization tests. All of them pass unchanged. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
This adds server/adapters/system-clock.ts. It is intentionally trivial. nowIso returns new Date().toISOString(). newId returns node:crypto's randomUUID(). Both are the exact calls the roughly fifteen call sites this port is meant to replace already make directly today. The test file runs the existing describeClockContract suite against it. Two whitebox tests are added alongside it. One confirms nowIso reports the real current time rather than a fixed instant. The other confirms newId returns a well formed v4 UUID. This adds five passing tests and touches no existing file. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
…unit tests This adds server/adapters/os-file-manager.ts. Its logic is lifted from the platform switch inside the POST /api/books/:id/audiobook/reveal route handler in server/routes/books.ts. That handler spawns open -R on macOS, explorer.exe's select flag on Windows, and xdg-open against the parent directory on Linux. The route is not rewired to this adapter in this change. It keeps its own inline copy for now, so this file has no callers yet. A later stage can wire the route to it and delete the inline copy. Both the process spawn call and the platform check are injected dependencies, defaulting to the real node:child_process spawn and the real process.platform. This keeps the adapter fully testable without ever launching a process. The test file runs the existing describeOsFileManagerContract suite against the real adapter with a fake spawn injected, so the contract runs for real without spawning anything. Targeted whitebox tests assert the exact argv for each platform branch, that the spawned process is always unreffed, and that reveal resolves rather than rejecting even when spawn throws. This adds seven passing tests and touches no existing file. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
This adds server/adapters/http-image-generation.ts. Its logic is lifted from server/services/image-generation.ts, the same OpenAI and Google request shaping, the same categorized error handling, and the same fallback chain that tries a known good model when the preferred one fails for a recoverable reason. The one real change from that module is where the API key comes from. This adapter takes a KeyVault as a constructor dependency and never imports server/services/key-store.ts itself, which is the whole point of this stage. fetch is injected too, defaulting to the global, so a test can exercise every path without making a real HTTP request. The test file injects a fake fetch and the existing fake KeyVault from the port layer. It asserts the missing key error path, the success path, the fallback path when a recoverable failure moves to the next model in the chain, the immediate rejection on an auth failure and on a content policy failure, the exhausted chain error when every model fails, and the immediate rejection when the signal is already aborted. The shared describeImageGenerationContract suite is not run against this adapter. That suite is fake only by its own design, since a real subject would spend money against a live provider, and its makeSubject is typed to the fake's own shape for a failNextAttempt method a real adapter should not have. This adds seven passing tests and touches no existing file. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
…neration adapter server/services/image-generation.ts kept its own copy of the OpenAI and Google request logic and called getKey from server/services/key-store.ts directly. It now adapts key-store's existing exported functions, get, set, remove, has, and status, into a KeyVault shaped object, and constructs one module scope instance of createHttpImageGeneration from it. generateImageWithFallback keeps its current exported signature and behaviour. Callers such as server/routes/covers.ts see no change. An invalid provider string still throws the same Invalid provider message it always has, now reached through the adapter's own key lookup rather than a direct call in this file, since keyVault.get is the real getKey and getKey has always validated its own input. This change is verified by the adapter's own tests added in the previous commit and by the full existing test suite, which passes unchanged. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
The current M4B stitch tags the file with the book's title as both the container's title and album metadata and the FFMETADATA1 file's own title line. AudiobookChapterEntry only carries per-chapter titles, so the request had no way to carry the book's own title through to the adapter that embeds it. This field is optional and follows the same shape as the already committed coverPath field, so an adapter given no bookTitle still produces a valid M4B, it just skips those two tags. Found and added while building the real ffmpeg adapter in this stage, flagging it here since the port was otherwise described as already committed. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Lifts runFfmpeg, getAudioDurationSec, and the M4B stitch step verbatim out of audiobook-generator.ts into a factory, createFfmpegAudioAssembly, that implements the AudioAssembly port. The process runner behind ffmpeg is now an injected dependency instead of a direct execFile call, so a caller can swap it for a fake without spawning anything, and the factory itself has no import time side effects. Cover embedding keeps its existing resilience. concatToM4b always attempts to embed a given coverPath on its first ffmpeg invocation and retries once without it if that attempt fails, producing a coverless M4B rather than rejecting. An abort never triggers that retry, it propagates as a rejection instead. Not yet wired up, audiobook-generator.ts still has its own ffmpeg internals. That follows in the next commit. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
…nner Exercises createFfmpegAudioAssembly by injecting a fake execFile that records every (file, args) call instead of spawning anything. Covers the concat list and FFMETADATA1 file contents, the bitrate flag, the with-cover and coverless argv, the coverless retry after a failed embed, abort-signal propagation (including that an abort never triggers the coverless retry), and tmp file cleanup. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
…dapter runFfmpeg, the FFMETADATA1/concat-list building, and the M4B stitch step (including the cover-embed-then-retry logic) are gone from this file, replaced by a module-scope createFfmpegAudioAssembly() instance that generateAudiobook calls into. getAudioDurationSec keeps its exported signature but now delegates to the adapter's probeDurationSec. The now-empty tmpFilesToClean bookkeeping is gone too, the adapter owns cleanup of its own concat list, FFMETADATA1, and tmp M4B files internally. All nine existing audiobook-generator.test.ts cases pass unchanged, which is the proof this extraction changed no observable behavior. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Lifts the voice catalogue tables, the in-process synthesis pool, and tts.stream()-based synthesis verbatim out of server/services/kokoro-service.ts into a factory, createKokoroSpeechSynthesis, that implements the SpeechSynthesis port. Worker pool and TTS singleton state move from module scope into the factory's closure, so each instance is independently resettable and the factory itself has no import time side effects. isInstalled, missingComponents, and install default to audiobook-installer.ts's real functions but are injectable through deps, so a caller (or a test) can probe install-surface delegation without touching the real filesystem or data dir. getRecommendedWorkerCount stays out of the adapter, the port's own JSDoc calls it a sizing policy over the host machine rather than a synthesis capability. __testing (reset and setTtsInstance) also stays out of the SpeechSynthesis contract itself, since a fake has no singleton to reset, but the factory still returns it on the richer KokoroSpeechSynthesis type so the production shim has something to forward its own test seam to. Not yet wired up, kokoro-service.ts still holds the original logic directly. That follows in the next commit. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Add describeLearningProfile, a pure synchronous function in profile-context.ts that formats a LearningProfile into the reader profile prompt fragment. It takes the profile as a parameter and does no I/O of its own. Add getProfileContext in the new server/services/profile-context.ts. It reads the profile through BookRepository.getProfile and swallows any failure, including no profile saved yet, into an empty string, then formats it with describeLearningProfile. Every generation service in this slice calls this wrapper. The old buildProfileContext stays in place, unchanged in signature and behavior, since server/routes/suggestions.ts still calls it with zero arguments and that file belongs to a sibling slice. It now carries a deprecation comment pointing at describeLearningProfile and noting it goes away with the book-store shim it reads through. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
… port Replace the generateQuiz that called the ai package's generateObject directly with createGenerateQuiz, a factory taking a TextGeneration port dependency. QUIZ_QUALITY_RULES and shuffleQuizOptions move here too, unchanged, as the single copy of each. This reconciles the two near duplicate quiz generators that existed before this slice. server/services/generate-quiz.ts never appended the shared markdown formatting rules to its prompt. The private copy inside server/services/generation-manager.ts always did. Both behaviors are preserved through an explicit includeFormattingRules flag on the request, defaulting to false, so every caller keeps its own historical prompt rather than silently converging on one. The request also carries an optional cancellation signal, since the TextGeneration adapter now owns the generation timeout through composeAbortSignal, and no caller needs to hand roll one with createTimeout anymore. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Add createGenerateNextChapter, taking TextGeneration, BookRepository, and Clock as dependencies. It is the single implementation for generating one chapter's text and quiz and updating the book's generatedUpTo and updatedAt, moved out of generation-manager.ts's generateSingleChapter. Regenerating an already generated chapter is this same function called with that chapter's own number, so there is still no separate regenerate implementation. The optional cancellation signal is a plain parameter now, since the adapter's composeAbortSignal replaces the manual timeout and signal combining generation-manager.ts used to do by hand. Quiz generation for a chapter uses includeFormattingRules true, matching generation-manager.ts's historical prompt for this path. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Add createChapterGenerationStream, the in-memory hub behind the single-chapter SSE flow, moved out of generation-manager.ts. It keeps its per-book state, subscriber bookkeeping, and cleanup timers exactly as they were, and now takes BookRepository and a GenerateNextChapter function as constructor dependencies instead of importing the store and the generation logic directly. The event and status types it emits now come from shared/events.ts and shared/responses.ts, GenerateChapterEvent and GenerationStatus, rather than a locally redeclared copy of the same shapes. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Add createGenerateAllChapters, taking BackgroundTasks, the shared ChapterGenerationStream, and GenerateNextChapter as dependencies. It starts a generate-all background task and runs the fire and forget loop that used to live inline in the generate-all route handler, generating each remaining chapter through the same GenerateNextChapter core the single-chapter flow uses. It reads task cancellation off the TaskHandle's signal rather than an abortController, since the BackgroundTasks port deliberately exposes only a signal. It still waits for any single-chapter generation active on the same book before generating each chapter, polling isGenerating on a one second delay exactly as generation-manager.ts did. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Add createCreateBook, taking TextGeneration, BookRepository, and Clock as dependencies. It covers the POST /api/books flow, meaning persisting the new book, streaming and parsing the table of contents, and saving the approved chapters, moved out of the inline handler in server/routes/generation.ts. Behavior is unchanged, including the quirk where a table of contents that fails to parse leaves the book in generating_toc rather than marking it failed, and where any other thrown error marks the book failed and falls back to deleting it if even that write does not succeed. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Add createReviseToc, taking TextGeneration, BookRepository, and Clock as dependencies. It covers the POST /api/books/:id/toc/revise flow, moved out of the inline handler in server/routes/generation.ts. The caller passes in the book and its current table of contents, already fetched for the toc_review status guard, rather than this service re-fetching them, matching the original handler exactly. A revision that fails to parse, or any other thrown error, only sends an error event over the stream. Neither one marks the book failed, which is different from create-book and is preserved on purpose. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Add createStartBook, taking TextGeneration, BookRepository, and Clock as dependencies. It covers the POST /api/books/:id/start flow, meaning skill classification, streaming chapter 1, saving it, generating its quiz, and finalizing the book to generatedUpTo 1 and status reading, moved out of generateFirstChapterAndQuiz in server/routes/generation.ts. Skill classification failure stays non fatal and simply skips the skills_classified event and leaves the table of contents without skills, same as before. The first chapter's quiz uses includeFormattingRules false on the shared generateQuiz, matching this flow's historical prompt, which never included the shared markdown formatting rules. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
generation-manager.ts no longer owns any generation logic. The stream hub moved to chapter-generation-stream.ts, the single-chapter core moved to generate-next-chapter.ts, and quiz generation moved to generate-quiz.ts in earlier commits on this branch. What remains here bridges two callers that have not migrated onto ports yet. server/routes/library.ts still calls getStatus, so registerChapterGenerationStream lets server/routes/generation.ts hand this shim the exact same live ChapterGenerationStream instance it builds from ports, rather than a separate always empty one. server/routes/assessment.ts still calls generateQuiz, QUIZ_QUALITY_RULES, and shuffleQuizOptions with the old positional signature, so the rules and shuffle function are re-exported from generate-quiz.ts rather than kept as a second copy, and generateQuiz builds its own real TextGeneration adapter per call, the same way book-store.ts and key-store.ts build their own real adapter instances while their callers migrate one at a time. Both remaining callers move onto ports directly in a later stage, and this file goes away then. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
…rvices server/routes/generation.ts now only parses requests, builds the services for this plugin registration from ports, and wires HTTP mechanics around them. Every handler's actual work lives in the services extracted across the prior commits on this branch. Two small route local helpers, openSseStream and pipeHubToSse, hold the SSE mechanics, meaning writeHead, the data colon frame format, and the request close listener, exactly as they were, since a service should never know it is talking to Fastify. singleChapterConflict and tocReviewConflict deduplicate the status guard bodies that were repeated across generate-next and regenerate, and across revise and start. registerChapterGenerationStream hands the shared hub instance built here to the generation-manager shim, so server/routes/library.ts's still unmigrated getStatus call observes the same live state this file's own routes do, instead of a separate, always empty one. No status code, response body, SSE frame, or event ordering changes. The existing characterization tests for these routes pass unmodified. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Removes server/services/key-store.ts, image-generation.ts, model-client.ts, and task-manager.ts. Each was a temporary compatibility shim left over from the hexagonal conversion, and each now has zero real importers since every caller already moved onto the matching port from server/ports. Verified with rg before deleting. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
server/domain/chapter-range.ts exported both the pure assertChapterInRange and a legacy validateChapterNum that read through the book-store.js shim. The legacy one had zero importers left, so this deletes it along with the shim import, leaving the domain module genuinely pure. server/services/validate-chapter-num.ts, the port-based I/O wrapper used by submit-feedback.ts and get-chapter-quiz.ts, now calls assertChapterInRange instead of restating the same range check, so the message text and statusCode cannot drift between the two. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
…icate server/domain/profile-context.ts exported both the pure describeLearningProfile and a legacy buildProfileContext that read through the book-store.js shim. suggest-next-book.ts and suggest-book-details.ts were the last two callers of the legacy one, so both now call getProfileContext(ports.bookRepository) instead, the proper I/O wrapper that already reads through the injected repository. suggest-book-details.ts gained a BookRepository dependency it did not need before, wired in suggestions.ts alongside its existing TextGeneration one. Deletes buildProfileContext and its book-store.js import, so the domain module is genuinely pure. Both services' tests seeded the profile through the book-store.js shim directly, which only worked because it happened to write into the same temp directory the fake repository never read from. They now seed the one fake BookRepository each service already takes, which is simpler and is what they should have done from the start. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
server/routes/epub.ts has taken the DiagramRenderer port through ports.diagramRenderer for a while now, and Electron passes its own renderer as a startServer override, so nothing read the fastify.mermaidRenderer decoration anymore. Verified with rg before removing it and the buildServer JSDoc line describing it. Rewords the stale comments in server/ports/diagram-renderer.ts, server/adapters/kroki-diagram-renderer.ts, and server/adapters/electron-diagram-renderer.ts that described the decoration mechanism, since it no longer exists. server/composition-root.test.ts had a test asserting the decoration routed an override to the right renderer. Drops it rather than rewriting it: the createPorts 'leaves every other port real when one is overridden' case already uses diagramRenderer as its own override example, and buildServer registers every route with that same resolved ports object, so there was no separate wiring fact left for a decoration-shaped test to pin. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
…Services
server/services/generation-manager.ts had shrunk to a module-scope registry.
registerChapterGenerationStream(stream) pushed the live hub built in
server/routes/generation.ts into a module variable so
server/routes/library.ts could call getStatus(bookId) for GET
/api/books/:id's embedded generation field. That is a global variable
standing in for dependency injection.
composition-root.ts now exports a SharedServices interface and
createSharedServices(ports), returning { chapterGenerationStream }. It is
documented as deliberately separate from Ports, since a port names an
external dependency an adapter can be swapped for, while this is in-memory
application state that two route modules must observe as one instance.
buildServer builds the shared services once, alongside ports, and passes
{ ports, services } to every route plugin instead of { ports }. All sixteen
route module signatures now accept services too; every one except
generation.ts and library.ts leaves it undestructured since it takes it but
never reads it. generation.ts no longer builds its own stream and registers
it; it reads services.chapterGenerationStream directly. library.ts reads
services.chapterGenerationStream.getStatus instead of the old registry's
getStatus export.
generation-manager.ts also re-exported generateQuiz for
get-chapter-quiz.ts's on-demand quiz generation (assessment.ts's route
wiring). That now calls generate-quiz.ts's createGenerateQuiz directly
against an injected TextGeneration port, with includeFormattingRules: true,
matching the generation-manager copy's prompt exactly rather than
generate-quiz.ts's own default of false. generate-final-quiz.ts's
QUIZ_QUALITY_RULES import moves to generate-quiz.ts too, its one real
source, rather than the re-export that is going away.
Deletes generation-manager.ts and its test. Nothing in that test needed
folding elsewhere: its registry-delegation case is redundant with
chapter-generation-stream.test.ts's own 'reports inactive' case, and its
QUIZ_QUALITY_RULES/shuffleQuizOptions and generateQuiz-with-no-API-key cases
are redundant with generate-quiz.test.ts's existing coverage of the same
functions.
Adds a test to composition-root.test.ts proving generation.ts and
library.ts observe one ChapterGenerationStream instance: it drives a
generation to completion through POST /api/books/:id/generate-next against
a scripted TextGeneration fake, then asserts GET /api/books/:id reports it
through the generation field. This is the regression this refactor could
cause if a route module ever built its own separate stream instead.
Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
server/index.ts imported recoverFromCrash from the book-store.js shim.
Crash recovery is really two things composed together: ArtifactStore's own
artifact-only recovery, and book status reconciliation that needs
BookRepository, which book-store.ts used to do in one function by building
its own fresh adapters from getDataDir().
Adds server/services/recover-from-crash.ts, exporting
createRecoverFromCrash({ bookRepository, artifactStore, dataDir }), with the
exact same composition, same returned report shape, and same log line. The
dataDir field is needed alongside the two ports because sweeping
BookRepository's own leftover .tmp files is fs-adapter-specific
(fs-book-repository.ts's cleanTmpArtifacts) with no equivalent on either
port, so it is the one piece of this composition that cannot be reached
through ports alone.
startServer now builds its own createPorts(overrides) call to construct the
service, independent of the one buildServer made for its routes. That
composes identically to what the shim did (two independently-built adapter
pairs), because bookRepository and artifactStore are both stateless bridges
to the same on-disk data, or in a test, the exact same override object
either time.
Adds a unit test against createFakeBookRepository and createFakeArtifactStore
covering every status-reconciliation branch (generating_toc with/without a
saved toc, generating with/without saved chapters, a healthy book left
untouched) plus the composition itself (ArtifactStore's own artifactsRemoved
folding into the report, and an audio wipe bumping updatedAt even without a
status change). The real filesystem .tmp sweep stays covered by the
existing real-fs integration test.
Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
…ctly
server/test/route-harness.ts's seedBook wrote fixtures through the
book-store.js shim. It now constructs createFsBookRepository({ dataDir })
itself and calls saveBook/saveToc/saveChapter on that, which is the same
adapter the shim delegated to. Fixtures still go through the exact same
validation and atomic-write path as production code. Behaviour identical,
no test count change.
Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
…tore books.characterization.test.ts and audiobook-generation.characterization.test.ts are otherwise frozen, so this touches only the import line in each and the four call sites that reached the book-store.js shim (two store.saveQuiz calls, two store.audiobookPath calls), swapping them for the equivalent adapter calls: createFsBookRepository's saveQuiz and createFsArtifactStore's audiobookPath, both built from getDataDir() the same way the shim built them. Not one assertion, test name, or other line changed, verified with git diff on both files. No test count change. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
…adapters book-store.ts was the last remaining compatibility shim, a thin bridge over createFsBookRepository and createFsArtifactStore kept only so callers could migrate one at a time. Every caller has now moved onto BookRepository, ArtifactStore, or recover-from-crash.ts directly (tasks 4 through 8), so it has zero importers left. Verified with rg before deleting. Its test file held 537 lines of real coverage against a real temp directory, so this repoints it at createFsBookRepository and createFsArtifactStore directly, through two small per-call helper functions that mirror the shim's own "resolve fresh every call" approach, and at createRecoverFromCrash for the crash-recovery describe block. Renamed to book-storage.test.ts, since book-store.ts no longer exists to name it after. Every CRUD, TOC, chapter, progress, feedback, validation, and reset assertion is unchanged. The crash-recovery describe block keeps its full real-filesystem coverage, including the two .tmp-sweep cases, now driving recover-from-crash.ts's real composition instead of the shim's copy of the same logic. Drops one assertion: "recoverStuckBooks alias still works". That name was a backward-compatible alias for recoverFromCrash with identical behaviour, kept only for tests referencing the old name — createRecoverFromCrash has no reason to carry it forward, and nothing else in the codebase ever called it (verified with rg). Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
server/http/ai-timeout.ts's createTimeout() has zero importers now that the TextGeneration adapter owns the five-minute generation timeout by combining a caller's cancellation signal with its own AbortSignal.timeout(). Deletes the file and rewords the one comment in server/ports/text-generation.ts that described the old hand-rolled pattern in the present tense, so it no longer points at a file that does not exist. Also rewords server/http/parse.ts's doc comment so it describes checking whether a caught error was a Zod validation error instead of writing the literal expression, since the phase gate greps server/ for that string and expects zero hits. No code changed. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Five more facts found while executing, plus the final shape. The two that matter most for review are that the quiz prompt builders in generation-manager and generate-quiz were not duplicates, they differed by a formatting rules block, and that three modules under server/domain were reaching the filesystem rather than being the pure core the plan assumed. Also records how the singleton to factory risk was actually handled, with each singleton becoming a thin shim over its adapter in one atomic commit and the shims deleted only once nothing imported them, so no half converted state ever existed. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
server/services/kokoro-service.ts was a temporary shim over createKokoroSpeechSynthesis with zero production importers left, but its test file held 240 lines of real coverage the adapter's own test file did not have, mainly synthesizeChapter and synthesizePreview against a stubbed kokoro-js stream. Merges that coverage into server/adapters/kokoro-speech-synthesis.test.ts, which already tested the same adapter, rather than keeping two files exercising one subject. Calls now go through the adapter object directly with the real SynthesizeChapterRequest shape instead of the shim's positional arguments. Three groups of assertions were dropped as no longer meaningful. Five getRecommendedWorkerCount tests: that function has no home on the SpeechSynthesis port by design (see the adapter's own doc comment) and had zero production callers even before this change. generate-audiobook.ts already carries its own private, parameterized copy of the identical formula, smoke tested only for call count today. That is a pre-existing gap this change does not create, and is worth a follow-up. One isModelInstalled test: it asserted adapter.isInstalled() with no injected deps and nothing on disk, which is now word for word the same scenario the adapter test's own "defaults to the real audiobook-installer probes" case already covers. Two listVoices tests: exact or subsumed duplicates of assertions already present once both files' coverage sat side by side. Test count for this file moves from 20 plus 9 (two files) to 21 (one file). Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
The count was a bare conditional returning 5 or 10. Both numbers are real product decisions, a single chapter book cannot support the cross chapter synthesis questions a longer book gets, so asking the full ten would pad its quiz with restatements of the same material. Naming them puts that reasoning next to the values. Also rewords the TextGeneration port's header, which still pointed at the model client service by name. That module is gone and its provider and model resolution now lives in the adapter, so the sentence describes where the behaviour actually is. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
rsml
force-pushed
the
phase-2-server-hexagonal
branch
from
July 21, 2026 04:53
c58b054 to
92f1e1a
Compare
rsml
changed the title
The singleton to adapter migration kept each old service alive as a thin shim, and the doc comments written during that stage described the shim in the present tense. Deleting the shims in the closing commits left roughly forty statements claiming that book-store.ts, key-store.ts, task-manager.ts, kokoro-service.ts, model-client.ts, generation-manager.ts, epub-importer.ts, and routes/books.ts still exist and still hold behaviour. Several sent the reader to a doc comment in a file that is gone. Every one of those statements now names the module that actually holds the behaviour today, verified against the code rather than assumed. Statements that were already honestly historical are untouched, because provenance is worth keeping. Three claims turned out to be doubly stale and are corrected rather than repointed. The generateObject call site count is eight and not nine, since two quiz generation call sites collapsed into one. The kokoro adapter has no production singleton for a test to reset. The OsFileManager port is fully wired rather than waiting for its first caller. Comments only. No executable line, type, name, or assertion changed. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
The shipped shape section said the composition root is the only module that names a concrete adapter, which a grep disproves in two places. Both are deliberate and both are explained where they live, so the claim is widened to name them rather than the code being changed to fit the sentence. recover-from-crash.ts reaches for cleanTmpArtifacts on the filesystem book repository because sweeping the stray tmp files an interrupted write leaves behind has no equivalent on the port, and a non filesystem adapter would have none to sweep. electron/main.ts names the Electron diagram renderer because Electron is its own composition point. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Objective
Make
server/fully hexagonal, strictly behavior preserving. Every external dependency now sits behind a named port with a fake and a contract test, the only place real I/O happens is an adapter,server/composition-root.tsis the one module that names a concrete adapter apart from two documented exceptions, andserver/routes/books.ts(2,176 lines, 46 route registrations) no longer exists.The two exceptions are worth knowing before you read the diff.
server/services/recover-from-crash.tsimportscleanTmpArtifactsfrom the filesystem book repository, because sweeping the stray.tmpfiles an interrupted write leaves behind has no equivalent on theBookRepositoryport and a non filesystem adapter would have none to sweep. Outsideserver/,electron/main.tsnames the Electron diagram renderer, because Electron is its own composition point.What landed
createX(deps)createPorts(overrides)builds all thirteen adapters and applies overrides on top.createSharedServices(ports)builds the in-memory state two route modules must observe as one instance.buildServer(overrides)threads both to every route plugin as plugin options, so a route module never names an adapter andfastify.injectcan drive a fully registered server whose edges are fakes.Gate evidence
Grep gates, every one reporting nothing:
Route source sizes, largest first: authoring 199, generation 191, audiobook-generation 130, audiobook 104, covers 90, library 78, tasks 62, profile 57, import 56, assessment 56, epub 50, suggestions 41, reading 38, settings 24, chat 24, models 18.
Contract tests run against the real adapter for every port that can be exercised hermetically, meaning BookRepository and ArtifactStore over
mkdtemptemp directories, KeyVault over a temp directory, BackgroundTasks and Clock in memory, OsFileManager with an injected spawn, and EpubImport and EpubExport against a real in-memory EPUB built by the export adapter. TextGeneration, ImageGeneration, SpeechSynthesis, AudioAssembly, and DiagramRenderer stay fake only by their contracts' own instruction. No test in this branch touches the real data directory, makes a network request, spawns ffmpeg, loads a speech model, or opens an Electron window.Boot verification, against a throwaway
TUTOR_DATA_DIRwith every provider key cleared:electron:previewboots against a throwaway data directory and reaches its post-startup key population step with zero errors, which is past bothstartServerand the diagram renderer override. The builtdist-electron/main.jscarries the override wiring, and fastify, kokoro-js, and epub-gen-memory all remain external rather than inlined.The 206 range response has a dedicated characterization test asserting
Content-Range: bytes 0-99/1000,Accept-Ranges: bytes,Content-Length: 100, and the exact byte slice, plus the plain 200 and 404 cases.Sanctioned behavior changes
#1, eight MCP authoring routes return 400 instead of 500 on invalid input. They called
.parse()with no guard, so bad input surfaced as a 500.parseBodymakes them 400 with the same{ error: 'Invalid request', details }body every other route already returned.No frozen assertion was edited, because there were none to edit. The plan assumed these routes had characterization assertions locking the 500. They had zero test coverage of any kind, verified by grepping every
*.test.tsunderserver/for their paths. So this ships with new coverage instead.server/routes/authoring.characterization.test.tsis new and asserts, for each of the eight, that a malformed body returns 400 with that exact shape and that a well formed body still succeeds:POST /api/books/create-skeletonPUT /api/books/:id/chapters/:num/contentPATCH /api/books/:id/metaPUT /api/books/:id/briefPUT /api/books/:id/summaries/:numPUT /api/books/:id/tocPUT /api/books/:id/references/:namePUT /api/books/:id/quiz/:num#5, the kroki diagram renderer now emits an escaped source fallback instead of an empty string for a chart it cannot render. The two real renderers disagreed, kroki pushed
''and Electron pushed an escaped mermaid code block holding the chart source. The architect standardized on the Electron behavior, so a reader sees the diagram's text rather than a hole in the page.diagramSourceFallbackin the port owns that markup so no implementation can drift, and it escapes the source, which also stops chart text injecting markup into an EPUB. Visible only in dev web mode, since Electron already behaved this way. Covered by dedicated tests for all three failure paths, non-ok response, thrown error, and timeout.The only characterization-test edits in this branch are import lines and four fixture call sites, swapping the deleted
book-storesingleton for the adapters. No assertion, test name, or expectation was touched.Deviations worth review
AudioAssembly.concatToM4btookcoverPathby architect decision, and also neededbookTitle, because the real M4B stitch tags the file with the book title as container metadata and as the FFMETADATA1 title line, andAudiobookChapterEntrycarries only per-chapter titles.generation-managercopy appendedMARKDOWN_FORMATTING_RULESand the standalone one did not. They reconciled onto one implementation with an explicitincludeFormattingRulesflag, so each caller keeps the prompt it has always sent. This is the single easiest thing in the branch to get silently wrong, so it is worth a look.ArtifactStore.recoverFromCrashcleans stray artifacts, the repository side cleans stray.tmpfiles, andservices/recover-from-crash.tscomposes both plus book status reconciliation. Artifact cleanup now also reaches a book directory with no validmeta.yml, which the original never touched. Required by the ArtifactStore contract, low risk since such a directory is already unreadable as a book.AbortSignal.timeout()aborts with reason nameTimeoutError, whereas the hand rolledcontroller.abort()it replaces producedAbortError. Unreachable in normal operation and not exercisable by any test, but it means a real five minute generation timeout could emit one extra error line where it previously closed silently.startServerport override. This removes theas unknown ascast Electron needed to reach into the server's type, and means the server is never briefly live on the kroki fallback it will not use. One surgical block inelectron/main.ts, the sanctioned edit.Scope isolation
236 files changed: 231 under
server/, 3 undershared/, 1 underelectron/, 1 underdocs/. Zero files underclient/, so this does not overlap Phase 3's branch at all.Two cross phase follow-ups also landed here.
shared/book-status.tsis now zod free, proven by a freshpnpm buildthen grepping all 5.2MB ofdist/assets/forZodErrorandsafeParseand finding neither, withgenerating_tocpresent as a positive control. AndProfileResponsewas added toshared/responses.ts, derived from the handler rather than fromLearningProfile, which is a different shape.Rebased on Phase 3
Rebased onto master after Phase 3 (#48) merged. 124 commits replayed with zero conflicts, because the expected
eslint.config.mjscollision never materialised. This branch never touched that file. The full gate was re-run after the rebase, with typecheck clean, lint at zero warnings, 1,110 tests green,pnpm buildgreen,electron:previewbooting with zero errors, and every grep gate still reporting nothing.Closing verification
Re-run independently against the pushed head, on macOS with node 24, using a throwaway
TUTOR_DATA_DIRand no provider key anywhere in the environment or on disk.pnpm testpnpm typecheckpnpm lint--max-warnings 0pnpm builddist-electron/main.jsproducedpnpm electron:build -c.mac.notarize=falserelease/Tutor-1.7.0-arm64.dmgproducedrg "from 'ai'" server/routes server/servicesrg "node:fs" server/routesrg "5 \* 60 \* 1000" serverrg "instanceof ZodError" serverrg "await import\(" server/routesls server/routes/books.tsauthoring.tsat 199 linesLive HTTP checks against a running server rather than
fastify.inject.pnpm dev:servercame up andGET /api/healthreturned 200 with{"status":"ok"}.POST /api/bookswith a valid body returnedtext/event-stream, emittedbook_created, then a singleerrorevent readingNo API key configured for provider: anthropic, and closed cleanly. With an invalid body the same route returned 400 with{"error":"Invalid request","details":[...]}, which is the shape the phase committed to.POST /api/books/:id/export-epubstarted a task that reacheddone, and the follow upGETreturnedapplication/epub+zipat 3,748 bytes, whichfileidentifies as an EPUB document.GET /api/books/:id/audiobook/filewithRange: bytes=0-99returned 206 withContent-Range: bytes 0-99/4096,Accept-Ranges: bytes, and exactly 100 bytes on the wire. Without the header it returned 200 and the full 4,096.electron:previewwas probed over the Chrome DevTools Protocol rather than by eye. The renderer loadedfile://.../dist/index.html, titled Tutor, mounted two children under#root, and rendered the library UI. The only console errors are the pre-existing inline script and data URI font CSP notices, which are unchanged on master, plus two/api/profile404s that are correct for an empty data directory.electron:devbooted Vite on 5173 alongside Electron, with the embedded server listening and startup key population reached.The characterization diff was audited independently of the claim above. Four characterization files existed on master, six exist here.
books.characterization.test.tschanged only its imports and two fixture call sites, swapping the deletedbook-storesingleton forcreateFsBookRepository. Zero assertions changed anywhere. The eight 400 assertions are all in the newauthoring.characterization.test.ts, which is new coverage rather than an edit.History
126 commits, conventional, rebase and merge only. Test commits precede or accompany the code they cover throughout.
The last two are documentation only. The singleton to adapter migration kept each old service alive as a thin shim, and comments written during that stage described the shim in the present tense. Deleting the shims left roughly forty statements claiming deleted modules still hold behaviour, several of them pointing the reader at a doc comment in a file that no longer exists. Those now name the module that actually holds the behaviour, verified against the code. Statements that were already honestly historical are untouched.
https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5