Skip to content

[P2] Execute ADR-0033's deletion of the unenforced tool.requiresConfirmation (safety-shaped, gated by nothing) #3715

Description

@os-zhuang

Surfaced by the #3686 preview-claim sweep; marked honestly in #3711 (dead + authorWarn). This issue tracks the prune, which ADR-0033 already resolved but never executed.

The state

tool.requiresConfirmation (packages/spec/src/ai/tool.zod.ts:100) is read by no execution path:

Path Behaviour
LLM tool set cloud service-ai/src/adapters/vercel-adapter.ts:263-272 — tools reach the model as name/description/parameters only
ToolRegistry.execute cloud tool-registry.ts:183-210 — no gate
POST /api/v1/ai/tools/:name/execute cloud tool-routes.ts:91,105 — has() then execute()
MCP bridge packages/mcp/src/mcp-server-runtime.ts:176 — sets destructiveHint from a hardcoded name list (:229-231), ignoring the flag

Every genuine requiresConfirmation read in all three repos is action.ai.requiresConfirmation — a different property, genuinely live (liveness/action.json, packages/runtime/src/action-execution.ts:264).

Why it is worse than inert

The authoring surface actively teaches reliance on it:

  • packages/spec/src/ai/tool.form.ts:36-42 — section titled "Access & safety", description "Permissions and confirmation requirements.", helpText "Ask user to approve before executing (for destructive actions)"
  • skills/objectstack-ai/SKILL.md:547 — "for destructive operations put a human in the loop — requiresConfirmation: true on the tool"
  • packages/spec/docs/MCP_GUIDE.md:358, packages/spec/README.md:136

An author (or an AI) marking a destructive tool requiresConfirmation: true gets no pause, no prompt, no gate. Same class as skill.permissions (pruned in #3704) and agent visibility (#1901) — a security/safety-shaped field that lies is dropped, not carried marked.

ADR-0033 already decided this

docs/adr/0033-…:

  • :31 — "The requiresConfirmation flag exists but is not enforced (the tool-call loop ignores it)"
  • :60 — "We delete the unenforced requiresConfirmation placeholder on metadata tools"

The rationale stands: the draft/publish workspace is the real approval gate — AI never publishes, every metadata mutation lands as a draft a human must publish. That is the enforced HITL boundary; the flag was a placeholder for a gate that got built somewhere better.

The decision was recorded and never executed.

Scope if approved (mirrors #3704 exactly)

  • packages/spec/src/ai/tool.zod.ts — the property
  • packages/spec/src/ai/tool.form.ts — the field; note the "Access & safety" section's only two fields are this and the already-dead permissions, so decide whether the section goes too
  • packages/spec/liveness/tool.json + the README count table
  • i18n: packages/platform-objects/src/apps/translations/{en,es-ES,ja-JP,zh-CN}.metadata-forms.generated.ts — regenerate (pnpm i18n:extract), and verify the diff touches only these keys
  • skills/objectstack-ai/SKILL.md:547 + MCP_GUIDE.md + README.md — replace the guidance with the real HITL path
  • objectui ToolPreview.tsx — the pill
  • 24 cloud *.tool.ts declarations that set it
  • changeset with FROM→TO: use an action with ai.requiresConfirmation + the approval queue; for metadata mutations the draft/publish gate already applies

Open question for the owner

Prune (execute ADR-0033), or wire it — i.e. make ToolRegistry.execute and the REST route honour the flag by routing through the same HITL queue actions use? ADR-0033's reasoning argues for prune, but that ADR predates the current tool surface; if first-class metadata tools are meant to be a first-class extension point, a per-tool confirmation gate may be worth building rather than deleting.

Refs #3711, #3704, ADR-0033.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions