Hacktoberfest 2026:维护者为十月标记出来的 issue,仍然开放、适合新手。 浏览 Hacktoberfest issue

Design rationale for the WO pending-approval gate (PR #642)

未关闭
#657 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

维护者通常 1 天内回复

还没有人认领这个 Issue。

评估

难度
4/5
预计耗时
3-5 天
新手友好度
25/100
Issue 类型
功能
描述清晰度
基本清楚
活跃度
活跃
技术栈
python
领域
api, backend

调研方向

The issue describes the pending approval gate, operator CLI, and tests, and names governance.py, main.py, couch.py, scripts/wo_pending.py, and docs/mcp-servers.md. Read those entry points and the linked PR #642 first; the issue says the gate should cover the three work order actions, fail closed, and require an identical retry after operator approval. The tests described should confirm a gated work order stays in WAPPR until approval.

由索引模型根据 Issue 内容生成。

描述

This lays out the reasoning behind each decision in #642, so it's discussable separately from the diff itself.

The gap this addresses

approve_workorder, close_workorder, and cancel_workorder apply their target status unconditionally today. docs/mcp-servers.md already documents this: "There is no status-transition guard." The only related control in the codebase is AOB_READONLY, which is a single on/off switch for the entire server, not specific to these three higher-consequence transitions. So right now, an agent with write access can approve, close, or cancel any work order with no human checkpoint at all, short of disabling writes entirely.

What this does and does not do

This PR adds a human approval checkpoint in front of those three calls. It does not add the status-transition check itself (for example, validating that WAPPR -> APPR is a legal transition). Those are two independent problems, and this issue is scoped to the first one. Happy to open a separate issue for the transition-guard gap if that would be useful.

Decisions and reasoning

Opt-in via env var (AOB_REQUIRE_APPROVAL), off by default. This mirrors the existing AOB_READONLY convention already in this server, so deployers who don't want this behavior see zero change, and deployers who do want it get a one-line config addition they're already used to from AOB_READONLY.

Scoped to exactly these three tools. These are the only WO tools flagged in the docs as having no transition guard, and the only ones that make an irreversible lifecycle change. Read tools and create_workorder aren't touched.

Argument digest plus one-shot pending -> approved/denied -> consumed state, instead of a simple per-tool allow/deny flag. A flag keyed only on tool name would mean approving one cancel_workorder call also silently approves a different cancel_workorder call with different arguments, a different wonum, a different reason, and so on. Hashing the actual call arguments (SHA-256, sorted-key JSON) and consuming the approval exactly once closes that gap: approving a $500 case never covers a $5,000,000 one just because it's the same tool. This part is conceptually based on a pattern I built for my own project, Runkite (github.com/getrunkite/runkite), reimplemented fresh in Python here rather than ported, since Runkite is BSL-1.1 and this is Apache-2.0, and it runs on Go over a different transport.

Fails closed if the pending store is unreachable. If the approval store can't be reached, the gate refuses the call rather than letting it through. An availability problem in the governance layer should not silently turn into a bypass of the governance layer.

Reuses the existing CouchDB client (couch.py) for the pending store. No new infrastructure or dependency, consistent with how the rest of the WO server already persists state.

A standalone operator CLI (scripts/wo_pending.py: list / approve / deny), rather than exposing approval through the MCP tool surface itself. The agent or client should never be able to approve its own pending action, only an operator with direct access to the pending store can. A separate CLI keeps that boundary explicit.

destructiveHint=True / idempotentHint=False added to the three gated tools' ToolAnnotations. These were previously unset on every write tool in this server, the one existing annotation usage in the file is readOnlyHint on a read tool. This simply extends the same mechanism to flag risk on the write side too, for any MCP client that respects these hints.

Shipped as one PR rather than split to fit the 300-line guideline. The gate, its tests, and the operator CLI aren't meaningfully separable: reviewing the gate without its tests weakens confidence in it, and shipping the CLI separately from the feature it operates on doesn't reduce real review surface, it just moves it to a second PR. Open to reconsidering if maintainers feel otherwise.

Testing approach

30 tests pass in total: 17 existing WO server tests plus 13 new ones covering the gate. Beyond isolated unit tests of governance.py, two of those new tests call main.approve_workorder() directly end to end and confirm the work order stays in WAPPR while gated, and only transitions after a real operator approval plus an identical retry, to make sure the wiring into main.py actually works, not just the gate logic in isolation.

Open for discussion

  • Is the AOB_REQUIRE_APPROVAL naming consistent with where this project wants to go, or is there a preferred naming or configuration convention for this kind of control?
  • Any preference on where operator approval should live long-term, CLI versus a future admin UI or API? This CLI is meant as a minimal starting point, not the final shape.
  • Anything in the fail-closed behavior that conflicts with how this server is expected to degrade under CouchDB outages elsewhere?
主要语言
Python
星标
2.4k
派生
360
平均合并
5 小时 52 分钟
30 天内合并 PR
34

环境准备

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

IBM/AssetOpsBench 的其他 Issue

查看 IBM/AssetOpsBench 的全部 Issue

相似的 Issue

更多 Python Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。