Repository navigation
feat(GTM-36): use the hosted agent in the web app for MozFest - #1353
Conversation
Seed managed-http agents from discovery into agents_system and talk to them through a new managed-http adapter, which posts the chat body to /v1/agent/chat through the authenticated app client and hands the UI message stream back unchanged. Attachments and quotes are hydrated into the text and file parts the endpoint accepts, assistant parts are filtered to text, and history is capped at 200 messages client-side. Rejected requests are serialized like the built-in pipeline's, so quota and rate-limit errors render normally. New chats resolve their agent as thread, then the user's remembered pick, then the agent discovery names as defaultAgentId, including chats opened before discovery returns on a first visit. Anonymous sessions now run discovery so MozFest visitors see the agent; deployments without ALLOW_ANONYMOUS_AGENT_DISCOVERY answer 403 and the list is cleared as before. The model picker stays hidden for the hosted agent, which chooses its own model.
Semgrep Security ScanNo security issues found. |
|
Preview environment destroyed 🧹 Stack |
PR Metrics
Updated Mon, 12 Oct 2026 20:02:01 GMT · run #3414 |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔭 thunder-deep-review (advisory)
Complements the other bots — surfaces only what they did not flag. Never approves, never requests changes, never gates merge.
head: 5b70788e874e · mode: deep · deferred 0 item(s) already reported by other bots (best-effort dedup)
Discovery could land while a new chat was hydrating, after the system agents were read but before the session existed, so a first-time MozFest visitor opened on the built-in agent. The chat store now keeps the discovered default and hydration reads it after its last await. Signing in started a second discovery without stopping the anonymous one, so a late anonymous 403 could clear the signed-in agents. Each bootstrap run now aborts its predecessor, and the seeder checks the signal before writing anything. A late default no longer replaces an agent the user picked in any unsent chat (agentChosenByUser), and the hosted-agent request filters parts and drops emptied messages before trimming, so the history always starts on a user turn.
There was a problem hiding this comment.
🔭 thunder-deep-review (advisory)
Complements the other bots — surfaces only what they did not flag. Never approves, never requests changes, never gates merge.
head: 8a20622c51b8 · mode: deep · deferred 0 item(s) already reported by other bots (best-effort dedup)
…t default The hosted-agent adapter built its URL with new URL(path, cloudUrl), which dropped a reverse-proxy prefix and made the request fail the backend check that attaches X-App-Version and X-App-Language. It now joins the discovery path under the backend root the same way the auth context derives it. When discovery answers 401/403 or names no default, unstarted chats that an earlier call moved onto the hosted agent kept pointing at it. A new store action, clearDiscoveredDefaultAgent, forgets the default and sends those chats back through resolveSessionAgent, leaving picked chats alone. resolveSessionAgent moves to its own module so the store can import it without a cycle, and loadAllAgents gives the bootstrap effect the same ordered agent list the hook uses.
There was a problem hiding this comment.
🔭 thunder-deep-review (advisory)
Complements the other bots — surfaces only what they did not flag. Never approves, never requests changes, never gates merge.
head: e508f5e18c0b · mode: deep · deferred 0 item(s) already reported by other bots (best-effort dedup)
Additional notes (couldn't anchor to a diff line)
src/hooks/use-bootstrap-system-agents.ts— 📐 Convention — A refresh that already wrote its rows can drop its wire-identity changessrc/db/seeding/seed-agents.ts— 📐 Convention — The discovery filter changed from an allowlist to a denylist
darkbanjo
left a comment
darkbanjo
left a comment
There was a problem hiding this comment.
did a deep pass, comments are tagged blocking / convention / nit. e508f5e covered the url prefix and the lost-default case, nice. a few things outside the diff:
- naming: the same setting is
rememberedAgentId,lastUsedAgentIdandselectedAgent,isOnFallbackcovers two meanings,'superseded'is really'aborted', andisAnonymousis a dep the effect never reads. - docs this makes false:
acp-agents.md's backend section (anon 403 at :246, haystack as the only managed provider at :272),configuration.md:201overstates who lands on the agent (anyone with a past pick keeps it),customize.md:84says the built-in is the default,multi-device-sync.md:70/export-format.md:75still say acp-only, and the e2e spec's hook pointer is stale. - hosted turns get stamped
engine: 'acp', and settings > agents shows the raw/v1/agent/chatas the endpoint. - separately, worth a ticket: a server can rewrite other plaintext routing columns the same way (
chat_threads.agent_idpointed at the hosted agent,models.provideroff tinfoil) and route content to itself. bringing routing columns under the e2ee map or the aad would close the class.
Blocking: the managed-http branch is taken only for system agents and a
synced custom row's type is clamped, seeding is back to an allowlist with an
exhaustive dispatch, the discovered default applies only to brand-new chats
(a persisted thread keeps its agent and a model pick counts as choosing the
built-in), applyDiscoveredDefaultAgent re-runs resolveSessionAgent against
the refreshed rows, and the request body is { id, role, parts } only so
encrypted metadata never leaves the device.
The hosted agent takes no attachments for now: the composer hides the attach
control and ignores dropped or pasted files for managed-http agents, and
files already in history are dropped with a could-not-be-delivered note. The
history rules (cap, per-role parts, start and end on user) live in
shared/hosted-agent-history.ts and are imported by the adapter and the
backend. The discovery url must start with a single slash and resolve to the
backend origin. AGENT_RUN_IN_PROGRESS gets its own copy and a delayed retry.
Hosted threads are created with isEncrypted 0, since that flag means
confidential inference and hosted content reaches the backend.
Also: naming unified around the remembered agent, the mozfest mention and
the dead adapter option removed, the hydration race test made real, shared
AgentType reused, bootstrap failures logged, and the docs Jordan listed
corrected.
…frontend-hosted-agent # Conflicts: # backend/src/hosted-agent/history.ts
|
Thanks for the deep pass. Everything tagged is addressed in 08c6e86 (main merged in aa16e55) and answered on its thread. The items outside the diff:
Two threads are left open for you because they carry product decisions: the default applying only to brand-new chats with no disclosure work for MozFest (every user there is new), and attachments being off on the hosted agent for now rather than building the byte budget. The pre-push reviewer also found two gaps I ticketed rather than fixed, both unreachable while the built-in agent is disabled: switching a private thread onto a non-built-in agent is unguarded (as it already was for ACP), and switching a hosted thread back to the built-in with a confidential default model throws on the next send. |
Folds in GTM-38's coded AGENT_FILES_UNSUPPORTED rejection so a request carrying a file part in any message gets the 400 and the agent_files_unsupported log before the shared rules or admission run, and keeps the backend's Fireworks wording in the self-hosting doc.
…frontend-hosted-agent # Conflicts: # backend/src/hosted-agent/history.test.ts # backend/src/hosted-agent/history.ts # backend/src/hosted-agent/routes.test.ts # docs/self-hosting/configuration.md
|
Since the round: #1359 (GTM-38) merged, so this branch now has main merged in (b3ac7f3) with one reconciliation commit (e90e086). The hosted-agent backend rejects file parts with the coded 400 ahead of the shared history rules, exactly as #1359 does on main, and the branch's own version of the shared rules sits on top. The diff against main for |
…r a cleared default Hosted turns stayed 'acp' in debug transcripts because the turn initialiser still hardcoded the engine; it now uses the same helper as the turn start, and a test checks the saved message after the turn finishes. Discovery state in the chat store is now pending, none or an agent, so hydration falls back to the cached is_default row only while discovery is pending and an explicit clear wins over a stale row. When the selected agent takes no attachments and files are already queued, sending is held and a translated banner says the files must be removed; the chips stay so they can be. Translations added to every catalog. The composer tests inject a no-op ACP warm-up so selecting a non-built-in agent in a test never dials out; the real hook's connection error was breaking every later test in the file in CI.
…nly if listed The chat store's discovery state went pending to done once and never reset, so after an account or server change a chat hydrating before the new answer took the previous run's result instead of the cached is_default row. The bootstrap effect now resets to pending before each run's fetch, and resolveSessionAgent accepts the discovered default only when it is among the available agents. Tests cover the reset on a server change, hydration using the row again after a reset, and a stale default being skipped.
| // A finished discovery's own list covers agents its write added after the read above. | ||
| const knownAgents = | ||
| agentDiscovery.status === 'done' | ||
| ? [...allAgents, ...agentDiscovery.agents.filter((agent) => !allAgents.some(({ id }) => id === agent.id))] |
There was a problem hiding this comment.
🚫 Blocking — Discovery's saved agent list can bring back a custom agent the user has since deleted
I traced this, and I think merging the whole discovery list into knownAgents reaches further than intended. agentDiscovery.agents is a snapshot from loadAllAgents (system rows, customs and the built-in), taken once per discovery run. Discovery only runs again when the user, cloud URL, db, http client or refresh changes. The merge then feeds every candidate in resolveSessionAgent: threadAgentId, lastUsedAgentId and the built-in check, not just the discovered default.
Here's the failure case. The user's selected_agent is custom agent X. Discovery runs, so X is in the snapshot. The user then deletes X in Settings, or another device's tombstone syncs in. deleteAgent only sets deletedAt and leaves selected_agent alone. When the user opens a new chat, allAgents no longer has X but the snapshot still does, so findAgent(lastUsedAgentId) returns the deleted agent and the chat opens on it. A persisted thread whose agentId is X does the same thing. Before this change those cases fell through the chain quietly, as the docs promise.
Could we use the discovery list only to vouch for the discovered default? For example, add agentDiscovery.defaultAgent to the list only when hydration's own read is missing it, rather than adding the whole snapshot.
| // Aborting it stops its late response (a 403 that clears the table, or a | ||
| // smaller anonymous list) from overwriting what the signed-in refresh wrote. | ||
| // Until this run answers, the last account's or server's result must not stand in for it. | ||
| useChatStore.getState().resetAgentDiscovery() |
There was a problem hiding this comment.
📐 Convention — Discovery isn't reset on sign-out or when the cloud URL is cleared
The reset sits after the if (!userId || !cloudUrl) return guard, so it only fires when a new run actually starts. Signing out drops userId to undefined before the next session arrives, and so does clearing the cloud URL. In both cases the cleanup aborts the old run but nothing resets the store.
Until the next session shows up, agentDiscovery keeps the previous account's done result and its agent list. A new chat that hydrates in that gap takes that default. Because knownAgents merges the same run's list, the new "only if listed" check can't reject it either. Could the reset move above the early return, or into the cleanup, so any change to these inputs clears the old answer?
| const knownAgents = | ||
| agentDiscovery.status === 'done' | ||
| ? [...allAgents, ...agentDiscovery.agents.filter((agent) => !allAgents.some(({ id }) => id === agent.id))] | ||
| : allAgents |
There was a problem hiding this comment.
agentDiscovery.agents keeps the full agent list from bootstrap. This merge adds missing entries back into the fresh database results, including custom agents the user has since deleted. When the user deletes their remembered agent and opens a new chat, resolveSessionAgent selects that deleted agent instead of falling back. Sending can reconnect to the agent they removed.
Keep custom agents tied to the fresh database results. Use the saved discovery list only to reconcile system agents added during hydration.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/chats/use-hydrate-chat-store.ts
Line: 255-258
Comment:
**Deleted agents return**
`agentDiscovery.agents` keeps the full agent list from bootstrap. This merge adds missing entries back into the fresh database results, including custom agents the user has since deleted. When the user deletes their remembered agent and opens a new chat, `resolveSessionAgent` selects that deleted agent instead of falling back. Sending can reconnect to the agent they removed.
Keep custom agents tied to the fresh database results. Use the saved discovery list only to reconcile system agents added during hydration.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
This branch was successfully deployed
2 participants
Why
#1336 made the backend advertise the hosted agent on
GET /v1/agentsas amanaged-httpdescriptor and name it indefaultAgentId. The web app filtered that descriptor out, so a MozFest visitor saw nothing. This is the frontend half of GTM-21. The only backend change ishosted-agent/history.tsreading its message rules from the newshared/hosted-agent-history.tsso the client and server shape history from one definition.What
src/acp/managed-http-adapter.ts): posts the turn to the descriptor's/v1/agent/chat, joined under the backend root the same way the auth context derives it so a reverse-proxy prefix survives (the path must start with a single/and resolve to the backend origin), through the authenticated app client (soAuthorization,X-Device-ID,X-App-VersionandX-App-Languagego along), forwards the abort signal so Stop works, and hands the UI message streamResponseback unchanged. Non-2xx responses are serialized into the same{ error, status, kind }JSON the built-in pipeline emits, so the existing quota and rate-limit UI rendersINFERENCE_QUOTA_EXCEEDEDandAGENT_RUN_IN_PROGRESSnormally.{ id, role, parts }only, so encryptedmetadatanever leaves the device. Quotes are hydrated into text; the hosted agent takes no attachments for now (the composer hides the attach control formanaged-httpagents and ignores dropped or pasted files; files already in a chat's history are dropped from the request with a "could not be delivered" note). The cap, per-role part allowlist, start-on-user and end-on-user rules live inshared/hosted-agent-history.tsand are imported by both sides.agents_systemacceptsmanaged-httprows and gains a nullableis_defaultcolumn (local-only PowerSync table, no migration). Discovery now runs for anonymous sessions too; deployments withoutALLOW_ANONYMOUS_AGENT_DISCOVERYanswer 403 and the table is cleared as before.selected_agent→ the discovered default → first agent → built-in; a persisted thread keeps its own agent, and a thread with noagent_id(automations, pre-agent_idthreads) never moves to the default. A model pick counts as choosing the built-in agent.applyDiscoveredDefaultAgentre-runs that resolution for unstarted, unpicked chats when discovery lands after/chats/newhydrated;clearDiscoveredDefaultAgentsends them back through it when discovery answers 401/403 or names no default. Hosted-agent threads are created withisEncrypted0, since the private-chat flag means Tinfoil-confidential inference and hosted content reaches the backend.managed-httpbranch is taken only for system agents; a synced custom row'stypeis clamped to the kinds a custom agent may have, and seeding is back to an allowlist ofmanaged-acpandmanaged-httpwith an exhaustive dispatch, so a server cannot flip someone's agent onto the hosted path.acp-agents.md,chat-runtime.md,configuration.md(replaces "the app starts honouring the default in an upcoming release").AGENT_RUN_IN_PROGRESSgets its own copy and a delayed retry instead of a terminal "too many requests". New strings are extracted to every catalog.Tests
Adapter (URL, headers, body, signal, response identity, relative
cloudUrl, 429 mapping, network error, part shaping, role filtering, history cap), connect dispatch, seeder (managed-http+is_default), hydrate precedence (integration and pure), store helper, bootstrap hook (anonymous discovery, remembered pick passed through), picker. Thirteen revert checks: each piece of wiring was reverted and its test failed.src/acp,src/chats,src/dal, hooks, components andshared/hosted-agent-history: 327 pass, 0 fail; backendsrc/hosted-agent: 47 pass, 0 failbun run i18n:checkpassesThe local root test failures in
encryption.test.tsandorg-escrow.test.tsreproduce on pristinemainunder Bun 1.4.2 and are unrelated.Manual test plan
backend/.env:AGENT_ENABLED=true,AGENT_MODEL=<Anthropic model with a price row>,ANTHROPIC_API_KEY,ALLOW_ANONYMOUS_AGENT_DISCOVERY=true; optionallyDISABLE_BUILT_IN_AGENT=trueto mimic MozFest. Restart the backend.http://localhost:1420:GET /v1/agentsreturns thehosted-agentdescriptor withdefaultAgentId; the first new chat opens on it; no model picker in the composer.POST /v1/agent/chatcarries the auth, version and language headers; the reply streams.>text part.ALLOW_ANONYMOUS_AGENT_DISCOVERY=false, restart, new private window: 403 on discovery, built-in only, chat works as on main.Left for follow-ups
/slug) and project instructions are not sent to the hosted agent; the adapter comment names the gap. Ticketed.isEncrypted1) thread onto a non-built-in agent is unguarded, as it already was for ACP agents; ticketed.Size
Against main: 66 files changed, 2438 insertions(+), 294 deletions(-), roughly half tests, catalogs and docs. Well over the 400-line guideline: the review round added the trust-boundary changes, the shared history module and the attachment handling on top of the original adapter and discovery work. A split was considered before the first round and again now; too late to do cleanly mid-review.