Skip to content

feat(worker): mark_processed accepts a batch of event_ids (worker DO + agent tool + proxy schemas + docs) - #246

Merged
liplus-lin-lay merged 2 commits into
mainfrom
245-enhancement-mark_processed-accepts-one-event-per-call-so-consuming-a-prs-own-events-costs-n-round-trips
Aug 2, 2026
Merged

feat(worker): mark_processed accepts a batch of event_ids (worker DO + agent tool + proxy schemas + docs)#246
liplus-lin-lay merged 2 commits into
mainfrom
245-enhancement-mark_processed-accepts-one-event-per-call-so-consuming-a-prs-own-events-costs-n-round-trips

Conversation

@liplus-lin-lay

Copy link
Copy Markdown
Member

Closes #245

mark_processed が 1 呼び出し 1 イベントしか受けなかったため、自己操作が生む 6〜10 件の到達確認イベントの消費が呼び出し回数に比例していた。event_ids: string[] を追加して往復を畳む。id 列挙型を採り、消す対象を AI が明示的に選ぶ現行の安全性は保つ(filter 一括消費は外部イベント誤消費の事故が起こりうるため採らない)。

受け入れ条件との対応

受け入れ条件 対応
複数 id を 1 呼び出しで処理でき、成功/失敗が id 単位で返る mark_processed({event_ids}){success, marked, failed, results[], purged} を返す。results は id ごとに {event_id, success, error?}
単数呼び出しの既存挙動が変わらないことがテストで固定 store.test.ts の既存 mark-processed テストに加え、未登録 id でも success: true を返す旧挙動を新規テストで固定
部分失敗時に成功分が確定していることがテストで固定 commits the successful ids when one id in the batch failsnot foundinvalid event_id を混ぜたバッチで、成功 id が processed のまま確定することを検証

設計判断

  • 部分失敗は tool error にしない。成功分のマークは既に確定しており、呼び出し側は失敗した id だけ再送すればよい。isError を立てると確定済みの成功が見えなくなる。
  • マルチアカウント時の id 単位判定。イベントは 1 つの store にのみ存在するので、アクセス可能な全 store に同じバッチを投げ、いずれかの store が一致した id を成功とマージする。全 store が取り逃した id のみ失敗。ここを取り違えると multi-account session で全 id が失敗扱いになるため、worker/src/mark-results.ts に切り出して DO ランタイム抜きで unit test を当てた。
  • purge はバッチ 1 回につき 1 回。id ごとに走らせない(往復を畳む目的そのもの)。
  • 単数形の応答 shape は凍結。バッチ形で導入した not found の厳格さを単数形に漏らさないことをテストで押さえた。

影響範囲

worker DO (store.ts) / agent tool (agent.ts) / 新規 mark-results.ts / MCP proxy 静的スキーマ 2 本 + manifest / docs 5 本。

リリース時の注意: proxy の静的スキーマ更新は再接続では反映されない。新 param を利用側に届けるには npm 再公開が要る。

テスト

  • worker/test/mark-results.test.ts(新規, 8 件)— cross-store マージの id 単位判定
  • worker/test/workers/store.test.ts(+7 件)— バッチ marking / 部分失敗の確定 / purge 1 回 / 空配列 / 冪等 / form 選択 / 単数形の非退行
  • ローカルで CI 相当(node --check / mcp-server tests / typecheck / wrangler dry-run / worker test)全通過

mark_processed accepted exactly one event per call, so consuming the 6-10
self-operation acknowledgement events a single PR generates cost one round
trip each. Add an `event_ids: string[]` form that collapses those round trips
while the caller still enumerates ids explicitly; filter-based bulk
consumption is deliberately not offered because it can silently consume
external events.

- store.ts: /mark-processed takes { event_ids } alongside { event_id }. Ids
  are marked one by one (each isolated by try/catch) before a single
  retention purge, so a failing id never undoes the ids that succeeded. The
  singular request keeps its exact prior response shape.
- agent.ts: the tool takes event_id or event_ids (1-100). Under multi-account
  fan-out the same batch goes to every accessible store, so the cross-store
  merge lives in the new mark-results.ts: an id marked by ANY store counts as
  marked, and only an id missed by every store failed.
- Partial failure is reported in the response body rather than as a tool
  error, because the successful marks are already committed and the caller
  needs to see which ids to retry.
- Proxy static schemas (mcp-server, local-mcp) and manifest advertise the new
  param. Reconnecting does not pick it up; the npm package must be republished.
- docs: F3.5 row + a batch-form note in both requirements specs, tool tables
  and polling flows in README / Home / mcp-server README.

単数呼び出しの既存挙動、部分失敗時に成功分が確定していること、バッチ 1 回
につき purge が 1 回だけ走ることをテストで固定した。マルチアカウント時の
id 単位判定は mark-results.ts の unit test で直接押さえている。

