feat(runtime): coordinate tool batches through resource authorities - #4542
feat(runtime): coordinate tool batches through resource authorities#4542Jarad-z wants to merge 6 commits into
Conversation
|
A few design thoughts after looking at this through the lens of classic concurrency and Tokio-style async I/O. Overall, I agree that overlapping local tool effects need deterministic coordination, and the scheduler implementation itself is coherent. The writer-fair rule (a later reader cannot bypass an earlier queued writer, while independent work can bypass) is a sensible async-RwLock policy. My main concern is that the current design may be combining two different responsibilities:
The concurrency problem is at the resource authorityThis is closer to an asynchronous I/O side-effect race than a shared-memory/executor scheduling problem. A Tokio executor schedules runnable futures; it does not inspect application resources to decide whether two futures touch the same Redis key or file. Readiness and exclusivity are owned below the executor by the I/O resource, an async Mutex/RwLock, a semaphore, or a dedicated resource-owning task/actor. The analogous Maka split would be: With this split, the batch runner could remain a simple fan-out/fan-in over lazy This is especially important because a batch-local scheduler cannot enforce the invariant against another batch, turn, agent, process, or any code path that reaches the same resource without going through this scheduler. The existing filesystem write lock already points toward the lower-level authority model; it also canonicalizes closer to the actual execution boundary. Tokio concepts map to different Maka primitivesI do not think one generic access graph should carry every responsibility:
In that model, the equivalent of a Tokio waker is simply the authority releasing a lease or consuming a command and waking the next waiter. We do not need work stealing or lock-free queue machinery here; the hard part is ownership and cancellation-safe I/O effects, not CPU scheduling. Concrete concerns in the current phase
Suggested simplificationI would frame the first milestone as:
Then introduce only three reusable coordination concepts when concrete users exist: A thin batch planner can still be valuable later to avoid wasted contention and improve observability, but it should be advisory rather than the sole correctness boundary. So my conclusion is: the PR has a reasonable scheduling algorithm, but I think the design would become both simpler and stronger if correctness moved to resource owners and the batch layer were narrowed to orchestration. At minimum, I would clarify the intended regression for undeclared tools and add real builtin behavior tests before merging this phase. |
M4n5ter
left a comment
There was a problem hiding this comment.
English
The problem is real, and the scheduling algorithm itself is coherent. However, I do not think resolveAccesses plus a batch-local ToolScheduler should become Maka’s long-term source of truth for resource correctness.
My preferred end state is:
thin deterministic batch sequencer
over
authority-prepared operations
The batch layer should own step admission, provider-order tickets, result slots, cancellation, and fail-stop behavior. Resource authorities should own canonical identity, mutual exclusion, revision/CAS checks, and the actual effect. In other words, the scheduler is still useful, but it should decide when a prepared operation may proceed—not independently decide what the real resource is.
Ideally, validated immutable arguments would be prepared once:
validated args
→ authority.prepare(...)
→ PreparedOperation { authoritative claims, execute() }
The same prepared operation would then drive both scheduling and execution. Files, session state, terminal/browser state, and provider capacity do not need to pretend to have identical semantics: they may use keyed leases, revisioned state/actors, or semaphores behind this lifecycle contract.
There are also two concrete correctness concerns in the current implementation:
-
The scheduler key is not the identity used at the execution boundary.
normalizeToolFilePathis explicitly lexical, and the builtin access resolvers use it directly. The filesystem executor later derives a canonical enforcement path independently. Consequently,Read("link/a")andEdit("real/a")can be considered independent by this scheduler even when a symlink makes them the same physical file. The architecture document says aliases should be canonicalized during preparation, but that does not currently happen.If the intended guarantee is ordering accesses to the same resource—not merely identical path spellings—the claim should come from the same filesystem authority used by execution, with a real builtin alias regression test.
-
A turn-fatal rejection releases the resource and starts queued conflicting work.
settleToolCallnormally resolves tool/business failures, while T1/T2 commit failures can reject. However,finishTaskdrains the queue after every rejection, and the backend only promotes the rejection after the entire batch has settled.This permits the following sequence:
A performs a write → A's T2 outcome commit fails → scheduler releases A's access → queued conflicting operation B starts → only after B settles does the turn surface A's fatal errorWaiting for already-active tasks to unwind is necessary; starting work that has not crossed T1 is not. A business failure may continue the queue, but a turn-fatal/infrastructure rejection should freeze dispatch, cancel queued entries, and then wait for active settlements.
A smaller contract concern is that resolveAccesses currently receives the original mutable executionInput before ToolRuntime snapshots and validates it. Its purity is only documentary. If this hook influences correctness, it should receive an immutable validated snapshot, or be replaced eventually by the prepared-operation seam above.
I am not suggesting that this PR must implement cross-turn coordination, every resource actor, or the complete target architecture. I would, however, address the identity mismatch and fatal-rejection behavior before treating this as a correctness foundation, and document resolveAccesses as a batch orchestration bridge rather than the final resource authority. The previously raised undefined => all fan-out regression also still needs an explicit compatibility decision and tests.
中文
这个问题确实值得解决,当前这套调度算法本身也说得通。不过我不希望 resolveAccesses + batch 内 ToolScheduler 就此变成 Maka 长期的资源正确性边界。
我更期待的最终形态是:
一层很薄、负责确定性编排的 batch sequencer
+
由各个资源权威准备好的 operation
batch 层负责 step admission、provider 顺序、结果槽位、取消和致命失败后的停止调度;文件系统、Session 状态、终端或浏览器等真正拥有资源的一层,负责确认资源身份、互斥、版本检查以及执行副作用。
换句话说,调度器仍然有价值,但它只该决定“什么时候可以执行”,不该自己猜“这个调用真正访问的是哪个资源”。
比较干净的契约应该是先校验并冻结参数,再由资源权威生成一份 operation:
校验后的参数
→ authority.prepare(...)
→ PreparedOperation { 权威资源声明, execute() }
之后调度和执行使用同一份结果,避免一边按字符串路径排队,另一边执行时又重新解析路径。文件、Session 状态、终端、浏览器和并发容量也不必硬塞进同一种锁模型:它们可以分别使用 keyed lease、revision/CAS、actor 或 semaphore,只共享准备、执行和完成这套生命周期。
当前实现里还有两个比较实在的正确性问题:
-
排队时认的资源,和真正执行时认的资源不是一回事。
normalizeToolFilePath只做字符串层面的路径整理,几个 builtin 的resolveAccesses直接拿它生成 key;真正写文件时,filesystem executor 又会单独解析 canonical path。这样一来,
Read("link/a")和Edit("real/a")即使通过符号链接指向同一个文件,也可能被调度器当成互不相关的任务并发执行。设计文档里写了别名应当在 preparation 阶段归一,但目前代码没有做到。如果这里承诺的是“同一资源按顺序执行”,而不只是“路径字符串恰好相同才按顺序执行”,那么资源 claim 应该来自实际执行所使用的 filesystem authority,并补一个真实 builtin 的路径别名测试。
-
遇到 Turn 级致命失败后,调度器仍会继续放行排队中的冲突任务。
settleToolCall会把普通工具错误变成正常的错误结果;真正 reject 的可能是 T1/T2 持久化失败之类的基础设施问题。但finishTask不区分失败类型,只要当前任务结束就释放资源并继续扫队列,而 backend 要等整个 batch 全部结束后才抛出致命错误。实际上可能发生:
A 已经修改文件 → A 的 T2 结果持久化失败 → 调度器释放 A 占用的资源 → 排队中的冲突操作 B 开始执行 → B 结束后,Turn 才报告 A 的致命错误已经在运行的任务需要等待它们正常收尾,但尚未跨过 T1 的任务没有理由继续启动。普通业务失败可以继续队列;一旦确认是 Turn 级基础设施错误,就应该停止派发、取消 queued tasks,然后等待 active tasks 结束。
还有一个相对小一些的契约问题:resolveAccesses 现在拿到的是原始、可变的 executionInput,之后 ToolRuntime 才会复制并校验参数。“必须是纯函数”目前只靠注释约束。如果它要参与正确性判断,至少应该传入已经校验并冻结的参数;长期则可以被上面的 prepared-operation 契约取代。
我的意思不是要求这个 PR 一步做完跨 Turn 协调、各种 actor 和完整终局架构。但如果要把这套机制当成后续资源调度的基础,我认为路径身份不一致和致命失败后继续启动任务这两点应该先处理;同时应把 resolveAccesses 定位成 batch 编排阶段的过渡接口,而不是最终的资源权威。前面已经有人提到的 undefined => all 导致 fan-out 退化,也仍然需要明确的兼容性决定和回归测试。
c980124 to
67b0881
Compare
|
@likun666661 @M4n5ter I pushed a substantial revision that follows the authority-owned correctness model from your reviews. The PR now uses immutable authority-prepared operations, canonical filesystem identity with process-wide exact/tree leases, fail-stop queued dispatch, real builtin/cross-batch tests, and explicit fan-out/E2E trade-offs. I also rebased onto current main and rewrote the PR description around the remaining compromises and follow-up authorities. When you have time, could you please take another look? |
|
重新 review 了当前 revision 1.
|
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Make all() process-wide across participating authorities and capture filesystem identity only after admission. Pin exact reads to admitted targets and cover cross-batch ordering, create-read chains, and process composition. Generated-by: Codex
67b0881 to
c7d2ac8
Compare
|
@likun666661 @M4n5ter I pushed revision c7d2ac8 and rebased it onto current main. This addresses the two blockers from the latest review:
I also added descriptor-pinned exact reads and updated the worker identity contract so Read and mutation effects use the admitted object rather than re-resolving an unchecked pathname. Verification after rebase:
The full Runtime dist suite still has the previously reported Windows-only harness/platform failures (/bin/echo, symlink privileges, SQLite EBUSY cleanup, and PTY timing), so I have kept that limitation explicit in the PR description. When you have time, could you please re-review the current revision? |
Summary
This revision keeps the useful part of the batch scheduler—a thin, deterministic sequencer—but moves resource correctness to resource authorities:
The filesystem authority owns canonical execution paths plus process-wide, writer-fair exact/tree read-write leases. A separate process admission plane now makes all() a real process-wide exclusive barrier against participating filesystem work, including direct execution and root/child Runtime compositions.
Preparation retains only stable canonical claims. Mutable filesystem identity is sampled after process and filesystem admission, once earlier conflicting owners have completed, and is then pinned through descriptor/CAS primitives for the actual effect. This allows legitimate ordered transitions such as Write(create) -> Read and atomic replacement -> Read without reopening the race between admission and execution.
The batch layer owns provider-order tickets, stable result slots, cancellation, and fail-stop dispatch. Provider order is only a deterministic tie-breaker for conflicting operations; it is not a general data dependency between tool calls in one assistant step.
Refs #4487.
What changed after review
Current compromises
Verification
Passed locally on Windows after rebasing onto current apache/main:
The complete Runtime dist suite was also attempted. It does not complete cleanly on this Windows host because of existing platform/test-harness cases involving /bin/echo, symlink privileges, SQLite EBUSY cleanup, and PTY/pipe timing. The affected and newly added suites above complete cleanly; the platform limitation is reported rather than hidden.
The detailed earlier filesystem lease matrix remains in docs/filesystem-read-tree-lease-test-report.md.
AI use
Tool(s) and scope: Codex assisted with architecture review, implementation, tests, rebase, and PR preparation. All contributed commits carry a Generated-by: Codex trailer.
Checklist
Does this PR entail a change in behavior?