mirror of
https://github.com/andrewyng/openworker.git
synced 2026-09-12 23:29:55 +00:00
port aisuite#380: slack installer pre-add, mcp oauth quarantine, automation toast, e2e fixes
Slack installer joins the workspace allow-list on managed connect; MCP interactive oauth only from explicit connects. Run-started toast over a new app-wide /ws/events socket; Automations e2e locators scoped to the account menu.
This commit is contained in:
@@ -184,6 +184,10 @@ def _slack_workspaces(secrets: SecretStore) -> list[dict[str, Any]]:
|
||||
"domain": profile.get("domain") or "",
|
||||
"allowed_users": list(profile.get("allowed_users") or []),
|
||||
"allow_all": bool(profile.get("allow_all")),
|
||||
# Who installed (authed_user) — the GUI marks their chip "you" and
|
||||
# keys the post-connect card's "your mentions get through" line.
|
||||
"installer_user_id": profile.get("slack_user_id") or "",
|
||||
"installer_name": profile.get("sender_name") or "",
|
||||
}
|
||||
for team_id, profile in sorted(
|
||||
_slack_team_profiles(secrets), key=lambda t: t[0]
|
||||
@@ -414,6 +418,15 @@ def managed_connect_slack_install(
|
||||
bot_token = form.get("access_token", "")
|
||||
if not team_id or not bot_token:
|
||||
return {"ok": False, "error": "missing team_id or bot token"}
|
||||
# A reinstall replaces the token but must not reset authorization state.
|
||||
existing = secrets.get(f"slack:team:{team_id}") or {}
|
||||
allowed = set(existing.get("allowed_users") or [])
|
||||
installer = form.get("slack_user_id", "")
|
||||
if installer:
|
||||
# Pre-add the installer (UX-027): connecting the workspace is consent to
|
||||
# talk to your own bot — without this, the connector's very first mention
|
||||
# comes from the installer and parks.
|
||||
allowed.add(installer)
|
||||
secrets.put(
|
||||
f"slack:team:{team_id}",
|
||||
{
|
||||
@@ -423,7 +436,7 @@ def managed_connect_slack_install(
|
||||
"bot_user_id": form.get("bot_user_id", ""),
|
||||
# The INSTALLER's Slack member id (authed_user) — who this workspace's
|
||||
# outbound posts speak for (attribution.py resolves + caches the name).
|
||||
"slack_user_id": form.get("slack_user_id", ""),
|
||||
"slack_user_id": installer,
|
||||
"team_id": team_id,
|
||||
"account": form.get("account", ""),
|
||||
# The workspace's slack.com subdomain (broker resolves it via auth.test)
|
||||
@@ -431,6 +444,9 @@ def managed_connect_slack_install(
|
||||
"domain": form.get("team_domain", ""),
|
||||
"scope": form.get("scope", ""),
|
||||
"connection_id": form.get("connection_id", ""),
|
||||
"allowed_users": sorted(allowed),
|
||||
"allow_all": bool(existing.get("allow_all")),
|
||||
"sender_name": existing.get("sender_name", ""),
|
||||
},
|
||||
)
|
||||
default = secrets.get("slack:default") or {}
|
||||
|
||||
+20
-5
@@ -41,14 +41,22 @@ class MCPManager:
|
||||
# so library/CLI construction without secrets keeps working.
|
||||
self._secrets = secrets
|
||||
|
||||
async def ensure(self, server: MCPServerDef) -> _Conn:
|
||||
"""Return a live connection for `server`, connecting (once) if needed."""
|
||||
async def ensure(self, server: MCPServerDef, *, interactive: bool = False) -> _Conn:
|
||||
"""Return a live connection for `server`, connecting (once) if needed.
|
||||
|
||||
`interactive=True` (explicit connect actions only) lets an OAuth server run
|
||||
the browser sign-in flow; the default refuses it — stored tokens and silent
|
||||
refresh still work, but a server that insists on re-authorization raises
|
||||
InteractiveAuthRequired instead of hijacking the user's browser.
|
||||
"""
|
||||
async with self._lock:
|
||||
existing = self._conns.get(server.name)
|
||||
if existing is not None:
|
||||
return existing
|
||||
ready: asyncio.Future = asyncio.get_running_loop().create_future()
|
||||
self._tasks[server.name] = asyncio.create_task(self._serve(server, ready))
|
||||
self._tasks[server.name] = asyncio.create_task(
|
||||
self._serve(server, ready, interactive=interactive)
|
||||
)
|
||||
conn = await ready # propagates connection errors
|
||||
self._conns[server.name] = conn
|
||||
return conn
|
||||
@@ -77,7 +85,9 @@ class MCPManager:
|
||||
self._tasks.clear()
|
||||
|
||||
# -- per-server lifecycle (one task owns enter+exit) ------------------------
|
||||
async def _serve(self, server: MCPServerDef, ready: asyncio.Future) -> None:
|
||||
async def _serve(
|
||||
self, server: MCPServerDef, ready: asyncio.Future, *, interactive: bool = False
|
||||
) -> None:
|
||||
try:
|
||||
async with AsyncExitStack() as stack:
|
||||
if server.transport == "http":
|
||||
@@ -92,7 +102,12 @@ class MCPManager:
|
||||
|
||||
if self._secrets is None:
|
||||
self._secrets = SecretStore()
|
||||
auth = build_auth(server.name, server.url, self._secrets)
|
||||
auth = build_auth(
|
||||
server.name,
|
||||
server.url,
|
||||
self._secrets,
|
||||
interactive=interactive,
|
||||
)
|
||||
read, write, *_ = await stack.enter_async_context(
|
||||
streamablehttp_client(
|
||||
server.url, headers=server.headers or None, auth=auth
|
||||
|
||||
+54
-4
@@ -87,6 +87,31 @@ class SecretStoreTokenStorage(TokenStorage):
|
||||
self._merge({"client_info": info.model_dump(mode="json", exclude_none=True)})
|
||||
|
||||
|
||||
class InteractiveAuthRequired(RuntimeError):
|
||||
"""The server wants a browser sign-in, but this context must not open one.
|
||||
|
||||
Interactive OAuth (browser + loopback wait) is an explicit-connect-only
|
||||
privilege: a background context that hit this — an engine turn, a tools
|
||||
listing — raises instead, and the caller skips the server. Without this, a
|
||||
server whose refresh token the vendor rejected (Atlassian rotates them
|
||||
aggressively) would hijack the user's browser from ANY code path that
|
||||
touched it — owner-hit 2026-07-20: an authorize page opened at app launch.
|
||||
"""
|
||||
|
||||
|
||||
def is_auth_required(exc: BaseException) -> bool:
|
||||
"""True if InteractiveAuthRequired is anywhere in the exception tree — the SDK
|
||||
transport runs in anyio task groups, so it often arrives wrapped in an
|
||||
ExceptionGroup (or chained as a cause) rather than bare."""
|
||||
if isinstance(exc, InteractiveAuthRequired):
|
||||
return True
|
||||
for sub in getattr(exc, "exceptions", None) or []: # ExceptionGroup
|
||||
if is_auth_required(sub):
|
||||
return True
|
||||
cause = exc.__cause__ or exc.__context__
|
||||
return is_auth_required(cause) if cause is not None else False
|
||||
|
||||
|
||||
# -- single-slot interactive flow ------------------------------------------------
|
||||
_pending: Optional[asyncio.Future] = None
|
||||
# The last authorize URL we sent the user to — surfaced over REST so the GUI can offer
|
||||
@@ -113,6 +138,22 @@ async def _open_browser(url: str) -> None:
|
||||
await asyncio.get_running_loop().run_in_executor(None, webbrowser.open, url)
|
||||
|
||||
|
||||
async def _refuse_browser(url: str) -> None:
|
||||
"""Non-interactive redirect handler: never open a browser, but keep the URL so
|
||||
the GUI's "reopen sign-in page" affordance still works after the refusal."""
|
||||
global last_authorize_url
|
||||
last_authorize_url = url
|
||||
raise InteractiveAuthRequired(
|
||||
"sign-in required — reconnect this server from its page"
|
||||
)
|
||||
|
||||
|
||||
async def _refuse_callback() -> tuple[str, Optional[str]]:
|
||||
raise InteractiveAuthRequired(
|
||||
"sign-in required — reconnect this server from its page"
|
||||
)
|
||||
|
||||
|
||||
async def _wait_for_callback() -> tuple[str, Optional[str]]:
|
||||
global _pending
|
||||
if _pending is not None and not _pending.done():
|
||||
@@ -130,9 +171,18 @@ async def _wait_for_callback() -> tuple[str, Optional[str]]:
|
||||
|
||||
|
||||
def build_auth(
|
||||
server_name: str, server_url: str, secrets: SecretStore
|
||||
server_name: str,
|
||||
server_url: str,
|
||||
secrets: SecretStore,
|
||||
*,
|
||||
interactive: bool = True,
|
||||
) -> OAuthClientProvider:
|
||||
"""The httpx auth for one OAuth MCP server (pass as streamablehttp_client(auth=…))."""
|
||||
"""The httpx auth for one OAuth MCP server (pass as streamablehttp_client(auth=…)).
|
||||
|
||||
`interactive=False` still uses stored tokens and silent refresh, but the moment
|
||||
the SDK wants a browser authorization it raises InteractiveAuthRequired instead
|
||||
of opening one — only explicit connect actions pass True.
|
||||
"""
|
||||
metadata = OAuthClientMetadata.model_validate(
|
||||
{
|
||||
"client_name": CLIENT_NAME,
|
||||
@@ -147,8 +197,8 @@ def build_auth(
|
||||
server_url=server_url,
|
||||
client_metadata=metadata,
|
||||
storage=SecretStoreTokenStorage(server_name, secrets),
|
||||
redirect_handler=_open_browser,
|
||||
callback_handler=_wait_for_callback,
|
||||
redirect_handler=_open_browser if interactive else _refuse_browser,
|
||||
callback_handler=_wait_for_callback if interactive else _refuse_callback,
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -1657,6 +1657,24 @@ def create_app(manager: SessionManager) -> FastAPI:
|
||||
finally:
|
||||
manager.unregister_session_client(session_id, ws.send_json)
|
||||
|
||||
@app.websocket("/ws/events")
|
||||
async def ws_events(ws: WebSocket) -> None:
|
||||
"""App-wide event stream (session-independent): the GUI keeps one open for
|
||||
pushes like automation_run_started (the UX-026 toast). Read-only — inbound
|
||||
frames are ignored; the receive loop just detects disconnect."""
|
||||
if not _origin_allowed(ws.headers.get("origin")):
|
||||
await ws.close(code=1008)
|
||||
return
|
||||
await ws.accept()
|
||||
manager.register_event_client(ws.send_json)
|
||||
try:
|
||||
while True:
|
||||
await ws.receive_text()
|
||||
except WebSocketDisconnect:
|
||||
pass
|
||||
finally:
|
||||
manager.unregister_event_client(ws.send_json)
|
||||
|
||||
return app
|
||||
|
||||
|
||||
|
||||
@@ -161,6 +161,9 @@ class SessionManager:
|
||||
# whoever drives the turn (foreground user_message, channel delivery, self-wake, resume).
|
||||
# Delivery itself is socket-independent — this only governs *live visibility*.
|
||||
self._session_clients: dict[str, set[Any]] = {}
|
||||
# App-wide event sockets (/ws/events): session-independent pushes — today the
|
||||
# automation-run-started toast (UX-026); badges could ride it later.
|
||||
self._event_clients: set[Any] = set()
|
||||
# Automation: scheduled tasks store + the tick scheduler (started in the lifespan).
|
||||
# The scheduler also resumes self-wake'd sessions each tick (extra_tick).
|
||||
self.task_store = TaskStore(base / "automation.db")
|
||||
@@ -833,9 +836,19 @@ class SessionManager:
|
||||
]
|
||||
try:
|
||||
conn = await self.mcp.ensure(server)
|
||||
except (
|
||||
Exception
|
||||
): # bad command / unreachable url — skip, don't break the session
|
||||
except Exception as exc:
|
||||
if mcp_oauth.is_auth_required(exc):
|
||||
# Stored tokens no longer refresh (vendor rotated/expired
|
||||
# them) — the non-interactive connect refused to open a
|
||||
# browser. Record it so the MCP page shows WHY the server is
|
||||
# dark; the session just runs without its tools.
|
||||
self._mcp_errors[server.name] = (
|
||||
"sign-in required — reconnect this server from its page"
|
||||
)
|
||||
logger.info(
|
||||
"mcp %s needs re-auth; skipped for this session", server.name
|
||||
)
|
||||
# else: bad command / unreachable url — skip, don't break the session
|
||||
continue
|
||||
callables = build_callables(
|
||||
server,
|
||||
@@ -914,7 +927,8 @@ class SessionManager:
|
||||
self._mcp_authorizing.add(name)
|
||||
self._mcp_errors.pop(name, None)
|
||||
try:
|
||||
conn = await self.mcp.ensure(server)
|
||||
# The ONE place a browser sign-in may start: an explicit connect.
|
||||
conn = await self.mcp.ensure(server, interactive=True)
|
||||
return {"ok": True, "tools": len(conn.tools)}
|
||||
except Exception as exc:
|
||||
self._mcp_errors[name] = str(exc) or exc.__class__.__name__
|
||||
@@ -2139,6 +2153,21 @@ class SessionManager:
|
||||
return {"ok": True}
|
||||
|
||||
# -- per-session live view --------------------------------------------------
|
||||
def register_event_client(self, send_cb: Any) -> None:
|
||||
self._event_clients.add(send_cb)
|
||||
|
||||
def unregister_event_client(self, send_cb: Any) -> None:
|
||||
self._event_clients.discard(send_cb)
|
||||
|
||||
async def broadcast_event(self, message: dict) -> None:
|
||||
"""Fan an app-wide event out to every /ws/events socket. Best-effort: a dead
|
||||
socket is dropped, never fatal to the caller."""
|
||||
for cb in list(self._event_clients):
|
||||
try:
|
||||
await cb(message)
|
||||
except Exception:
|
||||
self.unregister_event_client(cb)
|
||||
|
||||
def register_session_client(self, session_id: str, send_cb: Any) -> None:
|
||||
self._session_clients.setdefault(session_id, set()).add(send_cb)
|
||||
|
||||
@@ -2650,6 +2679,22 @@ class SessionManager:
|
||||
task_id=task.id, trigger=trigger
|
||||
) # __post_init__ sets run.session_id
|
||||
self.task_store.add_run(run) # mark "running"
|
||||
# UX-026: tell every open app window a SCHEDULED run just started (the 5s
|
||||
# top-right toast). Manual runs never come through here — the user is
|
||||
# already watching those live.
|
||||
await self.broadcast_event(
|
||||
{
|
||||
"type": "automation_run_started",
|
||||
"data": {
|
||||
"task_id": task.id,
|
||||
"task_title": task.title,
|
||||
"session_id": run.session_id,
|
||||
"workspace": task.workspace,
|
||||
"agent": task.agent,
|
||||
"trigger": trigger,
|
||||
},
|
||||
}
|
||||
)
|
||||
# Each run is a real, persisted conversation thread: it runs the instructions under its
|
||||
# own session id, then saves the transcript. The user can reopen that session and ask a
|
||||
# follow-up — the scheduled agent is no longer fire-and-forget.
|
||||
|
||||
Reference in New Issue
Block a user