feat(mcp): inline email auth — sign in from an MCP client with no API key (#848)

[obasilakis]

Summary

Lets an external user sign in to Trinity from inside their MCP client with the existing 6-digit email code — no pre-minted API key, no web-UI visit. They install a keyless connector config, call request_login(email), then verify_login(code), and can use the exposed playbooks of every agent shared with that address.

Flag-gated MCP_INLINE_AUTH_ENABLED, default OFF. With it off, behaviour is byte-identical to today.

Implements #848 (Part B of abilityai/trinity-enterprise#118). Design signed off by Eugene on the issue, 2026-06-28.

Commits (reviewable in order)

87b2abb3	Prereq: tool-visibility gate → allow-list. Independently valuable
a22be001	Anonymous session tier + request_login/verify_login
24538559	Connector tools serve a key-bound or email-verified caller
d9722a8b	Backend internal surface + docs

Please look hardest at these three things

1. INTERNAL_API_SECRET can now act as any verified email. This is the mechanism the "session, not a minted key" design requires — an email-verified session holds no credential, so the MCP server relays over /api/internal/mcp-auth/*. Every data call re-gates on db.email_has_agent_access(agent, email) + connector-enabled, so a compromised MCP server still cannot reach an agent the asserted email cannot. But this is a genuine widening of that secret's power and deserves a decision, not just a read.

2. A latent fail-open in the tool gate (87b2abb3). The operator gate was auth?.scope !== "connector" — it admitted every scope it had not heard of, including a null context. Not exploitable today (our authenticate throws rather than returning undefined), but it is a trap door sitting exactly where this feature steps, because fastmcp@4.4.0 #createSession does:

const allowedTools = auth ? this.#tools.filter(...canAccess...) : this.#tools;

Falsy auth skips filtering entirely. And only the stateless httpStream branch rejects an authenticate() returning undefined; the stateful branch we run does not. Now an explicit OPERATOR_SCOPES = {user, agent, system} allow-list.

3. Enumeration safety on request_login. One byte-identical body across known / unknown / malformed / rate-limited / backend-threw, and no audit row — wording, status, latency or an audit entry would each be an oracle for "is this address registered" (#186). /request is also not an open email relay: a code is only generated for an address Trinity already knows, and that lookup fails closed.

Design decisions worth knowing

• Session, not a minted key, per the sign-off. Cost: FastMCP sessions are per-connection, so a client restart requires signing in again. Documented in requirements §7.6 rather than hidden.
• The tool list is identical before and after login. A session's tools are resolved once at construction with no per-session refresh API, so gating visibility on login state would need a client reconnect. Login flips behaviour, not visibility.
• scope stays "anonymous" after login — the session still holds no key and must never satisfy operatorOnly. Pinned by test.
• The agent argument is a selector, not a grant. It picks among already-authorized agents; the backend re-gates every call, so a tampered session.agents cannot widen access.
• A denial is a flat 403, not an access_requests write — diverging from the channel gate deliberately, because this gate runs per tool call and would be a spam vector. An explicit request-access affordance is noted as deferred.

Test plan

• MCP server: 120/120 pass (npm test), tsc --noEmit clean. 30 new cases across tool-visibility.test.ts + inline-auth.test.ts
• Backend: 33/33 new (pytest tests/unit/test_848_mcp_inline_auth.py)
• Full backend unit suite deterministic (-p no:randomly): 4635 passed, 1 failed
• That 1 failure (test_1474_read_boundary_z.py::test_schedules_summary_last_run_at_normalized) verified pre-existing — fails identically in a clean worktree at HEAD with zero changes from this branch
• Randomized-order flake investigated and shown pre-existing: three runs on a clean tree produced 1 / 3 / 1+2-errors across test_1081_physical_meter.py and test_subscription_auto_switch_pingpong.py. This branch does not introduce it
• Manual: set MCP_INLINE_AUTH_ENABLED=true, connect a keyless client, sign in end-to-end

Not run

/review and /cso --diff were not run as separate passes. Given this is auth code on a network-exposed surface that widens the internal secret, both are worth running before merge.

Closes #848

🤖 Generated with Claude Code

[obasilakis]

/review complete — 7 findings fixed (50c3fd11)

Two independent adversarial passes (backend + mcp-server). Two findings were confirmed empirically by the reviewers, not just by reading.

Corrections to this PR's original description

Two things I asserted above were wrong. Both are now fixed, but the claims themselves were false when I wrote them:

1. "Flag-gated MCP_INLINE_AUTH_ENABLED, default OFF" — true only of the mcp-server. MCP_INLINE_AUTH_ENABLED did not exist in the backend at all and include_router was unconditional, so a surface that bypasses the email whitelist, creates accounts and dispatches agent chat answered on every install regardless of the flag. Now genuinely gated (404, so a disabled deploy doesn't advertise it), with a parametrized test over all four endpoints.

2. "A session's tool list is frozen at construction / no per-session refresh API" — false. FastMCPSession.toolsListChanged re-filters live sessions against current auth (dist/chunk-MDIESGNI.js:548-553), fanned by addTool/removeTool (:2202-2206) — which Trinity's own feat: per-agent MCP exposure flag — dynamic dedicated tool per exposed agent #846 reconciler fires every ~20s. The static-surface design stands, but for the opposite reason: a login-keyed gate would flip non-deterministically at reconciler timing. Corrected in requirements §7.6, the feature-flow doc, and three source files.

Critical

	Finding
Cross-user disclosure	/chat scoped idempotency as agent:{name} with a caller-supplied key, so two verified users of the same shared agent shared one (scope, key) — the second got the first's response snapshot and execution_id. Reachable by accident: MCP clients derive deterministic keys from call args. The old test replayed with one identity, proving caching but never isolation.
Ungated backend	Above.

High

• /verify poisoned a shared per-IP bucket. client_ip is always the MCP server, so all users collapsed into one bucket — 30 wrong codes from one anonymous client would lock inline login out fleet-wide and burn real web logins from the same egress. Exactly the DoS SEC: /api/token rate limiter is a platform-wide DoS primitive — 4 bad attempts locks out all users for 10 minutes (AISEC-H2) #591 removed. Now account-scoped only.
• /request timing oracle, measured 1.89×. The known branch did a committing INSERT the unknown branch didn't, so identical bytes still leaked membership. Moved behind BackgroundTasks.
• /request unbounded on the unknown branch — the per-address cap sits behind the known-check. Added a global ceiling, checked before the branch so it can't become its own differential.

Medium (mcp-server)

Empty-available agent passthrough · verify_login echoing backend errors (incl. the INTERNAL_API_SECRET env-var name) to unauthenticated callers · unbounded per-session Map · no timeouts on four fetch calls · unattributable audit rows (run_playbook's playbook name was being stamped as target_type:"agent").

Worth flagging: one fix broke the feature, silently

My first attempt at the timing oracle used asyncio.to_thread. Every code send then stopped: SQLite is thread-affine, _email_is_known raised in the worker, and its fail-closed handler swallowed it into "unknown address" — a total outage presenting as "the email never arrives", with a 202 still returned because the endpoint is deliberately non-signalling. Caught only because a test asserts the send actually fires. BackgroundTasks was the right primitive.

Tests

Backend 41 (was 33) · mcp-server 125 (was 120) · full backend suite -p no:randomly: 4643 passed, 1 failed (the pre-existing test_1474, previously verified in a clean worktree).

Three learnings.md entries: idempotency scope vs. caller identity; to_thread breaking SQLite hidden by a fail-closed handler; per-IP limiters behind a single-egress proxy.

Still open

AC6 is not implemented — "the default .mcp.json snippet in docs / UI works without a pre-filled API key." build_snippets requires a key and always embeds it; no keyless variant exists, and no UI surface hands one over. The config a collaborator needs is trivial ({"mcpServers":{"trinity":{"type":"http","url":"<mcp_url>"}}}) but is documented nowhere. Every mechanism behind it works — an owner just has no supported way to hand someone a working config. Needs a decision: build the snippet variant, document it only, or split to a follow-up.

/cso --diff still not run.

[github-actions]

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

[AndriiPasternak31]

Validated via /validate-pr. Requesting changes — the blockers are mergeability and review coverage, not defects I found in the code. To be explicit about scope: I ran the process gate (traceability, docs tiering, security/packaging scans, migration parity, test presence), not a line-by-line read of the auth path, so treat the absence of code findings below as "not yet reviewed", not "reviewed and clean".

What passes

Closes #848 resolves to an open P2 type-feature status-in-progress issue, so the merge-time promotion will fire. Docs are updated on both required surfaces — docs/memory/requirements/mcp.md and docs/memory/feature-flows/mcp-connector.md. A named test file (tests/unit/test_848_mcp_inline_auth.py) is present. No schema change, so the dual-track migration rule (#1183) doesn't apply. Secrets/email/IP scans clean; no new top-level backend module or os.getenv(), so neither packaging gap applies.

Blocking

1. Conflicting with dev. Good news: the only conflict is docs/memory/learnings.md — everything else auto-merges. That makes this the cheapest of the three conflicting PRs to unblock. Worth doing promptly, because until it rebases GitHub reports no checks at all (a conflicting PR has no refs/pull/N/merge, so CI has nothing to build), and right now there is zero CI signal on a new authentication path.

2. Needs /cso --diff before it lands, not just /review. The pipeline table puts a P2 feature at review + validate, CSO recommended — but the recommendation there is scoped by risk, and this PR adds a new sign-in path reachable from an MCP client with no pre-existing API key. That is precisely the "unless auth/security" carve-out. At +3538/−129 across 34 files it's also the largest auth change on the board; under the 50-file split threshold, but large enough that a structural pass is worth its cost.

3. No reviews at all yet. This has been open since 20 Jul with nothing recorded against it.

Sequence I'd suggest: rebase (one docs file) → let CI run → /review + /cso --diff → re-request. Happy to take the re-review.

[obasilakis]

Unblocked — merged, green, re-requesting review

Thanks for the process pass, and for being explicit about its scope — that distinction is what makes blocker 3 actionable rather than decorative.

1. Conflict — fixed

Merged dev in at f03cd5cb. Note this is the current dev tip, not the v0.8.5 point the branch had been sitting behind — so what CI just built is this feature against everything that has landed since 20 Jul, not against a snapshot. As you predicted, docs/memory/learnings.md was the only conflict.

23/23 checks pass (2 skipping: auto-merge, report). That is the first CI signal this branch has ever had, so the "zero CI on a new authentication path" gap is closed — including all six pytest shards, prod-image-smoke, pg-migrations and schema-parity.

Post-merge verification beyond CI: tests/unit/test_848_mcp_inline_auth.py 41/41, mcp-server suite 125/125.

2. /cso --diff — already done, six days before your review

This one was already satisfied when you wrote the review — e0bb25a5, 21 Jul, "CSO --diff audit of #848 — no findings above gate". The report is in this PR's own diff: docs/security-reports/cso-2026-07-21.md + .json. Easy to miss among 34 files, and I'd rather point at it than have you spend the cycle re-running it.

To be clear about what that audit covered, so you can judge whether it's still sufficient: it ran against e0bb25a5, which is pre-merge. If you want it re-run against the post-merge tree before you sign off, say so and I'll do it — but nothing in the merge touched the auth path, so I don't expect a delta.

I'm not arguing the carve-out — you're right that a new sign-in path reachable with no pre-existing API key is exactly the "unless auth/security" case. It just happens to have been paid already.

3. No reviews — still open, and it's the real one

Agreed, and this is the item I can't close myself. The only code review on record is my own /review (7 findings, fixed in 50c3fd11 — the two that matter are a cross-user idempotency replay on shared agents and a backend surface that was ungated despite the PR description claiming otherwise; both are called out in the comment above, including the two false claims in my original description).

Re-requesting your review. Taking you up on the offer.

Worth your attention in particular, since they're where the design load sits:

• services/mcp_auth_service.py — the enumeration-safety contract. /request answers one constant 202 with no audit row and, deliberately, no branch-dependent work on the request path at all (the known-check, the cap read and the code INSERT all run in a BackgroundTasks task after the response is flushed, because a committing write on only one branch measured ~1.9x even on in-process SQLite). If that reasoning is wrong, it's wrong in a way I can't see from inside it.
• assert_email_may_reach_agent — the per-call gate. The internal secret authenticates the caller, never the action; every data call re-checks the asserted email's own standing. This is the load-bearing claim of the whole design and deserves an adversarial read.
• src/mcp-server/src/tools/auth.ts — session upgrade is an in-place mutation of the object FastMCP hands every tool, and scope deliberately stays "anonymous" so an inline-verified session can never satisfy operatorOnly.

[AndriiPasternak31]

Re-validated via /validate-pr. One blocker, four lines. Prior blockers (conflict,
missing /cso pass) are cleared — and the CSO commit e0bb25a5 is the last
substantive one, so it covers the whole final diff.

Verified by execution, not by reading: MCP npm test 125/125 + tsc --noEmit
clean; backend pytest unit/test_848_mcp_inline_auth.py unit/test_118_mcp_connector_oss.py
56 passed; all 23 CI checks green including the base/head regression diff
across 3 seeds.

Blocking

• Wire MCP_INLINE_AUTH_ENABLED into backend.environment AND
  mcp-server.environment in BOTH docker-compose.yml and
  docker-compose.prod.yml — four wirings, because two processes read it
  (config.py:173, server.ts:201).

  Not a grep result — this is compose's own resolution with the flag
  explicitly set by the operator:
  
  ```
  $ MCP_INLINE_AUTH_ENABLED=true docker compose -f <file> config
  docker-compose.yml       backend      MCP_INLINE_AUTH_ENABLED     = <<ABSENT>>
                           mcp-server   MCP_INLINE_AUTH_ENABLED     = <<ABSENT>>
  docker-compose.prod.yml  backend      MCP_INLINE_AUTH_ENABLED     = <<ABSENT>>
                           mcp-server   MCP_INLINE_AUTH_ENABLED     = <<ABSENT>>
     (sibling MCP_AGENT_CHAT_PULL_ENABLED = false in all four, i.e. wired)
  ```
  
  Compose reads `.env` only for `${...}` interpolation, never to inject into a
  container; there is no `env_file:` and no Dockerfile `ENV` for it either. So
  `require_inline_auth_enabled` 404s the whole `/api/internal/mcp-auth/*`
  surface, the MCP server rejects keyless connections and registers no auth
  tools, and `keyless_snippets` never appears — permanently, with no operator
  lever. It fails *safe* (it makes the CSO report's axis-1 default-safe posture
  unconditional rather than opt-in), so this is not a security issue. It's that
  3.5k lines ship unable to be switched on. Copy the sibling one line up:
  `- MCP_AGENT_CHAT_PULL_ENABLED=${MCP_AGENT_CHAT_PULL_ENABLED:-false}`.
  
  Corroboration: the one unchecked box in the test plan is
  `[ ] Manual: set MCP_INLINE_AUTH_ENABLED=true, connect a keyless client` —
  the only step that would have caught this. Neither `/verify-local` nor CI can:
  both boot at defaults, so a flag that can never be turned on boots clean and
  goes green.
  

Should land with it

• architecture.md entries for the new surface: routers/mcp_auth.py in the
  router catalog, services/mcp_auth_service.py in the service catalog,
  tools/auth.ts + a request_login/verify_login row in the MCP tools table
  (Invariant #13 makes that table the canonical third-surface record), and the 4
  /api/internal/mcp-auth/{request,verify,playbooks,chat} endpoints.

  **Do not hand-edit the module counts.** They are already stale independent of
  this PR — the doc says 63 routers / 66 services / 22 tool modules; the tree has
  68 / 96 / 29. Reconciling those is `/validate-architecture` drift and belongs in
  its own pass, not here. (I got this wrong in my first read of this PR and asked
  for `63→64`; that would have written a still-wrong number.)
  

• MCP_INLINE_AUTH_TIMEOUT_MS → .env.example (+ compose for consistency).
  Functional via its 15000 default, so lower severity — same class as the blocker.

• feature-flows.md gets no Recent Updates row despite a substantial
  mcp-connector.md rewrite.

• Correct the fastmcp version citation, and test call-time enforcement.
  The code comments in server.ts and the CSO report both reason about
  fastmcp@4.4.0 internals with line citations (dist/chunk-MDIESGNI.js:548-553,
  :2202-2206, :572-574) — but package.json pins ^4.8.0 and
  package-lock.json resolves 4.8.0. The reasoning was verified against a
  version the project does not install.

  I re-verified both load-bearing behaviours on 4.8.0, and **the conclusion is
  unchanged**:
  - The falsy-auth filter-skip is still there (`chunk-JEOCXSB4.js:1857`
    `const allowedTools = auth ? this.#tools.filter(...) : this.#tools`), so the
    `87b2abb3` hardening is genuinely warranted on the shipped version.
  - Enforcement is real, not advertisement-only — though via a different
    mechanism than "checked at `tools/call`": `setupToolHandlers(tools)` closes
    over `toolsMap = new Map(tools.map(...))` built from the *filtered* list, and
    the `CallToolRequestSchema` handler does `toolsMap.get(name)` →
    `throw McpError(MethodNotFound)`. A filtered-out tool is absent from the call
    map, not merely hidden. `canAccess` itself is never re-invoked per call.
  
  Worth a test: `tool-visibility.test.ts` exercises the predicates in isolation
  only, so nothing in the suite would catch a future fastmcp change to
  advertisement-only filtering — the exact regression the allow-list defends
  against.
  

Non-blocking

• Surface mcp_inline_auth_enabled on GET /api/settings/feature-flags; both its
  closest siblings (mcp_agent_chat_pull_enabled:183, redelivery_governor_enabled:188)
  are there as observability-only entries — useful while a posture-changing flag soaks.
• CSO report filename is cso-2026-07-21.md; convention is cso-diff-YYYY-MM-DD.md.
• Carry the report's N1 note (set INTERNAL_API_SECRET explicitly in prod rather than
  the → SECRET_KEY fallback) into the release notes now that this surface exists.
• The anonymous scope is missing from the scope table at architecture.md:1967 — but
  so is connector already, so this is pre-existing practice, not a gap this PR
  introduced. Worth adding since anonymous is the security-relevant tier.
• The /request constant-time claim holds for time-to-response, but a Starlette
  BackgroundTask runs before the connection is released for keep-alive reuse, so with
  a pooled client the known/unknown delta is in principle observable on the next
  request over the same connection. Very low signal and the global cap bounds probing —
  not worth changing, noted so the claim's boundary is on record.

Scope of this review

Process gate + a targeted read of the auth path + the library-behaviour checks above.
I did not run an independent adversarial security pass — the CSO artifact in this PR
is the author's own, and my agreement with it is read-level. If independent assurance is
wanted on a keyless auth surface, a fresh-context /cso --diff is the instrument;
/verify-local is not (nothing here is in its blast radius: no base-image, no deps, no
migrations, no new top-level backend module).

The hardening in 87b2abb3 is correct and independently valuable.
make_inline_auth_scope folding the verified email in is right — MCP clients derive
deterministic keys from call args, so two users of one shared agent collide by design,
not just by malice. resolveAgent's refusal to guard on available.length > 0 is right
for the same reason. Test coverage is the strong part: seven non-happy-path classes
(enumeration safety, verify failure modes, access gate, cross-user replay, flag gate,
internal-secret gate, global rate limit) — no surface left happy-path-only.

[obasilakis]

Feedback addressed — blocker fixed, /cso --diff re-run, and one thing you should know about the fastmcp citation

Thanks for both passes, and specifically for measuring the flag rather than grepping it. That blocker was real and I would not have found it — every signal I had was green, and the one step that would have caught it is the box I left unchecked.

Blocking: MCP_INLINE_AUTH_ENABLED — fixed, all four wirings

Verified the way you found it, with the flag explicitly set:

docker-compose.yml       backend      MCP_INLINE_AUTH_ENABLED = true
docker-compose.yml       mcp-server   MCP_INLINE_AUTH_ENABLED = true
docker-compose.prod.yml  backend      MCP_INLINE_AUTH_ENABLED = true
docker-compose.prod.yml  mcp-server   MCP_INLINE_AUTH_ENABLED = true

You were also right that neither CI nor /verify-local can catch this class — both boot at defaults, so a flag that can never be turned on boots clean and goes green. So the fix isn't just the four lines: tests/unit/test_848_inline_auth_compose_wiring.py (11 cases) asserts all four wirings, the ${VAR:-false} shape (a hardcoded value would be the same un-switchable bug with extra steps; a non-false default would make a network-exposed keyless path opt-out), and the reader set itself — so a third process reading the flag without wiring it fails there. Mutation-checked: removing one wiring fails 2 tests.

Worth flagging for the team beyond this PR: #1876 landed on dev this same week for exactly this class (CANARY_ENABLED/CANARY_SLACK_WEBHOOK_URL never reaching the prod backend), joining #1039, #1056 and ent#31. Four occurrences says the per-PR /validate-pr grep isn't catching it — and I think I see why: that check greps for new backend os.getenv(), so it structurally misses a flag whose gate lives in the mcp-server (process.env). That's how this one passed the gate you ran. Recorded in learnings.md; the generalized guard is worth its own issue rather than more per-PR greps.

The fastmcp citation — you were right, and it had rotted one version further than you found

You caught 4.4.0-vs-4.8.0. By the time I looked, the dev merge had bumped the lock to 4.12.1 — and 4.4.0 turned out to be what my stale local node_modules held, not any version the project ever resolved. So the reasoning was against a version nobody installs, twice over.

I re-verified both load-bearing behaviours on the actually-installed 4.12.1 after npm ci, and your conclusion holds exactly: the falsy-auth filter-skip is still there (chunk-5BQXF2VT.js:1998-2000), so the 87b2abb3 hardening is warranted on the shipped version. Your mechanism correction was also right and I've adopted it — canAccess is never re-invoked per call; enforcement is by absence from the toolsMap that setupToolHandlers builds from the filtered list (:1181 → :1214-1220). All citations now lead with the symbol and treat the line as an "as of" locator, since the chunk filename moves between releases.

On your "worth a test" — agreed, and that's the actual fix here. Prose citations rot at the next dependabot bump; this one rotted twice before anyone noticed. tool-visibility.test.ts now boots a real FastMCP server and drives it with a real MCP client, asserting a filtered-out tool is rejected on call and that its body never executes. Behavioural, so it survives chunk renames. Mutation-checked against the pre-#848 deny-check.

That investigation also turned up a live error in learnings.md — the entry asserting canAccess filtering happens "once at session construction, so any log-in-and-new-tools-appear design needs a client reconnect." That's false (toolsListChanged re-filters live sessions; the #846 reconciler fires it every ~20s), and it's the second false claim this same investigation produced. The static-surface design stands, for the opposite reason. Corrected, with the generalizable trap written up: a design justified by an impossibility keeps looking correct long after the impossibility is disproved.

Should-land items

• architecture.md — routers/mcp_auth.py, services/mcp_auth_service.py, the tools/auth.ts row in the MCP tools table (Invariant feat: SMARTS trading pipeline with Telegram notifications and Miro visualization #13), all 4 /api/internal/mcp-auth/* endpoints, and the scope table gains connector + anonymous. Module counts untouched per your instruction. portal_delegate (ent#163) landed on dev mid-merge so the union carries all three — and it's the cleanest illustration of why the deny-check shape was untenable: not an MCP tool principal at all, yet !== "connector" would have advertised it every operator tool the day it appeared.
• MCP_INLINE_AUTH_TIMEOUT_MS — documented + wired, mcp-server only. The backend never reads it, so wiring it there would encode a wiring that shouldn't exist; the test asserts that asymmetry holds.
• feature-flows.md — Recent Updates row added.
• mcp_inline_auth_enabled on feature-flags — added, mirroring both siblings. It's also the cheap post-deploy check that the two halves agree, which is the confirmation that was missing here.
• CSO filename → cso-diff-2026-07-21-848.md (-848 matches cso-diff-2026-05-12-678.md precedent).
• N1 → carried into .env.example and requirements §7.6. There's no unreleased-notes file (/release generates at cut time), and .env.example is where an operator actually looks when enabling the flag.

/cso --diff re-run: cso-diff-2026-07-30-848.md

0 findings above the 8/10 gate. Not a formality — as you implied, the prior audit's default-safe posture was unconditional rather than opt-in because the flag couldn't be turned on. Now it can, so every gate previously "correct but unreachable" is live and I re-verified it against code: router-level gating of all 4 routes, assert_email_may_reach_agent before the idempotency claim and fail-closed, uniform 403 on both denial reasons, account-only limiters that provably never touch _ip_key, constant-time /request, name-only build_tool_description (#846), OPERATOR_SCOPES. Guards: 38 passed.

One real defect found and fixed (N2). routers/mcp_auth.py claimed the branch-dependent work runs "in a worker thread." It does not and must not — asyncio.to_thread there silently stops every code send (SQLite is thread-affine, _email_is_known raises in the worker, its own fail-closed handler swallows it into "unknown address") while the endpoint keeps returning 202. The router is the file you open first, so the natural "make these consistent" edit was to reintroduce a total silent outage. Not a security finding — the invited failure is availability, not a bypass — but a landmine on the enumeration-safety contract, and it was there because I wrote the router docstring before reverting the to_thread attempt.

Stated plainly in the report: this is me auditing my own change on a keyless auth surface. Same caveat as the 07-21 artifact. You named this as the outstanding gap and it remains outstanding — a fresh-context /cso --diff by someone who didn't write it is the instrument, and I can't self-serve it.

Also

Re-merged dev (it had moved 11 commits and gone CONFLICTING again). Three doc conflicts, all append-point collisions, resolved in date order.

Also closing your /review follow-up: AC6 is implemented — build_keyless_snippets + the ConnectorChannelPanel.vue surface exist and are flag-gated. My earlier "AC6 is not implemented / needs a decision" note is stale; no decision needed.

Verified: backend unit 5795 passed / 0 failed (the pre-existing test_1474 failure I'd flagged now passes — fixed by the dev merge, so that caveat is retired). mcp-server 127/127 (was 125) + tsc --noEmit clean, both against the real 4.12.1 after npm ci.

One note on the #1360 "newest ~20" cap in feature-flows.md: it's at 57 rows. Pre-existing drift, same category as the stale module counts — left alone.

Re-requesting review.

[obasilakis]

Re-conflicted against 78 new dev commits since your last pass, so this is a re-merge plus a walk of the checklist. Everything below is verified, not asserted.

The blocker

MCP_INLINE_AUTH_ENABLED is wired in all four places. Verified with your own instrument — compose's resolution with the flag explicitly set, not a grep:

docker-compose.yml       backend      MCP_INLINE_AUTH_ENABLED = "true"
docker-compose.yml       mcp-server   MCP_INLINE_AUTH_ENABLED = "true"
docker-compose.prod.yml  backend      MCP_INLINE_AUTH_ENABLED = "true"
docker-compose.prod.yml  mcp-server   MCP_INLINE_AUTH_ENABLED = "true"

tests/unit/test_848_inline_auth_compose_wiring.py is the guard, so a future compose edit that drops a wiring fails CI rather than shipping an un-switchable flag again.

Should-land items

• architecture.md — routers/mcp_auth.py in the router catalog, services/mcp_auth_service.py in the service catalog, auth.ts + a request_login/verify_login row in the MCP tools table, the four /api/internal/mcp-auth/* endpoints, and the anonymous scope row. Module counts left alone per your correction.
• MCP_INLINE_AUTH_TIMEOUT_MS → .env.example + both compose files.
• feature-flows.md — Recent Updates row added.
• fastmcp citations — corrected, and dev has since moved the pin again: package.json is now ^4.12.1 and the lock resolves 4.12.1, which is what the comments in server.ts and tool-visibility.test.ts cite. Both load-bearing behaviours re-checked there.
• Call-time enforcement is now tested. tool-visibility.test.ts boots a real FastMCP httpStream server with the real makeOperatorOnly/anonymousOnly predicates and drives it with a real MCP client, asserting a filtered-out tool is not merely hidden but uninvokable. Per your note it encodes the observable outcome, not the toolsMap mechanism — a fastmcp refactor that preserves enforcement stays green, one that drops it goes red.

Non-blocking

• mcp_inline_auth_enabled surfaced on GET /api/settings/feature-flags alongside its two siblings.
• CSO reports renamed to convention: cso-diff-2026-07-21-848.{md,json}, plus cso-diff-2026-07-30-848.{md,json} from the re-audit.
• N1 carried into docs/memory/requirements/mcp.md as an operational prerequisite (there is no open release-notes file — 0.8.5 is already cut — so it lands at the next cut from there).
• Your /request keep-alive note is on record and unchanged; no code change.

Merge resolutions

Six conflicts, all additive on both sides, both kept:

• main.py — mcp_auth_router + your agent_mcp_key_router (feature: first-class agent-scoped MCP keys — scope visibility, regeneration, and validation/enforcement of agent-key usage from agent containers #1854); both imports, both include_routers.
• models.py — both model blocks.
• types.ts — union widened to … | "portal_delegate" | "anonymous", keeping the feature: first-class agent-scoped MCP keys — scope visibility, regeneration, and validation/enforcement of agent-key usage from agent containers #1854 exhaustiveness comment verbatim. Worth noting the two changes agree: feature: first-class agent-scoped MCP keys — scope visibility, regeneration, and validation/enforcement of agent-key usage from agent containers #1854's "anything not listed must be treated as least-privileged, never as user" is the same argument as feat: MCP inline email auth — frictionless onboarding without a pre-existing Trinity account #848's OPERATOR_SCOPES allow-list, arrived at from the backend side.
• architecture.md — dev and this branch each added a connector scope row independently (neither existed at the merge base, as you spotted). Merged into one row carrying your two-route specificity and the connectorOnly/playbook-allow-list wording; both trailing paragraphs kept, since they cover different layers — mine the MCP-side OPERATOR_SCOPES gate, yours the backend-side guard allowlists.
• feature-flows.md, learnings.md — both entry sets.

Verification

• All 23 CI checks green on the merge commit, including the base/head regression diff across 3 seeds.
• Local pre-push: MCP tsc --noEmit clean, npm test 140/140; backend test_848_mcp_inline_auth + test_848_inline_auth_compose_wiring + test_118_mcp_connector_oss + test_186_enumeration_uniformity + test_models_centralized 100 passed; and your side of the merge — test_1854_agent_mcp_key + test_1310_auth_wiring + test_1310_auth_consolidation + test_1483_route_order — 95 passed, 1 skipped.

Re-requesting. The one thing this pass does not add is the independent adversarial security pass you flagged as optional — the CSO artifact in the PR is still the author's own.

[AndriiPasternak31]

Approving. Both prior blockers are cleared — verified by execution, not by reading the comment. This round I did the thing I owed you: the adversarial read of assert_email_may_reach_agent you asked for. It found two gaps. Neither is a blocker on this PR, and the reason why is the substantive part of this review.

Prior blockers — cleared

Compose resolution with the flag explicitly set, all four wirings:

docker-compose.yml       backend     ENABLED=true  TIMEOUT=<<ABSENT>>
docker-compose.yml       mcp-server  ENABLED=true  TIMEOUT=9999
docker-compose.prod.yml  backend     ENABLED=true  TIMEOUT=<<ABSENT>>
docker-compose.prod.yml  mcp-server  ENABLED=true  TIMEOUT=9999

The TIMEOUT asymmetry is correct and I checked it rather than assumed: no backend source reads MCP_INLINE_AUTH_TIMEOUT_MS, and test_848_inline_auth_compose_wiring.py pins that. Wiring it backend-side would encode a wiring that shouldn't exist.

Should-land items all present: architecture.md router catalog (:96), service catalog (:164), MCP tools table (:319), all four endpoints (:1181-1184), anonymous scope row (:2191); .env.example both vars + the N1 note; feature-flows.md row (:38); tool-visibility.test.ts boots a real FastMCP server and drives it with a real MCP client. Local: 96 passed across the four backend suites; 23/23 CI green.

The adversarial read: what "the email's own standing" does not cover

Your framing is that the internal secret authenticates the caller and the asserted email's own standing in Trinity authorizes the action. That claim holds for the grant — revoke a share and the next call fails, because assert_email_may_reach_agent re-reads it. It does not hold for account state. Two gates that every other sign-in path applies are absent here, and I confirmed both by execution rather than inference.

1. users.suspended_at (#995) does not reach this path. Seeded a suspended user on an agent's sharing allow-list, connector enabled, and drove the real flow:

[1] code row issued to SUSPENDED address: True
[2] /verify status=200 {"verified":true,"username":"suspended@example.com","agents":[{"name":"agent-a",...}]}
[3] users.suspended_at = '2026-02-01T00:00:00Z'
[GATE] assert_email_may_reach_agent ADMITTED suspended user

#995 is enforced only in dependencies.py:438 (JWT) and :460 (MCP key). This surface authenticates via verify_internal_secret and never reaches get_current_user; email_has_agent_access (db/agent_settings/sharing.py:235) resolves role/owner/sharing and never reads suspended_at.

2. The #5 MFA gate is not called. With a provider that requires a second factor:

[A] mfa_gate.gate_login -> CHALLENGE (2FA required)
[B] inline /verify status=200 {"verified":true,...,"agents":[...]}

gate_login has exactly two callers — routers/auth.py:374 (admin) and :649 (email). mcp_auth_service.verify_login_code reimplements the post-code half of the login path (get_or_create_email_user → update_last_login → return) and stops one step before the gate. So the inline path accepts the same first factor the web path deems insufficient on its own.

Why neither blocks

I went looking for the precedent before writing this up, and it changes the verdict. Telegram (telegram_adapter.py:825) and WhatsApp (whatsapp_adapter.py:654) already have the identical email-code inline login: same db.verify_login_code, same email_has_agent_access gate, no mfa_gate, no suspended_at. Both are worse than this PR on persistence — they write the verified email to telegram_chat_links / whatsapp_chat_links, so the binding survives restarts indefinitely, whereas #848's dies with the connection.

So this is a pre-existing platform class, not a defect this PR introduces. The PR docs explicitly cite Telegram's inline /login as the model (tools/auth.ts:25-30), and it followed that model faithfully and in a more conservative form. Holding it to a standard three existing surfaces don't meet would be the wrong call.

Worth a follow-up issue covering all four surfaces at once. The natural fix is one predicate — email_has_agent_access rejecting a suspended principal — which closes channels, public links and inline auth together; the MFA half is a separate decision, since an inline caller can't complete a challenge and the honest behaviour is probably to refuse and redirect to the web flow. Both are cheap, and neither belongs in this PR. Happy to file it if you'd rather not.

One thing to note for that issue: an inline session is never re-validated after verify_login (verifiedEmail is written once at tools/auth.ts:269 and has no TTL), so the per-call backend gate is the only revocation point. That's a good design — it just means whatever check gets added has to live in the gate, not only at verify, or suspension won't terminate an established session.

Non-blocking

architecture.md:1183 documents /api/internal/mcp-auth/playbooks as GET; routers/mcp_auth.py:190 defines it as POST. The three siblings are right.

What I checked that's correct

I went hunting for the fail-open you'd expect in this shape and found you'd already closed it: dynamic-agents.ts moved the #846 dedicated chat_with_<slug> tools from connectorDenied to operatorOnly. A deny-check there would have advertised every exposed agent's chat tool to the new anonymous tier — the same trap as the original operator gate, one layer down. That's the catch I'd have led this review with.

Also verified rather than assumed: authenticate mints an anonymous context only on an absent credential and still throws on an invalid one (server.ts:279-299); makeOperatorOnly is a genuine allow-list and requireAnonymous re-checks at execute time, so canAccess being advertisement-shaped doesn't matter; verifiedEmail() is gated on isAnonymous, so a connector key can't spoof it; unverified anonymous sessions hit NeedsLoginError rather than falling through; make_inline_auth_scope folds the email in, which is the right fix for the cross-user snapshot replay and matches derive_payment_key's shape.

The /request enumeration reasoning holds. Deferring all branch-dependent work rather than just the send is the correct reading of #186 — the oracle is the timing, not the body — and BackgroundTasks-not-to_thread is right for the reason you documented.