Repository navigation
Feature/applications site weibo - #2406
Conversation
📝 WalkthroughWalkthroughThe Sites API and workspace now support typed site and Weibo mini-program applications. The change adds discriminated response models, app-type filtering, tabbed rendering, typed creation prompts, accessible menus, built-in plugin staging, updated localization, and E2E coverage. ChangesTyped Applications
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant User
participant SitesWorkspace
participant SitesApi
participant SitesService
participant PlatformProjects
User->>SitesWorkspace: Select application type
SitesWorkspace->>SitesApi: Request typed application list
SitesApi->>SitesService: Forward app_type
SitesService->>PlatformProjects: Query matching projects
PlatformProjects-->>SitesService: Return project payloads
SitesService-->>SitesApi: Return typed response
SitesApi-->>SitesWorkspace: Render site or Weibo mini-program rows
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc4f81e192
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return | ||
| } | ||
|
|
||
| if (request.method === 'GET' && url.pathname === '/api/sites') { |
There was a problem hiding this comment.
In the e2e:desktop:plugins flow, CLOUD_ONLY is false, so the app uses the control server as its Backend and this handler synthesizes /api/sites responses. Consequently, the new Applications assertions never exercise the changed FastAPI endpoint, schema discrimination, service filtering, or upstream adapter; an incompatible Backend contract can pass this regression test. Start the real Backend with the existing Sites upstream mock and route this scenario through it instead.
AGENTS.md reference: wework/AGENTS.md:L34-L34
Useful? React with 👍 / 👎.
| setItems(current => [...current, ...response.items.filter(item => item.app_type === appType)]) | ||
| setTotal(response.total) |
There was a problem hiding this comment.
If a user clicks Load more and switches application tabs before that request finishes, the new tab's first-page request may resolve first, after which this old request unconditionally appends records from the previous type and overwrites the new total. The hidden stale records then inflate items.length, so the next request for the active tab uses an incorrect offset and can skip applications. Associate load-more requests with the current request generation/app type and ignore stale responses, as loadFirstPage already does.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/app/services/sites.py`:
- Around line 315-341: Update the updated_at assignment in the mini_program
construction within the relevant site conversion method to fall back to
created_at when the upstream value is missing or explicitly null, using a
falsy-safe fallback. Preserve the existing payload value when it is present and
valid so WeiboMiniProgramResponse.model_validate can process projects without
timestamps.
In `@docs/en/wework/developer-guide/wework-e2e-automation.md`:
- Around line 214-216: Update the supported query-parameter list for GET
/api/v1/projects/search in the developer guide to include app_type, matching the
mock behavior described above and the backend request contract.
In `@docs/zh/wework/developer-guide/wework-e2e-automation.md`:
- Line 205: Update the GET /api/v1/projects/search endpoint documentation to
list app_type among its supported parameters, and specify that its default value
is site. Keep the surrounding description of filtering and returned project
types consistent.
In `@wework/e2e/desktop/task-flow.e2e.mjs`:
- Around line 7867-7916: Configure the real Backend used by the typed
applications flow to use mock-sites-upstream-server.mjs, and remove the
/api/sites fixture handled in the desktop E2E request server. Preserve the
upstream mock exclusively for the external Sites project API, ensuring the test
exercises Backend conversion and filtering through real Backend requests.
In `@wework/src/components/common/ActionMenu.tsx`:
- Around line 110-118: Update handleTriggerKeyDown to store whether the menu was
opened with ArrowUp or ArrowDown, then use that direction in the menu-opening
focus logic around the existing first-enabled-item behavior at lines 186-193.
Focus the last enabled item when ArrowUp opens the menu and retain first enabled
item focus for ArrowDown, using the existing enabled-item collection and
preserving current keyboard handling.
In `@wework/src/components/sites/SitesWorkspace.tsx`:
- Around line 321-335: Update the experience button in the program status
rendering to use the existing mobile touch-target pattern, adding h-11 and
applying md:h-8 for larger screens while preserving its current inline-flex
styling and behavior.
- Around line 314-316: Update the AppID display in SitesWorkspace so the
separator is included in the localized t('weibo_app_id', ...) value rather than
hardcoded in JSX, and add matching weibo_app_id translations to the zh-CN and en
sites locale files with locale-appropriate punctuation.
- Around line 110-134: Update loadMore to participate in the existing requestId
guard used by loadFirstPage: capture the current request ID before calling
api.listSites, then verify it still matches before applying any response or
error state updates. Ensure stale responses cannot append items, overwrite
total, or alter availability/load-error state after an appType tab switch, while
preserving loading cleanup for the active request.
In `@wework/src/i18n/locales/zh-CN/common.json`:
- Line 267: Update the workspace_tab_sites translation in the Chinese locale to
a label distinct from workspace_tab_apps, using “应用管理” or another clear
equivalent while leaving workspace_tab_apps unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ad33dcb7-4ab3-4299-badc-74c4139c040f
📒 Files selected for processing (33)
backend/app/api/endpoints/sites.pybackend/app/schemas/site.pybackend/app/services/sites.pybackend/tests/api/test_sites_api.pydocs/en/wegent/user-guide/coding/priority-activity-filter.mddocs/en/wegent/user-guide/settings/experimental-features.mddocs/en/wework/developer-guide/wework-codex-plugins.mddocs/en/wework/developer-guide/wework-e2e-automation.mddocs/en/wework/plugins-and-skills.mddocs/zh/wegent/user-guide/coding/priority-activity-filter.mddocs/zh/wegent/user-guide/settings/experimental-features.mddocs/zh/wework/developer-guide/wework-codex-plugins.mddocs/zh/wework/developer-guide/wework-e2e-automation.mddocs/zh/wework/plugins-and-skills.mdwework/e2e/desktop/task-flow.e2e.mjswework/e2e/tests/upstream-mocks.spec.tswework/e2e/utils/mock-sites-upstream-server.mjswework/src/App.plugins.test.tsxwework/src/App.tsxwework/src/api/sites.test.tswework/src/api/sites.tswework/src/components/common/ActionMenu.tsxwework/src/components/layout/DesktopSidebar.test.tsxwework/src/components/layout/DesktopSidebar.tsxwework/src/components/sites/SitesWorkspace.test.tsxwework/src/components/sites/SitesWorkspace.tsxwework/src/features/plugins/pluginTrial.test.tswework/src/features/plugins/pluginTrial.tswework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/en/sites.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/i18n/locales/zh-CN/sites.jsonwework/src/pages/SitesPage.tsx
| Wework E2E 还会启动两个本机 loopback upstream mock。它们只替代 Wegent 之外的外部服务，不替代 Wegent Backend、Executor、Codex、`/api/sites`、`/api/apps/installed` 或 connector runtime API。 | ||
|
|
||
| `wework/e2e/utils/mock-sites-upstream-server.mjs` 模拟 Sites project API： | ||
| `wework/e2e/utils/mock-sites-upstream-server.mjs` 模拟 Sites project API，并根据 `app_type` 返回站点或微博小程序列表： |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
List app_type in the endpoint parameters.
Line 205 states that the mock filters by app_type. The GET /api/v1/projects/search entry below does not list app_type as a supported parameter. Add it and state that it defaults to site.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/zh/wework/developer-guide/wework-e2e-automation.md` at line 205, Update
the GET /api/v1/projects/search endpoint documentation to list app_type among
its supported parameters, and specify that its default value is site. Keep the
surrounding description of filtering and returned project types consistent.
| if (request.method === 'GET' && url.pathname === '/api/sites') { | ||
| const appType = url.searchParams.get('app_type') || 'site' | ||
| const query = url.searchParams.get('q')?.trim().toLowerCase() || '' | ||
| const items = | ||
| appType === 'weibo_mini_program' | ||
| ? [ | ||
| { | ||
| app_type: 'weibo_mini_program', | ||
| siteid: 'prj_e2e_weibo_mini', | ||
| taskid: 'prj_e2e_weibo_mini', | ||
| username: 'wework-desktop-e2e-cloud-user', | ||
| name: 'E2E Weibo Mini Program', | ||
| slug: 'prj_e2e_weibo_mini', | ||
| app_id: 'wx-e2e-weibo-mini', | ||
| status: 'experience', | ||
| version: '1.0.0', | ||
| experience_url: 'https://example.test/weibo-mini-experience', | ||
| thumbnail_url: null, | ||
| created_at: '2026-07-22T00:10:00Z', | ||
| updated_at: '2026-07-22T00:12:00Z', | ||
| }, | ||
| ] | ||
| : [ | ||
| { | ||
| app_type: 'site', | ||
| siteid: 'prj_e2e_product', | ||
| taskid: 'prj_e2e_product', | ||
| username: 'wework-desktop-e2e-cloud-user', | ||
| name: 'E2E Product Site', | ||
| slug: 'prj_e2e_product', | ||
| internal_url: 'https://sites.internal/e2e-product', | ||
| external_url: null, | ||
| publish_status: 'unpublished', | ||
| last_publish_error: null, | ||
| thumbnail_url: null, | ||
| created_at: '2026-07-22T00:00:00Z', | ||
| updated_at: '2026-07-22T00:00:00Z', | ||
| published_at: null, | ||
| }, | ||
| ] | ||
| const filteredItems = query | ||
| ? items.filter(item => item.name.toLowerCase().includes(query)) | ||
| : items | ||
| json(response, 200, { | ||
| items: filteredItems, | ||
| total: filteredItems.length, | ||
| offset: Number.parseInt(url.searchParams.get('offset') || '0', 10), | ||
| limit: Number.parseInt(url.searchParams.get('limit') || '20', 10), | ||
| }) | ||
| return |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Use the real Backend for the typed applications flow.
Lines 7867-7916 mock /api/sites in the desktop E2E server. The test at Lines 4523-4555 therefore bypasses Backend conversion and filtering for typed applications.
Configure the real Backend to use mock-sites-upstream-server.mjs. Remove this /api/sites fixture from the desktop E2E path. Keep the upstream mock only for the external Sites project API.
As per coding guidelines, E2E tests must use real Backend requests and must not replace integrations with mocks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/e2e/desktop/task-flow.e2e.mjs` around lines 7867 - 7916, Configure the
real Backend used by the typed applications flow to use
mock-sites-upstream-server.mjs, and remove the /api/sites fixture handled in the
desktop E2E request server. Preserve the upstream mock exclusively for the
external Sites project API, ensuring the test exercises Backend conversion and
filtering through real Backend requests.
Source: Coding guidelines
| const handleTriggerKeyDown = (event: ReactKeyboardEvent<HTMLButtonElement>) => { | ||
| if (disabled) return | ||
| if (event.key !== 'ArrowDown' && event.key !== 'ArrowUp') return | ||
| event.preventDefault() | ||
| if (!menuOpen) { | ||
| setMenuPosition(null) | ||
| setOpen(true) | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Focus the last enabled item when ArrowUp opens the menu.
Line 110 opens the menu for ArrowUp. Lines 186-192 always focus the first enabled item. Store the opening direction and focus the last enabled item for ArrowUp.
Proposed fix
+ const focusLastItemRef = useRef(false)
+
const handleTriggerKeyDown = (event: ReactKeyboardEvent<HTMLButtonElement>) => {
if (disabled) return
if (event.key !== 'ArrowDown' && event.key !== 'ArrowUp') return
event.preventDefault()
if (!menuOpen) {
+ focusLastItemRef.current = event.key === 'ArrowUp'
setMenuPosition(null)
setOpen(true)
}
}
...
- const target = document.activeElement === triggerRef.current ? items[0] : null
+ const target =
+ document.activeElement === triggerRef.current
+ ? items[focusLastItemRef.current ? items.length - 1 : 0]
+ : null
target?.focus()Also applies to: 186-193
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/components/common/ActionMenu.tsx` around lines 110 - 118, Update
handleTriggerKeyDown to store whether the menu was opened with ArrowUp or
ArrowDown, then use that direction in the menu-opening focus logic around the
existing first-enabled-item behavior at lines 186-193. Focus the last enabled
item when ArrowUp opens the menu and retain first enabled item focus for
ArrowDown, using the existing enabled-item collection and preserving current
keyboard handling.
| const loadMore = async () => { | ||
| setLoadingMore(true) | ||
| setLoadError(null) | ||
| try { | ||
| const response = await api.listSites({ | ||
| appType, | ||
| q: query, | ||
| offset: items.length, | ||
| limit: pageSize, | ||
| }) | ||
| setItems(current => [...current, ...response.items.filter(item => item.app_type === appType)]) | ||
| setTotal(response.total) | ||
| } catch (error) { | ||
| if (isSitesUnavailableError(error)) { | ||
| setUnavailable(true) | ||
| setItems([]) | ||
| setTotal(0) | ||
| setLoadError(null) | ||
| } else { | ||
| setLoadError(errorMessage(error, loadFailedMessage)) | ||
| } | ||
| } finally { | ||
| setLoadingMore(false) | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard loadMore against a stale response after a tab switch.
loadFirstPage increments and checks requestId, but loadMore does not participate in that scheme. If a user switches the application tab while a loadMore request is in flight, the resolved response still applies its state updates:
setTotal(response.total)overwrites the new tab's total. Line 757 usesitems.length < collection.totalto decide whether to render the load-more button, so the button can appear or disappear incorrectly for the new tab.- The
filteron line 120 captures the previousappType, so items of the previous type are appended to state. Line 420 hides them from the view, but they still inflatecollection.itemsand shift theitems.lengthoffset used by the nextloadMorecall, which can skip or duplicate a page.
Reuse the existing requestId ref in loadMore.
🛠️ Proposed fix
const loadMore = async () => {
+ const currentRequest = ++requestId.current
setLoadingMore(true)
setLoadError(null)
try {
const response = await api.listSites({
appType,
q: query,
offset: items.length,
limit: pageSize,
})
+ if (currentRequest !== requestId.current) return
setItems(current => [...current, ...response.items.filter(item => item.app_type === appType)])
setTotal(response.total)
} catch (error) {
+ if (currentRequest !== requestId.current) return
if (isSitesUnavailableError(error)) {
setUnavailable(true)
setItems([])
setTotal(0)
setLoadError(null)
} else {
setLoadError(errorMessage(error, loadFailedMessage))
}
} finally {
- setLoadingMore(false)
+ if (currentRequest === requestId.current) setLoadingMore(false)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const loadMore = async () => { | |
| setLoadingMore(true) | |
| setLoadError(null) | |
| try { | |
| const response = await api.listSites({ | |
| appType, | |
| q: query, | |
| offset: items.length, | |
| limit: pageSize, | |
| }) | |
| setItems(current => [...current, ...response.items.filter(item => item.app_type === appType)]) | |
| setTotal(response.total) | |
| } catch (error) { | |
| if (isSitesUnavailableError(error)) { | |
| setUnavailable(true) | |
| setItems([]) | |
| setTotal(0) | |
| setLoadError(null) | |
| } else { | |
| setLoadError(errorMessage(error, loadFailedMessage)) | |
| } | |
| } finally { | |
| setLoadingMore(false) | |
| } | |
| } | |
| const loadMore = async () => { | |
| const currentRequest = ++requestId.current | |
| setLoadingMore(true) | |
| setLoadError(null) | |
| try { | |
| const response = await api.listSites({ | |
| appType, | |
| q: query, | |
| offset: items.length, | |
| limit: pageSize, | |
| }) | |
| if (currentRequest !== requestId.current) return | |
| setItems(current => [...current, ...response.items.filter(item => item.app_type === appType)]) | |
| setTotal(response.total) | |
| } catch (error) { | |
| if (currentRequest !== requestId.current) return | |
| if (isSitesUnavailableError(error)) { | |
| setUnavailable(true) | |
| setItems([]) | |
| setTotal(0) | |
| setLoadError(null) | |
| } else { | |
| setLoadError(errorMessage(error, loadFailedMessage)) | |
| } | |
| } finally { | |
| if (currentRequest === requestId.current) setLoadingMore(false) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/components/sites/SitesWorkspace.tsx` around lines 110 - 134,
Update loadMore to participate in the existing requestId guard used by
loadFirstPage: capture the current request ID before calling api.listSites, then
verify it still matches before applying any response or error state updates.
Ensure stale responses cannot append items, overwrite total, or alter
availability/load-error state after an appType tab switch, while preserving
loading cleanup for the active request.
| <p className="mt-1 truncate text-sm leading-5 text-text-secondary"> | ||
| {t('weibo_app_id', 'AppID')}：{program.app_id} | ||
| </p> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Move the colon into the localized string.
Line 315 hardcodes the full-width colon ： outside the translation call. The English locale then renders AppID：1234567890 with CJK punctuation. Put the separator inside the localized value so each locale controls its own punctuation.
🛠️ Proposed fix
<p className="mt-1 truncate text-sm leading-5 text-text-secondary">
- {t('weibo_app_id', 'AppID')}：{program.app_id}
+ {t('weibo_app_id_value', { defaultValue: 'AppID：{{appId}}', appId: program.app_id })}
</p>Add the matching key to both wework/src/i18n/locales/zh-CN/sites.json and wework/src/i18n/locales/en/sites.json.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/components/sites/SitesWorkspace.tsx` around lines 314 - 316,
Update the AppID display in SitesWorkspace so the separator is included in the
localized t('weibo_app_id', ...) value rather than hardcoded in JSX, and add
matching weibo_app_id translations to the zh-CN and en sites locale files with
locale-appropriate punctuation.
| "workspace_tab_auxiliary": "工作区", | ||
| "workspace_tab_plugins": "插件", | ||
| "workspace_tab_sites": "站点", | ||
| "workspace_tab_sites": "应用", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use distinct Chinese labels for Applications and Apps.
workspace_tab_sites is translated as “应用”, and workspace_tab_apps is also “应用” on Line 270. Users cannot distinguish these workspace tabs by text.
Use a distinct label such as “应用管理” for workspace_tab_sites.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/i18n/locales/zh-CN/common.json` at line 267, Update the
workspace_tab_sites translation in the Chinese locale to a label distinct from
workspace_tab_apps, using “应用管理” or another clear equivalent while leaving
workspace_tab_apps unchanged.
add7278 to
5b40a01
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
wework/src/components/sites/ApplicationRows.tsx (3)
175-182: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMake
canOpenExperiencea boolean.
capabilities.has('open_experience') && program.experience_urlevaluates tostring | null | undefined | false. The ternary works, but the name states a boolean. Coerce the expression so the type matches the name.♻️ Proposed refactor
- const canOpenExperience = capabilities.has('open_experience') && program.experience_url + const canOpenExperience = capabilities.has('open_experience') && Boolean(program.experience_url)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/sites/ApplicationRows.tsx` around lines 175 - 182, Update canOpenExperience to explicitly coerce the existing capabilities.has('open_experience') and program.experience_url expression to a boolean, while leaving openExperience unchanged.
169-174: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winHoist the
Intl.DateTimeFormatinstance out of the component.The component builds a new formatter on every render. Formatter construction is the expensive part of
Intl. Declare one module-level constant and reuse it.♻️ Proposed refactor
+const UPDATED_AT_FORMAT = new Intl.DateTimeFormat(undefined, { + year: 'numeric', + month: 'short', + day: 'numeric', +}) + export function MiniProgramApplicationRow({ program, capabilities, }: { program: MiniProgram capabilities: ReadonlySet<ApplicationCapability> }) { const { t } = useTranslation('sites') - const updatedAt = new Intl.DateTimeFormat(undefined, { - year: 'numeric', - month: 'short', - day: 'numeric', - }).format(new Date(program.updated_at)) + const updatedAt = UPDATED_AT_FORMAT.format(new Date(program.updated_at))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/sites/ApplicationRows.tsx` around lines 169 - 174, Move the Intl.DateTimeFormat construction out of the component and declare a module-level formatter constant near the component definition. Update the updatedAt calculation in ApplicationRows to reuse this constant when formatting program.updated_at.
65-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign the new rows with the DESIGN.md typography scale and the mobile touch-target minimum.
Two guideline deviations exist in this new file:
- The publish button and the experience button are 32px tall (
h-8) with no minimum height for small screens. Both rows render on mobile, so both controls must reach 44px × 44px there.- The
h2elements usetext-base(16px) instead of a semanticheading-*role from the shared typography scale.As per coding guidelines: "Mobile controls must be at least 44px × 44px" and "Use the shared typography scale and semantic
heading-*,text-chat, andtext-coderoles."Also applies to: 106-112, 192-194, 203-207
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/sites/ApplicationRows.tsx` around lines 65 - 67, Update the h2 heading styling across the file (including the one at the shown line and at lines 106-112, 192-194, 203-207) to replace text-base with the appropriate semantic heading role from the shared typography scale instead of hardcoded font sizes. Additionally, update the publish button and experience button elements (which currently have h-8 height) to include minimum height constraints that ensure they reach the required 44px × 44px touch-target minimum on mobile screens.Source: Coding guidelines
backend/app/services/site_application_types.py (1)
70-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared identity fields of the two handlers.
Both
parseimplementations repeat the same mapping forsiteid,taskid,username,name,slug,thumbnail_url, andcreated_at. Move that mapping into one protected helper onApplicationTypeHandlerand let each handler add its own fields. A new application type then only declares its differences.As per coding guidelines: "Before adding code, search for and reuse existing components, services, utilities, and patterns; extract shared logic instead of duplicating it."
♻️ Sketch
class ApplicationTypeHandler(ABC): def _common_fields(self, payload: dict[str, Any], *, username: str) -> dict[str, Any]: project_id = payload.get("id") snapshot = payload.get("snapshot") return { "app_type": self.app_type, "siteid": project_id, "taskid": project_id, "username": username, "name": payload.get("title"), "slug": project_id, "thumbnail_url": ( snapshot if isinstance(snapshot, str) and snapshot else None ), "created_at": payload.get("created_at"), }Also applies to: 108-124
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/services/site_application_types.py` around lines 70 - 87, Extract the duplicated identity mapping from both parse implementations into a protected ApplicationTypeHandler._common_fields helper, using the existing app_type, payload, and username values to populate siteid, taskid, username, name, slug, thumbnail_url, and created_at. Update each parse handler to reuse this helper and add only its type-specific fields, while preserving the existing publish and URL fields.Source: Coding guidelines
wework/src/components/sites/useApplicationTypeDefinitions.ts (1)
32-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog the discovery failure before falling back to local defaults.
The
catchblock discards the error. When/sites/app-typesfails, the hook silently shows every locally defined application type with its full local capability set. A user then sees a Mini Programs tab that an older Backend cannot serve, and there is no evidence in the console to diagnose it. Add a focused diagnostic log in thecatch.As per coding guidelines: "Diagnose problems using logs, actual code, and other concrete evidence first; when evidence is insufficient, add focused diagnostic logging before changing behavior and do not guess."
♻️ Proposed change
- .catch(() => { - if (!cancelled) setDescriptors(null) + .catch(error => { + console.warn('[Wework Applications] application type discovery failed', error) + if (!cancelled) setDescriptors(null) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/sites/useApplicationTypeDefinitions.ts` around lines 32 - 39, Add a focused diagnostic log in the catch handler for the listApplicationTypes request, including the caught error and clear context that application-type discovery failed before setDescriptors(null) applies the local fallback. Preserve the existing cancellation check and fallback behavior.Source: Coding guidelines
wework/src/pages/SitesPage.tsx (1)
146-231: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit
handleCreateinto focused units.
handleCreatenow spans about 85 lines and carries five responsibilities: strategy resolution, device selection, sync verification, trial queuing, and a six-branch error-to-message mapper. Extract the error mapper into a module-level function that returns the translation key and fallback, and extract the sync verification into a helper. The main flow then reads as a short sequence.As per coding guidelines: "functions should remain focused, preferably under 50 lines."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/pages/SitesPage.tsx` around lines 146 - 231, Refactor handleCreate into focused helpers: extract a module-level error-mapping function that returns the translation key and fallback for each existing error branch, and extract the plugin synchronization validation into a helper covering the current success criteria and SitesDeviceSyncConfirmationError behavior. Keep strategy resolution, device selection, queuing, navigation, and cleanup behavior unchanged while reducing handleCreate below 50 lines.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/app/services/site_application_types.py`:
- Around line 122-124: Update the updated_at assignment in the response payload
construction to fall back to created_at when payload.get("updated_at") is null
or otherwise falsy, while preserving a provided truthy timestamp.
In `@wework/src/components/sites/ApplicationRows.tsx`:
- Line 196: Update the translation usage in ApplicationRows so the separator is
included in the locale string rather than hardcoded outside t(); pass
program.app_id through the translation interpolation parameter, and update the
English and Chinese locale values to use their appropriate colon typography with
the appId placeholder.
---
Nitpick comments:
In `@backend/app/services/site_application_types.py`:
- Around line 70-87: Extract the duplicated identity mapping from both parse
implementations into a protected ApplicationTypeHandler._common_fields helper,
using the existing app_type, payload, and username values to populate siteid,
taskid, username, name, slug, thumbnail_url, and created_at. Update each parse
handler to reuse this helper and add only its type-specific fields, while
preserving the existing publish and URL fields.
In `@wework/src/components/sites/ApplicationRows.tsx`:
- Around line 175-182: Update canOpenExperience to explicitly coerce the
existing capabilities.has('open_experience') and program.experience_url
expression to a boolean, while leaving openExperience unchanged.
- Around line 169-174: Move the Intl.DateTimeFormat construction out of the
component and declare a module-level formatter constant near the component
definition. Update the updatedAt calculation in ApplicationRows to reuse this
constant when formatting program.updated_at.
- Around line 65-67: Update the h2 heading styling across the file (including
the one at the shown line and at lines 106-112, 192-194, 203-207) to replace
text-base with the appropriate semantic heading role from the shared typography
scale instead of hardcoded font sizes. Additionally, update the publish button
and experience button elements (which currently have h-8 height) to include
minimum height constraints that ensure they reach the required 44px × 44px
touch-target minimum on mobile screens.
In `@wework/src/components/sites/useApplicationTypeDefinitions.ts`:
- Around line 32-39: Add a focused diagnostic log in the catch handler for the
listApplicationTypes request, including the caught error and clear context that
application-type discovery failed before setDescriptors(null) applies the local
fallback. Preserve the existing cancellation check and fallback behavior.
In `@wework/src/pages/SitesPage.tsx`:
- Around line 146-231: Refactor handleCreate into focused helpers: extract a
module-level error-mapping function that returns the translation key and
fallback for each existing error branch, and extract the plugin synchronization
validation into a helper covering the current success criteria and
SitesDeviceSyncConfirmationError behavior. Keep strategy resolution, device
selection, queuing, navigation, and cleanup behavior unchanged while reducing
handleCreate below 50 lines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 40d5a23d-dace-4925-b418-3538c4c47bd8
📒 Files selected for processing (37)
backend/app/api/endpoints/sites.pybackend/app/schemas/site.pybackend/app/services/site_application_types.pybackend/app/services/sites.pybackend/tests/api/test_sites_api.pydocs/en/wegent/user-guide/coding/priority-activity-filter.mddocs/en/wegent/user-guide/settings/experimental-features.mddocs/en/wework/developer-guide/wework-codex-plugins.mddocs/en/wework/developer-guide/wework-e2e-automation.mddocs/en/wework/plugins-and-skills.mddocs/zh/wegent/user-guide/coding/priority-activity-filter.mddocs/zh/wegent/user-guide/settings/experimental-features.mddocs/zh/wework/developer-guide/wework-codex-plugins.mddocs/zh/wework/developer-guide/wework-e2e-automation.mddocs/zh/wework/plugins-and-skills.mdwework/e2e/desktop/task-flow.e2e.mjswework/e2e/tests/upstream-mocks.spec.tswework/e2e/utils/mock-sites-upstream-server.mjswework/src/App.plugins.test.tsxwework/src/App.tsxwework/src/api/sites.test.tswework/src/api/sites.tswework/src/components/common/ActionMenu.tsxwework/src/components/layout/DesktopSidebar.test.tsxwework/src/components/layout/DesktopSidebar.tsxwework/src/components/sites/ApplicationRows.tsxwework/src/components/sites/SitesWorkspace.test.tsxwework/src/components/sites/SitesWorkspace.tsxwework/src/components/sites/applicationTypeDefinitions.tsxwework/src/components/sites/useApplicationTypeDefinitions.tswework/src/features/plugins/pluginTrial.test.tswework/src/features/plugins/pluginTrial.tswework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/en/sites.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/i18n/locales/zh-CN/sites.jsonwework/src/pages/SitesPage.tsx
🚧 Files skipped from review as they are similar to previous changes (24)
- wework/src/components/layout/DesktopSidebar.test.tsx
- wework/src/i18n/locales/en/common.json
- wework/src/components/layout/DesktopSidebar.tsx
- docs/en/wework/developer-guide/wework-e2e-automation.md
- docs/en/wegent/user-guide/settings/experimental-features.md
- docs/zh/wework/developer-guide/wework-e2e-automation.md
- wework/src/App.tsx
- wework/src/i18n/locales/en/sites.json
- docs/en/wegent/user-guide/coding/priority-activity-filter.md
- wework/src/features/plugins/pluginTrial.ts
- docs/zh/wegent/user-guide/coding/priority-activity-filter.md
- wework/e2e/utils/mock-sites-upstream-server.mjs
- wework/src/i18n/locales/zh-CN/common.json
- wework/src/i18n/locales/zh-CN/sites.json
- docs/en/wework/plugins-and-skills.md
- wework/e2e/tests/upstream-mocks.spec.ts
- wework/src/features/plugins/pluginTrial.test.ts
- wework/src/components/sites/SitesWorkspace.test.tsx
- wework/e2e/desktop/task-flow.e2e.mjs
- wework/src/components/common/ActionMenu.tsx
- wework/src/App.plugins.test.tsx
- wework/src/components/sites/SitesWorkspace.tsx
- backend/app/services/sites.py
- docs/zh/wegent/user-guide/settings/experimental-features.md
| "created_at": created_at, | ||
| "updated_at": payload.get("updated_at", created_at), | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Handle an explicit null updated_at from upstream.
payload.get("updated_at", created_at) returns the default only when the key is absent. If upstream sends "updated_at": null, the value becomes None, MiniProgramResponse.model_validate fails, and the whole listing page is rejected as an invalid project. Use a truthiness fallback so a null value falls back to created_at.
🛡️ Proposed fix
- "updated_at": payload.get("updated_at", created_at),
+ "updated_at": payload.get("updated_at") or created_at,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "created_at": created_at, | |
| "updated_at": payload.get("updated_at", created_at), | |
| } | |
| "created_at": created_at, | |
| "updated_at": payload.get("updated_at") or created_at, | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/app/services/site_application_types.py` around lines 122 - 124,
Update the updated_at assignment in the response payload construction to fall
back to created_at when payload.get("updated_at") is null or otherwise falsy,
while preserving a provided truthy timestamp.
| {program.name} | ||
| </h2> | ||
| <p className="mt-1 truncate text-sm leading-5 text-text-secondary"> | ||
| {t('mini_program_app_id', 'AppID')}：{program.app_id} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Move the separator into the translation string.
The full-width colon ： is hardcoded outside t(). The English locale then renders AppID：1234567890, which is incorrect for English typography. Put the separator inside each locale value.
🐛 Proposed fix
- <p className="mt-1 truncate text-sm leading-5 text-text-secondary">
- {t('mini_program_app_id', 'AppID')}：{program.app_id}
- </p>
+ <p className="mt-1 truncate text-sm leading-5 text-text-secondary">
+ {t('mini_program_app_id', 'AppID：{{appId}}', { appId: program.app_id })}
+ </p>Then set the English value to AppID: {{appId}} and the Chinese value to AppID：{{appId}}.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/components/sites/ApplicationRows.tsx` at line 196, Update the
translation usage in ApplicationRows so the separator is included in the locale
string rather than hardcoded outside t(); pass program.app_id through the
translation interpolation parameter, and update the English and Chinese locale
values to use their appropriate colon typography with the appId placeholder.
5b40a01 to
b5d9833
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
backend/app/api/endpoints/sites.py (1)
59-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConfigure Ruff for FastAPI routes in
pyproject.toml.
Query(...)andDepends(...)are FastAPI dependency declarations, but the project lint config does not document howB008applies to them. Add a project-level fastapi/dependency rule setting or a scoped suppression, then ensurebackend/app/api/endpoints/sites.pypasses the configured Ruff check.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/api/endpoints/sites.py` around lines 59 - 63, Update the Ruff configuration in pyproject.toml to explicitly handle B008 for FastAPI Query and Depends declarations, using the project’s dependency-call rule setting or a narrowly scoped suppression. Ensure the route signature in sites.py, including app_type, q, offset, limit, and current_user, passes the resulting Ruff check without broader lint suppression.Source: Linters/SAST tools
backend/tests/services/test_installed_plugin_upload.py (1)
225-229: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd type hints to the changed test signatures.
Add
-> Nonetotest_application_plugins_are_registered_as_builtins. Addplugin_name: strto each parameter introduced by the parameterization.As per coding guidelines, Python code must use type hints.
Also applies to: 232-237, 263-271, 511-522
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/services/test_installed_plugin_upload.py` around lines 225 - 229, Add the `-> None` return annotation to `test_application_plugins_are_registered_as_builtins`, and annotate every parameter introduced by the affected parameterized tests with `plugin_name: str`, including the tests near the additional referenced sections. Preserve the existing test behavior and parameterization.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/fetch-builtin-plugins.mjs`:
- Around line 48-60: Update the archive download flow around
response.arrayBuffer() to read response.body incrementally, track accumulated
bytes, and stop once MAX_ARCHIVE_SIZE_BYTES would be exceeded before buffering
more data. Add a request deadline, and when terminating early consume or cancel
the remaining response stream as appropriate; preserve the existing
oversized-archive error for both declared and streamed sizes.
---
Nitpick comments:
In `@backend/app/api/endpoints/sites.py`:
- Around line 59-63: Update the Ruff configuration in pyproject.toml to
explicitly handle B008 for FastAPI Query and Depends declarations, using the
project’s dependency-call rule setting or a narrowly scoped suppression. Ensure
the route signature in sites.py, including app_type, q, offset, limit, and
current_user, passes the resulting Ruff check without broader lint suppression.
In `@backend/tests/services/test_installed_plugin_upload.py`:
- Around line 225-229: Add the `-> None` return annotation to
`test_application_plugins_are_registered_as_builtins`, and annotate every
parameter introduced by the affected parameterized tests with `plugin_name:
str`, including the tests near the additional referenced sections. Preserve the
existing test behavior and parameterization.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a7e2633d-21f9-4d75-a993-9fe61066b8f3
📒 Files selected for processing (51)
.github/workflows/publish-image.yml.github/workflows/snapshot-image.yml.gitignorebackend/app/api/endpoints/sites.pybackend/app/schemas/site.pybackend/app/services/builtin_plugin_registry.pybackend/app/services/site_application_types.pybackend/app/services/sites.pybackend/init_data/README.mdbackend/tests/api/test_sites_api.pybackend/tests/services/test_installed_plugin_upload.pydocs/en/wegent/user-guide/coding/priority-activity-filter.mddocs/en/wegent/user-guide/settings/experimental-features.mddocs/en/wework/developer-guide/wework-codex-plugins.mddocs/en/wework/developer-guide/wework-e2e-automation.mddocs/en/wework/plugins-and-skills.mddocs/zh/wegent/user-guide/coding/priority-activity-filter.mddocs/zh/wegent/user-guide/settings/experimental-features.mddocs/zh/wework/developer-guide/wework-codex-plugins.mddocs/zh/wework/developer-guide/wework-e2e-automation.mddocs/zh/wework/plugins-and-skills.mdpackage.jsonscripts/builtin-plugin-staging.mjsscripts/fetch-builtin-plugins.mjsscripts/fetch-builtin-sites-plugin.mjsscripts/sync-builtin-plugins.mjsscripts/sync-builtin-sites-plugin.mjswework/e2e/desktop/task-flow.e2e.mjswework/e2e/tests/upstream-mocks.spec.tswework/e2e/utils/mock-sites-upstream-server.mjswework/src/App.plugins.test.tsxwework/src/App.tsxwework/src/api/sites.test.tswework/src/api/sites.tswework/src/components/common/ActionMenu.tsxwework/src/components/layout/DesktopSidebar.test.tsxwework/src/components/layout/DesktopSidebar.tsxwework/src/components/sites/ApplicationRows.tsxwework/src/components/sites/SitesWorkspace.test.tsxwework/src/components/sites/SitesWorkspace.tsxwework/src/components/sites/applicationTypeDefinitions.test.tsxwework/src/components/sites/applicationTypeDefinitions.tsxwework/src/components/sites/useApplicationTypeDefinitions.tswework/src/features/plugins/builtinPlugins.tswework/src/features/plugins/pluginTrial.test.tswework/src/features/plugins/pluginTrial.tswework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/en/sites.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/i18n/locales/zh-CN/sites.jsonwework/src/pages/SitesPage.tsx
💤 Files with no reviewable changes (2)
- scripts/fetch-builtin-sites-plugin.mjs
- scripts/sync-builtin-sites-plugin.mjs
🚧 Files skipped from review as they are similar to previous changes (31)
- wework/src/components/layout/DesktopSidebar.test.tsx
- wework/src/App.tsx
- wework/src/components/layout/DesktopSidebar.tsx
- wework/e2e/tests/upstream-mocks.spec.ts
- wework/src/i18n/locales/en/common.json
- wework/src/features/plugins/pluginTrial.test.ts
- docs/zh/wegent/user-guide/settings/experimental-features.md
- docs/zh/wegent/user-guide/coding/priority-activity-filter.md
- docs/en/wework/developer-guide/wework-e2e-automation.md
- docs/en/wework/plugins-and-skills.md
- wework/src/features/plugins/pluginTrial.ts
- wework/src/components/sites/applicationTypeDefinitions.tsx
- docs/zh/wework/developer-guide/wework-e2e-automation.md
- wework/src/api/sites.test.ts
- wework/src/components/sites/useApplicationTypeDefinitions.ts
- backend/tests/api/test_sites_api.py
- wework/src/components/sites/SitesWorkspace.test.tsx
- wework/src/pages/SitesPage.tsx
- wework/src/i18n/locales/en/sites.json
- backend/app/services/site_application_types.py
- docs/en/wegent/user-guide/settings/experimental-features.md
- wework/src/i18n/locales/zh-CN/common.json
- wework/src/components/sites/ApplicationRows.tsx
- docs/en/wegent/user-guide/coding/priority-activity-filter.md
- wework/src/i18n/locales/zh-CN/sites.json
- wework/e2e/utils/mock-sites-upstream-server.mjs
- backend/app/services/sites.py
- backend/app/schemas/site.py
- wework/src/App.plugins.test.tsx
- wework/src/components/common/ActionMenu.tsx
- wework/src/components/sites/SitesWorkspace.tsx
| const contentLength = Number(response.headers.get("content-length") ?? "0"); | ||
| if (contentLength > MAX_ARCHIVE_SIZE_BYTES) { | ||
| throw new Error( | ||
| `${definition.label} plugin archive exceeds ${MAX_ARCHIVE_SIZE_BYTES} bytes.`, | ||
| ); | ||
| } | ||
|
|
||
| const archive = Buffer.from(await response.arrayBuffer()); | ||
| if (archive.length > MAX_ARCHIVE_SIZE_BYTES) { | ||
| throw new Error( | ||
| `${definition.label} plugin archive exceeds ${MAX_ARCHIVE_SIZE_BYTES} bytes.`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file exists and size =="
if [ -f scripts/fetch-builtin-plugins.mjs ]; then
wc -l scripts/fetch-builtin-plugins.mjs
sed -n '1,140p' scripts/fetch-builtin-plugins.mjs | cat -n
else
echo "scripts/fetch-builtin-plugins.mjs not found"
fd -a 'fetch-builtin-plugins\.mjs'
fi
echo
echo "== references to fetch-builtin-plugins and MAX_ARCHIVE_SIZE_BYTES =="
rg -n "fetch-builtin-plugins|MAX_ARCHIVE_SIZE_BYTES|fetchBuiltin|builtin.*plugin|arrayBuffer|response\.body" . -g '!node_modules' -g '!dist' -g '!build' || trueRepository: wecode-ai/Wegent
Length of output: 20703
🌐 Web query:
Node.js 18+ global fetch response arrayBuffer consumes body buffer response.body stream API headers content-length
💡 Result:
In Node.js (v18 and later), the global fetch implementation follows the Fetch API standard, which dictates that the response body is a stream [1][2]. When you call response.arrayBuffer() (or other consumption methods like .json(), .text(), or .blob()), the underlying response body stream is read to completion, and the body is marked as consumed [3][4][1]. Key points regarding this behavior: 1. Body Consumption: Once response.arrayBuffer() is called, the bodyUsed property of the Response object is set to true [5][3]. Any subsequent attempt to consume the body (e.g., calling another .json() or .arrayBuffer() on the same response object) will result in a TypeError because the stream has already been locked and exhausted [3][1]. 2. Handling Multiple Consumptions: If you need to access the body multiple times, you must use response.clone() before consuming it [3][1]. Cloning creates a new Response object that shares the same underlying data but allows for independent consumption [1]. 3. Streaming vs. Buffering: The use of streams is an efficiency feature intended to prevent loading large responses entirely into memory [1]. While response.arrayBuffer() buffers the entire body into memory to return an ArrayBuffer, using response.body (the raw stream) allows you to process data in chunks without full buffering, provided you handle the stream events yourself [1][6]. 4. Content-Length Headers: When fetch is used, the Content-Length header in a request or response is often managed by the underlying HTTP stack. When you provide a stream as a request body, the Content-Length is typically not automatically set because the total size is unknown; you may need to set this header manually depending on your specific requirements [7]. For responses, Content-Length is generally provided by the server if known; if you are constructing a response from a stream, you must set it manually if it is required by the receiver [7].
Citations:
- 1: https://thenewtoys.dev/blog/2020/07/23/why-make-fetch-response-a-stream/
- 2: doc: add section describing differences between Node.js fetch and WHATWG Fetch Standard nodejs/node#60744
- 3: TypeError: body used already for - should be able to extract twice node-fetch/node-fetch#533
- 4: https://developer.mozilla.org/en-US/docs/Web/API/Response/arrayBuffer
- 5: https://github.com/node-fetch/node-fetch
- 6: .size option not supported on res.body node-fetch/node-fetch#1149
- 7: Content-Length header not sent with DELETE request with a body node-fetch/node-fetch#101
Enforce the archive limit while streaming the body.
response.arrayBuffer() reads the whole body before the archive.length check, so a missing or small Content-Length lets a chunked archive grow past MAX_ARCHIVE_SIZE_BYTES in a Buffer. Read response.body incrementally, cap the total at MAX_ARCHIVE_SIZE_BYTES, add a request deadline, and consume the remaining stream when stopping early.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/fetch-builtin-plugins.mjs` around lines 48 - 60, Update the archive
download flow around response.arrayBuffer() to read response.body incrementally,
track accumulated bytes, and stop once MAX_ARCHIVE_SIZE_BYTES would be exceeded
before buffering more data. Add a request deadline, and when terminating early
consume or cancel the remaining response stream as appropriate; preserve the
existing oversized-archive error for both declared and streamed sizes.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes