Skip to content

feat(agent): select workspaces by path, not by directory selection - #6632

Merged
M4n5ter merged 4 commits into
rust-previewfrom
fix/workspace-lookup-tool
Nov 6, 2026
Merged

M4n5ter merged 4 commits into
rust-previewfrom
fix/workspace-lookup-tool

Conversation

@UncertaintyDeterminesYou4ndMe

@UncertaintyDeterminesYou4ndMe UncertaintyDeterminesYou4ndMe commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Closes #6630.

Callers no longer hand the Host an AgentDirectorySelection (rootId, segments, directoryIdentity); they name a directory by path, and the Host resolves and identifies it. Models asked to create a conversation in a directory had been guessing those three values (K2.8 Preview sent the SHA-256 of the empty string; K3.1 Pro spent 59 tool calls disassembling the binary for it), and the UI and ACP had been walking the directory tree to look them up.

Resolution lives in maka-workspace-support: a leading ~ is replaced by the home directory the owning service passes in (the Agent owner reads it once from the process environment), . and .. are resolved before symbolic links are followed, the most specific configured location holding the path wins when locations overlap, managed worktrees and private Host paths are excluded, and directory_identity is captured as the directory is opened. Workspace.directory_identity stays as it was: Host-persisted state that validate checks so a session fails closed when its directory is replaced on disk.

Inputs that now take a path:

  • agent.create / SessionCreate workspace: isolated_git, project and directory; session is unchanged.
  • selectWorkspace ({path}), projects kind=directory, project register and relocate.
  • import locations and copy.
  • WorkHub resolve, workhub_tasks route targets and route_preset, and the graph routing.

directories stays for tree browsing: roots, and children keyed by path + after. AgentDirectory carries name and path; AgentProject and AgentProjectLocation lost their selection. The workspace picker browses by path, and "open folder" only has to make sure a location holds the path. ACP's list::select tree walk is gone; NewSession passes the cwd.

Desktop recent workspaces keep only the path; the stored workspace field of older entries is ignored.

Migrating local data

The code keeps no compatibility readers. Plugin-data records written before this change no longer decode: Agent import sources (location.directory), worktree allocations (selection), WorkHub coordinator intents and task targets (workspace). A one-off script rewrites them in place: migrate-pr6632-workspace-paths.py.

# Stop Maka and back up the state root first.
python3 migrate-pr6632-workspace-paths.py ~/.maka/application/host          # report only
python3 migrate-pr6632-workspace-paths.py ~/.maka/application/host --apply  # write

It takes each database's writer lock and refuses while a Host holds it. Import sources and allocations take the path already stored on the same record. WorkHub records hold none, so the script asks for --root root-N=/path for each configured workspace location they name (root-1 is the first). Any other record still holding a directory selection aborts the run before anything is written.

Verification: workspace-support unit tests (paging with inaccessible children; path, ./.., overlapping locations, outside/missing/file/relative/managed; ~ expansion), the native suites for workspaces, projects, imports, worktrees, WorkHub, skills, session context, file browsing, MCP and the Bot, the CLI ACP tests, the crash integration test, the desktop desktop_new_session suite, SDK/bundle/workbench type checks and the workbench script tests.

AI use

  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code wrote the change and this description; its commits carry Generated-by: Claude Code. Antigravity wrote the review follow-up (remaining selection callers, Windows path comparison, blocking resolution, WorkHub creation check), the migration script and its section here; its commit carries Generated-by: Antigravity.

@M4n5ter

M4n5ter commented Nov 4, 2026

Copy link
Copy Markdown
Member
English

Thanks for tracking this down and putting up a draft quickly!

Looking closer at the contract, caller-supplied AgentDirectorySelection ({ rootId, segments, directoryIdentity }) is itself unnecessary ceremony—we don't maintain backward compatibility on internal contracts, so instead of adding a WorkspaceLookup model tool around it, let's remove DirectorySelection as an input contract and take path directly:

  • Keep directory_identity strictly as Host-captured internal state: Workspace.directory_identity is valuable once persisted on the Host (directory::capture when binding + validate afterwards so we fail closed if a directory is replaced on disk while a session lives), not as something callers need to look up and echo back (rootId is just a transient root-N index, and directoryIdentity is not an authorization capability).
  • Take path directly across workspace-selecting inputs: Have WorkspaceSelection in agent.create / SessionCreate, project mutations, WorkHub graph Routing, etc. accept path directly. Pass home: Option<&Path> in rather than reading std::env::var_os("HOME") inside maka-workspace-support, expand leading ~, canonicalize, check containment against workspace_roots (preferring the most specific matching root when roots overlap) and exclusion of managed_worktrees, and atomically capture directory_identity when initializing/opening the workspace.
  • Keep directories strictly for UI tree browsing: Key directories pagination by path + after instead of DirectorySelection, do not expose WorkspaceLookup as a model tool, and delete ACP's list::select tree-walking workaround in products/maka/acp/src/session/list.rs.
简体中文

感谢快速定位问题并提交初版实现!

仔细审视现有契约后,让调用方传入 AgentDirectorySelection({ rootId, segments, directoryIdentity })本身就是不必要的繁琐设计——内部契约不保留向后兼容,因此与其围绕它新增一个 WorkspaceLookup 模型工具,不如直接移除作为入参契约的 DirectorySelection,统一改为接收 path:

  • 将 directory_identity 严格保留为 Host 内部采集的状态:Workspace.directory_identity 的价值在于由 Host 持久化持有(绑定工作区时执行 directory::capture,后续通过 validate 核对,确保会话存续期间磁盘目录被替换时安全拒绝),而不在于让调用方先查询再原样回传(rootId 只是临时的 root-N 下标,directoryIdentity 也不是权限凭据)。
  • 所有选择工作区的入参直接接收 path:让 agent.create / SessionCreate 中的 WorkspaceSelection、项目变更接口、WorkHub 图编排的 Routing 等直接接收 path。由调用方传入 home: Option<&Path>(不要在 maka-workspace-support 内直接读取 std::env::var_os("HOME"))以展开前导 ~,执行规范化,校验其落在 workspace_roots 内(若根目录重叠则优先匹配最长前缀)且排除 managed_worktrees,并在初始化/打开工作区时原子采集 directory_identity。
  • directories 仅保留给 UI 目录树浏览:directories 分页改用 path + after,不再使用 DirectorySelection;不向模型暴露 WorkspaceLookup 工具,同时一并删掉 ACP 在 products/maka/acp/src/session/list.rs 中逐层爬树的 list::select 绕路逻辑。

@UncertaintyDeterminesYou4ndMe UncertaintyDeterminesYou4ndMe changed the title feat(agent): let models look up a directory's workspace selection feat(agent): select workspaces by path, not by directory selection Nov 4, 2026
@UncertaintyDeterminesYou4ndMe

UncertaintyDeterminesYou4ndMe commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor Author

Reworked along those lines in e74cf5b (on top of the earlier commit, for squash) and the description is rewritten.

  • DirectorySelection is gone from the caller side. agent.create (all three directory kinds), selectWorkspace, projects by directory, project register/relocate, import locations and copy, WorkHub resolve, task route targets and the graph routing take path.
  • maka-workspace-support resolves it: ~ from a home the Agent owner passes in, ./.. lexically then canonicalize, most specific overlapping root, managed worktrees and private paths excluded, directory_identity captured as the directory is opened. Workspace.directory_identity and validate are unchanged.
  • directories is UI browsing only, children keyed by path + after; no WorkspaceLookup tool. ACP's list::select is deleted and NewSession passes the cwd.

Not done: the task-creation check in WorkHub now compares the created session's path with the route's resolved path rather than identities, since the worker resolves the path on its side. Say if you would rather it re-resolve through the worker's directories.

@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.

This is a focused review of WorkHub coordinator recovery and workspace-identity handling at dcb77eec7a9aaa82967440838f0d836bb7b9ff25, not a complete review of all 44 changed files. I found two P2 issues, detailed inline: existing coordinator intents no longer decode, and new pending intents lose the directory identity needed to prevent retargeting during recovery.

The path-only public interface removes the need for callers to construct Host-owned directory selections. The new resolver expands the supplied home directory, normalizes and canonicalizes paths, selects a configured location and captures a directory identity. That interface change does not require dropping the Host's durable physical-identity expectation after it has resolved a path.

I read the old/current coordinator intent declarations, their plugin-data write/read and coordinator/resolve dispatch, the pending-to-create boundary, Owner::create and project-workspace creation, and the workspace resolver/initializer. A controlled Rust decoder check reproduces the object-to-string incompatibility using the actual declarations; recovery metadata is represented opaquely and the directory-identity adapter uses a supplied valid-format string, so this is not a native Root-reopen test. A separate check runs the actual resolve_path function against real directories and confirms that replacement preserves the canonical path while changing the physical directory identity. The two inline comments explain how these observations reach the production recovery paths.

The actual Git merge against rust-preview at c00ee9921a3df098ca9c9723d850c34307476370 succeeds. CI 41860448684 is still running at publication; its dependency audit 41860449457 succeeds. I did not run the complete native Host or verify all new UI, ACP, import, task-routing and worktree paths. The remaining persisted task-target/allocation shape changes also need a complete compatibility review. The author-reported model behavior and test results remain the author's evidence.


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

operation: RecoveryRef,
workspace: crate::contract::DirectorySelection,
/// The coordinator's directory as the caller named it.
workspace: String,

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.

P2: Read or migrate the existing durable coordinator intent before requiring a string.

The previous build writes Intent.workspace as a DirectorySelection object (rootId, segments, directoryIdentity) under this same WorkHub plugin-data key. The new intent() immediately deserializes that record into this struct, so it returns workhub_intent_invalid for an existing record. Both coordinator and resolve call intent() before observe(): this also prevents returning an already-created, otherwise healthy coordinator, not just resuming an unfinished creation.

A controlled Rust codec check using the actual old/new Intent declarations accepts the old workspace object with the old decoder and fails with invalid type: map, expected a string with the new decoder. Unrelated recovery metadata is opaque in that check; the production write/read and pre-observation order were checked separately in this file. The description's no-compatibility statement does not provide migration or operator recovery for these persisted records.

Add a versioned legacy reader or migration which preserves the original operation and already-created coordinator, and test reopening a Root with an actual pre-change intent. Do not silently discard the intent and create another coordinator.


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

