feat: refactor frontend console and upgrade node 26#22
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough本 PR 升级 CI/Docker 到 Node 26，新增并统一 AppError 错误模型，重构前端状态与组合函数（草稿、pending、session、sortable），拆分编辑器子组件并模块化样式，同时补充 E2E 与单元测试覆盖。 Changes基础设施与文档
错误处理与状态管理
API 与工具
路由与认证
组件重构与集成
无障碍与样式
测试补充
Estimated code review effort:
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This PR successfully implements a comprehensive frontend refactor and Node.js upgrade from 24 to 26. The changes have been thoroughly reviewed across 54 modified files.
Key Changes Reviewed
- ✅ Node.js Upgrade: Consistently updated from Node 24 to Node 26 across Dockerfile, workflows (.nvmrc, verify.yml, publish workflows)
- ✅ Frontend Refactor: Clean separation of concerns with new composables (useEditorDrafts, useSortableList, usePendingMap, useUserRuntime)
- ✅ Error Handling: Proper refactor from
errorMessage()totoAppError()with consistent AppError usage - ✅ Routing: Added authentication guards with proper meta fields and navigation logic
- ✅ State Management: Improved session management with
ensureSessionReady()and proper promise deduplication - ✅ New Utilities: Added reorder.ts and clipboard.ts with proper validation and bounds checking
- ✅ Test Coverage: Comprehensive unit and e2e tests for new functionality
Security Review
- ✅ CSRF token handling remains secure with proper retry logic on 403 responses
- ✅ Credentials properly included in all API requests
- ✅ Authentication state properly managed and validated
- ℹ️ Pre-existing
chmod 777 /app/datain Dockerfile (line 64) is maintained - while this is a security consideration, it's not introduced by this PR
Code Quality
- Clean separation of concerns with well-structured composables
- Proper TypeScript typing throughout
- Consistent error handling patterns
- Good test coverage for critical paths
No blocking defects found. The refactor is well-executed with proper attention to maintainability and security. Ready for merge after CI verification passes.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request executes the first phases of a frontend refactor, centralizing authentication and routing logic via router guards and establishing a unified AppError system. The changes include modularizing console components, enhancing accessibility with ARIA attributes, and restructuring the global CSS into a modular directory. It also improves state management for editor drafts and user lists while expanding test coverage. Feedback indicates that the specified Node.js version 26 is invalid and will break the build, and suggests adding a unique key to the links editor to prevent state persistence issues when switching between users.
![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: 514d7d4931
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
This PR upgrades the frontend runtime/tooling to Node 26 and completes a multi-step frontend refactor focused on routing/auth, error modeling, state boundaries, sortable interactions, accessibility semantics, and stylesheet modularization—backed by expanded unit + e2e coverage.
Changes:
- Upgrade Node runtime across Docker + GitHub workflows and add
web/.nvmrcfor local consistency. - Refactor frontend: router-level auth guards, centralized
AppError/toAppErrorhandling, composable boundary cleanup, draft state aggregation, and shared sortable/reorder utilities + components (with improved a11y roles/ARIA). - Expand unit/e2e tests for error handling, drafts, reorder, clipboard, toast timing, and console flows (create/duplicate/save/delete).
Reviewed changes
Copilot reviewed 54 out of 54 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| web/tests/unit/state.test.ts | Adds unit coverage for AppError / toAppError / errorMessage behavior. |
| web/tests/unit/reorder.test.ts | Adds tests for new reorder/move helpers. |
| web/tests/unit/feedback.test.ts | Adds coverage for status toast replacement + timer reset. |
| web/tests/unit/editor-drafts.test.ts | Adds coverage for per-user draft aggregation and normalization logic. |
| web/tests/unit/clipboard.test.ts | Adds coverage for clipboard availability, success, and failure behavior. |
| web/tests/e2e/app.spec.ts | Expands Playwright coverage for login failure, redirects, validation/normalization, confirm dialogs, and delete flows. |
| web/src/views/LoginView.vue | Uses inline errors for login failures; minor redirect cleanup; improves ARIA roles/live regions. |
| web/src/views/ConsoleView.vue | Refactors console orchestration, adds last-created tracking, simplifies selection validation, and moves reorder handling to list component. |
| web/src/utils/time.ts | Removes unused timestamp formatting utility. |
| web/src/utils/reorder.ts | Introduces shared reorder primitives (moveIndexByOffset, moveIndexBefore, reorderList). |
| web/src/utils/clipboard.ts | Changes clipboard copy API to return {url, copied} and throws AppError on failure. |
| web/src/utils/bytes.ts | Removes unused bytes formatting utility. |
| web/src/styles/layout.css | Extracts layout-related styles from monolithic stylesheet. |
| web/src/styles/index.css | Adds a central CSS entrypoint importing split stylesheets. |
| web/src/styles/feedback.css | Extracts feedback/toast/dialog styles and pointer-events behavior. |
| web/src/styles/console.css | Extracts console-specific styles (sidebar, editor, dialog, drag states). |
| web/src/styles/base.css | Extracts global tokens, typography, controls, and shared utilities. |
| web/src/styles.css | Removes the monolithic global stylesheet in favor of split CSS files. |
| web/src/router/index.ts | Adds auth/guest route meta + a global guard; changes default redirects to /console. |
| web/src/main.ts | Switches style import to ./styles/index.css. |
| web/src/composables/useUsers.ts | Moves to AppError model, simplifies pending API, and removes cross-composable resets. |
| web/src/composables/useUserRuntime.ts | Moves to AppError model and new pending API; refines error propagation. |
| web/src/composables/useSortableList.ts | Adds shared drag/drop + reorder state helper for user and link lists. |
| web/src/composables/useSession.ts | Adds ensureSessionReady and promise de-duping; migrates to AppError model and new pending API. |
| web/src/composables/usePendingMap.ts | Simplifies pending interface (run) and throws AppError for re-entrancy. |
| web/src/composables/useEditorDrafts.ts | Reworks draft state to a per-user aggregate record with computed derived state. |
| web/src/composables/state.ts | Introduces AppError, updates async state error return type, and adds toAppError. |
| web/src/composables/index.ts | Re-exports useSortableList. |
| web/src/components/shell/ConfirmDialog.vue | Adds an in-app confirm dialog with ARIA semantics and focus management. |
| web/src/components/shell/AppShell.vue | Removes unused shell “chrome” element. |
| web/src/components/console/UserListPane.vue | Migrates to confirm dialog deletion and shared sortable logic; improves list semantics and messaging. |
| web/src/components/console/UserListItem.vue | Adds sortable handle component, ARIA current/labels, and drag state classes. |
| web/src/components/console/SortableHandleButton.vue | Adds reusable sortable handle with keyboard shortcuts + drag behavior. |
| web/src/components/console/LinkRowsEditor.vue | Migrates link list to shared sortable logic and adds SR announcements/instructions. |
| web/src/components/console/LinkRowItem.vue | Uses sortable handle component and improves ARIA labels for inputs/actions. |
| web/src/components/console/FeedbackBanner.vue | Adds ARIA roles/live regions and close button labels for toasts. |
| web/src/components/console/EditorWorkspace.vue | Splits workspace into smaller components and adapts to new clipboard result + error handling. |
| web/src/components/console/EditorSaveActions.vue | New component for save CTA section. |
| web/src/components/console/EditorPublicRouteCard.vue | New component for public route display + copy action. |
| web/src/components/console/EditorLinksSection.vue | New component for link editing section, including loading/errors. |
| web/src/App.vue | Removes component-level redirect logic; gates rendering on sessionReady. |
| web/src/api/types.ts | Removes unused AppInfoResponse. |
| web/src/api/meta.ts | Removes unused meta API client. |
| web/src/api/client.ts | Wraps API error strings as AppError when HTTP responses are not OK. |
| web/src/api/auth.ts | Throws AppError for non-401 failures to align with new error model. |
| web/.nvmrc | Pins local Node version to 26. |
| Dockerfile | Upgrades build stage image to node:26-trixie. |
| README.md | Links the new FRONTEND_REFACTOR_PLAN.md. |
| FRONTEND_REFACTOR_PLAN.md | Adds the documented phased frontend refactor plan. |
| CHANGELOG.md | Updates release automation note to Node 26. |
| .github/workflows/verify.yml | Uses Node 26 for web verification jobs; removes Node-24 forcing env. |
| .github/workflows/review.yml | Removes Node-24 forcing env. |
| .github/workflows/publish-release.yml | Uses Node 26 for release jobs; removes Node-24 forcing env. |
| .github/workflows/publish-preview.yml | Removes Node-24 forcing env. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
PreviewImage: Commit: Preview image was published. No preview server deployment is configured. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Dockerfile (1)
1-73:⚠️ Potential issue | 🟠 Major | ⚡ Quick win容器以 root 用户运行存在安全风险。
Trivy 静态分析工具标记了一个安全问题（DS-0002）：Dockerfile 未指定非 root 用户，容器默认以 root 权限运行。这违反了最小权限原则，降低了安全防护水平。
尽管
/app/data目录已设置为chmod 777，但仍建议添加专用的非 root 用户来运行应用程序。🔒 建议的安全加固方案
FROM debian:trixie-slim RUN apt update && \ DEBIAN_FRONTEND=noninteractive apt install -y --no-install-recommends \ ca-certificates \ curl \ libc-bin \ libc6 \ sqlite3 \ tzdata && \ apt clean && \ rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* ENV TZ=Asia/Shanghai \ RUST_LOG=info \ RUST_BACKTRACE=1 \ HOST=0.0.0.0 \ PORT=8080 \ WEB_DIST_DIR=/app/dist \ DATABASE_URL=sqlite:///app/data/substore.db?mode=rwc RUN ln -snf /usr/share/zoneinfo/Asia/Shanghai /etc/localtime && echo "Asia/Shanghai" > /etc/timezone WORKDIR /app -RUN mkdir -p /app/data /app/dist && chmod 777 /app/data +RUN mkdir -p /app/data /app/dist && \ + groupadd -r submora && \ + useradd -r -g submora -d /app -s /sbin/nologin submora && \ + chown -R submora:submora /app COPY --from=app-binary /app/target/release/submora /app/submora COPY --from=web-assets /app/web/dist /app/dist +USER submora + HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \ CMD /usr/bin/curl -f http://localhost:8080/healthz || exit 1 EXPOSE 8080 CMD ["/app/submora"]🤖 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 `@Dockerfile` around lines 1 - 73, The container runs as root; create a dedicated non-root user and group (e.g., app user) in the Dockerfile, chown the runtime dirs (/app, /app/data, /app/dist) to that user after files are copied, tighten permissions (avoid 777) and add a USER directive before the CMD so /app/submora runs as the non-root user; update the steps around RUN mkdir -p /app/data /app/dist && chmod 777 /app/data, COPY --from=app-binary /app/target/release/submora /app/submora and COPY --from=web-assets /app/web/dist /app/dist to perform chown and proper chmod, and ensure the HEALTHCHECK/CMD still work under the new user.
🧹 Nitpick comments (6)
web/src/components/console/EditorPublicRouteCard.vue (1)
29-32: 💤 Low value建议动态调整复制按钮的
aria-label与
EditorSaveActions组件类似，复制按钮的aria-label固定为"复制公共入口"，但视觉文本会在copyPending时切换为"复制中…"，可能造成辅助技术用户与视觉用户感知不一致。建议将
aria-label改为动态绑定以反映当前状态。♻️ 建议的动态 aria-label 实现
class="button button--ghost" data-testid="editor-copy-public" type="button" :disabled="props.disabled || props.copyPending" - aria-label="复制公共入口" + :aria-label="props.copyPending ? '复制中' : '复制公共入口'" `@click`="emit('copy')"🤖 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 `@web/src/components/console/EditorPublicRouteCard.vue` around lines 29 - 32, The aria-label for the copy button in EditorPublicRouteCard.vue is static while the visible text toggles with props.copyPending; update the button to compute its aria-label dynamically (e.g., based on props.copyPending) so screen readers match the visual state—locate the button using the emit('copy') handler and props.copyPending reference and bind aria-label to a computed/string expression that yields "复制中…" when copyPending is true and "复制公共入口" (or equivalent) when false.web/src/components/console/EditorSaveActions.vue (1)
19-22: 💤 Low value建议动态调整
aria-label以匹配视觉状态当前实现中，
aria-label固定为"保存链接"，但按钮的可见文本会在pending状态下变为"保存中…"。这可能导致屏幕阅读器用户听到的状态与视觉用户看到的不一致。可选方案：
- 移除
aria-label，让按钮文本自行提供无障碍标签- 将
aria-label改为动态绑定：:aria-label="props.pending ? '保存中' : '保存链接'"♻️ 建议的动态 aria-label 实现
class="button button--primary" data-testid="editor-save" type="button" :disabled="props.disabled" - aria-label="保存链接" + :aria-label="props.pending ? '保存中' : '保存链接'" `@click`="emit('save')"🤖 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 `@web/src/components/console/EditorSaveActions.vue` around lines 19 - 22, The fixed aria-label "保存链接" can mismatch the visible text when props.pending is true; update the button in EditorSaveActions.vue to either remove the static aria-label so the visible label is announced, or make it dynamic by binding it to props.pending (e.g., set :aria-label to return "保存中" when props.pending and "保存链接" otherwise) so screen readers hear the same state as the visual text; ensure the change references the button element that uses props.pending and the emitted 'save' click handler.web/src/views/ConsoleView.vue (1)
119-125: 💤 Low value两个相同条件的
if分支可以合并。此处对
selectedUsername.value === username重复判断了两次，且没有中间副作用会改变该条件，可以合并为一个分支让逻辑更清晰。♻️ 建议合并的 diff
- if (selectedUsername.value === username) { - runtime.reset(); - } - - if (selectedUsername.value === username) { - await router.replace("/console"); - } + if (selectedUsername.value === username) { + runtime.reset(); + await router.replace("/console"); + }🤖 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 `@web/src/views/ConsoleView.vue` around lines 119 - 125, The two identical conditionals checking selectedUsername.value === username should be merged into a single if: inside that one block call runtime.reset() and then await router.replace("/console") (preserving the await and execution order), replacing the duplicated if statements so the condition is evaluated once and both actions occur sequentially; update the block containing selectedUsername.value, username, runtime.reset, and router.replace accordingly.web/src/composables/state.ts (1)
5-13: 💤 Low value可以直接使用原生
Error.cause，避免覆写字段。ECMAScript 2022 起
Error构造函数原生支持cause（通过new Error(message, { cause })），调试器/Error#toString/序列化工具也都已经识别。这里手动声明readonly cause: unknown并默认null，会遮蔽原生字段且与生态约定（默认undefined、且cause仅在显式传入时存在）不一致。可以简化为：♻️ 简化为原生 cause
export class AppError extends Error { - readonly cause: unknown; - - constructor(message: string, cause: unknown = null) { - super(message); - this.name = "AppError"; - this.cause = cause; - } + constructor(message: string, cause?: unknown) { + super(message, cause === undefined ? undefined : { cause }); + this.name = "AppError"; + } }请确认项目
tsconfig的target与运行时（Node 26 / 现代浏览器）已对应到 ES2022 及以上，以便原生Error.cause可用。🤖 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 `@web/src/composables/state.ts` around lines 5 - 13, The AppError class currently declares and assigns a custom readonly cause field which shadows the native Error.cause; remove the manual declaration and assignment and instead pass cause via the Error constructor options (i.e., call super(message, { cause })) in AppError's constructor and update the constructor signature (constructor(message: string, cause?: unknown)) so cause is optional/undefined by default; also ensure tsconfig target is ES2022+ (or runtime is Node 26/modern browsers) so native Error.cause is available.web/src/composables/useUserRuntime.ts (1)
61-67: 💤 Low value错误归一化与重新抛出 LGTM；建议保留原始 cause 是可选改进。
setError(toAppError(error).message)会基于 message 重新构造一个AppError，丢失了原始cause。如果未来需要在上层日志/监控中追踪根因，可以考虑让setError支持直接接收AppError或在setError内部new AppError(message, cause)。当前流程对用户可见行为无影响。🤖 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 `@web/src/composables/useUserRuntime.ts` around lines 61 - 67, The current catch block normalizes errors with toAppError(error) but then calls linksState.setError(toAppError(error).message), which discards the original AppError.cause; change the call so setError accepts an AppError (or update setError to accept either a message or an AppError) and pass the full AppError instance instead of only the message—update usage in the catch branch (where loadToken/activeLoadToken and drafts.selectedUsername.value vs username are checked) to call linksState.setError(toAppError(error)) so the original cause is preserved for logging/monitoring.web/src/components/console/LinkRowsEditor.vue (1)
122-127: 💤 Low valuearia-live 公告区可能漏报相同消息。
由于
announcement是单一字符串 ref，连续两次相同文案（例如同方向再按一次方向键移动到边界后又触发同样的提示）不会触发屏幕阅读器重新朗读。如有此场景，可在内容末尾追加一个不可见的循环计数（或不可见空格切换）以强制刷新。请验证目标场景下是否存在「相同公告需要被多次朗读」的实际可访问性需求；若没有，则可忽略此建议。
🤖 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 `@web/src/components/console/LinkRowsEditor.vue` around lines 122 - 127, The aria-live announcement may not re-read identical consecutive messages because `announcement` is a single string ref; change the code that updates `announcement` (the place where you call announcement.value = ...) to append a small, invisible toggle (e.g., a sequential counter or alternating zero-width space) when the new message equals the current one so the DOM text actually changes; update the logic that sets `announcement` in LinkRowsEditor.vue (the ref named `announcement` and the handler that announces key/drag moves) to include this invisible change so the aria-live <p aria-live="polite" ...>{{ announcement }}</p> is retriggered without altering visible text.
🤖 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 `@Dockerfile`:
- Line 1: The Dockerfile currently uses a non-LTS base image ("FROM
node:26-trixie AS web-assets"); change this to an LTS Node 24 image for
production stability (e.g., "node:24" or a Debian/Alpine variant like
"node:24-bullseye" or "node:24-slim") by replacing the FROM line so the build
uses Node 24 LTS instead of Node 26 current.
In `@web/src/components/console/UserListItem.vue`:
- Around line 44-46: The dragleave handler currently forwards every dragleave to
emit('dragLeave', props.user.username), causing flicker when the pointer crosses
child nodes; replace the inline `@dragleave` call with a named handler (e.g.,
onDragLeave) and implement onDragLeave in the <script setup> to check
event.relatedTarget and return early if the relatedTarget is still contained by
event.currentTarget, otherwise call emit("dragLeave", props.user.username);
update the template to use `@dragleave`="onDragLeave" so only true leaves fire
clearDropTarget.
In `@web/src/components/shell/ConfirmDialog.vue`:
- Around line 36-46: The dialog currently sets initial focus (cancelButton) but
lacks a focus trap and focus restoration; update the component to (1) save
document.activeElement before opening when props.open transitions true, (2) add
a dialogRoot ref and an onKeydown handler bound to the dialog <section> that
handles Tab/Shift+Tab to trap focus inside the dialog and handles Escape to
close (instead of relying on backdrop keydown), and (3) when props.open becomes
false restore focus to the previously-saved element; use the existing watch on
props.open, cancelButton, and nextTick to coordinate setting focus after mount
and to tear down the trap and restore focus on close.
In `@web/src/composables/useSession.ts`:
- Around line 28-47: ensureSessionReady currently reuses an in-flight
restoreSessionPromise even when force=true, causing callers to receive stale
session data; change the logic in ensureSessionReady so that when force===true
you do not directly return the existing restoreSessionPromise — instead await
the existing in-flight promise (if any) and then start a fresh restore by
assigning restoreSessionPromise = (async () => { try { return await
restoreSession(); } finally { sessionReady.value = true; restoreSessionPromise =
null; } })(); and return that new promise; keep the existing sessionReady.value
and restoreSessionPromise cleanup behavior and use the same restoreSession
function and sessionState.data references.
In `@web/src/styles/base.css`:
- Around line 156-166: The .sr-only rule uses the deprecated clip property;
replace the clip declaration with a modern, accessible alternative by removing
clip: rect(0, 0, 0, 0) and adding clip-path: inset(50%) (optionally with
-webkit-clip-path fallback) so the element remains visually hidden but
discoverable by screen readers; keep the existing positioning, size, overflow,
white-space, padding, margin and border declarations in the .sr-only class and
ensure the new clip-path line is added alongside them.
In `@web/src/utils/clipboard.ts`:
- Around line 15-24: The current clipboard write uses only a navigator.clipboard
existence check; ensure you also verify that navigator.clipboard.writeText is a
callable function before attempting to use it so the code gracefully returns {
url, copied: false } instead of throwing. Update the logic in
web/src/utils/clipboard.ts around the navigator/clipboard handling to check
typeof navigator.clipboard.writeText === "function" (or similar) and
early-return copied: false when it's not available, keeping the existing
try/catch for runtime errors and referencing navigator, clipboard, and writeText
to locate the change.
In `@web/src/views/LoginView.vue`:
- Around line 22-29: The try/catch in submitLogin currently wraps both await
login(...) and await router.replace(...), which can silently swallow navigation
errors; narrow the error handling so only the login call is inside the try/catch
(keep loginPassword.value reset in the login success path), then call
router.replace(redirectTarget) outside that catch so routing errors surface (or
add a separate catch/log for router.replace if you want explicit feedback).
Locate submitLogin, the login(...) call, loginPassword.value, and
router.replace(...) to make this change.
---
Outside diff comments:
In `@Dockerfile`:
- Around line 1-73: The container runs as root; create a dedicated non-root user
and group (e.g., app user) in the Dockerfile, chown the runtime dirs (/app,
/app/data, /app/dist) to that user after files are copied, tighten permissions
(avoid 777) and add a USER directive before the CMD so /app/submora runs as the
non-root user; update the steps around RUN mkdir -p /app/data /app/dist && chmod
777 /app/data, COPY --from=app-binary /app/target/release/submora /app/submora
and COPY --from=web-assets /app/web/dist /app/dist to perform chown and proper
chmod, and ensure the HEALTHCHECK/CMD still work under the new user.
---
Nitpick comments:
In `@web/src/components/console/EditorPublicRouteCard.vue`:
- Around line 29-32: The aria-label for the copy button in
EditorPublicRouteCard.vue is static while the visible text toggles with
props.copyPending; update the button to compute its aria-label dynamically
(e.g., based on props.copyPending) so screen readers match the visual
state—locate the button using the emit('copy') handler and props.copyPending
reference and bind aria-label to a computed/string expression that yields "复制中…"
when copyPending is true and "复制公共入口" (or equivalent) when false.
In `@web/src/components/console/EditorSaveActions.vue`:
- Around line 19-22: The fixed aria-label "保存链接" can mismatch the visible text
when props.pending is true; update the button in EditorSaveActions.vue to either
remove the static aria-label so the visible label is announced, or make it
dynamic by binding it to props.pending (e.g., set :aria-label to return "保存中"
when props.pending and "保存链接" otherwise) so screen readers hear the same state
as the visual text; ensure the change references the button element that uses
props.pending and the emitted 'save' click handler.
In `@web/src/components/console/LinkRowsEditor.vue`:
- Around line 122-127: The aria-live announcement may not re-read identical
consecutive messages because `announcement` is a single string ref; change the
code that updates `announcement` (the place where you call announcement.value =
...) to append a small, invisible toggle (e.g., a sequential counter or
alternating zero-width space) when the new message equals the current one so the
DOM text actually changes; update the logic that sets `announcement` in
LinkRowsEditor.vue (the ref named `announcement` and the handler that announces
key/drag moves) to include this invisible change so the aria-live <p
aria-live="polite" ...>{{ announcement }}</p> is retriggered without altering
visible text.
In `@web/src/composables/state.ts`:
- Around line 5-13: The AppError class currently declares and assigns a custom
readonly cause field which shadows the native Error.cause; remove the manual
declaration and assignment and instead pass cause via the Error constructor
options (i.e., call super(message, { cause })) in AppError's constructor and
update the constructor signature (constructor(message: string, cause?: unknown))
so cause is optional/undefined by default; also ensure tsconfig target is
ES2022+ (or runtime is Node 26/modern browsers) so native Error.cause is
available.
In `@web/src/composables/useUserRuntime.ts`:
- Around line 61-67: The current catch block normalizes errors with
toAppError(error) but then calls linksState.setError(toAppError(error).message),
which discards the original AppError.cause; change the call so setError accepts
an AppError (or update setError to accept either a message or an AppError) and
pass the full AppError instance instead of only the message—update usage in the
catch branch (where loadToken/activeLoadToken and drafts.selectedUsername.value
vs username are checked) to call linksState.setError(toAppError(error)) so the
original cause is preserved for logging/monitoring.
In `@web/src/views/ConsoleView.vue`:
- Around line 119-125: The two identical conditionals checking
selectedUsername.value === username should be merged into a single if: inside
that one block call runtime.reset() and then await router.replace("/console")
(preserving the await and execution order), replacing the duplicated if
statements so the condition is evaluated once and both actions occur
sequentially; update the block containing selectedUsername.value, username,
runtime.reset, and router.replace accordingly.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0da0fc6a-534d-4581-a576-ebdb62e6d800
📒 Files selected for processing (54)
.github/workflows/publish-preview.yml.github/workflows/publish-release.yml.github/workflows/review.yml.github/workflows/verify.ymlCHANGELOG.mdDockerfileFRONTEND_REFACTOR_PLAN.mdREADME.mdweb/.nvmrcweb/src/App.vueweb/src/api/auth.tsweb/src/api/client.tsweb/src/api/meta.tsweb/src/api/types.tsweb/src/components/console/EditorLinksSection.vueweb/src/components/console/EditorPublicRouteCard.vueweb/src/components/console/EditorSaveActions.vueweb/src/components/console/EditorWorkspace.vueweb/src/components/console/FeedbackBanner.vueweb/src/components/console/LinkRowItem.vueweb/src/components/console/LinkRowsEditor.vueweb/src/components/console/SortableHandleButton.vueweb/src/components/console/UserListItem.vueweb/src/components/console/UserListPane.vueweb/src/components/shell/AppShell.vueweb/src/components/shell/ConfirmDialog.vueweb/src/composables/index.tsweb/src/composables/state.tsweb/src/composables/useEditorDrafts.tsweb/src/composables/usePendingMap.tsweb/src/composables/useSession.tsweb/src/composables/useSortableList.tsweb/src/composables/useUserRuntime.tsweb/src/composables/useUsers.tsweb/src/main.tsweb/src/router/index.tsweb/src/styles.cssweb/src/styles/base.cssweb/src/styles/console.cssweb/src/styles/feedback.cssweb/src/styles/index.cssweb/src/styles/layout.cssweb/src/utils/bytes.tsweb/src/utils/clipboard.tsweb/src/utils/reorder.tsweb/src/utils/time.tsweb/src/views/ConsoleView.vueweb/src/views/LoginView.vueweb/tests/e2e/app.spec.tsweb/tests/unit/clipboard.test.tsweb/tests/unit/editor-drafts.test.tsweb/tests/unit/feedback.test.tsweb/tests/unit/reorder.test.tsweb/tests/unit/state.test.ts
💤 Files with no reviewable changes (8)
- web/src/utils/time.ts
- .github/workflows/review.yml
- web/src/components/shell/AppShell.vue
- web/src/utils/bytes.ts
- web/src/api/types.ts
- web/src/api/meta.ts
- .github/workflows/publish-preview.yml
- web/src/styles.css
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d9e7f3e6d
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
Dockerfile (1)
1-74: 建议添加非 root 用户以提升容器安全性。容器当前以 root 身份运行。建议添加
USER指令以非 root 用户身份运行应用，这是容器安全最佳实践。示例实现：# 在 WORKDIR /app 之后添加 RUN groupadd -r appuser && useradd -r -g appuser appuser && \ chown -R appuser:appuser /app USER appuser需要确保
/app/data和/app/dist对该用户可写。🤖 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 `@Dockerfile` around lines 1 - 74, The image currently runs as root; add a non-root user and switch to it after the workdir and after directories/files are created/copied so the app runs with least privilege: create a system group/user (e.g., appuser) via groupadd/useradd, chown -R appuser:appuser for /app and its subdirs (ensure ownership for paths created by RUN mkdir -p /app/data /app/dist and files added by COPY --from=app-binary /app/target/release/submora and COPY --from=web-assets /app/web/dist), then add a USER appuser line before the HEALTHCHECK/EXPOSE/CMD so submora runs as that user; ensure the created user has write permission to /app/data and /app/dist and adjust any file modes if necessary.web/tests/unit/sortable-list.test.ts (1)
3-14: ⚡ Quick win可选建议：增加测试覆盖面。
当前测试验证了核心的键盘重排序场景。为了更全面的覆盖，可以考虑添加以下测试用例：
dropBefore函数的各种场景syncLength在索引越界时的行为- 拖拽生命周期函数（
startDrag、finishDrag等）的状态转换- 边界情况（空列表、无效索引等）
不过当前的测试对于基础功能验证已经足够。
🤖 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 `@web/tests/unit/sortable-list.test.ts` around lines 3 - 14, Add additional unit tests to increase coverage around useSortableList: write separate specs for dropBefore to verify inserting before/after at start/middle/end and when target is equal to source; test syncLength behavior when passed lengths smaller than, equal to, and greater than current length (including negative and out-of-range indexes); test drag lifecycle methods startDrag and finishDrag to ensure draggingIndex and dropTargetIndex transition from null to expected indexes and back to null; and add edge-case tests for empty lists and invalid indices to confirm no state leakage after reorderByOffset, dropBefore, syncLength, startDrag, and finishDrag operations.
🤖 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 @.github/workflows/publish-release.yml:
- Line 116: The workflow is pinned to Node 26 (node-version: 26) which is a
Current release—change the Node runtime to the Active LTS version by replacing
the node-version: 26 entry with node-version: 24 (or 24.x) so the
publish-release.yml uses Node 24 LTS; update any related setup references that
read node-version to the new value to keep the workflow consistent.
In @.github/workflows/verify.yml:
- Line 53: The CI uses Node 26 for the `node-version: 26` entries (affecting the
`web` and `e2e` jobs), which may destabilize workflows; change those
`node-version: 26` occurrences to `node-version: 24` to match the LTS used in
`publish-release.yml` so CI runs use Node 24 LTS for stability and
reproducibility.
In `@web/src/composables/useSortableList.ts`:
- Around line 14-27: The syncLength function can leave dropTargetIndex set while
draggingIndex was cleared, causing inconsistent state; update syncLength so that
when draggingIndex.value is out of range (>= length) you call resetDragState()
(or otherwise clear both draggingIndex and dropTargetIndex together) instead of
only nulling draggingIndex.value, ensuring draggingIndex, dropTargetIndex and
any other drag state are reset consistently; locate this logic in syncLength and
reference draggingIndex.value, dropTargetIndex.value and resetDragState() when
making the change.
In `@web/src/styles/base.css`:
- Line 68: 将 text-rendering 的关键字值从 optimizeLegibility 改为小写以满足 Stylelint 的
value-keyword-case 规则；在 base.css 中找到 text-rendering: optimizeLegibility; 并替换为
text-rendering: optimizelegibility; 保存后重新运行样式检查以确认通过。
In `@web/tests/unit/state.test.ts`:
- Around line 27-35: The test claims we should preserve an AppError's cause but
the implementation (createAsyncDataState / state.setError) currently stores only
appError.message in a string ref; fix by changing the error ref type from string
| null to AppError | null inside createAsyncDataState, update state.setError to
store and return the full AppError object (not just message), and update the
unit test to assert state.error.value === appError (and that
state.error.value.cause === nativeError) so the AppError and its cause are
actually preserved; ensure any callers expecting a string are updated to read
.message when needed.
---
Nitpick comments:
In `@Dockerfile`:
- Around line 1-74: The image currently runs as root; add a non-root user and
switch to it after the workdir and after directories/files are created/copied so
the app runs with least privilege: create a system group/user (e.g., appuser)
via groupadd/useradd, chown -R appuser:appuser for /app and its subdirs (ensure
ownership for paths created by RUN mkdir -p /app/data /app/dist and files added
by COPY --from=app-binary /app/target/release/submora and COPY --from=web-assets
/app/web/dist), then add a USER appuser line before the HEALTHCHECK/EXPOSE/CMD
so submora runs as that user; ensure the created user has write permission to
/app/data and /app/dist and adjust any file modes if necessary.
In `@web/tests/unit/sortable-list.test.ts`:
- Around line 3-14: Add additional unit tests to increase coverage around
useSortableList: write separate specs for dropBefore to verify inserting
before/after at start/middle/end and when target is equal to source; test
syncLength behavior when passed lengths smaller than, equal to, and greater than
current length (including negative and out-of-range indexes); test drag
lifecycle methods startDrag and finishDrag to ensure draggingIndex and
dropTargetIndex transition from null to expected indexes and back to null; and
add edge-case tests for empty lists and invalid indices to confirm no state
leakage after reorderByOffset, dropBefore, syncLength, startDrag, and finishDrag
operations.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ceb33331-8c66-485d-8b1a-8a87d83924f6
📒 Files selected for processing (25)
.github/workflows/publish-release.yml.github/workflows/verify.ymlDockerfileweb/src/components/console/EditorPublicRouteCard.vueweb/src/components/console/EditorSaveActions.vueweb/src/components/console/EditorWorkspace.vueweb/src/components/console/LinkRowItem.vueweb/src/components/console/LinkRowsEditor.vueweb/src/components/console/UserListItem.vueweb/src/components/shell/ConfirmDialog.vueweb/src/composables/state.tsweb/src/composables/useSession.tsweb/src/composables/useSortableList.tsweb/src/composables/useUserRuntime.tsweb/src/composables/useUsers.tsweb/src/router/index.tsweb/src/styles/base.cssweb/src/utils/clipboard.tsweb/src/utils/reorder.tsweb/src/views/ConsoleView.vueweb/src/views/LoginView.vueweb/tests/unit/clipboard.test.tsweb/tests/unit/reorder.test.tsweb/tests/unit/sortable-list.test.tsweb/tests/unit/state.test.ts
✅ Files skipped from review due to trivial changes (1)
- web/tests/unit/clipboard.test.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- web/src/components/console/EditorPublicRouteCard.vue
- web/src/components/console/EditorSaveActions.vue
- web/src/components/shell/ConfirmDialog.vue
- web/src/composables/state.ts
- web/src/composables/useUserRuntime.ts
- web/src/router/index.ts
- web/tests/unit/reorder.test.ts
- web/src/components/console/LinkRowItem.vue
- web/src/components/console/EditorWorkspace.vue
- web/src/utils/clipboard.ts
- web/src/utils/reorder.ts
- web/src/components/console/LinkRowsEditor.vue
- web/src/composables/useSession.ts
- web/src/views/ConsoleView.vue
- web/src/composables/useUsers.ts
- web/src/components/console/UserListItem.vue
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f8e109ee7
ℹ️ 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".
vansour
dismissed stale reviews from coderabbitai[bot] and coderabbitai[bot]
Addressed in follow-up commits and all checks are passing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification