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
8 changes: 8 additions & 0 deletions .changeset/32993-approval-recall-resubmit-ai-exposure.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@objectstack/plugin-approvals': patch
---

Clause-②: yes (widening)
Metadata-protocol: none

A submitter's agent can now recall and resubmit its own approval request over MCP. `approval_recall` and `approval_resubmit` on `sys_approval_request` declare `ai: { exposed: true, description, requiresConfirmation: true }`, the same shape the four approver decisions got, so `list_actions` lists them and `run_action` runs them once the call carries `confirm: true`. Each forwards to its approvals route as the caller, so the approval service admits the same people the console's buttons admit: the submitter, and, for recalling a pending request, the administrator who may override it. Anyone else is refused (recall in the `approval_recall_not_submitter` sentence) and the request does not move. Remind, request-info and comments stay unexposed.
13 changes: 11 additions & 2 deletions content/docs/ai/connect-mcp.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,17 @@ can reach is the **default exposure policy (v1)**:
same people the console's buttons admit: a current approver of the request,
and, for approve, reject and reassign, the administrator who may override it.
Anyone else, the submitter
included, is refused and the request stays pending. The submitter's own
levers (remind, recall, resubmit), request-info and comments are not exposed.
included, is refused and the request stays pending.
- **A submitter's recall and resubmit are exposed to the submitter's agent.**
Recall (withdraw a pending request, or abandon one sent back for revision) and
resubmit (send a request that was sent back for revision into review again)
are exposed the same way, with `ai.requiresConfirmation: true`, and forward to
their approvals routes as the caller. The approval service admits the
submitter, and for recalling a pending request the administrator who may
override it; anyone else is refused (`FORBIDDEN`) and the request does not
move. A recalled request is closed: to send the record for approval again,
the agent uses the app's own submit action. Remind, request-info and comments
are not exposed.

<Callout type="info">
This is the default policy, not a ceiling. A finer, **metadata-declared** exposure
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,15 +169,16 @@ describe('sys_approval_request declared actions', () => {
// `ActionAiSchema.description` is the LLM-facing contract (min 40 chars,
// required when `ai.exposed`) — the same name one level down. Putting the
// question there would arm a tool description while the dialog fell back to
// its generic line. `approval_reject` is exposed to AI since #32442, so its
// tool description exists — and must be its own text, never the question.
// its generic line. `approval_reject` is exposed to AI since #32442 and
// `approval_recall` since #32993, so each has a tool description — and it
// must be its own text, never the question.
for (const name of ['approval_reject', 'approval_recall']) {
const a = byName(name);
const question = templateSlotSource(a.description);
expect(question, `${name}.description`).toMatch(/\?/);
expect(a.ai?.description ?? '', `${name}.ai.description`).not.toContain(question);
expect(a.ai?.description, `${name}.ai.description`).toEqual(expect.any(String));
expect(a.ai.description, `${name}.ai.description`).not.toContain(question);
}
expect(byName('approval_recall').ai?.description).toBeUndefined();
});

it('reassign collects the new approver via a field-backed sys_user picker keyed as `to`', () => {
Expand Down Expand Up @@ -252,10 +253,15 @@ describe('sys_approval_request AI exposure (#32442)', () => {
approval_reassign: 'reassign',
approval_send_back: 'revise',
};
// [#32993] The submitter's side of the same process.
const SUBMITTER_LEVERS: Record<string, string> = {
approval_recall: 'recall',
approval_resubmit: 'resubmit',
};

it('exposes exactly the four approver decisions to AI', () => {
it('exposes exactly the four approver decisions and the submitter\'s recall and resubmit to AI', () => {
const exposed = actions.filter((a) => a.ai?.exposed === true).map((a) => a.name).sort();
expect(exposed).toEqual(Object.keys(DECISIONS).sort());
expect(exposed).toEqual([...Object.keys(DECISIONS), ...Object.keys(SUBMITTER_LEVERS)].sort());
});

it('every exposed decision asks for the human\'s confirmation and carries its own tool description', () => {
Expand All @@ -278,6 +284,19 @@ describe('sys_approval_request AI exposure (#32442)', () => {
}
});

it('[#32993] recall and resubmit ask for the human\'s confirmation, name the submitter, and forward to their own routes', () => {
for (const [name, verb] of Object.entries(SUBMITTER_LEVERS)) {
const a = byName(name);
expect(a.ai.requiresConfirmation, `${name}.ai.requiresConfirmation`).toBe(true);
expect(a.ai.description.length, `${name}.ai.description length`).toBeGreaterThanOrEqual(40);
expect(a.ai.description).toMatch(/submitter/);
expect(a.ai.description).toMatch(/confirm/);
expect(a.type).toBe('api');
expect(a.method).toBe('POST');
expect(a.target).toBe(`/api/v1/approvals/requests/{id}/${verb}`);
}
});

it('reassign tells the agent what `to` is', () => {
expect(byName('approval_reassign').ai.paramHints?.to?.description).toMatch(/user id/);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -604,9 +604,15 @@ export const SysApprovalRequest = ObjectSchema.create({
// `visible` is not that gate and the door does not read it. Each declares
// `requiresConfirmation: true`, ADR-0011's bucket C for a consequential,
// bounded act (`sys_user.ban_user` is the precedent), so `run_action`
// refuses until the call carries the confirmation. The submitter levers,
// request-info and the thread reply stay unexposed: the ruling named the
// four decisions.
// refuses until the call carries the confirmation.
//
// [#32993] The submitter's side of the same process: recall and resubmit
// are exposed the same way, so the submitter's agent can withdraw a request
// to fix the record and send a returned one back into review. Same door,
// same gate: `ApprovalService.recall` admits the submitter (and, on a
// pending request, the override holder) and refuses anyone else with
// `approval_recall_not_submitter`; `ApprovalService.resubmit` admits the
// submitter alone. Remind, request-info and the thread reply stay unexposed.
actions: [
{
name: 'approval_approve',
Expand Down Expand Up @@ -833,6 +839,15 @@ export const SysApprovalRequest = ObjectSchema.create({
locations: ['record_section'],
successMessage: 'Recalled "{{ result.request.record_title }}"',
refreshAfter: true,
// [#32993] Exposed to AI for the request's submitter. See the note above
// the actions array. The tool description is its own text, never the
// dialog question in `description`.
ai: {
exposed: true,
description:
'Withdraw an approval request (the record: a sys_approval_request id) that the user submitted, with an optional comment, while it is pending or was sent back for revision. Approvers can no longer act on it and the record is unlocked for editing; a recalled request is closed, so sending the record for approval again goes through the app\'s own submit action. Only the submitter of the request, or an administrator who may override a stuck pending request, may recall it; anyone else is refused. Ask the human to confirm before calling.',
requiresConfirmation: true,
},
},
{
name: 'approval_resubmit',
Expand All @@ -848,6 +863,14 @@ export const SysApprovalRequest = ObjectSchema.create({
locations: ['record_section'],
successMessage: 'Resubmitted "{{ result.request.record_title }}"',
refreshAfter: true,
// [#32993] Exposed to AI for the request's submitter. See the note above
// the actions array.
ai: {
exposed: true,
description:
'Send an approval request (the record: a sys_approval_request id) that an approver sent back for revision (status returned) into review again, after the user revised the record, with an optional note on what changed. It applies only to a returned request. Only the submitter of the request may resubmit it; anyone else is refused. Ask the human to confirm before calling.',
requiresConfirmation: true,
},
},

// ── Thread reply ────────────────────────────────────────────────
Expand Down
117 changes: 101 additions & 16 deletions packages/qa/dogfood/test/approval-decide-over-mcp.dogfood.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,32 @@
// negative pins below are the service's gate observed through the live MCP
// transport, not a second gate.
//
// [#32993] The submitter's side of the same process: `approval_recall` and
// `approval_resubmit` declare the same `ai` shape, forwarded to their routes as
// the caller, so `ApprovalService` decides who may recall or resubmit.
//
// Real `SecurityPlugin`, real flows, real approvals plugin, real MCP plugin. A
// one-step flow waits on one approver, addressed by email.
// one-step flow waits on one approver, addressed by email, with an ADR-0044
// revise window (send back → returned → resubmit).
//
// ⭐ the approver's `list_actions` lists the four decisions on
// `sys_approval_request`, attested `aiExposed`, and none of the
// submitter's levers;
// ⭐ `list_actions` lists the four decisions and the submitter's recall and
// resubmit on `sys_approval_request`, attested `aiExposed`, and none of
// the other levers;
// ⭐ a member on no slate is refused, and the request stays pending;
// ⭐ the submitter is refused, and the request stays pending;
// ⭐ an AI service account acting for itself (ADR-0151, its own API key, on
// no slate) is refused, and the request stays pending;
// ⭐ the approver is refused `428` until the call carries `confirm: true`;
// ⭐ confirmed, the approver's call decides: the request is approved and
// the decision is recorded once, as the approver's.
// the decision is recorded once, as the approver's;
// ⭐ [#32993] the submitter's recall is refused `428` until confirmed, then
// withdraws the pending request, recorded once as the submitter's;
// ⭐ [#32993] a caller who is not the submitter (the approver) is refused
// recall in `approval_recall_not_submitter`'s words, and the request
// stays pending;
// ⭐ [#32993] on a request sent back for revision, the submitter edits the
// record and resubmits over MCP (confirmed): round 2 opens a pending
// request; unconfirmed, or from the approver, nothing moves.
//
// The read half of the card (an approver listing their pending requests over
// MCP) is not here: the MCP object tools refuse every `sys_*` object, so this
Expand All @@ -40,6 +53,7 @@ import { defineStack, defineFlow } from '@objectstack/spec';
import { ObjectSchema, Field } from '@objectstack/spec/data';
import { PermissionSetSchema, type PermissionSet } from '@objectstack/spec/security';
import { AI_ACTION_CONFIRMATION_MEMBER } from '@objectstack/spec/contracts';
import { BUILTIN_OPERATION_MESSAGES } from '@objectstack/spec/system';
import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security';
import { catalogPermissionSet } from './catalog-permission-set.js';

Expand All @@ -53,6 +67,9 @@ const APPROVER = 'apmcp-approver@verify.test';
const STRANGER = 'apmcp-stranger@verify.test';

const DECISIONS = ['approval_approve', 'approval_reassign', 'approval_reject', 'approval_send_back'];
// [#32993] The submitter's recall and resubmit, exposed beside the decisions.
const EXPOSED = [...DECISIONS, 'approval_recall', 'approval_resubmit'].sort();
const RECALL_REFUSAL = BUILTIN_OPERATION_MESSAGES.en!.approval_recall_not_submitter!;

const leave = ObjectSchema.create({
name: OBJECT,
Expand All @@ -75,13 +92,17 @@ const flow = defineFlow({
label: 'Manager',
config: { approvers: [{ type: 'user', value: APPROVER }], behavior: 'first_response', lockRecord: false },
},
// ADR-0044 revise window, so a request can be sent back (`returned`) and resubmitted.
{ id: 'rework', type: 'approval_revise', label: 'Awaiting Revision' },
{ id: 'approved', type: 'end', label: 'Approved' },
{ id: 'rejected', type: 'end', label: 'Rejected' },
],
edges: [
{ id: 'e1', source: 'start', target: 'manager' },
{ id: 'e2', source: 'manager', target: 'approved', label: 'approve' },
{ id: 'e3', source: 'manager', target: 'rejected', label: 'reject' },
{ id: 'e4', source: 'manager', target: 'rework', label: 'revise' },
{ id: 'e5', source: 'rework', target: 'manager', label: 'resubmit', type: 'back' },
],
});

Expand Down Expand Up @@ -114,6 +135,7 @@ describe('an approver\'s agent decides a pending approval request over MCP (#324
let approver: string;
let stranger: string;
let approverId: string;
let submitterId: string;
let accountKey: string;

const idOf = async (email: string): Promise<string> =>
Expand All @@ -138,14 +160,17 @@ describe('an approver\'s agent decides a pending approval request over MCP (#324
try { payload = JSON.parse(text); } catch { payload = { message: text }; }
return { isError: rpc.result?.isError === true, payload, text };
};
const approve = (as: Caller, requestId: string, confirm: boolean) =>
/** `run_action` on one request's action, confirmed or not. */
const act = (as: Caller, actionName: string, requestId: string, confirm: boolean, comment: string) =>
tool(as, 'run_action', {
actionName: 'approval_approve',
actionName,
objectName: REQUEST,
recordId: requestId,
params: { comment: 'approved over MCP' },
params: { comment },
...(confirm ? { [AI_ACTION_CONFIRMATION_MEMBER]: true } : {}),
});
const approve = (as: Caller, requestId: string, confirm: boolean) =>
act(as, 'approval_approve', requestId, confirm, 'approved over MCP');

const requestRow = async (requestId: string) =>
rowsOf(await ql.find(REQUEST, { where: { id: requestId }, context: SYS.context }))[0];
Expand Down Expand Up @@ -190,6 +215,8 @@ describe('an approver\'s agent decides a pending approval request over MCP (#324
}
approverId = await idOf(APPROVER);
expect(approverId).not.toBe('');
submitterId = await idOf(SUBMITTER);
expect(submitterId).not.toBe('');

// An AI service account acting for itself (ADR-0151): registered by the
// administrator, its key minted for it, on no slate.
Expand All @@ -208,15 +235,17 @@ describe('an approver\'s agent decides a pending approval request over MCP (#324
await stack?.stop?.();
});

it('⭐ the approver\'s agent lists the four decisions, attested and asking for confirmation, and none of the other levers', async () => {
it('⭐ the approver\'s and the submitter\'s agents list the decisions, recall and resubmit, attested and asking for confirmation, and none of the other levers', async () => {
await openRequest('listed');
const listed = await tool({ bearer: approver }, 'list_actions', {});
expect(listed.isError, listed.text).toBe(false);
const onRequest = (listed.payload.actions as any[]).filter((a) => a.objectName === REQUEST);
expect(onRequest.map((a) => a.name).sort()).toEqual(DECISIONS);
for (const row of onRequest) {
expect(row.aiExposed, `${row.name} is attested as exposed`).toBe(true);
expect(row.requiresConfirmation, `${row.name} asks for confirmation`).toBe(true);
for (const [who, token] of [['approver', approver], ['submitter', submitter]] as const) {
const listed = await tool({ bearer: token }, 'list_actions', {});
expect(listed.isError, listed.text).toBe(false);
const onRequest = (listed.payload.actions as any[]).filter((a) => a.objectName === REQUEST);
expect(onRequest.map((a) => a.name).sort(), `${who}'s list_actions`).toEqual(EXPOSED);
for (const row of onRequest) {
expect(row.aiExposed, `${row.name} is attested as exposed`).toBe(true);
expect(row.requiresConfirmation, `${row.name} asks for confirmation`).toBe(true);
}
}
});

Expand Down Expand Up @@ -264,4 +293,60 @@ describe('an approver\'s agent decides a pending approval request over MCP (#324
expect((await requestRow(requestId)).status).toBe('approved');
expect((await decisionsOn(requestId)).map((r) => [r.action, r.actor_id])).toEqual([['approve', approverId]]);
});

it('⭐ [#32993] the submitter\'s recall is refused until confirmed, then withdraws the pending request, recorded once as the submitter\'s', async () => {
const requestId = await openRequest('recall');
const unconfirmed = await act({ bearer: submitter }, 'approval_recall', requestId, false, 'fixing the lines');
expect(unconfirmed.isError, unconfirmed.text).toBe(true);
expect(unconfirmed.text).toContain('ACTION_CONFIRMATION_REQUIRED');
await expectUndecided(requestId, 'an unconfirmed recall');

const confirmed = await act({ bearer: submitter }, 'approval_recall', requestId, true, 'fixing the lines');
expect(confirmed.isError, confirmed.text).toBe(false);
expect((await requestRow(requestId)).status).toBe('recalled');
expect((await decisionsOn(requestId)).map((r) => [r.action, r.actor_id])).toEqual([['recall', submitterId]]);
});

it('⭐ [#32993] a caller who is not the submitter is refused recall in the service\'s own words, and the request stays pending', async () => {
const requestId = await openRequest('recall-not-submitter');
const answer = await act({ bearer: approver }, 'approval_recall', requestId, true, 'not mine to recall');
expect(answer.isError, answer.text).toBe(true);
expect(answer.payload?.error?.code, answer.text).toBe('FORBIDDEN');
expect(answer.text).toContain(RECALL_REFUSAL);
await expectUndecided(requestId, 'a recall by the approver');
});

it('⭐ [#32993] on a request sent back for revision, the submitter edits the record and resubmits over MCP: round 2 opens a pending request', async () => {
const requestId = await openRequest('resubmit');
const recordId = String((await requestRow(requestId)).record_id);
const sentBack = await act({ bearer: approver }, 'approval_send_back', requestId, true, 'fix the dates');
expect(sentBack.isError, sentBack.text).toBe(false);
expect((await requestRow(requestId)).status).toBe('returned');

const edited = await stack.apiAs(submitter, 'PATCH', `/data/${OBJECT}/${recordId}`, { name: 'resubmit (dates fixed)' });
expect(edited.status, await edited.clone().text()).toBe(200);

const pendingOn = async () =>
rowsOf(await ql.find(REQUEST, { where: { object_name: OBJECT, record_id: recordId, status: 'pending' }, context: SYS.context }));

// Unconfirmed, or from anyone but the submitter, nothing moves.
const unconfirmed = await act({ bearer: submitter }, 'approval_resubmit', requestId, false, 'dates fixed');
expect(unconfirmed.isError, unconfirmed.text).toBe(true);
expect(unconfirmed.text).toContain('ACTION_CONFIRMATION_REQUIRED');
const byApprover = await act({ bearer: approver }, 'approval_resubmit', requestId, true, 'not mine');
expect(byApprover.isError, byApprover.text).toBe(true);
expect(byApprover.payload?.error?.code, byApprover.text).toBe('FORBIDDEN');
expect((await requestRow(requestId)).status).toBe('returned');
expect(await pendingOn(), 'no round 2 before the submitter\'s confirmed resubmit').toEqual([]);

const confirmed = await act({ bearer: submitter }, 'approval_resubmit', requestId, true, 'dates fixed');
expect(confirmed.isError, confirmed.text).toBe(false);
const round2 = await pendingOn();
expect(round2, 'the resubmit opened round 2').toHaveLength(1);
expect(round2[0].id).not.toBe(requestId);
// One row each, in no stored order (the read is unsorted).
expect((await decisionsOn(requestId)).map((r) => `${r.action}:${r.actor_id}`).sort()).toEqual(
[`resubmit:${submitterId}`, `revise:${approverId}`].sort(),
);
});
});
Loading