A connect screen on the device where the reading happens - #600
Merged
Conversation
`BookInsightsSection` called `@textstack/shared`'s `insightsApi`. Every shared API
client routes through an `authFetch` configured by `initApi()`, and only the
mobile app calls `initApi` — the web has its own client because its token lives in
a cookie, not a header. So the call threw "API not initialized" before reaching
the network, the component swallowed it with `.catch(() => {})`, and
`insights.length === 0` returned null.
The конспект section — the destination of the entire assistant handoff, the place
a conclusion written by an outside assistant is supposed to appear — has never
rendered on the web at all. Silently, for its whole life. Proved with a throwaway
test asserting the shared call rejects on web before fixing it.
Its own unit test was green throughout, because it mocked `@textstack/shared` and
thereby replaced the broken dependency with a working one. A test that mocks the
thing that is broken cannot see it.
Fixed with a web-local `api/insights.ts` on the web's own authFetch, matching how
every other web API module works. The component test now mocks that instead.
Added `noSharedApiOnWeb.test.ts`, which fails if any non-test web source imports a
shared API client — the class of bug rather than this instance. Mutation-tested:
re-adding the original import makes it fail with the file and symbol named, and
removing it makes it pass.
The `.catch(() => {})` stays: silence is right for a network failure on a
supplementary panel. It just cannot tell that apart from a wiring mistake, which
is what the import guard is for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011rgEMvYYi4Egj99dVtvm6E
Four PRs merged and deployed, and an honest list of what is left in the order it should be done — plus the three things only the owner can do, one of which (Android developer verification, 30 Sep) is unrelated to this feature and more urgent than all of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011rgEMvYYi4Egj99dVtvm6E
The web page shipped in #598; the phone had nothing. The key is account-level, so one minted here works in Claude Desktop and vice versa — which is why this is worth having regardless of how the open question about mobile connector support lands. `canConnectAssistant` is a new named capability rather than an inline check. capabilityLiterals.test.ts fails the build on `user?.isGuest`, and rightly: the reason this one is account-only is sharper than the others. A key reaches the whole library from outside the app and lives until revoked, while a guest's identity is three SecureStore keys that vanish with the app — a key minted by one would outlive every means of revoking it. apps/mobile/vitest.config.ts is narrowed to `src/lib/**/*.test.ts` and says outright that components are not in scope, so a test for this screen would not run. Everything worth asserting therefore moved to `packages/shared/src/lib/mcpConnect.ts` — the config template, the default key name, the live-key filter — where both platforms use it and it is covered. The screen stays dumb. Its tests parse the generated config as JSON rather than matching substrings: a snippet that is subtly malformed fails the client at launch with nothing naming the cause. That move also removes a real risk of drift. The template and the default name existed only in the web component; two platforms handing out two different snippets for the same key is two chances to be wrong. Networking stays per-platform on purpose. Mobile uses the shared api client it initialises; web keeps its own because its token is a cookie. Mixing those two layers is exactly the defect that kept the insights section from ever rendering on the web, and `noSharedApiOnWeb.test.ts` now guards that boundary from the other side. Strings went into shared, so the golden fixtures on both sides needed the hand edit their guards demand — 20 keys, visible in the diff rather than regenerated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011rgEMvYYi4Egj99dVtvm6E
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.
#598 shipped the connect page on the web; the phone had nothing. The key is
account-level — one minted here works in Claude Desktop and vice versa — so this
is worth having regardless of how the open question about mobile connector
support lands.
Also in here: a bug that had nothing to do with mobile and everything to do with
this feature.
The web insights section had never rendered
BookInsightsSectioncalled@textstack/shared'sinsightsApi. Every shared APIclient routes through an
authFetchconfigured byinitApi(), and only themobile app calls
initApi— the web has its own client because its token livesin a cookie. So the call threw "API not initialized" before reaching the network,
the component swallowed it with
.catch(() => {}), andinsights.length === 0returned null.
The конспект section is the destination of the entire assistant handoff — the
place a conclusion written by an outside assistant is supposed to appear. It had
never rendered on the web at all, silently, for its whole life. Its own unit test
was green throughout, because it mocked
@textstack/sharedand thereby replacedthe broken dependency with a working one.
Fixed with a web-local
api/insights.ts, plusnoSharedApiOnWeb.test.ts, whichfails if any non-test web source imports a shared API client — the class rather
than the instance. Mutation-tested: re-adding the original import makes it
fail naming the file and symbol; removing it makes it pass.
The screen
Entry from the profile menu. Create a key → shown once → copy the key, or copy a
claude_desktop_config.jsonwith the key already substituted. Server URL is shownseparately for clients that only ask for that. Revoke from the list, which says
used recently/never used— the only signal a reader has that a connector isactually reaching us.
canConnectAssistantis a new named capability, not an inline check:capabilityLiterals.test.tsfails the build onuser?.isGuest, and the reasonthis one is account-only is sharper than the others — a key reaches the whole
library from outside the app and lives until revoked, while a guest's identity is
three SecureStore keys that vanish with the app. A key minted by a guest would
outlive every means of revoking it.
Why the logic is in
packages/sharedapps/mobile/vitest.config.tsis narrowed tosrc/lib/**/*.test.tsand saysoutright that components are not in scope, so a test for this screen would not
run. Everything worth asserting therefore lives in
packages/shared/src/lib/mcpConnect.ts— config template, default key name,live-key filter — where both platforms use it and it is covered. The screen stays
dumb, and honestly uncovered rather than pretend-covered.
That also removed a real drift risk: the template and the default name existed
only inside the web component, so two platforms could have handed out two
different snippets for the same key.
Its tests parse the generated config as JSON rather than matching substrings.
A subtly malformed snippet fails the client at launch with nothing that names the
cause.
Networking stays per-platform on purpose — mobile on the shared client it
initialises, web on its own — which is the boundary the new guard enforces.
Verified
369 mobile, 706 web, 447 shared tests; tsc clean on web, admin and mobile. Golden
fixtures on both sides hand-edited, as their guards demand: 20 keys visible in the
diff rather than regenerated.
🤖 Generated with Claude Code
https://claude.ai/code/session_011rgEMvYYi4Egj99dVtvm6E