Skip to content

fix(workers): admit same-tool jobs against a shared running limit - #183

Merged
LIghtJUNction merged 1 commit into
integrate/all-open-20260929from
fix/tool-concurrency-20260929
Sep 29, 2026
Merged

LIghtJUNction merged 1 commit into
integrate/all-open-20260929from
fix/tool-concurrency-20260929

Conversation

@LIghtJUNction

Copy link
Copy Markdown
Member

摘要

补齐 docs/DESIGN_CONTRACT_INVENTORY.md 中确认的缺口 F162.ToolSpec.concurrency:每个 EffectWorker 进程内是串行的,但多个 worker 可以同时领取同一工具的不同 pending job,原先没有跨 worker 的上限。

  • 原子准入,在同一个写事务里完成:EffectWorker._claim 在处理过期 lease 的那个写事务里(SQLite 用 BEGIN IMMEDIATE,PostgreSQL 用全局 advisory lock),按工具 id 统计仍为 running 的 job。不用进程内锁,因为进程内锁不是所有 worker 共享的事实。
  • 跳过已满的工具,继续往后找:工具已达上限时跳过该 pending job,继续按 next_at,id 找下一个可领取的,所以其他工具和非 tool job 不会被堵住。
  • lease 语义不变:running 且 lease 过期的 job 仍然先被标为 uncertain,不计入 running。
  • 历史工具字节不变,默认上限 1:上限读自 job 所记录的工具修订 JSON。已发布修订、descriptor()、ToolSpec、签名、request_id、账本和 receipt 字节都不改;修订里没有 concurrency 字段就按 1 处理,比之前更严格。
  • 非法值直接拒绝:显式 concurrency 必须是 1..32 的 int(bool 不算)。其他值会被 read_tool 拒绝(invalid_tool_concurrency),领取时该 job 被标为 failed,不会执行,也不会变成无上限。
  • 不改 SQL schema,不新增 Manager/Repository,只用现有 jobs.body 里的 JSON。

不包括:docs/DESIGN_CONTRACT_INVENTORY.{md,json} 带有 review-commit 标记的审计条目这次没有更新。

测试

在 tests/test_tools.py 里扩展了测试,走真实的 PostgreSQL installed 夹具;第二个 worker 使用独立的 Application 实例:

  • 同一工具已有 1 个 running 时,第二个 worker 领不到同一工具的 job,但仍能领取另一个工具(tool_curl)的 job 和一个非 tool job;第一个 job 结束后,被跳过的 job 才能被领取。
  • 修订显式声明 concurrency=2 时,允许第二个,拒绝第三个。
  • 缺字段的历史描述按 1 处理;running 计数按工具 id 在各修订之间共享。
  • 已存历史修订的字节等于 canonical(descriptor('dns')),且不含 concurrency。
  • 非法值 0, 33, -1, True, '2', 1.5, None 都会被拒绝,job 标为 failed,runner 没有被调用。

新测试在实现前已确认失败。本地(PostgreSQL 由 conftest 通过 initdb 启动)所有涉及 EffectWorker / _claim / save_job / read_tool 的测试文件:315 passed。

Made with Cursor

Each EffectWorker serializes its own loop, but several workers could claim
different pending jobs of the same tool at once, so a tool had no
cross-worker concurrency bound (F162.ToolSpec.concurrency). A process-local
lock cannot express that fact; only the jobs table seen under the claim's
write transaction (BEGIN IMMEDIATE on SQLite, the global advisory lock on
PostgreSQL) is shared by every worker.

The claim now counts live running jobs per tool id in the same write
transaction that expires stale leases, and skips any pending tool job whose
tool is already at its limit, continuing in next_at,id order so other tools
and non-tool jobs are not blocked. Expired leases still become uncertain
first and are not counted.

The limit comes from the job's captured tool revision. Published revisions
have no concurrency field and keep their bytes, signatures and receipts;
absence means 1 (tighter than before). An explicit value must be an int in
1..32; anything else is rejected (invalid_tool_concurrency) by read_tool and
fails the job at claim without executing it, never becoming unbounded.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copilot AI balanced review requested due to automatic review settings September 29, 2026 14:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
🔒 Security Review ✅ Completed 2026-09-29T14:20:25.491785Z bf07d48 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@LIghtJUNction
LIghtJUNction merged commit 84071b8 into integrate/all-open-20260929 Sep 29, 2026
10 of 14 checks passed
LIghtJUNction added a commit that referenced this pull request Sep 29, 2026
Preserves the current integration branch plus main 22b5e86 and #182 bf1a166. The tree is the previously reviewed pre-quality integration tree; only pyproject.toml required explicit conflict resolution. #183 and #184 are already present through the first parent.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants