Skip to content

feat(wo): add optional pending-approval gate for work-order writes - #642

Open
sharanharsoor wants to merge 1 commit into
IBM:mainfrom
sharanharsoor:feature/wo-pending-approval-gate
Open

sharanharsoor wants to merge 1 commit into
IBM:mainfrom
sharanharsoor:feature/wo-pending-approval-gate

Conversation

@sharanharsoor

Copy link
Copy Markdown

What

approve_workorder, close_workorder, and cancel_workorder currently apply their
target status unconditionally. docs/mcp-servers.md already documents this: "There
is no status-transition guard." This PR adds a separate, opt-in human checkpoint in
front of those three calls, it does not add the transition check itself. The two are
independent and composable.

How it works

  • Off by default. Set AOB_REQUIRE_APPROVAL=1 to enable, same convention as the
    existing AOB_READONLY.
  • When enabled, the first call to a gated tool returns a pending result with an
    action_id instead of running.
  • An operator reviews and approves or denies it with the new scripts/wo_pending.py
    CLI (list / approve / deny), which talks to the same CouchDB-backed pending
    store the server uses.
  • The agent must then repeat the identical call. The approval is bound to a SHA-256
    digest of the call's arguments and consumed exactly once, so approving one call
    never covers a different one (different wonum, different reason, etc.).
  • If the pending store is unreachable, the gate fails closed (refuses) rather than
    letting the call through.
  • Added destructiveHint=True / idempotentHint=False to the three gated tools'
    ToolAnnotations (previously unset on any write tool in this server).

Why

Looked into whether this server has any human-in-the-loop mechanism for its
destructive write tools before building this. It doesn't, AOB_READONLY is the only
related control, and it's a binary on/off that isn't specific to these three
higher-risk transitions. This adds a lightweight, opt-in checkpoint without touching
default behavior for anyone not using the env var.

Testing

  • 30/30 WO server tests pass (19 existing + 11 new), including two end-to-end tests
    that call main.approve_workorder() directly and confirm the work order stays
    WAPPR while gated and only transitions after operator approval + an identical
    retry.
  • Verified real MCP tool registration (mcp.list_tools()) picks up the new
    annotations correctly, and that AOB_READONLY=1 still behaves as before alongside
    this change.
  • ruff format --check clean; no new ruff check findings beyond pre-existing
    typing-style debt already present throughout this file.

Size

This is ~570 lines against the project's stated 300-line preference. Splitting it
would mean reviewing the gate without its tests, or shipping the operator CLI
separately from the feature it operates on, so it's kept as one PR.

approve_workorder, close_workorder, and cancel_workorder currently apply
their target status unconditionally (documented in docs/mcp-servers.md:
'There is no status-transition guard'). This adds a separate, opt-in
human checkpoint in front of those three calls.

Off by default (AOB_REQUIRE_APPROVAL=1 to enable), same convention as
the existing AOB_READONLY. When enabled, the first call to a gated tool
returns an action_id instead of running. An operator approves or denies
it with scripts/wo_pending.py; the agent must then repeat the identical
call. The approval is bound to a SHA-256 digest of the call's arguments
and consumed exactly once, so approving one call never covers a
different one (a different wonum, different reason, etc.), and the
pending store being unreachable fails closed instead of letting the
call through.

Also adds destructiveHint=True to the three gated tools' ToolAnnotations
(previously unset on any write tool in this server; the one existing
usage is on a read-only tool).

This does not add the status-transition check itself. It adds a human
checkpoint in front of whatever transition the agent requests. The two
are independent and composable.

30/30 existing + new WO server tests pass with the gate both on and off.

Signed-off-by: Sharan Harsoor <sharanharsoor@gmail.com>
@DhavalRepo18

Copy link
Copy Markdown
Collaborator

@sharanharsoor Maybe it's better to create a git issue for any code changes, and we can discuss the reasoning behind certain decisions.

This branch has not been deployed

No deployments
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