From 0ac034d7d330f589bea683e7210461b344523c4b Mon Sep 17 00:00:00 2001 From: "tao.chen" <93983997+taochen-ct@users.noreply.github.com> Date: Fri, 21 Aug 2026 11:31:07 +0800 Subject: [PATCH] fix(scripts): sequence-token race + comment/import cleanup (Codex ffec234 follow-up) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review of ffec234 flagged: 1. **MEDIUM — loadScriptCount in-flight race on workspace switch.** Previous implementation used `if (get().scriptCountLoading) return` to dedupe. That meant switching workspaces WHILE a fetch was in flight dropped the new fetch entirely; the stale response from the previous workspace then overwrote state, leaving the dashboard showing workspace A's total while the user is on workspace B. Fix: drop the dedupe-via-flag, use a module-level `_scriptCountSeq` counter. Every call increments, captures the seq at start, and the response/finally block only mutates state when `_scriptCountSeq === seq` — stale responses are silently dropped. Rapid workspace switches each get their own fetch; only the latest response wins. 2. **LOW — scriptCount scope comment was misleading.** "范围与 list_scripts(parent_path=\"\") 对齐" is wrong: the count includes all descendant depths, not just root-level. Behaviour is correct for the dashboard's "全部脚本" intent but the comment would mislead the next maintainer. Rewritten to explicitly call out that the count is the UNION across all parent_path depths, with the rationale for each design choice. 3. **LOW — local `from backend.scripts import _escape_like_pattern` inside `resources.py` keyword-search block.** The "avoid cycle" justification was false: scripts.py and resources.py don't import each other at module level. Moved to the top-of-file import block. Skipped: - get_workspace_tree `like_prefix` not run through the escape helper (user_id is a 26-char Crockford ULID so no `_`/`%` can appear, but the invariant is not documented at the call site). Pre-existing pattern; out of scope for this round. Verified: pytest 65 passed; pnpm typecheck clean. --- backend/src/backend/resources.py | 2 +- backend/src/backend/scripts.py | 17 ++++++++++++----- .../platform/state/scriptWorkspaceStore.ts | 13 +++++++++++-- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/backend/src/backend/resources.py b/backend/src/backend/resources.py index c7a8fa5..7522f7b 100644 --- a/backend/src/backend/resources.py +++ b/backend/src/backend/resources.py @@ -28,6 +28,7 @@ from backend.dependencies import ( database_session, request_context, ) +from backend.scripts import _escape_like_pattern from backend.schemas import ( CompleteResourceUploadRequest, CreateResourceUploadRequest, @@ -402,7 +403,6 @@ async def list_resources( if keyword: # Escape LIKE metacharacters so a search like "100%" or "my_file" # doesn't act as a wildcard. The outer "%...%" wildcards stay raw. - from backend.scripts import _escape_like_pattern # local import: avoid cycle escaped = _escape_like_pattern(keyword.strip()) statement = statement.where( DataResources.resource_name.like(f"%{escaped}%", escape="\\") diff --git a/backend/src/backend/scripts.py b/backend/src/backend/scripts.py index d98405b..aaabd7a 100644 --- a/backend/src/backend/scripts.py +++ b/backend/src/backend/scripts.py @@ -1138,11 +1138,18 @@ async def list_scripts( # 避免被 listScripts 的懒加载语义污染。该路由必须在 /scripts/{script_id} 之前声明 # ——FastAPI 按声明顺序匹配,否则 `count` 会被当作 script_id 命中 get_script。 # -# 范围与 list_scripts(parent_path="") 对齐:INNER JOIN 到 StorageObjects 以排除 -# 孤儿脚本(其 current_object_id 没有 joinable row),按用户子树 -# (workspace/{user_id}/) 过滤。否则: -# - 含孤儿 → 数字虚高 -# - workspace 范围 → 多成员工作区里 dashboard 会显示用户看不见的脚本 +# Scope: equals the UNION of list_scripts across every parent_path +# within the user's subtree, NOT just the root-level call. We +# intentionally include deeper descendants because the dashboard's +# "全部脚本" / "工作副本" counts reflect the whole workspace, not +# only the top level. +# +# Implementation choices and why: +# - INNER JOIN to StorageObjects so orphans (current_object_id has no +# joinable row) are excluded. +# - User subtree filter so multi-member workspaces don't show counts +# the requester can't see. +# - No NOT-LIKE filter because the count wants descendants too. @router.get("/api/v1/scripts/count") async def count_scripts( context: RequestContext = Depends(request_context), diff --git a/frontend/app/features/platform/state/scriptWorkspaceStore.ts b/frontend/app/features/platform/state/scriptWorkspaceStore.ts index 32fa0f7..cdae484 100644 --- a/frontend/app/features/platform/state/scriptWorkspaceStore.ts +++ b/frontend/app/features/platform/state/scriptWorkspaceStore.ts @@ -40,6 +40,7 @@ let _api: WorkspaceBoundApi | null = null; let _previewController: AbortController | null = null; let _previewRequest = 0; let _pythonEditorOpeningIds = new Set(); +let _scriptCountSeq = 0; export const bindScriptWorkspaceApi = (api: WorkspaceBoundApi | null) => { _api = api; @@ -397,17 +398,25 @@ export const useScriptWorkspaceStore = create((set, get) => { loadScriptCount: async () => { const api = requireApi(); - if (get().scriptCountLoading) return; + // Always fire — don't dedupe via the loading flag. Rapid workspace + // switches would otherwise drop the new fetch and leave the + // dashboard showing the previous workspace's count. The sequence + // counter below discards stale responses instead. + const seq = ++_scriptCountSeq; set({ scriptCountLoading: true, scriptCount: null }); try { const total = await api.countScripts(); + if (_scriptCountSeq !== seq) return; // a newer fetch superseded us set({ scriptCount: total }); } catch { + if (_scriptCountSeq !== seq) return; // Leave previous value in place; the dashboard already tolerates // a stale count by rendering `scriptCount ?? 0`. Don't toast — // the dashboard's other metrics are best-effort. } finally { - set({ scriptCountLoading: false }); + if (_scriptCountSeq === seq) { + set({ scriptCountLoading: false }); + } } },