labels: vec![],
workspace: crate::contract::WorkspaceSelection::Directory {
selection: intent.workspace,
path: intent.workspace,

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.

P2: Keep the first directory's physical identity in the durable intent used for resumed creation.

For a new intent, resolve() calls workspace::selected, but only tests Ok(Some(_)) and discards the returned Located.identity; the durable record keeps just the caller's path. If execution stops after that record commits and before coordinator creation, replacing the directory at the same path causes retry to reach this branch with the original operation but a different directory. Owner::create → create_project_workspace → workspace::initialize captures that replacement's new identity, with no original identity left to reject it. The original DirectorySelection carried that expectation across the same boundary.

I checked the actual new resolve_path function with a real rename-and-recreate operation: the resolved canonical path stays equal while the directory inode changes; retaining the original identity would detect the replacement. This is a controlled filesystem check plus the inspected production call chain, not a full Host restart test. Persisted Workspace validation after a Session exists does not cover this earlier pending-intent window.

Keep the public path-only input, but freeze the Host-resolved path and identity internally before recording the intent and require that identity when resuming creation. Add a stop/reopen/replace-directory regression which refuses the replacement under the original operation.


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

@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.

A further P2 is now confirmed in the running CI for dcb77eec7a9aaa82967440838f0d836bb7b9ff25: the shared real-Host fixture still sends the removed directory-selection input. This supplements the two recovery findings above; it remains a scoped result rather than completion of the 44-file review.

In CI 41860448684, desktop shards 1/4, 2/4 and 3/4 each fail all five tests at support.rs:464 while preparing agent.create: {"kind":"directory","selection":null} fails the new workspace schema. The unchanged fixture at support.rs:326 reads roots["roots"][0]["selection"], but this PR removes that output field and requires a path input. The side-pane test also builds that old request directly at desktop_new_session.rs:669.

I compared the actual source and read all three failed job logs (126686086921, 126686087188, 126686086980). On the current target c00ee992, the matching five tests in desktop shard 1/4 pass in CI 41853005558, job 126681643056; I read its original passing log as the control. This attributes these 15 failures to the incomplete caller/fixture migration in this PR, separately from the older external-agent UI wait.

Please migrate the shared fixture and remaining direct test callers to path and rerun the desktop matrix. These failed cases stop during fixture setup and do not validate the intended UI behaviors. I did not rerun the full native application locally.


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

@@ -377,7 +386,6 @@ pub enum Directories {
pub struct Directory {
pub name: String,
pub path: String,

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.

P2: Migrate the shared real-Host fixture and direct test callers when removing Directory.selection.

Fixture::open_with_configuration in terminal-shell/tests/real_host/support.rs:326 still sends workspace: {kind:"directory", selection: roots["roots"][0]["selection"]}. With this output field removed, indexing produces null, and the new WorkspaceSelection::Directory { path } schema rejects the input before the test UI starts. desktop_new_session.rs:669 repeats the old request directly.

Exact-head CI 41860448684 jobs 126686086921 / 126686087188 / 126686086980 each have 0 passes and 5 failures with that same selection:null error at support.rs:464. The same five shard-1 cases pass on target c00ee992 in job126681643056, and the helper source itself is unchanged. This is a PR-introduced CI regression. Update these callers to use the returned path and run the affected matrix so these tests can exercise their actual UI scenarios.


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

@M4n5ter

M4n5ter commented Nov 5, 2026

Copy link
Copy Markdown
Member
English

Thanks for the clean rework—the overall contract direction looks great! A few concrete issues to fix before merging, plus notes on WorkHub and storage migration:

  1. Unmigrated "selection" callers (causes the CI desktop-tests / root-tests failures):

    • CI tests:
      • products/maka/terminal-shell/tests/real_host/support.rs:326
      • products/maka/terminal-shell/tests/real_host/desktop_new_session.rs:669
      • products/maka/cli/tests/integration/candidate/crash.rs:98
    • Offline benchmark harness (not run in CI, good to update alongside):
      • products/maka/cli/examples/host_benchmark_client.rs:505-508
      • scripts/rust/host-comparison/compare.py:1340-1375 (can pass path directly now instead of walking directories)
  2. Windows \\?\ verbatim prefix mismatch in plugins/workspace/support/src/lib.rs:
    On Windows, root.directory.path() (PublishedDirectory::open), resolve_path(...), managed (owned_root().canonical_path()), and excluded all carry std::fs::canonicalize's \\?\C:\... prefix, whereas Located.path / selected.path strips \\?\ via host_path (dunce::simplified). Turning selected.path / located.path back into Path and comparing against \\?\ paths breaks Component-based prefix matching on Windows:

    • query_excluding (lib.rs:238, 249-250, 278): Path::new(&selected.path).strip_prefix(root.directory.path()) and capture(root, &Path::new(&selected.path).join(name)) will always fail on Windows, and Path::new(&selected.path).starts_with(excluded) will always return false. Use the canonical path from line 234 directly for starts_with, strip_prefix, and .join(name).
    • initialize (lib.rs:332-335) & resolve (lib.rs:449-451): shadowing canonical path with PathBuf::from(&located.path) before path.starts_with(&managed) / path.starts_with(&excluded) makes those exclusion checks always false on Windows. Keep the canonical path (or only convert to host_path for wire output).
    • Unit test lib.rs:605: assert_eq!(found.path, canonical.to_str().unwrap()) will also fail on Windows unless canonical goes through host_path.
  3. Blocking resolve_path on the async executor in plugins/workspace/support/src/lib.rs:
    query_excluding runs resolve_path inside tokio::task::spawn_blocking, while initialize (lib.rs:326) and resolve (lib.rs:444) call resolve_path (which runs blocking canonicalize() + is_dir()) on the async runtime thread right before spawn_blocking. Please move resolve_path inside the spawn_blocking closures there as well (and feel free to keep maka_plugin_core::json::check_delivery in fits at lib.rs:216).

  4. On your WorkHub task-creation check question (plugins/agent/native/src/workhub/tasks/preparation.rs:339-349):
    As context, neither standalone workhub nor graph currently has any product-side callers—standalone workhub will eventually be removed as its coordination capabilities fold into Bot, and we'll separately evaluate whether graph is still needed (out of scope for this PR). So let's keep the WorkHub/graph touchpoints minimal here:

    • No need to re-resolve through the worker's directories, and let's also drop the local maka_workspace_support::resolve_path(...) call in prepare_delivery (the worker's create is already the authority resolving path, and local resolve_path runs blocking FS I/O in async fn and hits the same dunce vs \\?\ mismatch on Windows). Checking selected.worktree's shape alone (selected.worktree.as_ref().is_some_and(|binding| binding.directory() == std::path::Path::new(&selected.path)) when *isolated_workspace, and selected.worktree.is_none() otherwise) is sufficient.
    • In workhub.rs:246, storing located.path in Intent.workspace is plenty (no need for extra identity checks there).
  5. Persisted storage compatibility:
    Please ignore the automated review comment asking for legacy DirectorySelection fallback readers in code—we don't keep backward-compatibility shims for internal persisted formats. Where persisted plugin_data records change shape and would fail to deserialize on existing local DBs (e.g., import_sources ImportLocation, :worktree Allocation, or workhub Intent/Target—note ImportSource.path and Allocation.binding.source already sit on the same JSON record), linking a small one-off migration script Gist in the PR is enough.

简体中文

感谢快速完成重构,整体契约方向非常清晰!合并前还有几处具体问题需要处理,顺便回答 WorkHub 校验与本地存储迁移的问题:

  1. 漏迁移的 "selection" 调用点(当前 CI desktop-tests / root-tests 失败的直接原因):

    • CI 测试:
      • products/maka/terminal-shell/tests/real_host/support.rs:326
      • products/maka/terminal-shell/tests/real_host/desktop_new_session.rs:669
      • products/maka/cli/tests/integration/candidate/crash.rs:98
    • 离线基准脚本(不在 CI 中运行,建议一并改齐):
      • products/maka/cli/examples/host_benchmark_client.rs:505-508
      • scripts/rust/host-comparison/compare.py:1340-1375(现在可以直接传 path,无需再逐层遍历 directories)
  2. plugins/workspace/support/src/lib.rs 在 Windows 下的 \\?\ 前缀不匹配问题:
    在 Windows 上,root.directory.path()(PublishedDirectory::open)、resolve_path(...)、managed(owned_root().canonical_path())以及 excluded 都带有 std::fs::canonicalize 产出的 \\?\C:\... 前缀;而 Located.path / selected.path 经过 host_path(dunce::simplified)剥离了 \\?\。把 selected.path / located.path 转回 Path 再与带 \\?\ 的路径做比较,在 Windows 上会因 Prefix 组件不同而失效:

    • query_excluding(lib.rs:238, 249-250, 278):Windows 下 Path::new(&selected.path).strip_prefix(root.directory.path()) 和 capture(root, &Path::new(&selected.path).join(name)) 会恒定失败,且 Path::new(&selected.path).starts_with(excluded) 恒为 false。请直接使用第 234 行拿到的规范 path 做 starts_with、strip_prefix 和 .join(name)。
    • initialize(lib.rs:332-335)与 resolve(lib.rs:449-451):在 path.starts_with(&managed) / path.starts_with(&excluded) 之前用 PathBuf::from(&located.path) 覆盖 path,会导致这些排除检查在 Windows 下恒为 false。保留规范 path(仅在构造对外返回值时使用 host_path)即可。
    • 单测 lib.rs:605:assert_eq!(found.path, canonical.to_str().unwrap()) 在 Windows 下也会因 \\?\ 前缀不一致而失败,需先对 canonical 走 host_path。
  3. plugins/workspace/support/src/lib.rs 在异步线程上执行阻塞 resolve_path:
    query_excluding 把 resolve_path 放进了 tokio::task::spawn_blocking,但 initialize(lib.rs:326)和 resolve(lib.rs:444)在进入 spawn_blocking 之前直接在异步执行器线程上调了 resolve_path(含阻塞的 canonicalize() + is_dir())。请把这两处也移入各自的 spawn_blocking 闭包内(另外 lib.rs:216 的 fits 也建议保留原来的 maka_plugin_core::json::check_delivery)。

  4. 关于你提到的 WorkHub 创建任务校验问题(plugins/agent/native/src/workhub/tasks/preparation.rs:339-349):
    补充个背景:目前独立的 workhub 和 graph 在产品侧都没有调用方——后续独立的 workhub 会移除并将所需协调能力合入 Bot,graph 是否保留也会单独评估(均不在本 PR 范围内)。因此这个 PR 里对 WorkHub/graph 保持最小改动即可:

    • 不需要再去调 worker 的 directories,同时建议直接删掉 prepare_delivery 里的本地 maka_workspace_support::resolve_path(...)(worker 的 create 本身就是解析 path 的权威边界,协调者本地调 resolve_path 既在 async fn 中做了阻塞 I/O,在 Windows 下同样会遇到 dunce 与 \\?\ 不匹配)。只校验 selected.worktree 的形态(*isolated_workspace 时检查 selected.worktree.as_ref().is_some_and(|binding| binding.directory() == std::path::Path::new(&selected.path)),否则检查 selected.worktree.is_none())即可。
    • workhub.rs:246 里把 Intent.workspace 存成 located.path 就够了,无需额外加 identity 核对。
  5. 关于持久化存储格式变更:
    自动 Review 要求在代码里保留旧版 DirectorySelection 兼容读取的评论请直接忽略——我们不在代码里为内部存储格式保留向后兼容分支。对于因持久化格式变更可能导致既有本地库反序列化报错的 plugin_data 记录(如 import_sources 的 ImportLocation、:worktree 的 Allocation,以及 workhub 的 Intent/Target——其中 ImportSource.path 和 Allocation.binding.source 本身就在同一条 JSON 记录上),在 PR 里附一个一次性迁移脚本 Gist 即可。

@github-actions github-actions Bot added the effort/XL Under 2500 readable lines label Nov 5, 2026
SessionCreate takes a directory workspace as rootId, segments and
directoryIdentity, which no model tool returned, so models asked for a
conversation in a directory guessed them. The agent service's
directories query, which the workspace picker uses, is now the model
tool WorkspaceLookup, and it resolves an absolute path (~ allowed) to
the directory with its selection within the configured locations, or
to null with the locations listed. The Bot holds it with its other
coordination tools, and the SessionCreate description says where a
selection comes from.

Closes #6630.

Generated-by: Claude Code
Callers named a workspace as rootId, segments and directoryIdentity,
values only the Host could know, so models asked for a conversation in
a directory guessed them and the UI and ACP walked the directory tree
to look them up. Workspace-selecting inputs now take a path: the Host
expands a leading ~, normalizes and canonicalizes it, finds the most
specific configured location holding it, excludes managed worktrees
and private paths, and captures the directory identity as it opens the
directory. Workspace.directory_identity stays Host-persisted state that
validate checks. The directories query remains for UI browsing, paged
by path; the ACP tree walk is removed, and so is the WorkspaceLookup
tool this branch had added.

Closes #6630.

Generated-by: Claude Code
Migrate the remaining callers that still sent a directory selection: the
real-Host desktop fixtures, the crash integration test, the Host
benchmark client and the host-comparison script. Recent workspaces keep
only the path, which the Host resolves on creation.

maka-workspace-support now compares canonical paths with canonical roots
and exclusions, so Windows verbatim prefixes no longer defeat the
managed-worktree and private-path checks, and resolves paths inside the
blocking task rather than on the executor. WorkHub freezes the
Host-resolved path in its coordinator intent and checks only the shape
of a worker's created workspace, leaving path resolution to the worker.

Generated-by: Antigravity
@M4n5ter
M4n5ter force-pushed the fix/workspace-lookup-tool branch from dcb77ee to 6c6a4cb Compare November 6, 2026 01:06
@M4n5ter
M4n5ter merged commit 445767c into rust-preview Nov 6, 2026
19 checks passed
@M4n5ter
M4n5ter deleted the fix/workspace-lookup-tool branch November 6, 2026 01:18

@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.

I reviewed the final 59-file diff at 6c6a4cbc62e6771d40b56924cd4721553b3711d2, including the path resolver, Agent/WorkHub inputs and persisted records, project/import/worktree handling, ACP, recent-workspace storage, UI callers, generated contracts and tests. I separated the rebased changes with git range-diff and checked the final follow-up commit. The merged source tree at 445767cc23447e97411457d7affd10e2f01b5abf is identical to this PR head.

One previously reported P2 remains: a pending WorkHub coordinator intent still drops the first directory's physical identity. Canonicalizing its path fixes relative/home-path reinterpretation, but does not prevent replacement at that same canonical path. The inline comment updates the finding for this final commit.

The other two earlier findings have changed:

  • The shared real-Host fixture and direct callers now use path. I read the original logs from all four desktop jobs in the successful exact-head CI: 5 + 5 + 5 + 4 tests pass. This resolves the earlier selection:null setup failure; I did not rerun the native application locally.
  • Existing records still require migration, but there is now a documented manual migration script. I ran its actual downloaded bytes on disposable SQLite samples: report-only, four record transformations with revision increments, idempotence, refusal of an unmapped root, refusal of an unknown selection-bearing record, and refusal while the writer lease is held. An extracted Rust codec accepts its transformed coordinator intent and preserves the opaque original operation; an unmigrated object remains rejected. This verifies the explicit migration path on synthetic records, not a complete pre-upgrade Root reopen or a valid native recovery operation. Users must stop Maka, back up the state root and perform the documented migration; there is no automatic compatibility reader.

Local verification also ran the actual four conversation-group tests on Node 22: all pass; substituting the pre-change grouping module makes one fail. A separate Rust check used the actual final Intent declaration and resolve_path function with a real rename/recreate operation: after persisting the canonical path, it resolves to the same pathname but a different inode. Keeping the original identity would reject that replacement. Recovery metadata and physical identity have controlled adapters in this check; it is not a native Host restart test.

I did not verify live model selection behavior, complete old-state migration, native UI operation or Windows/macOS path behavior locally. The model observations in the PR description remain the author's evidence. The PR is already merged; the old pre-merge conflicts no longer apply. At this check the successor Rust commit is a61a85d1; its CI is still running, so this review does not claim that successor is fully validated.


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

let candidate = Intent {
operation,
workspace: input.workspace,
workspace: located.path,

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.

P2: Preserve the original physical identity alongside the canonical coordinator path.

located.path now freezes the Host-resolved pathname, but located.identity is still discarded. If the durable intent commits and creation stops before a Session exists, renaming that directory away and creating another directory at the same path leaves an intent with the original operation and no expected identity. On retry, resolve() skips first-time selection, then Owner::create → create_project_workspace → workspace::initialize captures the replacement's identity.

I reran a controlled Rust check with the actual final Intent declaration and resolve_path: a serialized/reloaded intent keeps the canonical path and original opaque operation, while a real rename/recreate produces a different inode at that path. An original-identity guard rejects the replacement. This is source-chain inspection plus an actual filesystem/codec check, not a native Host restart.

Persist the Host-captured identity in the intent and require it before resumed creation; canonical path equality alone does not close the previously reported recovery window.


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

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

Labels

effort/XL Under 2500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants