Skip to content

feat(desktop): create projects from folders dropped into the sidebar - #6090

Open
YayoiNanoka wants to merge 2 commits into
apache:mainfrom
YayoiNanoka:feat/sidebar-folder-drop
Open

YayoiNanoka wants to merge 2 commits into
apache:mainfrom
YayoiNanoka:feat/sidebar-folder-drop

Conversation

@YayoiNanoka

@YayoiNanoka YayoiNanoka commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Problem: The project sidebar cannot register folders dragged from the file manager, so users must open the project picker even when the folder is already at hand.

Solution: Resolve the native File path with Electron webUtils.getPathForFile, validate an absolute directory in main, and call the existing catalog.register(path). A small drop zone highlights the project list with an outline and a hint. Folder batches reuse archived-project confirmation, share one in-flight guard, and report failures in one toast. Folder drops are disabled while an existing registration or archived-project confirmation is pending.

Result: Dropping folders adds projects to the local Host using the same naming and identity rules as New Project, including while a remote task is open. Existing projects are reused, archived projects can be restored, and the current conversation or draft stays selected. The sidebar no longer advertises an available drop target while registration is busy, and accepts folder drops again when the operation finishes.

Folder-name naming and the frosted effect are deferred. This PR makes no protocol or storage changes. Existing task-row drag handlers are unchanged; a compact regression in the existing ProjectGroups story checks their interaction with the enabled folder-drop zone.

Screenshots

Captured from an isolated Electron fixture with native folder drag input.

Before dragging During folder drag
Project sidebar before a folder drag Project sidebar with an outline and folder drop hint

Verification

  • Repository lint, format, build, typecheck, and desktop/UI unused-code checks passed.
  • 90 focused desktop and UI tests passed on the current revision, covering local registration, partial batch failures, archived recovery, pending drop availability, duplicate-operation prevention, and existing sidebar interactions.
  • Native Electron E2E passed on macOS: a real Unicode directory retains its native File backing across contextBridge and is registered by the local Host.
  • The retained ProjectGroups browser regression and accessibility audit passed: the enabled zone registers Files, allows internal Session drags, and blocks text/plain and text/uri-list drops before row handlers. Removing the non-file capture guard made this regression fail; removing the pending gate made the controller regressions fail.
  • Strict renderer architecture, artifact/E2E budgets, surface inventory, shell-hook, ASF-header, and protocol checks passed. The Runtime Host epoch remains 222, identical to the base.

CI passed for commit de9ee770a. Physical file-manager drags on Windows and Linux were not exercised locally.

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented the change, tests, and automated validation. The commit includes Generated-by: Codex.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No
点击展开中文

概述

问题: 项目侧栏无法注册从文件管理器拖入的文件夹。即使文件夹已经在手边,用户仍需打开项目选择器。

解决方法: 通过 Electron 的 webUtils.getPathForFile 取得原生 File 路径,在主进程验证绝对路径和目录类型,再调用现有 catalog.register(path)。项目列表使用简单描边和文字提示接收拖放。多个文件夹复用归档恢复确认、共用一个单飞保护,并将失败汇总为一次提示。已有注册操作或归档恢复确认尚未结束时,禁用文件夹拖放。

效果: 拖入文件夹即可在本机 Host 添加项目,命名与身份规则和“新建项目”一致,当前打开远程任务时也一样。已有项目会被复用,归档项目可以确认恢复,当前会话和草稿保持选中。注册繁忙时,侧栏不再显示可用的拖放目标;操作结束后重新接受文件夹拖放。

文件夹命名和毛玻璃效果留到后续。本 PR 不修改协议或存储。现有任务行的拖拽处理保持不变;在已有 ProjectGroups story 中保留精简回归测试,验证它们与启用状态下的文件夹拖放区域之间的交互。

截图

上方展示拖拽前与拖入文件夹时的侧栏,来自隔离 Electron 测试窗口和原生目录拖拽输入。

验证

  • 仓库 lint、format、构建、类型检查和 desktop/UI 无用代码检查通过。
  • 当前提交的 90 项定向桌面端和 UI 测试通过,覆盖本机注册、批量部分失败、归档恢复、注册期间拖放可用性、重复操作保护和现有侧栏交互。
  • macOS 原生 Electron E2E 通过:包含中文的真实目录以原生 File 穿过 contextBridge,最终由本机 Host 注册。
  • 保留在 ProjectGroups 中的浏览器回归测试及无障碍检查通过:启用状态下的拖放区域可以注册 Files、放行内部 Session 拖拽,并在任务行处理前拦截 text/plain 和 text/uri-list。移除非文件捕获保护后,该回归测试正确失败;移除 pending 门控后,控制器回归测试也正确失败。
  • 严格渲染层架构检查、产物与 E2E 预算、界面清单、shell hook、ASF 文件头和协议检查通过。Runtime Host epoch 保持 222,与基线一致。

提交 de9ee770a 的 CI 已通过。本地未验证 Windows、Linux 文件管理器的实际拖拽。

AI 使用

Codex 对实现、测试和自动验证作出了实质贡献,提交包含 Generated-by: Codex。

检查清单

  • 测试覆盖本次改动,并能在缺少对应实现时失败。
  • lint、format、类型检查及受影响测试均在本地通过。
  • 本次包含上述用户可见的行为变化。

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

At c2f5ca1b5755c71bdf59eb32a2feb56a68686638, no reproducible P0–P2 issue was established. One P3 concern remains:

[P3] Allocate the next unused compatibility epoch after rebasing. This PR, #6088, #5969 and #5732 independently claim epoch 221 while current main remains 220. Their strict protocol extensions differ, so matching numbers do not guarantee compatible binaries. The later merge must use the next unused epoch, preserving the earlier extensions; 222 is appropriate after one 221 change, not a permanently reserved value. The inline comment identifies the declaration.

The change implements one coherent folder-registration path. Preload resolves the filesystem-backed File through Electron's webUtils.getPathForFile; main validates an absolute directory before contacting the local Host. Opening a remote conversation does not redirect registration to that remote Host. The existing project catalog remains the sole identity authority: initialName applies atomically only to a new identity, repeated drops preserve custom names and Git worktree identity, and archived projects retain the existing restore confirmation. No storage migration or separate project registry is introduced.

The UI distinguishes external folder drops from internal Session moves, suppresses overlapping registration, handles each folder independently, reports skipped files and partial failures, refreshes the catalog, and reveals the last successful project without switching the current conversation or draft. The overlay uses the existing list and theme tokens rather than a separate sidebar layout.

Validation at this head:

  • Nine complete affected and adjacent test files passed 129 tests, with no skips. These cover local authority while a remote task is active, native-path resolution and forged/synthetic File rejection, directory validation, partial batches, pending/lifecycle guards, archived recovery, and atomic naming without renaming existing projects.
  • The actual Electron directory-drop test passed on Linux under Xvfb. CDP directory-drag input retained a real filesystem-backed File through contextBridge, preload and main, and the running Host registered the real Unicode directory path and name. This is native Electron boundary evidence, not a physical file-manager gesture.
  • Six real Chromium runs covered the folder-drop and project-group stories in light, dark and forced-color modes. Story assertions and accessibility checks passed for empty-space hit testing, visible-viewport centering in empty/scrolled lists, resize during dragging, and unchanged row/scroll geometry. These synthetic browser Files validate presentation, not native filesystem provenance.
  • A clean normal build, Desktop and Host type checks, production renderer and Storybook builds, scoped Biome, strict renderer architecture, locale/shell-hook checks, whitespace checks, the epoch guard and the E2E budget check passed. The two author-provided light/dark Storybook images were inspected.

#6088 and #6090 are independent branches from 32b17504, not stacked dependencies. Their combined tree conflicts in packages/runtime-host/src/protocol/index.ts; the other overlapping files merge automatically. I also built a local combination of #6088 at 0c6455f6, this head and current main 6c63fa05, resolving only the epoch declaration to 222 and retaining both hasUnread and initialName. All 404 tests across 13 complete protocol/catalog/navigation/sidebar files passed with no skips; Desktop/Host type checks and the production renderer build passed, and the same native Electron folder-drop test passed again on the combination. This conditional combination is not an author change and does not resolve the competing epochs of #5969 or #5732.

The head alone merges cleanly with main 6c63fa057ed97916b77db3ff002a13636d6a564a; that separate synthetic merge was not built. CI for this exact head remains queued. The full monorepo suite, physical file-manager gestures and independent Windows/macOS execution were not run. Feature adoption and merging remain human decisions.

// Increment when the same protocol version no longer guarantees safe Client-Host
// interoperability. Mismatches are rejected before domain commands are admitted.
export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 220 as const;
export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 221 as const;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] Allocate a distinct epoch after the competing protocol change lands

Current main remains epoch 220, while this PR, #6088, #5969 and #5732 independently advertise 221 for different strict protocol inputs. This PR's initialName extension is not interchangeable with those other extensions merely because the epochs match. The #6088/#6090 combination also conflicts at this declaration. Please rebase after the first relevant merge, preserve every landed protocol extension, and use the next unused epoch (222 after one 221 merge). This head's own 220→221 epoch guard passes; the concern is merge-order compatibility.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged. Before merging, we will rebase this PR onto the latest main, preserve all protocol extensions that have landed, and update the compatibility epoch to the next unused value based on the actual merge order. We will not treat 222 as a reserved value.

— Posted by Codex on behalf of the PR author.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review from c2f5ca1b to c6e533a94fd5b0566286e79f7b88253b5855e609. The one new commit, c6e533a9, refreshes docs/astryx-surface-file-inventory.paths (+1) and docs/astryx-surface-file-inventory.md (+2/−1) to include this PR's new surface. No source files changed. I found no new P0–P3 issue. CI on this exact head passed, and the branch merges cleanly with main.

The earlier P3 still applies: this PR, #5969, #6091 and #5732 all declare compatibility epoch 221, so whichever merges later needs to rebase onto the next unused epoch.

@YayoiNanoka
YayoiNanoka force-pushed the feat/sidebar-folder-drop branch from c6e533a to df4e530 Compare October 16, 2026 09:40

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

At df4e530ee111af284e8cbadd62e4258b4cc03215, I found no new P0–P2 issue in the increment from c6e533a9.

The three commits are rebased versions of the existing feature, centered frosted hint and inventory refresh—not two newly added behavioral changes. The hint commit 09bf1332 is patch-equivalent to the earlier c2f5ca1b. The rebase adapts locale additions to main's separate locale objects, preserves main's project-row host attributes, updates the inventory to 341 files, and raises the protocol epoch. The drop-zone, registration controller and storage implementations remain unchanged; the author-added sidebar CSS is byte-equivalent across the rebase.

The initialName extension is preserved end to end. The actual current decoder accepts it, while the actual main decoder rejects it; both still accept ordinary registration. Moving from main's epoch 222 to 223 is therefore appropriate. A P3 integration caveat remains at packages/runtime-host/src/protocol/index.ts:110: open #6091 at 870297e6 independently uses 223 for a different, revision-bound heatmap contract. That branch rejects this branch's named registration. Whichever incompatible change lands second must rebase and use the next unused epoch; do not treat 223 as reserved for either branch. These are production-decoder checks, not a live mixed-peer connection test.

A fresh dependency installation, normal clean build:test, Host and all four Desktop type checks passed. 413 tests in 16 targeted files passed without skips, covering real directory/catalog registration and naming, duplicate/archive handling, controller lifetime and partial failures, preload conversion, external-file versus task drags, main's project menus, session-catalog behavior and protocol contracts. Surface/Windows inventories, strict renderer architecture, full formatting, locale hygiene, whitespace and the epoch guard passed; the E2E budget check also passed. I did not rerun native Electron drag input, browser geometry, cross-platform or full-repository tests.

CI remains red on this head because the artifact ledger omits marked@18.0.11. The local check reproduces it, and all dependency manifests, the lockfile and the ledger match main 331340ab, so this is inherited rather than a new P2 from the rebase. #6129 repairs that main ledger; rebase after it lands. The current head is not all green, and feature adoption/merging still require a human decision.

// Increment when the same protocol version no longer guarantees safe Client-Host
// interoperability. Mismatches are rejected before domain commands are admitted.
export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 222 as const;
export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 223 as const;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] Keep epoch equality meaningful after the other branch lands

Open #6091 at 870297e6 independently declares 223 for revision-bound annual heatmap requests. It still rejects project.catalog.mutate with this branch's initialName, although this branch accepts that input. The move above current main's 222 is correct, but whichever incompatible branch lands second needs a rebase and the next unused epoch; otherwise different contracts share an admission epoch.

@YayoiNanoka
YayoiNanoka force-pushed the feat/sidebar-folder-drop branch from df4e530 to 8cf193e Compare October 16, 2026 14:11

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.Incremental review: df4e530e..8cf193e2, a rebase onto main 2c429077 (#6129). git range-diff shows the second and third commits as patch-identical. The first commit differs only in the base AppShell nonTriviaTokens value in renderer-architecture.json, now 7280 from #6129 instead of 7285, while this PR's own count of 7269 is unchanged. I found no P0–P2 issue, and CI test passes on this head. The epoch-223 P3 shared with #6091 still applies.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @YayoiNanoka, thanks! Dropping a folder onto the sidebar to create a project is a nice touch, and I'm happy with the direction. It's quite a bit larger than the feature needs, though (about 1,190 lines). Could we trim it to a minimal version, around 300–350 lines including tests?

  • Could you drop the initialName protocol/storage change and the epoch bump to 223? It names drop-created projects after the folder, while the "New project" button uses the repository-name rule, so the two ways of creating a project end up inconsistent. If we want folder-name naming, let's do it in its own PR for both paths. Dropping it also avoids the epoch clash with #6091.
  • Keep the essentials:
    • the preload webUtils.getPathForFile method
    • a small main handler that checks the path is absolute and a directory, then calls the existing catalog.register(path)
    • a simple loop that reuses resolveProjectRegistration for archived projects, with one error toast
    • an onDropProjectFolders callback next to onNewProject
    • a small drop zone with an outline highlight
    • the copy
    • the e2e test, which is the one real proof the dropped file path crosses contextBridge
  • These can be follow-ups or dropped:
    • the host-lifecycle fencing during the drag (a single in-flight guard is enough)
    • the projectCreation provider refactor
    • scrolling to the new project
    • the frosted overlay
    • the stories
    • most of the large UI test
  • The row-level SESSION_DRAG_MIME guards change existing task-drag behaviour. If they're needed, could you send them as a separate fix with a short explanation?

Thanks!


中文

谢谢!拖文件夹到侧边栏建项目这个细节很好,方向我没问题。不过目前约 1190 行,比这个功能需要的大不少,能不能精简到 300–350 行左右(含测试)?

  • 能否去掉 initialName 的协议/存储改动和 223 版本号? 它让拖进来的项目用文件夹名命名,而"新建项目"按钮用的是仓库名规则,两种建项目方式会不一致。如果需要按文件夹命名,我们单独提一个 PR,两条路径一起改。去掉后也就不和 #6091 撞版本号了。
  • 保留必需的部分:preload 的 webUtils.getPathForFile;主进程里校验绝对路径和目录后调用现有 catalog.register(path) 的小处理函数;复用 resolveProjectRegistration 处理归档项目的简单循环和一个错误提示;在 onNewProject 旁边加 onDropProjectFolders 回调;带描边高亮的小放置区;文案;以及 e2e 测试(这是文件路径真能穿过 contextBridge 的唯一实证)。
  • 可以后续再做或去掉的:拖动过程中的 Host 生命周期防护(一个单飞保护就够)、projectCreation provider 重构、滚动到新项目、毛玻璃浮层、stories、以及大部分大型 UI 测试。
  • 行级的 SESSION_DRAG_MIME 判断会改变现有任务拖拽的行为,如果确实需要,能否单独提一个修复并简单说明原因?

谢谢!

Resolve native File paths in preload, validate local directories in main, and reuse catalog naming and archived-project recovery. Process a folder batch with one shared in-flight guard and error report, keeping the current task and draft selected. Show an outline and drop hint in the project list.

Generated-by: Codex
@YayoiNanoka
YayoiNanoka force-pushed the feat/sidebar-folder-drop branch from 8cf193e to aa922a4 Compare October 17, 2026 07:46
@YayoiNanoka

Copy link
Copy Markdown
Contributor Author

Thanks @Astro-Han! The PR has been simplified following your suggestions in aa922a404. It now keeps the native folder-drop path, existing project registration and archived-project recovery, a single in-flight guard, and a simple outline highlight. The protocol/storage changes and epoch bump, lifecycle fencing, provider refactor, automatic reveal, frosted overlay, stories, and duplicate tests have been removed; existing task-row drag handlers are unchanged.

Folder-name naming and the frosted effect are deferred to follow-up work. Drop-created projects now use the same naming rules as New Project.

CI is passing, including the native Electron E2E test. The PR description and screenshots have also been updated. Could you please take another look?

点击展开中文

谢谢!已在 aa922a404 中按建议精简,保留原生文件夹拖入、现有项目注册和归档恢复、单飞保护及简单描边提示。协议与存储修改、epoch 升级、生命周期保护、provider 重构、自动定位、毛玻璃遮罩、stories 和重复测试均已移除,现有任务行拖拽处理保持不变。

文件夹命名和毛玻璃效果留到后续处理,当前拖入项目沿用“新建项目”的命名规则。

CI 已通过,包括原生 Electron E2E 测试,PR 正文和截图也已更新。麻烦再看一下,谢谢!

Posted by Codex on behalf of @YayoiNanoka.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Full review of the rewritten head aa922a40 (one commit on merge-base face9409; main has moved one unrelated commit since, to d14cec57). The diff is +388/-20 across 22 files. About 36 of those lines are license headers and 7 are generated inventory and budget lines. The earlier head 8cf193e2 was +1186/-51.

Ablation request, item by item

  • initialName protocol/storage change and epoch bump: removed. Nothing under packages/runtime-host or storage changes, and the epoch stays at 222 on both this head and main. There is no protocol impact and no collision with #6091. Drop-created projects now go through plain catalog.register(path), so they are named the same way as projects from New project.
  • Preload webUtils.getPathForFile: followed. addDroppedProject maps a thrown or empty path to not-directory.
  • Small main handler: followed. addDroppedDirectory rejects remote hosts, rejects non-string or non-absolute paths with a TypeError, returns not-directory when stat says it is not a directory, calls the existing catalog.register(path), and returns archived with the id. It does not change the Host selection.
  • Simple sequential loop with resolveProjectRegistration and one error toast: followed. Failures are collected per file into a single reportError. The single in-flight guard reuses projectMutationPendingRef, so the New project picker shares the lock. isCurrent is () => true, so the lifecycle fencing is gone.
  • onDropProjectFolders next to onNewProject: followed. It is passed through app-shell, SessionNavigationProvider and SessionRailData, and there is no projectCreation refactor.
  • Small drop zone with outline highlight: followed. project-folder-drop-zone.tsx is 71 lines including the header, plus 23 lines of CSS. There is no frosted overlay, no reveal/scroll and no stories.
  • Copy: followed. There is one hint string and one error string, each in 3 locales.
  • e2e: kept. It does a native CDP drag of a real directory with a non-ASCII name and checks that the Host catalog admits it.
  • Large UI test: removed entirely. The remaining tests are a service test (+32), a controller test (+47) and the e2e (+68).
  • Row-level SESSION_DRAG_MIME guards: removed. The project and session row handlers are byte-identical to main; session-history-list.tsx only wraps the list.

The only unrequested change is a small cleanup in app-shell that pulls out selectedHost (about 8 changed lines, no behaviour change).

Correctness and security

  • Renderer-supplied path. Main validates the type, that the path is absolute and that it is a directory before it registers anything. A compromised renderer could register any local directory without going through the chooser. However, projects:registerDirectory (root plus segments) already gives the renderer a chooser-free registration path, and registering a project does not read the folder's contents. I don't consider this a new trust boundary.
  • Drag handling. The zone sets data-maka-file-drop-target only when it is enabled, which means project view with a local Host that can choose client directories. That lets the window drop guard pass the event through, and the zone then takes over the guard's job for anything that is not a session drag: it calls preventDefault and stopPropagation in the capture phase, so non-file drops still cannot navigate the window. Only drags that include Files light up the zone or call the callback.
  • Internal session drags. Drags carrying application/x-maka-session return before the zone does anything, so the existing row move handlers still receive them. Rows still key off the module-level draggingSessionId, as on main.
  • Non-folder files. A dropped regular file reaches main and comes back as not-directory, which shows as one line in the combined toast.
  • Archived projects. An archived project gets the existing confirm, then restore through the local hostRef, keeping its original name. The controller test covers this.
  • Error paths. Errors from stat (ENOENT/EACCES), the type check and the IPC layer are caught per file, so one bad item does not abort the batch. A refresh failure is caught as well.
  • OS drops always go to the local Host, even while a remote task is selected, and the remote draft and target are left unchanged. The test asserts both.

Findings

  • P3 (use-task-entry-controller.ts:434): a drop that lands while another project mutation is pending, such as an earlier drop batch or a New project registration, is silently ignored. The zone still highlights and accepts it, so the user gets no feedback. This is acceptable for v1, and either disabling the zone while pending or showing a toast would fix it.
  • P3, test gap: nothing tests the drop zone together with an enabled onDropProjectFolders. The existing session-history-project-drop tests do not pass the callback, so the zone is disabled there. As a result, the claim that an enabled zone lets session-row moves through and swallows non-file drags is checked only by reading the code. A short unit test would cover it. The maintainer asked to drop "most of" the UI test, not all of it.

I found no P0–P2 issues.

CI and mergeability
The test run 39014888187 is green on aa922a40. Its test plan includes the new e2e spec and the Desktop e2e tier reports 38 passed. The PR is mergeable, with state BLOCKED pending review. I did not run the tests locally.

}, [copy.archivedProjectCancel, copy.archivedProjectDescription, copy.archivedProjectRestore, copy.archivedProjectTitle, copy.readPathFailedFallback, copy.selectDirectoryFailedTitle, input.confirm, locale, refreshAfterProjectMutation, reportError, service, toast]);