Closes #245
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 2, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
github-webhook-mcp 55164ec Aug 02 2026, 06:51 PM

…ndary

Self-review found the spec claiming a per-id error value the tool cannot
emit. mergeMarkResults seeds every id with "not found" and only ever promotes
successes -- it never reads a store's error string -- so the DO's
"invalid event_id" verdict never reaches the caller through mark_processed.
The schema still admitted the input that produces it: z.array(z.string())
accepts "", which then misses in every store and is reported as "not found",
a misleading verdict for a malformed request.

Closed by removing the degree of freedom, not by adding error-propagation
plumbing:

- agent.ts: z.string().min(1) on the array ITEM, so an empty id is rejected
  by schema instead of becoming a silent "not found".
- Proxy static schemas (mcp-server, local-mcp): minLength 1 on items, keeping
  the advertised contract aligned with the worker.
- docs: the F3.5 per-id-verdict bullet in both requirements specs now states
  "not found" as the only error value at the tool surface.
- mark-results.ts: comment naming why store-side error strings are not
  propagated, so the merge contract is not re-broken later.

DO-level markOne keeps its guard: it is an internal route with its own test
and should stay defensive independently of what the tool boundary admits.

tool 表層の contract と spec の記述が食い違っていた箇所を、記述側ではなく
入力側を締めることで揃えた。zod の挙動(空文字/空配列/上限超過の reject)は
実行して確認済み。

Refs #245

@liplus-lin-lay liplus-lin-lay left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

AI self-review (auto mode)

判定: pass。merge する。

検証は報告の受け取りではなく差分の実読で行った。CI は head SHA 55164ec に対して 3 本すべて green、mergeable state は CLEAN。

受け入れ条件

3 件すべてテストで固定されていることを確認した。

  • id 単位の成功/失敗mark_processed({event_ids})results[]{event_id, success, error?} を返す。
  • 単数形の非退行 — 応答 shape が凍結されている。存在しない id でも success: true を返す旧挙動が新規テストで固定され、バッチ形で導入した not found の厳格さが単数形に漏れないことが押さえられている。
  • 部分失敗時の成功分確定 — 失敗を混ぜたバッチで、成功 id が processed のまま確定することを検証。

設計判断の確認

マルチアカウント時の id 単位判定が本件の勘所。 イベントは 1 つの store にのみ存在するため、全 store への fan-out では「見つからない」が正常応答として返る。これを失敗と読むと multi-account セッションで全 id が失敗扱いになる。実装は「いずれかの store が一致した id を成功、全 store が取り逃した id のみ失敗」を採り、この判定を worker/src/mark-results.ts に切り出して DO ランタイム抜きの unit test 8 件を当てている。落とし穴を落とし穴として分離した形で、妥当。

部分失敗を tool error にしない判断に同意する。 isError を立てると確定済みの成功が呼び出し側から見えなくなり、バッチ全体の再送を誘発する。失敗した id だけを再送できる現行の形が正しい。

purge がバッチ 1 回につき 1 回。 往復を畳む目的と整合。

self-review で差し戻した点(修正済み)

初版は仕様書 F3.5 に「errornot found または invalid event_id」と記していたが、invalid event_id は tool 表層に到達しない。mergeMarkResults は store 側の error 文字列を読まず成功のみを昇格させるため、失敗理由は常に not found に潰れる。一方 z.array(z.string()) は空文字を通すので、空 id は実際には malformed request でありながら not found として報告される経路が開いていた。

docs が source of truth である以上、実装が出せない挙動を仕様が謳っている状態は欠陥として扱う。修正はエラー伝播の配管追加ではなく境界での自由度削除を指示した — z.array(z.string().min(1))(item 側の min)と両 proxy schema の items.minLength: 1、および仕様記述の訂正。DO 側 markOne のガードは内部経路の防御として非対象のまま残している。mark-results.ts に「store 側 error を意図的に伝播しない」旨のコメントが入り、再発時の再導入インセンティブも潰されている。

リリース時の申し送り

proxy の静的スキーマ更新は再接続では反映されない。新 param を利用側に届けるには npm 再公開が必要。merge とリリースの間で失われやすい種類の制約であり、リリース時に確認する。

@liplus-lin-lay
liplus-lin-lay merged commit 2683619 into main Aug 2, 2026
3 checks passed
@liplus-lin-lay
liplus-lin-lay deleted the 245-enhancement-mark_processed-accepts-one-event-per-call-so-consuming-a-prs-own-events-costs-n-round-trips branch August 2, 2026 18:52
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.

enhancement: mark_processed accepts one event per call, so consuming a PR's own events costs N round trips

1 participant