You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "Jarad-z (via GitHub)" <gi...@apache.org> on 2026/09/02 06:41:54 UTC
[PR] feat(runtime): make local tool-batch scheduling resource-aware [maka]
Jarad-z opened a new pull request, #4542:
URL: https://github.com/apache/maka/pull/4542
## Summary
- add call-level resource access declarations with conservative fail-closed normalization
- schedule one provider tool-call batch with writer fairness, independent-task bypass, cancellation safety, and stable result ordering
- preserve `exclusive_step` as a precomputed control-plane admission before resource scheduling
- declare file access for Read, Write, Edit, FormatJson, Glob, Grep, and multi-file apply_patch
- include the phase-oriented scheduler architecture and test matrix
Refs #4487
This implements the first phase described by the architecture document. Session-state resources, UI/browser authorities, MCP/provider capacity, and cross-batch coordination remain follow-up work under #4487.
## Verification
- `npm --workspace @maka/core run build`
- `npm --workspace @maka/storage run build`
- `npm --workspace @maka/runtime run typecheck`
- `npm --workspace @maka/runtime run build`
- affected Runtime suites: 258 tests passed
- Biome check on all changed Runtime source and test files
- `git diff --check`
Not run cleanly: the complete Runtime `test:dist` suite on Windows. Existing platform-specific cases fail on `/bin/echo`, symlink permissions, and SQLite cleanup; the affected suites listed above pass.
## AI use
Select exactly one:
- [ ] No generative tool made a substantive contribution
- [x] Generative tooling made a substantive contribution
Tool(s) and scope: Codex contributed the architecture review, Runtime implementation, tests, and PR preparation. The commit carries the required `Generated-by: Codex` trailer.
## Checklist
- [x] Tests cover the change and fail without it
- [x] Lint, format, typecheck and the affected suites pass locally
Does this PR entail a change in behavior?
- [x] Yes — described under Summary above
- [ ] No
## Review focus
The scheduler is intentionally batch-local. Unknown or failed access declarations serialize through `all`; rejected `exclusive_step` siblings declare `none` and still settle through ToolRuntime to preserve durable, model-visible refusal results.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(runtime): coordinate tool batches through resource authorities [maka]
Posted by "Jarad-z (via GitHub)" <gi...@apache.org>.
Jarad-z commented on PR #4542:
URL: https://github.com/apache/maka/pull/4542#issuecomment-5544213078
@likun666661 @M4n5ter I pushed revision c7d2ac87f9b6dfa9a636abe9e89c1c7a03a8179a and rebased it onto current main.
This addresses the two blockers from the latest review:
1. all() now takes writer-fair process-exclusive admission, while participating filesystem paths take process-shared admission. Direct execution, independent ToolCallBatches, and root/child Runtime Host compositions share the same coordinator. Explicit none() remains outside the barrier.
2. Filesystem preparation now retains only the stable canonical claim. Mutable target identity is sampled after process/filesystem admission, immediately before the pinned effect. Real Write(create) -> Read, replacement -> Read, ApplyPatch chains, and cross-batch cases now pass while post-admission replacement still fails closed.
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:
- @maka/core, @maka/storage, @maka/mcp, @maka/runtime, and @maka/runtime-host builds pass
- affected authority/filesystem/worker/batch/host matrix: 120 passed, 0 failed
- Biome check and git diff --check pass
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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(runtime): coordinate tool batches through resource authorities [maka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #4542: feat(runtime): coordinate tool batches through resource authorities
URL: https://github.com/apache/maka/pull/4542
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(runtime): coordinate tool batches through resource authorities [maka]
Posted by "Jarad-z (via GitHub)" <gi...@apache.org>.
Jarad-z commented on PR #4542:
URL: https://github.com/apache/maka/pull/4542#issuecomment-5556576601
@likun666661 @M4n5ter I rebased the PR again onto apache/main at a03dd577bbe38a2d602879fef42bfeddf85af56a. The new head is 117232fde4c5c5dbfffababb91f17a6a41ab7e0e.
The only source conflict came from the upstream AiSdkBackend/AiSdkTurn split in #4779. I kept that new boundary and moved the provider-batch scheduler plus Code Mode prepared-operation path into AiSdkTurn; the authority/admission behavior is unchanged.
Fresh verification on the rebased head:
- @maka/core, @maka/storage, @maka/mcp, @maka/runtime, and @maka/runtime-host builds pass
- deterministic affected matrix: 79 passed, 0 failed
- provider-batch scheduling and Code Mode integration: passed
- root/child process-admission composition: passed
- Biome on all 60 changed TypeScript files and git diff --check: passed
The broader Windows run still exhibits the already documented platform-only cases (/bin/echo, symlink privileges, open-file rename EPERM, SQLite EBUSY, and PTY/pipe timing). GitHub now reports the PR as mergeable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(runtime): make local tool-batch scheduling resource-aware [maka]
Posted by "likun666661 (via GitHub)" <gi...@apache.org>.
likun666661 commented on PR #4542:
URL: https://github.com/apache/maka/pull/4542#issuecomment-5506018216
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:
1. **Correctness:** preventing corruption or inconsistent observation of a resource.
2. **Orchestration:** deciding which otherwise-safe tasks should start now for determinism, fairness, or throughput.
### The concurrency problem is at the resource authority
This 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:
```text
AiSdkBackend / batch runner
- creates lazy ToolTasks
- preserves admission and result slots
- propagates cancellation
Resource authority
- canonicalizes the real resource identity
- owns mutual exclusion / version checks
- defines the linearization point
- wakes the next waiter when the resource becomes available
Underlying effect
- filesystem / session state / terminal / browser / remote service
```
With this split, the batch runner could remain a simple fan-out/fan-in over lazy `run()` closures. A task that is not ready awaits its resource authority instead of being kept out of a central batch scheduler. Correctness would still hold if the batch planner were disabled.
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 primitives
I do not think one generic access graph should carry every responsibility:
- Files: keyed, writer-fair async RwLease at the filesystem authority, plus the existing inode/version/atomic-write safeguards.
- Todo/Goal/Plan: immutable state + revision CAS, or a session-state actor.
- Terminal/browser: one resource-owning actor per session/tab/window, with commands and reply promises.
- Web/MCP/subagent capacity: semaphore/capacity policy, not fabricated resource conflicts.
- Multi-resource atomic operations: an explicit transaction/`acquireMany` contract if atomicity is truly required. Holding multiple batch-local accesses prevents some interleavings, but does not by itself provide crash atomicity or cross-batch isolation.
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
1. **`resolveAccesses` omission => `all` is a behavioral change, not only a safe default.** This PR only adds precise declarations to file tools. Existing tools such as WebSearch and agent spawning appear to remain undeclared, so multiple calls that previously fanned out can now serialize behind `all`. In particular, the existing subagent concurrency limiter may effectively become batch-local concurrency 1. If this is intentional, it should be an explicit compatibility decision with regression tests; if not, it is a fairly large hidden throughput regression.
2. **The first demonstrated problem is narrower than the architecture.** Mutation/mutation on one file is already serialized by `withFileWriteLock`. The immediate uncovered cases are read/write, search/write, multi-file interleaving, and cross-tool cases such as Bash versus file tools. I would prefer real end-to-end tests using builtin `Read`/`Edit`/`Grep`/`apply_patch`, not only the synthetic `ScheduledFile` integration test, so the user-visible failure and the new guarantee are falsifiable.
3. **Provider array order needs an explicit semantic status.** Calls in one assistant step are concurrent in the sense that the model cannot observe an earlier result before emitting the later call. Provider order can be a useful deterministic tie-breaker, but it should not silently become a data-dependency mechanism. If a later call must consume an earlier result, it belongs in the next model step.
4. **`all` should ideally name a real coarse authority.** “Conflicts with everything in the universe” creates unrelated head-of-line blocking. `workspace:{id}`, `session:{id}`, and `provider:{id}` make the ownership and blast radius explicit. Unknown provider-only work should not automatically conflict with local filesystem work unless a verified contract requires it.
### Suggested simplification
I would frame the first milestone as:
> Make builtin filesystem operations linearizable against overlapping reads and writes at the filesystem authority. Use provider order only as a deterministic tie-breaker for same-batch conflicts. Preserve fan-out for tools whose resource/capacity contract is independent.
Then introduce only three reusable coordination concepts when concrete users exist:
```text
ResourceLease keyed Mutex/RwLock semantics
ResourceActor single owner + bounded mailbox
CapacityPermit semaphore/backpressure
```
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(runtime): coordinate tool batches through resource authorities [maka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4542:
URL: https://github.com/apache/maka/pull/4542#issuecomment-6008973713
<!-- maka-lifecycle:stale-warning -->
This pull request has had no new commits for 30 days and has been marked stale. It will be closed in 7 days unless a new commit is pushed.
Comments do not reset this timer: only a new commit does. If the pull request is intentionally long-lived, a maintainer can apply the `pinned` label.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(runtime): coordinate tool batches through resource authorities [maka]
Posted by "likun666661 (via GitHub)" <gi...@apache.org>.
likun666661 commented on PR #4542:
URL: https://github.com/apache/maka/pull/4542#issuecomment-5539525468
重新 review 了当前 revision `67b0881999d802d78207e6ae8bd6568a9436d95d`。上轮提到的 canonical filesystem identity 和 turn-fatal rejection 后继续派发的问题已经得到实质修复，整体架构也已经收敛到 authority-prepared operation。不过当前仍有两个 correctness 问题需要处理：
## 1. `all()` 目前只在单个 batch 内互斥，并不是 process-wide global barrier
[`allResourceAuthority`](https://github.com/apache/maka/blob/67b0881999d802d78207e6ae8bd6568a9436d95d/packages/runtime/src/preparation/placeholder-authorities.ts#L44-L51) 只生成 `{ kind: 'all' }` claim；而 [`settleToolCallBatch`](https://github.com/apache/maka/blob/67b0881999d802d78207e6ae8bd6568a9436d95d/packages/runtime/src/tool-call-batch.ts#L46-L50) 每次调用都会创建一个新的、batch-local `ToolScheduler`。
这意味着不同 batch/session 中的 `all` 彼此不互斥，也不会与 process-wide `FilesystemLeaseCoordinator` 协调。当前显式映射到 `all` 的 `Bash`、`maka_computer`、`request_sandbox_boundary`，以及 registry miss 的未知/dynamic tool，都可能与另一个 batch 的真实 `Read`/`Write` 并发执行。
我用两个独立的 `settleToolCallBatch()` 做了复现：batch A 持有一个尚未释放的 `Bash/all` operation，batch B 执行真实 builtin `Write`。结果 `Write` 在 `all` 仍 active 时就完成了。
因此现在的 `all` 只能称为 batch-global，不能满足 PR 文案和代码注释中的 “global serialization / fail closed”，也没有覆盖 PR 强调的 cross-batch correctness。建议让 coarse `all` 和各 process-scoped authority 进入同一个 process-level admission plane，并补一个跨 batch/session 的 `all -> Read/Write` 阻塞测试。
## 2. eager identity capture 会把合法的前序 filesystem effect 识别成外部 target replacement
[`settleToolCallBatch`](https://github.com/apache/maka/blob/67b0881999d802d78207e6ae8bd6568a9436d95d/packages/runtime/src/tool-call-batch.ts#L51-L61) 会在任何 operation 开始执行前并行 prepare 整个 batch。filesystem authority 在 prepare 阶段捕获当前 target identity（[`filesystem-executor.ts#L515-L527`](https://github.com/apache/maka/blob/67b0881999d802d78207e6ae8bd6568a9436d95d/packages/runtime/src/filesystem-executor.ts#L515-L527)），取得 lease 后重新 resolve，并要求 identity 未发生变化（[`filesystem-executor.ts#L558-L567`](https://github.com/apache/maka/blob/67b0881999d802d78207e6ae8bd6568a9436d95d/packages/runtime/src/filesystem-executor.ts#L558-L567)）。
我使用真实 builtin filesystem owner 运行同一 batch：
```text
Write("created.txt", "created")
Read("created.txt")
```
Write 成功且文件已经创建，但后续 Read 因 prepare 时 identity 为 `missing`、执行时 identity 为 `file`，以 `FilesystemPreparedTargetChangedError` 失败。现有 `3 calls: queued writer prevents a later reader from bypassing it` 测试没有发现这一点，因为 `ControlledFilesystemWorker` 的 Write 只返回成功结果，并不实际修改磁盘。
PR 已明确不把 provider order 当作通用数据依赖，但对于已经因资源冲突被强制排序的 filesystem operations，目前的实现仍把前序 owner 自己造成的预期变化与 lease 外的篡改混为一谈。除了文案中记录的 `Bash -> Write`，`Write/create -> Read`、atomic replacement 后的 Read，以及类似 patch/create 链路也会遇到同样问题。
建议把稳定的 canonical claim/lease key preparation 与可变 identity snapshot 分开：先准备不会变化的 claims，在 operation 真正获得 authority admission、前序冲突 owner 已完成后捕获或刷新 identity，同时继续保证 admission 后到 effect 之间的 race closure。并建议用真实 filesystem backend 增加 create→read、replace→read 回归测试，而不是只依赖不修改磁盘的 controlled worker。
## Verification
本地验证通过：
- `@maka/core`、`@maka/storage`、`@maka/runtime`、`@maka/mcp`、`@maka/runtime-host` 相关构建
- PR 文档中的定向矩阵：62 tests passed, 0 failed
- `git diff --check`
- 与当前 `main` 的本地 merge-tree 无冲突
截至 2026-09-04，当前 head 的 CI、Dependency audit 和 Windows recovery 都仍为 `action_required`，尚未产生实际 CI 结果。
第一个问题直接破坏 cross-batch correctness 和 `all` 的 fail-closed 语义，因此我认为当前 revision 仍不宜合并。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(runtime): coordinate tool batches through resource authorities [maka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4542:
URL: https://github.com/apache/maka/pull/4542#issuecomment-6137965959
<!-- maka-lifecycle:stale-close -->
This pull request has been closed because it received no new commits during the 7-day grace period. This is not a judgement on the merit of the change; it only keeps the review queue aligned with work that is actively in flight.
The branch and review history are preserved. Push a new commit and reopen the pull request whenever the work is ready to continue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(runtime): coordinate tool batches through resource authorities [maka]
Posted by "Jarad-z (via GitHub)" <gi...@apache.org>.
Jarad-z commented on PR #4542:
URL: https://github.com/apache/maka/pull/4542#issuecomment-5538795865
@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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(runtime): coordinate tool batches through resource authorities [maka]
Posted by "hqhq1025 (via GitHub)" <gi...@apache.org>.
hqhq1025 commented on code in PR #4542:
URL: https://github.com/apache/maka/pull/4542#discussion_r3942897297
##########
packages/runtime/src/filesystem-executor.ts:
##########
@@ -485,7 +924,9 @@ function createWorkspaceFilesystemExecutor(
? {
...common,
action: 'delete' as const,
- ...(expectedIdentity ? { approvedIdentity: expectedIdentity } : {}),
+ ...(toExpectedIdentity(target.identity)
Review Comment:
[P1] Preserve the admitted `missing` identity for local deletes. `toExpectedIdentity({ kind: 'missing' })` becomes `undefined`, so this branch omits `approvedIdentity` and `compareAndDeleteEntry` performs an unconditional unlink. I reproduced the admission-to-effect race by preparing a delete for a missing file, creating the file in `beforeTargetEffectForTest`, and continuing: the operation returned `completed` and deleted the newly created file. The worker path correctly sends the literal `missing` sentinel and rejects an appeared target. Please carry the same sentinel through the local workspace path so a delete cannot remove a file created after admission.
##########
packages/runtime/src/preparation/default-tool-authorities.ts:
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { allResourceAuthority, noneResourceAuthority } from './placeholder-authorities.js';
+import {
+ processResourceAdmissions,
+ type ProcessResourceAdmissionCoordinator,
+} from '../process-resource-admission.js';
+import type { ToolAuthorityRegistration } from './tool-authority-registry.js';
+
+/** Stable Kimi-policy decisions for tools outside the filesystem domain. */
+export const EXPLICIT_NONE_TOOL_AUTHORITY_IDS = Object.freeze([
+ 'WebSearch',
+ 'WebFetch',
+ // Code Mode is an orchestration container. Its nested leaf calls prepare and
+ // acquire their own authorities; the container itself owns only cell capacity.
+ 'exec',
+ 'agent_spawn',
+ 'agent_list',
+ 'agent_output',
+ 'view_agent_graph',
+ 'agent_swarm_status',
+ // Turn-scoped immutable inventory + in-memory ranking: no resource lease.
+ 'SkillSearch',
+ // These implementations already own their cross-call correctness through
+ // domain admission lanes, transactions, leases, or state-machine gates.
+ // Keep them outside Scheduler ordering until their precise ResourceAuthority
+ // adapters are completed; execute still enters the real implementation.
+ 'StopBackgroundTask',
+ 'WriteStdin',
+ 'todo_read',
+ 'todo_write',
+ 'SearchHistory',
+ 'ReadHistory',
+ 'ScheduledTask',
+ 'GoalSet',
+ 'GoalClear',
+ 'GoalStatus',
+ 'GoalPause',
+ 'GoalResume',
+ 'SubmitPlan',
+ 'update_plan',
+ 'cancel_plan',
+] as const);
+
+export const EXPLICIT_ALL_TOOL_AUTHORITY_IDS = Object.freeze([
+ 'Bash',
+ 'update_agent_graph',
+ 'yield_agent_graph',
+ 'AskUserQuestion',
Review Comment:
[P1] Do not hold the process-wide exclusive admission while waiting for a human response. `AskUserQuestion` (and the adjacent sandbox-boundary request) can remain parked indefinitely, while `allResourceAuthority` wraps the entire effect in `withExclusive`. In a production-registry probe, a pending question left `activeExclusive=true` and a shared filesystem operation in another batch did not start until the answer resolved. One unattended question therefore freezes every participating filesystem operation across all Sessions. This needs an interaction/session-specific authority, or the process-exclusive admission must be released before parking for the user.
##########
packages/runtime/src/workspace-executor.ts:
##########
@@ -436,7 +454,7 @@ export class LocalWorkspaceExecutor implements WorkspaceExecutor {
input.semantics === 'entry'
? (await resolveCanonicalDirectoryEntryTarget(input.cwd, input.path)).path
: (await canonicalPathUnderCwd(input.cwd, input.path)).path;
- return { key: path };
+ return { key: path, canonicalPath: path };
Review Comment:
[P2] This path-derived key does not coordinate hard-link aliases. `realpath` collapses symlinks, but two different hard-link paths retain different canonical strings while addressing the same inode. I reproduced a held `Read(a.txt)` and concurrent `Write(b.txt)` where `b.txt` is a hard link to `a.txt`: the write completed before the read was released, and the read returned the new content. Include stable object identity in the conflict key (or conservatively serialize aliases) so the advertised read/write exclusion also covers hard links.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(runtime): coordinate tool batches through resource authorities [maka]
Posted by "hqhq1025 (via GitHub)" <gi...@apache.org>.
hqhq1025 commented on code in PR #4542:
URL: https://github.com/apache/maka/pull/4542#discussion_r3942897783
##########
packages/runtime/src/ai-sdk-turn.ts:
##########
@@ -2235,59 +2239,129 @@ export class AiSdkTurn {
await loadDurableTurnEvents();
}
const toolsByName = new Map(providerTools.map((tool) => [tool.name, tool]));
- const settlementOutcomes = await Promise.allSettled(
- returnedToolCalls.map(async (toolCall) => {
- if (toolCall.providerExecuted) {
- throw new Error(
- `Provider-executed tool call "${toolCall.toolName}" is outside the main-agent tool loop`,
- );
- }
- const sandboxBoundaryAttempt = isProviderSandboxBoundaryAttempt(toolCall);
- const deniedBoundaryRequest =
- toolRuntime.hasSandboxBoundaryDenial() &&
- toolCall.toolName.toLowerCase() === REQUEST_SANDBOX_BOUNDARY_TOOL_NAME;
- if (deniedBoundaryRequest) {
- toolRuntime.forceSandboxBoundaryFinalization();
- }
- const blockedToolCall = sandboxBoundaryFinalizationStep || deniedBoundaryRequest;
- const requestedTool = blockedToolCall
- ? undefined
- : toolsByName.get(toolCall.toolName);
- const tool = requestedTool ?? toolsByName.get(INVALID_TOOL_NAME);
- if (!tool) throw new Error('Runtime invalid-tool fallback is unavailable');
- const unavailableError = sandboxBoundaryFinalizationStep
- ? 'Sandbox boundary finalization does not permit tool execution.'
- : deniedBoundaryRequest
- ? SANDBOX_BOUNDARY_DENIED_FOR_TURN
- : 'returned tool is unavailable';
- return await toolRuntime.settleToolCall({
- tool,
- turnId,
- stepId: providerStepId,
- toolCallId: toolCall.toolCallId,
- // Provider metadata is persisted verbatim into an immutable
- // RuntimeEvent, and a field the response did not carry
- // arrives as an explicit `undefined` — which JSON drops, so
- // the event no longer reads back as it was written and the
- // store refuses it. One refusal took every tool-calling turn
- // with it.
- ...(toolCall.providerOptions !== undefined
- ? {
- providerOptions: stripUndefinedDeep(toolCall.providerOptions),
- }
- : {}),
- input:
- requestedTool !== undefined
- ? toolCall.input
- : {
- tool: toolCall.toolName,
- error: unavailableError,
- ...(sandboxBoundaryAttempt ? { sandboxBoundaryAttempt: true } : {}),
- },
- abortSignal: turnAbortController.signal,
- eventSink: queue,
- });
- }),
+ const preparedToolCalls = returnedToolCalls.map((toolCall) => {
+ const sandboxBoundaryAttempt = isProviderSandboxBoundaryAttempt(toolCall);
+ const deniedBoundaryRequest =
+ toolRuntime.hasSandboxBoundaryDenial() &&
+ toolCall.toolName.toLowerCase() === REQUEST_SANDBOX_BOUNDARY_TOOL_NAME;
+ if (deniedBoundaryRequest) {
+ toolRuntime.forceSandboxBoundaryFinalization();
+ }
+ const blockedToolCall = sandboxBoundaryFinalizationStep || deniedBoundaryRequest;
+ const requestedTool = blockedToolCall
+ ? undefined
+ : toolsByName.get(toolCall.toolName);
+ const tool = requestedTool ?? toolsByName.get(INVALID_TOOL_NAME);
+ if (!tool) throw new Error('Runtime invalid-tool fallback is unavailable');
+ const unavailableError = sandboxBoundaryFinalizationStep
+ ? 'Sandbox boundary finalization does not permit tool execution.'
+ : deniedBoundaryRequest
+ ? SANDBOX_BOUNDARY_DENIED_FOR_TURN
+ : 'returned tool is unavailable';
+ return {
+ toolCall,
+ tool,
+ input:
+ requestedTool !== undefined
+ ? toolCall.input
+ : {
+ tool: toolCall.toolName,
+ error: unavailableError,
+ ...(sandboxBoundaryAttempt ? { sandboxBoundaryAttempt: true } : {}),
+ },
+ syntheticWithoutEffect:
+ blockedToolCall || requestedTool === undefined || toolCall.providerExecuted,
+ };
+ });
+ const admissionCandidates = preparedToolCalls.filter(
+ ({ toolCall }) => !toolCall.providerExecuted,
+ );
+ const candidateAdmissions = toolRuntime.admitToolCallBatch(
+ admissionCandidates.map(({ tool }) => tool),
+ providerStepId,
+ );
+ let admissionIndex = 0;
+ const admittedToolCalls = preparedToolCalls.map((prepared) => ({
+ ...prepared,
+ admission: prepared.toolCall.providerExecuted
+ ? ({ kind: 'admitted' } as const)
+ : candidateAdmissions[admissionIndex++]!,
+ }));
+ const settlementOutcomes = await settleToolCallBatch(
Review Comment:
[P2] Preserve the causal fatal rejection when consuming this batch. When a later active task throws the turn-fatal error, `ToolScheduler` rejects an earlier queued conflicting task with the generic frozen/cancelled error. The existing provider-order `find` below then selects that earlier cancellation and discards the real T1/commit-boundary exception. A three-slot reproduction selected `Tool Scheduler is frozen; queued task ... was cancelled` while the later slot contained `T1 ROOT CAUSE`. Track the scheduler's fatal cause or skip derivative cancellation rejections when selecting the error to surface.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org