Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions .changeset/tool-requires-confirmation-removed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
---
"@objectstack/spec": minor
"@objectstack/platform-objects": patch
---

feat(spec)!: remove `tool.requiresConfirmation` — a safety flag nothing enforced (#3715, ADR-0033 §2)

`ToolSchema.requiresConfirmation` accepted `true` and no execution path ever read
it. Not the LLM tool set (a tool reaches the model as name/description/parameters
only), not `ToolRegistry.execute`, not `POST /ai/tools/:name/execute`, and not the
MCP bridge — which derives `destructiveHint` from a hardcoded 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, which is the case ADR-0049 exists for: an author gates a
destructive tool, sees the flag accepted, and ships believing a human is in the
loop. It is 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 this one.

## Migration

- **FROM:** `requiresConfirmation: true` on a tool definition
- **TO:** put the operation behind an action and set `ai.requiresConfirmation:
true` there — that is the flag the HITL approval queue reads
(`packages/runtime/src/action-execution.ts`) and the only path that actually
stops execution.
- For AI *metadata* mutations there is nothing to migrate: the ADR-0033
draft/publish workspace is the gate — nothing is live until a human publishes.

**`ToolSchema` is now `.strict()`.** This is load-bearing, not tidying. Removing a
key from a non-strict schema swaps one silent no-op for another: zod strips the
key wordlessly, the author keeps writing it, and the safety flag goes on meaning
nothing — the "silent strip" ADR-0032 / #1535 closed for objects. The retired key
now **rejects**, and the error carries the FROM → TO above, because a parse error
is the one channel every consumer bumping `@objectstack/spec` is guaranteed to
hit.

Strictness applies to *all* unknown keys on a tool definition, so a typo
(`buildIn`, `catagory`) is now a located parse error instead of a silently
dropped field.

Also removed: the Studio form row, its four generated locale bundles (the
`en`/`zh-CN`/`ja-JP`/`es-ES` strings still promised *"Ask user to approve before
executing (for destructive actions)"* — a translated false promise), the
liveness-ledger entry, and the generated reference-doc row.

objectui's `ToolPreview.tsx` reads the field via `!!d.requiresConfirmation`, so it
degrades to "not shown" with no error; removing that badge is a follow-up in that
repo.
1 change: 0 additions & 1 deletion content/docs/references/ai/tool.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ AI tool definition. [READ-ONLY PROJECTION — not an execution entry point] Auth
| **parameters** | `Record<string, any>` | ✅ | JSON Schema for tool parameters |
| **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). |
| **objectName** | `string` | optional | Target object name (snake_case) |
| **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. |
| **permissions** | `string[]` | optional | Required permission-set capabilities |
| **active** | `boolean` | ✅ | Whether the tool is enabled |
| **builtIn** | `boolean` | ✅ | Platform built-in tool flag |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1693,10 +1693,6 @@ export const enMetadataForms: NonNullable<TranslationData['metadataForms']> = {
label: "Output Schema",
helpText: "Output schema for validation (optional)"
},
requiresConfirmation: {
label: "Requires Confirmation",
helpText: "Ask user to approve before executing (for destructive actions)"
},
permissions: {
label: "Permissions",
helpText: "Required permissions to use this tool"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1693,10 +1693,6 @@ export const esESMetadataForms: NonNullable<TranslationData['metadataForms']> =
label: "Esquema de salida",
helpText: "Esquema de salida para validación (opcional)"
},
requiresConfirmation: {
label: "Requiere confirmación",
helpText: "Pide aprobación al usuario antes de ejecutar (para acciones destructivas)"
},
permissions: {
label: "Permisos",
helpText: "Permisos necesarios para usar esta herramienta"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1693,10 +1693,6 @@ export const jaJPMetadataForms: NonNullable<TranslationData['metadataForms']> =
label: "出力スキーマ",
helpText: "検証用出力スキーマ(任意)"
},
requiresConfirmation: {
label: "確認が必要",
helpText: "実行前にユーザー承認を求める(破壊的アクション用)"
},
permissions: {
label: "権限",
helpText: "このツールの使用に必要な権限"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1693,10 +1693,6 @@ export const zhCNMetadataForms: NonNullable<TranslationData['metadataForms']> =
label: "输出 Schema",
helpText: "用于校验的输出结构(可选)"
},
requiresConfirmation: {
label: "需要确认",
helpText: "在执行前请求用户确认(用于破坏性操作)"
},
permissions: {
label: "权限",
helpText: "使用此工具所需的权限"
Expand Down
8 changes: 5 additions & 3 deletions packages/spec/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,11 @@ export const objectStackMCP = MCPServerConfigSchema.parse({
],
handler: 'flows.create_record',
sideEffects: 'write',
// NOTE: declarative only — nothing pauses on this flag (#3715). A real
// human-in-the-loop gate is `ai.requiresConfirmation` on the underlying
// action (+ the approval queue), or `approval: 'always'` on an MCP binding.
// NOTE: an MCP capability-descriptor hint — surfaced to the client, but
// nothing server-side pauses on it. (The `ToolSchema` field of the same
// name was removed in 16.x — #3715, ADR-0033 §2.) A real human-in-the-loop
// gate is `ai.requiresConfirmation` on the underlying action (+ the
// approval queue), or `approval: 'always'` on an MCP binding.
requiresConfirmation: true,
},
{
Expand Down
7 changes: 5 additions & 2 deletions packages/spec/docs/MCP_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -359,8 +359,11 @@ For operations with side effects, gate them with something that is enforced:
`approval: 'always'` on the MCP tool binding, or `ai.requiresConfirmation` on
the underlying **action** (+ the HITL approval queue).

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

### 4. Security

Expand Down
2 changes: 1 addition & 1 deletion packages/spec/liveness/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ EOF
| 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) |
| position | 4 | – | – | – | (role's ADR-0090 successor) fully live |
| 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) |
| 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 |
| 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 |
| 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) |
| dataset | 19 | – | 0 | – | `measures.certified` (declared-but-unenforced governance flag) REMOVED in 16.0 (#2377) |
| page | 16 | – | – | 1 | fully live + one planned |
Expand Down
7 changes: 0 additions & 7 deletions packages/spec/liveness/tool.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,6 @@
"authorHint": "Display-only — no runtime groups, filters or routes tools by category.",
"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."
},
"requiresConfirmation": {
"status": "dead",
"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)",
"authorWarn": true,
"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.",
"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."
},
"permissions": {
"status": "dead",
"evidence": "tool.form.ts only; not on AIToolDefinition, no consumer",
Expand Down
4 changes: 3 additions & 1 deletion packages/spec/src/ai/tool.form.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ export const toolForm = defineForm({
collapsible: true,
collapsed: true,
fields: [
{ field: 'requiresConfirmation', helpText: 'NOT ENFORCED (#3715) — nothing pauses for confirmation on this flag. For a real gate use the action-level `ai.requiresConfirmation` + approval queue; AI metadata edits are already gated by draft/publish.' },
// `requiresConfirmation` was REMOVED from ToolSchema (#3715, ADR-0033
// §2). A safety flag no path enforced is false compliance; the real
// gate is the action-level `ai.requiresConfirmation` + approval queue.
{ field: 'permissions', widget: 'string-tags', helpText: 'NOT ENFORCED — tool invocation is not permission-gated by this list. Gate the underlying action via permission sets (ADR-0066), or restrict the agent that exposes the tool.' },
],
},
Expand Down
42 changes: 38 additions & 4 deletions packages/spec/src/ai/tool.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ describe('ToolSchema', () => {
expect(result.name).toBe('list_records');
expect(result.active).toBe(true);
expect(result.builtIn).toBe(false);
expect(result.requiresConfirmation).toBe(false);
});

it('should accept full tool', () => {
Expand All @@ -64,7 +63,6 @@ describe('ToolSchema', () => {
},
},
objectName: 'support_case',
requiresConfirmation: true,
permissions: ['case.create', 'support.agent'],
active: true,
builtIn: false,
Expand All @@ -74,7 +72,6 @@ describe('ToolSchema', () => {
expect(result.name).toBe('create_case');
expect(result.category).toBe('action');
expect(result.objectName).toBe('support_case');
expect(result.requiresConfirmation).toBe(true);
expect(result.permissions).toEqual(['case.create', 'support.agent']);
});

Expand Down Expand Up @@ -162,7 +159,6 @@ describe('defineTool', () => {

expect(tool.active).toBe(true);
expect(tool.builtIn).toBe(false);
expect(tool.requiresConfirmation).toBe(false);
});

it('should throw on invalid tool name', () => {
Expand All @@ -173,4 +169,42 @@ describe('defineTool', () => {
parameters: {},
})).toThrow();
});

// ── #3715 / ADR-0033 §2 — the retired `requiresConfirmation` safety flag ──
// Removing a key from a NON-strict schema would swap one silent no-op for
// another (zod strips it wordlessly). These pin the loud rejection AND the
// prescription it must carry — the parse error is the one channel a consumer
// bumping @objectstack/spec is guaranteed to hit.

it('REJECTS the retired `requiresConfirmation` instead of silently stripping it', () => {
const authored = {
name: 'delete_everything',
label: 'Delete Everything',
description: 'Destructive',
parameters: {},
requiresConfirmation: true,
};
expect(() => ToolSchema.parse(authored)).toThrow(/requiresConfirmation/);
expect(() => defineTool(authored as never)).toThrow();
});

it('the rejection names the REAL gate, not merely the removal', () => {
let message = '';
try {
ToolSchema.parse({
name: 't', label: 'T', description: 'd', parameters: {}, requiresConfirmation: true,
});
} catch (e) {
message = String((e as Error).message);
}
// FROM → TO: the action-level flag is the only path that stops execution.
expect(message).toMatch(/ai\.requiresConfirmation/);
expect(message).toMatch(/#3715/);
});

it('rejects an unrelated unknown key too (strictness is not special-cased)', () => {
expect(() => ToolSchema.parse({
name: 't', label: 'T', description: 'd', parameters: {}, notAToolField: 1,
})).toThrow(/notAToolField/);
});
});
Loading
Loading