diff --git a/CLAUDE.md b/CLAUDE.md index a38caa9..e4fbdfd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,57 +1,49 @@ # Repository Guide -## Current architecture +## Architecture -- `frontend`: React Router SPA. Production files are built in `nginx/Dockerfile`. -- `backend`: public FastAPI API and internal S3 storage API in one process. -- `runtime`: shared Jupyter lifecycle, MySQL edit leases and short-lived in-memory access tickets. -- `schedule`: APScheduler, MySQL JobStore, MySQL Outbox polling and DAG execution. -- `common`: SQLAlchemy models, database/session helpers, IDs and object-store helpers. -- `migrations`: Alembic schema and seed migrations. -- `nginx`: static frontend, `/api/` proxy and authenticated `/jupyter/` proxy. +- `frontend` — React Router v8 SPA; production bundle built in `nginx/Dockerfile`. +- `backend` — public FastAPI API + internal S3 storage API in one process. +- `runtime` — Jupyter lifecycle, MySQL edit leases, short-lived in-memory access tickets. +- `schedule` — APScheduler, MySQL JobStore, MySQL Outbox polling and DAG execution. +- `common` — SQLAlchemy models, database/session helpers, IDs, object-store helpers. +- `migrations` — Alembic schema + seed migrations. +- `nginx` — static frontend, `/api/` proxy, authenticated `/jupyter/` proxy. Redis and the former separate Storage API container are intentionally removed. ## Commands -From the repository root: +From repo root: ```bash -# Local Python workspace uv sync --all-packages - -# Static Python check python -m compileall common/src backend/src runtime/src schedule/src - -# Database migration uv run --package backend alembic upgrade head -# Full Docker stack cp .env.example .env docker compose config docker compose up -d --build ``` -Frontend development: +Frontend: ```bash cd frontend pnpm install pnpm dev -pnpm typecheck +pnpm typecheck # runs `react-router typegen && tsc` pnpm build ``` ## Service rules -- Browser traffic enters through Gateway only. -- Frontend API calls use same-origin `/api/v1/...` paths. -- Backend writes `schedule_runs` and `outbox_events`, then performs best-effort HTTP dispatch to Schedule Executor. -- Schedule Executor always polls pending MySQL Outbox rows, so HTTP dispatch failure does not lose a task. -- Cron jobs are persisted by APScheduler in MySQL table `apscheduler_jobs`. +- Browser traffic enters through Gateway only; frontend uses same-origin `/api/v1/...`. +- Backend writes `schedule_runs` and `outbox_events`, then best‑effort HTTP-dispatches to Schedule Executor. The executor always polls pending MySQL Outbox rows, so dispatch failure does not lose a task. +- Cron jobs persisted in MySQL `apscheduler_jobs`. - Runtime must stay single-replica while file leases and Jupyter tickets use the simplified implementation. - Never expose the internal Jupyter token to the browser. -- Never delete Docker volumes when preserving MySQL or storage data is required. +- Never delete Docker volumes when preserving MySQL or storage data. ## Main entrypoints @@ -63,19 +55,39 @@ schedule/src/schedule/main.py nginx/default.conf.template ``` -## Engineering notes from recent platform-employee work +## Engineering notes -These are hard-won lessons from the `GET/POST/PATCH/DELETE /api/v1/platform/employees` rollout. Read before touching platform auth, soft-delete, or `Users.platform_role_id` flows. +Hard-won lessons. Read the relevant bullet before touching the named area. -- **Reuse `system_admin_context` and the in-file `_*_admins` helpers.** Self-protection (cannot disable/demote/delete self) and the last-admin guard for `Users.platform_role_id` mirror the workspace pattern. `_count_active_system_admins(session, exclude_user_id=...)` lives in `backend/src/backend/platform.py`; do not reinvent the count in the handler. -- **PATCH guard order is load-bearing.** Always check self-protection, then `leaves_admin_pool`, then the count. Putting the self-demotion check before the last-admin check looks equivalent but lets the test mock bypass the count helper when `is_current_system_admin` happens to be False. The last-admin check must run first. -- **Delete on already-soft-deleted users returns 404, not 409.** `delete_platform_employee` collapses `user is None or user.is_deleted != 0` into a single 404 "用户不存在" raise. This intentionally differs from `DELETE /workspaces/{id}` which returns 409 for already-disabled. Document both to avoid reviewer pushback. -- **PATCH cannot null-out `platform_role_id`.** `PlatformEmployeeUpdate.role_code: Literal["admin","developer"]` (not `Optional`). To clear the platform role, add a separate endpoint or a different field — do not loosen the Literal. -- **DELETE cascade covers `WorkspaceMembers` only.** It writes `is_deleted=1, deleted_at=now` on `WorkspaceMembers` rows where `user_id = :uid AND is_deleted=0`. It does not touch `Workspaces`. Document that boundary explicitly. -- **Codex MCP on this machine may fail with `InvalidParameter`** even when prompts include the required `model: "kimi-k2.7-code"`, `sandbox: "danger-full-access"`, `approval-policy: "on-request"`. The upstream proxy rejects the request before our wrapper can recover. Fall back to local implementation rather than retrying — three consecutive failures indicate a transport issue, not a prompt issue. -- **SQLAlchemy 2.0 `compile(literal_binds=True)` uppercases keywords.** `"from roles" in text` will miss the table reference; use case-insensitive matching (`text.lower()`) when building a mock session's `scalar` dispatcher, or test for `"roles.role_id"` / `"roles.role_code"` instead. -- **Mocking `Depends`-style helpers requires async callables.** `_load_role_by_code` and `_count_active_system_admins` are awaited; substituting them with a sync `lambda` raises `TypeError: object int can't be used in 'await' expression`. Wrap mocks in `async def` factories. -- **Mock response ordering matters for re-reads.** `update_platform_employee` queries `current_role` (before write) and then `response_role` (after write). A scalar mock that returns a single fixed value will make the response use the pre-write role. Track call order or look up by `user.platform_role_id` post-write. -- **Mock users must declare every attribute the handler writes.** `Users.deleted_at` is not in the column defaults; `SimpleNamespace(user_id=..., ...)` will raise `AttributeError` on `target.deleted_at = now`. Set `user.deleted_at = None` explicitly when building mocks for delete tests. -- **Documentation review is part of the task.** API.md descriptions must distinguish "clearing" from "demoting" `platform_role_id`, separate workspace 409 from user 404 semantics, and avoid language like "platform developer role" when only one shared `roles` table exists. The reviewer or a future agent will catch these inconsistencies. -- **Frontend coupling is intentionally conservative.** `frontend/app/components/admin/UserManagementPage.tsx` and `api.ts` still call `/api/v1/admin/employees`. Do not migrate them in the same change as a platform endpoint addition — the contract surface is intentionally duplicated. \ No newline at end of file +### Platform auth / soft-delete (`/api/v1/platform/employees`) + +- **Reuse `system_admin_context` + `_*_admins` helpers** in `backend/src/backend/platform.py`. Self-protection and the last-admin guard for `Users.platform_role_id` mirror the workspace pattern. `_count_active_system_admins(session, exclude_user_id=...)` is already there; do not reinvent the count in the handler. +- **PATCH guard order is load-bearing.** Always check `is_current_system_admin` last-admin first, then `leaves_admin_pool`, then self-demotion. Reversing the order lets test mocks bypass the count helper. +- **404 vs 409 on already-soft-deleted:** `delete_platform_employee` returns a single 404 "用户不存在" for both `user is None` and `user.is_deleted != 0`. This intentionally differs from `DELETE /workspaces/{id}` which returns 409 for already-disabled. Document both in API.md. +- **PATCH cannot null-out `platform_role_id`.** `PlatformEmployeeUpdate.role_code: Literal["admin","developer"]` (not `Optional`). Add a separate endpoint if clearing is needed — never loosen the Literal. +- **DELETE cascade covers `WorkspaceMembers` only** — writes `is_deleted=1, deleted_at=now` on `WorkspaceMembers` rows for the user. Does not touch `Workspaces`. + +### Backend test mocks (SQLAlchemy 2.0 / pytest-asyncio) + +- **`compile(literal_binds=True)` uppercases keywords.** `"from roles" in text` misses the table reference; match `text.lower()` or test for `"roles.role_id"` / `"roles.role_code"` directly. +- **`Depends`-style helpers are awaited.** `_load_role_by_code` and `_count_active_system_admins` need `async def` mocks — sync `lambda` raises `TypeError: object int can't be used in 'await' expression`. +- **Mock response ordering matters for re-reads.** `update_platform_employee` reads `current_role` before write then `response_role` after. A scalar mock returning a fixed value will return the pre-write role in the response — track call order or look up by `user.platform_role_id` post-write. +- **Mock users must declare every attribute the handler writes.** `Users.deleted_at` is not in column defaults; `SimpleNamespace(user_id=...)` raises `AttributeError` on `target.deleted_at = now`. Set `user.deleted_at = None` explicitly. + +### Frontend state + routing (zustand + React Router v8) + +Lessons from splitting `frontend/app/features/platform/ModelPlatformApp.tsx` (1057 → 121 lines) into zustand stores + nested routes. + +- **Barrel-file CSS imports vanish on refactor.** `features/admin/AdminPages.tsx` was a barrel that side-effect-imported `admin.css` + `dashboard.css`. When route files started importing `DashboardPage` / `SystemAdminPage` directly, the CSS disappeared silently. Fix: each page component imports its own CSS at the top — `DashboardPage` needs **both** `admin.css` (`.dashboard-page`, `.dashboard-hero`, `.dashboard-metrics`, `.dashboard-actions`) **and** `dashboard.css` (`.dashboard-grid`). `UserManagementPage` / `ProjectManagementPage` / `SystemAdminPage` only need `admin.css`. Don't put CSS imports in route files; let the components own their styles. `SchedulePage` already follows this pattern with `schedule.css`. +- **Module-level zustand store + `bindApi(api)`** for auth-dependent APIs. Keep a module-level `_api` ref; expose `bindScriptWorkspaceApi(api)`; layout calls it in `useEffect([api])`. Actions read `_api` internally — no api param on every call. `bindScriptWorkspaceApi(null)` in cleanup avoids stale refs on logout. +- **Lifecycle hooks belong in the layout, not in route components.** Heartbeats (active edit session + cached sessions), the 10-min cleanup timer, and `beforeunload` lock-release must mount at the layout level — navigating to `/schedules` otherwise unmounts them and cached locks expire. Pattern: store exposes `tickHeartbeats()` / `tickCleanup()` / `releaseActiveOnUnload()`; the layout hook just owns the `setInterval` and `addEventListener`. +- **`{ current: T | null }` module-level handle for non-subscribing consumers.** Sidebar reads "is there an active edit session?" without subscribing to the store — expose a plain `{ current: ... }` object at module scope and update it synchronously inside the store's `setEditSession` action. +- **zustand `StateCreator` enforces declared action signatures.** Declaring `loadLatestVersion: () => Promise` while accidentally returning a cleanup function from the implementation makes `tsc` reject the whole store with TS2345. Match the declared type exactly. +- **Nested routes in React Router v8.** Use `route("", "layout.tsx", [route("x", "x.tsx"), ...])` from `@react-router/dev/routes`. URLs stay flat; the layout renders ``. Don't use `route("*", ...)` as a wildcard — it skips the nested children config. +- **Route ids are derived from file paths.** The same file cannot be referenced by two route entries (`index("X.tsx")` + `route("y", "X.tsx")` → `duplicate route id`). To make `/` redirect to `/workbench`, create a tiny `RootIndex.tsx` that renders `` rather than reusing `DashboardRoute.tsx`. +- **An unmatched nested child leaves `` blank → white screen inside the layout.** Always cover `/` either with `index(...)` or by letting the parent layout `` on a location check. `/login` is the only top-level path that escapes this trap. +- **`pnpm typecheck` runs `react-router typegen && tsc`.** Type errors from the generated `+types/...` files surface here too. New route files must be registered in `routes.ts` first. + +### Frontend coupling + +- **`UserManagementPage.tsx` and `api.ts` still call `/api/v1/admin/employees`.** Don't migrate them in the same change as a `/api/v1/platform/employees` addition — the contract surface is intentionally duplicated. \ No newline at end of file diff --git a/frontend/app/components/admin/DashboardPage.tsx b/frontend/app/components/admin/DashboardPage.tsx index bcbd9c0..ea6d16a 100644 --- a/frontend/app/components/admin/DashboardPage.tsx +++ b/frontend/app/components/admin/DashboardPage.tsx @@ -1,6 +1,9 @@ import Icon from "../../components/common/Icon"; import { useAuth } from "../../context/AuthContext"; +import "../../styles/admin.css"; +import "../../styles/dashboard.css"; + export function DashboardPage({ scriptCount, online, diff --git a/frontend/app/components/admin/ProjectManagementPage.tsx b/frontend/app/components/admin/ProjectManagementPage.tsx index 1b3ec65..4560c97 100644 --- a/frontend/app/components/admin/ProjectManagementPage.tsx +++ b/frontend/app/components/admin/ProjectManagementPage.tsx @@ -3,6 +3,8 @@ import { useEffect, useState } from "react"; import { ApiRequestError, type Employee, type Workspace } from "../../services/api"; import { useApi, useAuth } from "../../context/AuthContext"; import Icon from "../common/Icon"; + +import "../../styles/admin.css"; import { UserMultiSelect } from "./UserMultiSelect"; const EMPTY_PROJECT_FORM = { diff --git a/frontend/app/components/admin/UserManagementPage.tsx b/frontend/app/components/admin/UserManagementPage.tsx index 5c1ddcb..42a88cd 100644 --- a/frontend/app/components/admin/UserManagementPage.tsx +++ b/frontend/app/components/admin/UserManagementPage.tsx @@ -4,6 +4,8 @@ import { ApiRequestError, type Employee } from "../../services/api"; import { useApi, useAuth } from "../../context/AuthContext"; import Icon from "../../components/common/Icon"; +import "../../styles/admin.css"; + const EMPTY_FORM = { username: "", display_name: "", diff --git a/frontend/app/components/common/Topbar.tsx b/frontend/app/components/common/Topbar.tsx index 538258e..22e7dcd 100644 --- a/frontend/app/components/common/Topbar.tsx +++ b/frontend/app/components/common/Topbar.tsx @@ -108,8 +108,7 @@ export function Topbar({ className="text-button" type="button" onClick={() => { - onLogout(); - window.location.assign("/login"); + void onLogout(); }} > 登出 diff --git a/frontend/app/features/admin/SystemAdminPage.tsx b/frontend/app/features/admin/SystemAdminPage.tsx index c4afe8a..8ab1d8e 100644 --- a/frontend/app/features/admin/SystemAdminPage.tsx +++ b/frontend/app/features/admin/SystemAdminPage.tsx @@ -4,6 +4,8 @@ import Icon from "../../components/common/Icon"; import { UserManagementPage } from "../../components/admin/UserManagementPage"; import { ProjectManagementPage } from "../../components/admin/ProjectManagementPage"; +import "../../styles/admin.css"; + export function SystemAdminPage({ onNotify, onConnectionChange, diff --git a/frontend/app/features/admin/SystemAdminRoute.tsx b/frontend/app/features/admin/SystemAdminRoute.tsx new file mode 100644 index 0000000..e70ea5b --- /dev/null +++ b/frontend/app/features/admin/SystemAdminRoute.tsx @@ -0,0 +1,15 @@ +import { useScriptWorkspaceStore } from "../platform/state/scriptWorkspaceStore"; +import { useUiStore } from "../platform/state/uiStore"; +import { SystemAdminPage } from "./SystemAdminPage"; + +export default function SystemAdminRoute() { + const setApiOnline = useScriptWorkspaceStore((s) => s.setApiOnline); + const pushToast = useUiStore((s) => s.pushToast); + + return ( + + ); +} \ No newline at end of file diff --git a/frontend/app/features/platform/DashboardRoute.tsx b/frontend/app/features/platform/DashboardRoute.tsx index f28f6fb..4c2ac86 100644 --- a/frontend/app/features/platform/DashboardRoute.tsx +++ b/frontend/app/features/platform/DashboardRoute.tsx @@ -3,8 +3,6 @@ import { useNavigate } from "react-router"; import { DashboardPage } from "../../components/admin/DashboardPage"; import { useScriptWorkspaceStore } from "./state/scriptWorkspaceStore"; -import "../../styles/dashboard.css"; - export default function DashboardRoute() { const scripts = useScriptWorkspaceStore((s) => s.scripts); const apiOnline = useScriptWorkspaceStore((s) => s.apiOnline); diff --git a/frontend/app/features/platform/ModelPlatformApp.tsx b/frontend/app/features/platform/ModelPlatformApp.tsx deleted file mode 100644 index 34246c6..0000000 --- a/frontend/app/features/platform/ModelPlatformApp.tsx +++ /dev/null @@ -1,122 +0,0 @@ -import { useEffect, useState } from "react"; -import { Outlet, useLocation, useNavigate } from "react-router"; - -import { Sidebar } from "../../components/common/Sidebar"; -import { Toast } from "../../components/common/Toast"; -import { Topbar } from "../../components/common/Topbar"; -import { useApi, useAuth } from "../../context/AuthContext"; -import { useEditSessionLifecycle } from "./hooks/useEditSessionLifecycle"; -import { - bindScriptWorkspaceApi, - editSessionHandle, - useScriptWorkspaceStore, -} from "./state/scriptWorkspaceStore"; -import { useUiStore } from "./state/uiStore"; - -import "../../styles/platform.css"; - -type ActivePage = "home" | "scripts" | "schedules" | "system"; - -function pageFromPath(pathname: string): ActivePage { - const page = pathname.replace(/^\/+|\/+$/g, ""); - return ["scripts", "schedules", "system"].includes(page) - ? (page as ActivePage) - : "home"; -} - -function pathForPage(page: ActivePage): string { - return page === "home" ? "/workbench" : `/${page}`; -} - -export default function ModelPlatformApp() { - const { currentWorkspace } = useAuth(); - if (!currentWorkspace) { - return ( -
-
- 加载中… -
-
- ); - } - return ; -} - -function AuthenticatedLayout() { - const api = useApi(); - const location = useLocation(); - const navigate = useNavigate(); - const activePage = pageFromPath(location.pathname); - const auth = useAuth(); - const { - user, - workspaces, - currentWorkspace, - setCurrentWorkspace, - logout, - } = auth; - - const apiOnline = useScriptWorkspaceStore((s) => s.apiOnline); - const endEditing = useScriptWorkspaceStore((s) => s.endEditing); - const selectScript = useScriptWorkspaceStore((s) => s.selectScript); - - const toast = useUiStore((s) => s.toast); - const dismissToast = useUiStore((s) => s.dismissToast); - const workspaceMenuOpen = useUiStore((s) => s.workspaceMenuOpen); - const setWorkspaceMenuOpen = useUiStore((s) => s.setWorkspaceMenuOpen); - - const [sidebarCollapsed, setSidebarCollapsed] = useState(false); - - // 绑定 api 到 script workspace store - useEffect(() => { - bindScriptWorkspaceApi(api); - return () => { - bindScriptWorkspaceApi(null); - }; - }, [api]); - - // 心跳 / cleanup / 切页结束编辑 / 卸载前释放 - useEditSessionLifecycle({ activePage }); - - // toast 自动消失 - useEffect(() => { - if (!toast) return; - const timer = window.setTimeout(dismissToast, 3200); - return () => window.clearTimeout(timer); - }, [toast, dismissToast]); - - return ( -
- navigate(pathForPage(page))} - onToggleCollapse={() => setSidebarCollapsed((v) => !v)} - onEndEditing={() => void endEditing(true)} - onSelectScript={selectScript} - editSessionRef={editSessionHandle} - /> - -
- - - -
- - -
- ); -} \ No newline at end of file diff --git a/frontend/app/features/platform/RootIndex.tsx b/frontend/app/features/platform/RootIndex.tsx new file mode 100644 index 0000000..269f448 --- /dev/null +++ b/frontend/app/features/platform/RootIndex.tsx @@ -0,0 +1,10 @@ +import { Navigate } from "react-router"; + +/** + * 根路径 "/" 命中时直接跳到 /workbench。 + * 抽出独立文件是为了不和 route("workbench", ...) 撞 route id + * (React Router v8 基于文件路径生成 id, 同一文件不能被两个路由引用)。 + */ +export default function RootIndex() { + return ; +} \ No newline at end of file diff --git a/frontend/app/routes.ts b/frontend/app/routes.ts index d74832f..ef49b0c 100644 --- a/frontend/app/routes.ts +++ b/frontend/app/routes.ts @@ -1,8 +1,9 @@ -import {type RouteConfig, route} from "@react-router/dev/routes"; +import {type RouteConfig, index, route} from "@react-router/dev/routes"; export default [ route("login", "routes/login.tsx"), route("", "routes/platform.tsx", [ + index("features/platform/RootIndex.tsx"), route("workbench", "features/platform/DashboardRoute.tsx"), route("scripts", "features/platform/ScriptsPage.tsx"), route("schedules", "features/schedules/SchedulesPageRoute.tsx"), diff --git a/frontend/app/routes/platform.tsx b/frontend/app/routes/platform.tsx index 6f9b64c..ac20865 100644 --- a/frontend/app/routes/platform.tsx +++ b/frontend/app/routes/platform.tsx @@ -1,5 +1,33 @@ +import { useEffect, useState } from "react"; +import { Outlet, useLocation, useNavigate } from "react-router"; + import type { Route } from "./+types/platform"; -import ModelPlatformApp from "../features/platform/ModelPlatformApp"; +import { Sidebar } from "../components/common/Sidebar"; +import { Toast } from "../components/common/Toast"; +import { Topbar } from "../components/common/Topbar"; +import { useApi, useAuth } from "../context/AuthContext"; +import { useEditSessionLifecycle } from "../features/platform/hooks/useEditSessionLifecycle"; +import { + bindScriptWorkspaceApi, + editSessionHandle, + useScriptWorkspaceStore, +} from "../features/platform/state/scriptWorkspaceStore"; +import { useUiStore } from "../features/platform/state/uiStore"; + +import "../styles/platform.css"; + +type ActivePage = "home" | "scripts" | "schedules" | "system"; + +function pageFromPath(pathname: string): ActivePage { + const page = pathname.replace(/^\/+|\/+$/g, ""); + return ["scripts", "schedules", "system"].includes(page) + ? (page as ActivePage) + : "home"; +} + +function pathForPage(page: ActivePage): string { + return page === "home" ? "/workbench" : `/${page}`; +} export function meta({}: Route.MetaArgs) { return [ @@ -8,6 +36,95 @@ export function meta({}: Route.MetaArgs) { ]; } -export default function PlatformRoute() { - return ; +export default function PlatformLayout() { + const { currentWorkspace } = useAuth(); + if (!currentWorkspace) { + return ( +
+
+ 加载中… +
+
+ ); + } + return ; } + +function AuthenticatedLayout() { + const api = useApi(); + const location = useLocation(); + const navigate = useNavigate(); + const activePage = pageFromPath(location.pathname); + const auth = useAuth(); + const { + user, + workspaces, + currentWorkspace, + setCurrentWorkspace, + logout, + } = auth; + + const apiOnline = useScriptWorkspaceStore((s) => s.apiOnline); + const endEditing = useScriptWorkspaceStore((s) => s.endEditing); + const selectScript = useScriptWorkspaceStore((s) => s.selectScript); + + const toast = useUiStore((s) => s.toast); + const dismissToast = useUiStore((s) => s.dismissToast); + const workspaceMenuOpen = useUiStore((s) => s.workspaceMenuOpen); + const setWorkspaceMenuOpen = useUiStore((s) => s.setWorkspaceMenuOpen); + + const [sidebarCollapsed, setSidebarCollapsed] = useState(false); + + // 绑定 api 到 script workspace store + useEffect(() => { + bindScriptWorkspaceApi(api); + return () => { + bindScriptWorkspaceApi(null); + }; + }, [api]); + + // 心跳 / cleanup / 切页结束编辑 / 卸载前释放 + useEditSessionLifecycle({ activePage }); + + // toast 自动消失 + useEffect(() => { + if (!toast) return; + const timer = window.setTimeout(dismissToast, 3200); + return () => window.clearTimeout(timer); + }, [toast, dismissToast]); + + return ( +
+ navigate(pathForPage(page))} + onToggleCollapse={() => setSidebarCollapsed((v) => !v)} + onEndEditing={() => void endEditing(true)} + onSelectScript={selectScript} + editSessionRef={editSessionHandle} + /> + +
+ + + +
+ + +
+ ); +} \ No newline at end of file