const addDroppedProjectFolders = useCallback(async (files: File[]): Promise<void> => {
if (!files.length || projectMutationPendingRef.current || !localHost?.capabilities.chooseClientDirectory) return;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: if a drop lands while projectMutationPendingRef is set (an earlier batch or a New project registration), it returns silently, but the zone still highlighted and accepted it. Either disable the zone while pending (for example, gate onDropProjectFolders on !pending) or show a short toast.

Gate the existing folder-drop capability on the shared pending state. Extend controller checks for disabled and restored admission, and retain browser capture-order coverage in the existing project-list story for text, file, and session drags.

Generated-by: Codex

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.Incremental review: aa922a40..de9ee770, one commit (+54/-2). Our first P3 is fixed, and I found no P0–P2 issue. CI test passes, and the PR is mergeable.

canDropProjectFolders is now !pending && …, and pending is in the memo dependencies. The drop zone therefore stops claiming drops while a folder batch or a "New project" pick is in flight, so a drop is no longer accepted visually and then silently ignored. The controller tests assert the flag flips to false during both the batch and the picker, and back to true after them.

The rest of the commit is a Storybook story for the drop zone (+48). The earlier request asked to leave stories out of the first version, but this one is small and also covers our second P3 (no rendered test of the zone), so it is fine to keep.

@YayoiNanoka

Copy link
Copy Markdown
Contributor Author

Thanks @Astro-Han for reviewing the follow-up! Both P3s have been addressed in de9ee770a:

  • Folder drops are disabled while project registration or archived-project confirmation is pending, and re-enabled once the operation finishes.
  • A compact regression in the existing ProjectGroups story covers the enabled drop zone: Files registration, internal Session drag passthrough, and capture-phase blocking of text/link drops.

CI passed on this commit, including Desktop E2E and Storybook smoke. The change stays small (+54/-2, with only two production-code lines adjusted). Could you please take another look and let me know if anything else is needed before merging?

点击展开中文

谢谢复查!两个 P3 已在 de9ee770a 中按建议修复:

  • 项目注册或归档恢复确认期间禁用文件夹拖放,操作结束后恢复。
  • 在已有 ProjectGroups story 中保留精简回归测试,覆盖启用状态下的 Files 注册、内部 Session 拖拽放行,以及捕获阶段对文本和链接拖拽的拦截。

该提交的 CI 已通过,包括 Desktop E2E 和 Storybook smoke。本次保持精简,新增 54 行、删除 2 行,产品代码仅调整两行。麻烦再看一下,确认合并前是否还有需要调整的地方,谢谢!

Posted by Codex on behalf of @YayoiNanoka.

@github-actions github-actions Bot added effort/M Under 500 readable lines and removed effort/XL Under 2500 readable lines labels Oct 18, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants