Repository navigation
refactor(architecture): Shared stream contracts, per-stream Redis channels and extraction race fixes - #76
Merged
Conversation
- Rewrite docs/reference/architecture/overview.md from the code: packages, hosted vs local, API module map with the gate/turn/store/bus rules, and a reply sequence diagram. packages/ARCHITECTURE.md now points there and labels the old PNG as aspirational. - CLAUDE.md: real module tree and web routes, architecture rules, Redis roles, refresh tokens are not rotated, MCP at packages/mukti-mcp. - Knowledge-tracing README: replace the removed cache with the KnowledgeStateStore consistency model. - Drop the removed cache-stats endpoint from RFC-0001 and RFC-0007; add a code-drift note to the RFC-0007 design. - Fix broken mukti-mcp-server links in README, RFC-0001 and the MCP package docs.
- thought-map dialogue queue spec: expect the scaffold-state guard argument, and test model validation through process() now that it lives in SocraticTurnEngine. - e2e: the old suites never ran (ESM uuid/@scalar not transformed) and asserted a removed GET / controller. New app.e2e-spec boots startApi in local mode on an embedded MongoDB and checks the versioned prefix, health, helmet, CORS, the error envelope, the removed cache-stats route, thought-map->canvas via service, and that 12 concurrent knowledge updates on one concept all land. - That concurrency test found the CAS retry cap of 5 was too low under burst load (11/12 landed). KnowledgeStateStore now backs off with jitter, allows 20 conflicts, and does not count record creation against the budget. - CI runs the e2e target in the quality gate.
…muktiai/contracts New type-only workspace package packages/mukti-contracts (private, never published, Nx type-check target). API and web import it with import type, so it is erased from runtime JS. Contracts now also pin the branch-suggestion and extraction streams. API services alias them. The web drops its looser suggestion type and the casts that hid drift from tsc, and its compile-time check asserts both streams plus that suggestedType stays renderable. Mutation-tested: adding, removing or renaming a payload field, or widening or narrowing suggestedType, each fails web tsc. The published @muktiai/api ships only dist/start.d.ts, its public entry, so internal declarations never reference the private package (verified with npm pack). Dockerfiles copy the new manifest so bun links it. Docs updated.
Each stream now publishes on its own channel (mukti:stream:{namespace}:{key}) instead of the shared mukti:stream-events, so an event reaches only instances that hold a connection for that stream rather than every instance.
StreamBus gains track/untrack. The chat, dialogue and extraction stream services call track on their first local connection for a key and untrack when the last one closes (remove or cleanup). RedisStreamBus reference-counts these and subscribes or unsubscribes the channel; the in-process bus ignores them.
Order within a job is unchanged: one publisher per process and Redis keeps per-publisher, per-channel order.
The fake Redis in the spec now honours channel subscriptions, and a new extraction cross-instance test closes a coverage gap. Removing any single track or untrack call fails the spec.
Checked against a real redis-server: 201 events in order, no leakage to other instances, unsubscribe on last close, and resubscribe after the server kills pub/sub clients.
…cribes
With per-stream channels an instance receives a stream's events only after its SUBSCRIBE lands. Map extraction jobs are enqueued before the client opens the SSE, so a fast job could publish preview and complete in that gap and the client would hang with no result. Reproduced against a real redis-server: the client received nothing.
StreamBus.track now returns a promise that resolves once the subscription is in place (one shared subscribe per stream; resolves even if the subscribe fails, after logging). MapExtractionService.addConnection returns { close, ready } and each connection holds its own bus reference.
The extraction SSE waits for ready, re-fetches the job, and if it completed or failed in the meantime replays the stored result, skipping event types already delivered live and dropping live events after the replay. The finished-job replay used by late subscribers is now one helper shared with this path.
Chat and dialogue are unchanged: their jobs are enqueued after the stream is open, and suggestions poll BullMQ.
Tests: controller race cases (completed, failed, still running, live complete first, live preview then replay) and bus readiness cases. Removing the re-check, the dedupe or the end guard each fails the suite. The real-Redis repro now delivers preview and complete.
The extraction SSE re-check (047f9da) loads the BullMQ job, then asks getState(). getState() reads Redis live, but returnvalue and failedReason are the snapshot taken by getJob(). A job that completes between the two reports "completed" with no returnvalue, and the client got "Extraction job result missing mapId" instead of its map. Found by a hosted acceptance run: two real API processes (node dist/main) sharing MongoDB and Redis, instance A behind a 40 ms Redis link, jobs enqueued on B and streamed from A. 2 of 3 extractions failed that way before this change; 9 of 9 succeed after it across three runs. The replay helper now reloads the job once when the state is final but the snapshot has no result yet. Covered by a controller test that fails on the previous code.
…ithout OAuth secrets AuthModule picks its providers from isLocalMode() at import time. The static import ran before beforeAll set MUKTI_LOCAL, so GoogleStrategy was registered and threw in CI, where there are no Google credentials. Locally, .env hid this.
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
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.
Summary
Follow-ups to the architecture fixes already on
main(through7456728): shared SSE contracts, per-stream Redis channels for the stream bus, two extraction race fixes, plus docs and a real e2e suite.Changes
@muktiai/contracts(packages/mukti-contracts): new private, type-only package. It defines the chat, node-chat, suggestion and extraction stream events. The API's stream services use it directly, and the web's casts that hid mismatches are gone. A compile-time check in the web failstscif a field changes. Both Dockerfiles copy its manifest.mukti:stream:<kind>:<id>), instead of every instance receiving every event onmukti:stream-events. Event order within a job is kept.test/app.e2e-spec.ts, which boots the realstartApiagainst an embedded MongoDB. CI now runs it. This suite found a lost-update bug in the knowledge-state store's retry loop, which is fixed here.docs/reference/architecture/overview.mdfrom the code with Mermaid diagrams. CorrectedCLAUDE.md, the knowledge-tracing README and RFC-0001 and RFC-0007. Fixed broken MCP links.bun.lock(ioredis).Behaviour changes
main, noted here for reviewers).GET /knowledge-tracing/admin/cache-statsis removed.Verification
nx type-check,lintandtestpass for the API, web and contracts projects.next buildpasses.npm packships onlydist/start.d.tsas the API's typings. I ran the Dockerfile build stages by hand on a cleangit archive. I did not run an actualdocker buildbecause the Docker daemon was down.