Skip to content

Commit 4d00b13

Browse files
authored
feat(spec)!: remove tool.requiresConfirmation — a safety flag nothing enforced (#3715, ADR-0033 §2) (#3876)
ToolSchema.requiresConfirmation accepted `true` and no execution path ever read it: not the LLM tool set, not ToolRegistry.execute, not POST /ai/tools/:name/execute, and not the MCP bridge (hardcoded destructive-name list). Setting it on a destructive tool produced NO pause. For an ordinary dead property that is untidy. For a SAFETY property it is false compliance — the case ADR-0049 exists for. Made worse by the near-miss: action.ai.requiresConfirmation has the same name and DOES work, so the mistake reads as correct in review. ADR-0033 §2 already resolved to delete it. Migration — FROM `requiresConfirmation: true` on a tool, TO an action carrying `ai.requiresConfirmation: true`, the flag the HITL approval queue actually reads. For AI metadata mutations nothing changes: the ADR-0033 draft/publish workspace is the gate. ToolSchema is now .strict(), which is load-bearing rather than tidying. Removing a key from a non-strict schema swaps one silent no-op for another — zod strips it wordlessly and the safety flag goes on meaning nothing. The retired key now REJECTS with the FROM -> TO prescription attached, via a TOOL_RETIRED_KEY_GUIDANCE map wired through a $ZodErrorMap (house pattern: ObjectCapabilities, tenancy). Typos on a tool definition are now located parse errors too. Also removed: the Studio form row; its four generated locale bundles, which still promised "Ask user to approve before executing (for destructive actions)" — a translated false promise nothing would have caught; the liveness-ledger entry (tool dead 5 -> 4); and the generated reference-doc row. A follow-up commit fixed the skills/objectstack-ai defineTool example that check:skill-examples caught, and disambiguated the MCP capability-descriptor field of the same name.
1 parent 33f5e23 commit 4d00b13

14 files changed

Lines changed: 152 additions & 59 deletions

File tree

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/platform-objects": patch
4+
---
5+
6+
feat(spec)!: remove `tool.requiresConfirmation` — a safety flag nothing enforced (#3715, ADR-0033 §2)
7+
8+
`ToolSchema.requiresConfirmation` accepted `true` and no execution path ever read
9+
it. Not the LLM tool set (a tool reaches the model as name/description/parameters
10+
only), not `ToolRegistry.execute`, not `POST /ai/tools/:name/execute`, and not the
11+
MCP bridge — which derives `destructiveHint` from a hardcoded name list. Setting
12+
it on a destructive tool produced **no pause**.
13+
14+
For an ordinary dead property that is untidy. For a **safety** property it is
15+
false compliance, which is the case ADR-0049 exists for: an author gates a
16+
destructive tool, sees the flag accepted, and ships believing a human is in the
17+
loop. It is made worse by the near-miss — `action.ai.requiresConfirmation` has
18+
the same name and **does** work, so the mistake reads as correct in review.
19+
ADR-0033 §2 already resolved to delete this one.
20+
21+
## Migration
22+
23+
- **FROM:** `requiresConfirmation: true` on a tool definition
24+
- **TO:** put the operation behind an action and set `ai.requiresConfirmation:
25+
true` there — that is the flag the HITL approval queue reads
26+
(`packages/runtime/src/action-execution.ts`) and the only path that actually
27+
stops execution.
28+
- For AI *metadata* mutations there is nothing to migrate: the ADR-0033
29+
draft/publish workspace is the gate — nothing is live until a human publishes.
30+
31+
**`ToolSchema` is now `.strict()`.** This is load-bearing, not tidying. Removing a
32+
key from a non-strict schema swaps one silent no-op for another: zod strips the
33+
key wordlessly, the author keeps writing it, and the safety flag goes on meaning
34+
nothing — the "silent strip" ADR-0032 / #1535 closed for objects. The retired key
35+
now **rejects**, and the error carries the FROM → TO above, because a parse error
36+
is the one channel every consumer bumping `@objectstack/spec` is guaranteed to
37+
hit.
38+
39+
Strictness applies to *all* unknown keys on a tool definition, so a typo
40+
(`buildIn`, `catagory`) is now a located parse error instead of a silently
41+
dropped field.
42+
43+
Also removed: the Studio form row, its four generated locale bundles (the
44+
`en`/`zh-CN`/`ja-JP`/`es-ES` strings still promised *"Ask user to approve before
45+
executing (for destructive actions)"* — a translated false promise), the
46+
liveness-ledger entry, and the generated reference-doc row.
47+
48+
objectui's `ToolPreview.tsx` reads the field via `!!d.requiresConfirmation`, so it
49+
degrades to "not shown" with no error; removing that badge is a follow-up in that
50+
repo.

content/docs/references/ai/tool.mdx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ AI tool definition. [READ-ONLY PROJECTION — not an execution entry point] Auth
4040
| **parameters** | `Record<string, any>` || JSON Schema for tool parameters |
4141
| **outputSchema** | `Record<string, any>` | optional | [EXPERIMENTAL — not enforced] JSON Schema for tool output. Keys are folded into the tool description only; outputs are not validated (liveness #1878/#1893). |
4242
| **objectName** | `string` | optional | Target object name (snake_case) |
43-
| **requiresConfirmation** | `boolean` || [EXPERIMENTAL — not enforced] Require user confirmation before execution. NOTHING pauses on this flag (#3715) — use the action-level `ai.requiresConfirmation` + approval queue for a real gate. |
4443
| **permissions** | `string[]` | optional | Required permission-set capabilities |
4544
| **active** | `boolean` || Whether the tool is enabled |
4645
| **builtIn** | `boolean` || Platform built-in tool flag |

packages/platform-objects/src/apps/translations/en.metadata-forms.generated.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1693,10 +1693,6 @@ export const enMetadataForms: NonNullable<TranslationData['metadataForms']> = {
16931693
label: "Output Schema",
16941694
helpText: "Output schema for validation (optional)"
16951695
},
1696-
requiresConfirmation: {
1697-
label: "Requires Confirmation",
1698-
helpText: "Ask user to approve before executing (for destructive actions)"
1699-
},
17001696
permissions: {
17011697
label: "Permissions",
17021698
helpText: "Required permissions to use this tool"

packages/platform-objects/src/apps/translations/es-ES.metadata-forms.generated.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1693,10 +1693,6 @@ export const esESMetadataForms: NonNullable<TranslationData['metadataForms']> =
16931693
label: "Esquema de salida",
16941694
helpText: "Esquema de salida para validación (opcional)"
16951695
},
1696-
requiresConfirmation: {
1697-
label: "Requiere confirmación",
1698-
helpText: "Pide aprobación al usuario antes de ejecutar (para acciones destructivas)"
1699-
},
17001696
permissions: {
17011697
label: "Permisos",
17021698
helpText: "Permisos necesarios para usar esta herramienta"

packages/platform-objects/src/apps/translations/ja-JP.metadata-forms.generated.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1693,10 +1693,6 @@ export const jaJPMetadataForms: NonNullable<TranslationData['metadataForms']> =
16931693
label: "出力スキーマ",
16941694
helpText: "検証用出力スキーマ(任意)"
16951695
},
1696-
requiresConfirmation: {
1697-
label: "確認が必要",
1698-
helpText: "実行前にユーザー承認を求める(破壊的アクション用)"
1699-
},
17001696
permissions: {
17011697
label: "権限",
17021698
helpText: "このツールの使用に必要な権限"

packages/platform-objects/src/apps/translations/zh-CN.metadata-forms.generated.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1693,10 +1693,6 @@ export const zhCNMetadataForms: NonNullable<TranslationData['metadataForms']> =
16931693
label: "输出 Schema",
16941694
helpText: "用于校验的输出结构(可选)"
16951695
},
1696-
requiresConfirmation: {
1697-
label: "需要确认",
1698-
helpText: "在执行前请求用户确认(用于破坏性操作)"
1699-
},
17001696
permissions: {
17011697
label: "权限",
17021698
helpText: "使用此工具所需的权限"

packages/spec/README.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,11 @@ export const objectStackMCP = MCPServerConfigSchema.parse({
133133
],
134134
handler: 'flows.create_record',
135135
sideEffects: 'write',
136-
// NOTE: declarative only — nothing pauses on this flag (#3715). A real
137-
// human-in-the-loop gate is `ai.requiresConfirmation` on the underlying
138-
// action (+ the approval queue), or `approval: 'always'` on an MCP binding.
136+
// NOTE: an MCP capability-descriptor hint — surfaced to the client, but
137+
// nothing server-side pauses on it. (The `ToolSchema` field of the same
138+
// name was removed in 16.x — #3715, ADR-0033 §2.) A real human-in-the-loop
139+
// gate is `ai.requiresConfirmation` on the underlying action (+ the
140+
// approval queue), or `approval: 'always'` on an MCP binding.
139141
requiresConfirmation: true,
140142
},
141143
{

packages/spec/docs/MCP_GUIDE.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -359,8 +359,11 @@ For operations with side effects, gate them with something that is enforced:
359359
`approval: 'always'` on the MCP tool binding, or `ai.requiresConfirmation` on
360360
the underlying **action** (+ the HITL approval queue).
361361

362-
> ⚠️ `requiresConfirmation` on the **tool definition** is declared but read by
363-
> no execution path — it produces no pause (#3715).
362+
> ⚠️ The `requiresConfirmation` in these examples is an **MCP capability
363+
> descriptor** hint — surfaced to the client in `list_actions` so it *may* ask,
364+
> but nothing server-side pauses on it. Distinct from the `ToolSchema` field of
365+
> the same name, which was REMOVED in the 16.x line (#3715, ADR-0033 §2)
366+
> precisely because a safety flag no path enforces is false compliance.
364367
365368
### 4. Security
366369

packages/spec/liveness/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ EOF
376376
| permission | 32 || 0 || CRUD/FLS/RLS live; dead `contextVariables` REMOVED (ADR-0105 D11 — RLS resolves only the `current_user.*` built-ins plus runtime-staged `rlsMembership` sets) |
377377
| position | 4 |||| (role's ADR-0090 successor) fully live |
378378
| agent | 13 | 5 | 1 || dead `tenantId` + `planning.strategy`/`allowReplan` REMOVED (#2377) — only `planning.maxIterations` live; autonomy tier experimental; `knowledge` CORRECTED to dead 2026-07 — `search_knowledge` takes `sourceIds` from the LLM's tool-call args, never from the agent record (#1878 §3 recheck) |
379-
| tool | 5 | 1 | 5 || the whole authoring surface is inert: `permissions` (not permission-gated), plus `category`/`requiresConfirmation`/`active`/`builtIn` CORRECTED to dead 2026-07 (#3686 sweep). ⚠️ `requiresConfirmation` is SAFETY-shaped and unenforced on every path — ADR-0033 already resolved to delete it |
379+
| tool | 5 | 1 | 4 || the whole authoring surface is inert: `permissions` (not permission-gated), plus `category`/`active`/`builtIn` CORRECTED to dead 2026-07 (#3686 sweep). `requiresConfirmation` REMOVED (#3715, ADR-0033 §2) — SAFETY-shaped and unenforced on every path, so it was false compliance, not merely dead; ToolSchema is now `.strict()` so the retired key REJECTS with the FROM → TO prescription instead of being silently stripped |
380380
| skill | 8 || 1 || `permissions` REMOVED 2026-07 (never gated anything — owner call was prune, #3704); `triggerPhrases` CORRECTED to dead — phrases are never matched against user messages; activation is `triggerConditions` + the agent's `skills[]` + explicit /skill-name pinning (#3686 sweep) |
381381
| dataset | 19 || 0 || `measures.certified` (declared-but-unenforced governance flag) REMOVED in 16.0 (#2377) |
382382
| page | 16 ||| 1 | fully live + one planned |

packages/spec/liveness/tool.json

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,6 @@
3838
"authorHint": "Display-only — no runtime groups, filters or routes tools by category.",
3939
"note": "RE-VERIFIED 2026-07 (#3686 preview-claim sweep): the prior `live` verdict cited only a metadata-admin PREVIEW panel, which echoes what the author typed."
4040
},
41-
"requiresConfirmation": {
42-
"status": "dead",
43-
"evidence": "zero reads off any TOOL definition in any repo; every requiresConfirmation read is action.ai.requiresConfirmation (a different, live property). Ignored by the LLM tool set (vercel-adapter.ts:263-272), ToolRegistry.execute (tool-registry.ts:183-210), POST /ai/tools/:name/execute (tool-routes.ts:91,105) and the MCP bridge (framework packages/mcp/src/mcp-server-runtime.ts:176, which uses a hardcoded destructive-name list)",
44-
"authorWarn": true,
45-
"authorHint": "NOT ENFORCED — nothing pauses for confirmation on this flag, on any path. To gate a destructive operation, put it behind an action with `ai.requiresConfirmation` + the HITL approval queue, which is the only path that actually stops execution.",
46-
"note": "RE-VERIFIED 2026-07 (#3686 preview-claim sweep): the prior `live` verdict cited only a metadata-admin PREVIEW panel, which echoes what the author typed. SAFETY-SHAPED: tool.form.ts:36-42 places it in a section titled 'Access & safety' with helpText 'Ask user to approve before executing (for destructive actions)', and skills/objectstack-ai/SKILL.md:547 tells authors to rely on it — so the authoring surface actively teaches a guarantee nothing delivers. ADR-0033 (:31, :60) already recorded it as never-enforced and resolved to DELETE the placeholder (the draft/publish workspace is the real approval gate); that deletion was never executed. OWNER CALL 2026-07: the prune-or-wire decision is DEFERRED (#3715), but the false promise stops now — the spec `.describe()`, the Studio form section (formerly titled \"Access & safety\", whose copy told authors to use it for destructive operations) and the SKILL.md / MCP_GUIDE / README guidance all now say NOT ENFORCED and point at the action-level gate."
47-
},
4841
"permissions": {
4942
"status": "dead",
5043
"evidence": "tool.form.ts only; not on AIToolDefinition, no consumer",

0 commit comments

Comments
 (0)