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
171 changes: 169 additions & 2 deletions lib/Service/ApprovalService.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
use OCA\Integriq\Service\Helper\FlowToken;
use OCA\OpenRegister\Db\ObjectEntity;
use OCA\OpenRegister\Service\ObjectService as ORObjectService;
use OCA\OpenRegister\Service\Task\TaskService as ORTaskService;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\IGroupManager;
use OCP\IURLGenerator;
Expand Down Expand Up @@ -106,6 +107,11 @@ class ApprovalService {
* @param ExecutionTraceService|null $executionTraceService Persists the traced run's execution_trace at
* suspension/resume (execution-trace REQ-004). Nullable + defaulted so
* pre-existing positional test instantiations keep working unmodified.
* @param ORTaskService|null $taskService OpenRegister's shared task service: every suspension mirrors ONE
* shared task through it and every decision closes that mirror
* (hitl-on-shared-tasks D-1). Nullable + defaulted for the same
* positional-test reason; absent, no mirror exists and the approval
* flow is unchanged.
*/
public function __construct(
private readonly ORObjectService $objectService,
Expand All @@ -115,6 +121,7 @@ public function __construct(
private readonly IURLGenerator $urlGenerator,
private readonly LoggerInterface $logger,
private readonly ?ExecutionTraceService $executionTraceService = null,
private readonly ?ORTaskService $taskService = null,
) {
}//end __construct()

Expand Down Expand Up @@ -195,6 +202,7 @@ public function suspend(ObjectEntity $endpoint, ObjectEntity $rule, FlowToken $f
}
}

$record = $this->mirrorIntoSharedTask(approvalRequest: $record);
$this->notifyApprovers(approvalRequest: $record);

return $record;
Expand Down Expand Up @@ -243,6 +251,7 @@ public function suspendForSynchronization(
schema: self::SCHEMA
);

$record = $this->mirrorIntoSharedTask(approvalRequest: $record);
$this->notifyApprovers(approvalRequest: $record);

return $record;
Expand Down Expand Up @@ -296,6 +305,7 @@ public function suspendForFlow(ObjectEntity $flowRun, int $resumeStepOrder, arra
schema: self::SCHEMA
);

$record = $this->mirrorIntoSharedTask(approvalRequest: $record);
$this->notifyApprovers(approvalRequest: $record);

return $record;
Expand Down Expand Up @@ -349,6 +359,7 @@ public function suspendForSubscription(
schema: self::SCHEMA
);

$record = $this->mirrorIntoSharedTask(approvalRequest: $record);
$this->notifyApprovers(approvalRequest: $record);

return $record;
Expand Down Expand Up @@ -575,13 +586,16 @@ public function completeApproval(
$data['comment'] = $comment;
}

return $this->objectService->saveObject(
$saved = $this->objectService->saveObject(
object: $data,
register: self::REGISTER,
schema: self::SCHEMA,
uuid: $approvalRequest->getUuid()
);

$this->closeSharedTask(data: $data, outcome: 'transition:approved', actorUid: $approver->getUID());

return $saved;
}//end completeApproval()

/**
Expand Down Expand Up @@ -618,13 +632,24 @@ public function reject(ObjectEntity $approvalRequest, IUser $approver, string $c
$data['rejectedAt'] = (new DateTime())->format('c');
$data['comment'] = $comment;

return $this->objectService->saveObject(
$saved = $this->objectService->saveObject(
object: $data,
register: self::REGISTER,
schema: self::SCHEMA,
uuid: $approvalRequest->getUuid()
);

// The mirror ends the way the record did: dead-lettered when
// onReject routed the record there, plainly rejected otherwise
// (hitl-on-shared-tasks D-4).
$mirrorOutcome = 'transition:rejected';
if ($data['status'] === 'dead_letter') {
$mirrorOutcome = 'dead_letter';
}

$this->closeSharedTask(data: $data, outcome: $mirrorOutcome, actorUid: $approver->getUID());

return $saved;
}//end reject()

/**
Expand Down Expand Up @@ -791,6 +816,148 @@ public function notifyApprovers(ObjectEntity $approvalRequest): void {

}//end notifyApprovers()

/**
* Mirror a just-created, pending approval_request into ONE shared
* OpenRegister task (hitl-on-shared-tasks D-1/D-2): approver group as
* candidate group, requester, expiry, and the record's
* onTimeout/onReject when they are in the shared vocabulary, so the
* shared timer sweep owns the mirror's expiry (D-3). The created task's
* uuid is written back onto the record as `taskUuid`.
*
* A failure here is logged and swallowed: the approval flow is the
* system of record and MUST NOT be gated by the mirror (D-5).
*
* @param ObjectEntity $approvalRequest The pending approval_request.
*
* @return ObjectEntity The record, carrying `taskUuid` when the mirror was created.
*
* @spec openspec/changes/hitl-on-shared-tasks/specs/hitl-on-shared-tasks/spec.md#requirement-every-suspension-mirrors-one-shared-task
*/
private function mirrorIntoSharedTask(ObjectEntity $approvalRequest): ObjectEntity {
if ($this->taskService === null) {
return $approvalRequest;
}

$data = $approvalRequest->getObject();
$actor = (string)($data['requesterUserId'] ?? '');
if ($actor === '') {
$actor = 'integriq';
}

try {
$task = $this->taskService->import(
data: $this->sharedTaskData(data: $data, approvalRequestId: (string)$approvalRequest->getUuid()),
actor: $actor
);

$data['taskUuid'] = (string)$task->getUuid();

return $this->objectService->saveObject(
object: $data,
register: self::REGISTER,
schema: self::SCHEMA,
uuid: $approvalRequest->getUuid()
);
} catch (Throwable $e) {
$this->logger->warning(
'ApprovalService: could not mirror the approval into the shared task service: ' . $e->getMessage(),
['approvalRequest' => $approvalRequest->getUuid()]
);

return $approvalRequest;
}
}//end mirrorIntoSharedTask()

/**
* The shared-task payload a pending approval_request mirrors to.
*
* `onTimeout`/`onReject` travel only when they are in the shared
* vocabulary (`skip`|`error`|`dead_letter`); anything else stays an
* app-local behaviour and the mirror carries none.
*
* @param array $data The approval_request object data.
* @param string $approvalRequestId The record uuid the task links back to.
*
* @return array<string, mixed> The task creation payload.
*
* @spec openspec/changes/hitl-on-shared-tasks/specs/hitl-on-shared-tasks/spec.md#requirement-every-suspension-mirrors-one-shared-task
*/
private function sharedTaskData(array $data, string $approvalRequestId): array {
$payload = [
'state' => 'enabled',
'title' => 'Approval request',
'description' => 'Approve or reject this request in Integriq. Your decision resumes the suspended run.',
'performerType' => 'user',
'appId' => 'integriq',
'metadata' => [
'kind' => 'approval_request',
'approvalRequestId' => $approvalRequestId,
],
];

if ((string)($data['approverGroup'] ?? '') !== '') {
$payload['candidateGroups'] = [(string)$data['approverGroup']];
}

if ((string)($data['requesterUserId'] ?? '') !== '') {
$payload['requester'] = (string)$data['requesterUserId'];
}

if ((string)($data['expiresAt'] ?? '') !== '') {
$payload['expiresAt'] = (string)$data['expiresAt'];
$onTimeout = (string)($data['onTimeout'] ?? '');
if (in_array($onTimeout, ['skip', 'error', 'dead_letter'], true) === true) {
$payload['onTimeout'] = $onTimeout;
}
}

$onReject = (string)($data['onReject'] ?? '');
if (in_array($onReject, ['skip', 'error', 'dead_letter'], true) === true) {
$payload['onReject'] = $onReject;
}

return $payload;
}//end sharedTaskData()

/**
* Close the mirrored shared task after a decision resolved the record
* (hitl-on-shared-tasks D-4), through the shared outcome path: the
* decision was already authorized by this service's own two-layer model,
* and the mirror has no assignee for a completion check to pass.
*
* A missing mirror (`taskUuid` absent: pre-seam rows, or a failed
* mirror) and a mirror already closed by the shared sweep are both
* fine; any failure is logged and swallowed (D-5).
*
* @param array $data The resolved approval_request object data.
* @param string $outcome The shared outcome (`transition:approved`, `transition:rejected` or `dead_letter`).
* @param string $actorUid The deciding user's uid, recorded as the source.
*
* @return void
*
* @spec openspec/changes/hitl-on-shared-tasks/specs/hitl-on-shared-tasks/spec.md#requirement-a-decision-closes-the-mirrored-task
*/
private function closeSharedTask(array $data, string $outcome, string $actorUid): void {
$taskUuid = (string)($data['taskUuid'] ?? '');
if ($this->taskService === null || $taskUuid === '') {
return;
}

try {
$this->taskService->applyTimerOutcome(
uuid: $taskUuid,
outcome: $outcome,
source: 'integriq:' . $actorUid,
reason: sprintf("Approval request resolved as '%s'.", (string)($data['status'] ?? ''))
);
} catch (Throwable $e) {
$this->logger->warning(
'ApprovalService: could not close the mirrored shared task: ' . $e->getMessage(),
['taskUuid' => $taskUuid]
);
}
}//end closeSharedTask()

/**
* Strip sensitive headers (at minimum `Authorization`) from a FlowToken
* snapshot's request slots before persisting it — security-hard
Expand Down
56 changes: 56 additions & 0 deletions openspec/changes/hitl-on-shared-tasks/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# Design: HITL approvals on the shared task service

## D-1: a mirror, not a migration

The approval_request record keeps owning suspend/resume orchestration; the
shared task mirrors its human-facing half (who must act, by when, with what
consequence). Four callers compose resume orchestration around
`ApprovalService` today; moving the record itself is follow-up work with its
own change. The mirror gives the fleet inbox, notification and expiry
machinery a real row NOW without touching any resume path.

## D-2: the mirror is created on the trusted path

`TaskService::import()` (in-process, trusted) rather than `create()`: the
mirror names a requester that is not the acting identity, and it is created
by a service, not over HTTP. The acting identity passed is the requester
when known, else the app id.

## D-3: OpenRegister owns the mirror's expiry

The mirrored task carries `expiresAt` and the record's `onTimeout` (when it
is one of `skip`/`error`/`dead_letter`), so the shared timer sweep closes it
with the declared behaviour. integriq's `ApprovalTimeoutSweepJob` keeps
resolving the approval_request itself. The two sweeps run at the same 300s
cadence and both are idempotent, so the pair converges without coordination:
the record ends `expired`/`dead_letter`, the task ends through its declared
behaviour. Retiring the app-local sweep for mirrored rows is follow-up 1.

## D-4: decisions close the mirror through the outcome path

`completeApproval()` closes the mirror with `transition:approved`;
`reject()` with `transition:rejected`, or `dead_letter` when the record's
`onReject` routed the record there. The outcome path
(`applyTimerOutcome()`) is chosen over `complete()` because the mirror has
no assignee: the decision was authorized by integriq's own two-layer model
(action matrix + approver group) before the close, and re-running the task
service's assignee check against a pooled mirror would refuse a decision
that already happened. The source names the deciding user
(`integriq:<uid>`).

## D-5: the mirror never gates the approval flow

Creation, linking and closing of the mirror are each wrapped: a failure is
logged as a warning and the approval flow proceeds. A missing `taskUuid`
(pre-seam rows, or a failed mirror) simply means no mirror to close. The
inverse guarantee is OpenRegister's: `applyTimerOutcome()` on an
already-terminal task returns it unchanged, so a decision racing the shared
sweep cannot double-close.

## D-6: tests stub the real signatures

integriq's suite runs without the OpenRegister app. The stubs added for
`OCA\OpenRegister\Service\Task\TaskService` and `OCA\OpenRegister\Db\Task`
copy the REAL signatures (`import(array $data, ?string $actor): Task`,
`applyTimerOutcome(string $uuid, string $outcome, string $source, string
$reason): Task`), because a fake that agrees with the caller cannot fail.
55 changes: 55 additions & 0 deletions openspec/changes/hitl-on-shared-tasks/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# HITL approvals on the shared task service

## Why

integriq carries its own human-in-the-loop machinery: `approval_request`
objects with `expiresAt`, `onTimeout` and `onReject`, a 300s
`ApprovalTimeoutSweepJob`, and an imperative approver notification. The
fleet now has one task service in OpenRegister, and wave 2 of the
consolidation moved exactly these semantics into it
(openregister `task-expiry-and-outcomes`): tasks declare `onTimeout` and
`onReject` in the same vocabulary, and the shared sweep enforces `expiresAt`.
Keeping a second copy here means two sweeps, two vocabularies and an
approval inbox nobody shares.

## What changes

This change is the ADOPTION SEAM, not the full retirement. The
`approval_request` record stays the system of record for suspend/resume
orchestration (FlowToken snapshots, resume ordering, consumption), because
that orchestration is composed by `EndpointService`, `FlowRunnerService`,
`SynchronizationService` and two controllers, and ripping it out in the same
PR that introduces the seam would be a half-delete.

- Every `ApprovalService::suspend*()` also creates ONE shared task through
OpenRegister's `TaskService::import()`: candidate group, requester,
`expiresAt`, `onTimeout`, `onReject` and a metadata link to the
approval_request. The task uuid lands on the approval_request as
`taskUuid`.
- Expiry of the mirrored task is OWNED by OpenRegister's timer sweep: the
task declares `onTimeout`, so the shared machinery closes it. integriq's
own sweep keeps resolving the `approval_request` record.
- `completeApproval()` and `reject()` close the mirrored task with the
matching outcome (`approved`, `rejected`, or `dead_letter` when the
record's `onReject` said so), so the shared inbox never shows a decided
approval as open.
- A mirror failure never fails the approval flow: created, closed or
skipped, the approval_request behaviour is unchanged.

## Follow-ups (tracked, not in this PR)

1. Listen to OpenRegister's `TaskTransitionedEvent` for mirrored tasks and
resolve the approval_request from the task side, then retire
`ApprovalTimeoutSweepJob` for mirrored rows.
2. Drive approve/reject from the shared task inbox (task-first), reducing
`ApprovalsController` to the resume orchestration.
3. Translate the mirrored task's title and description.

## Impact

- Affected specs: hitl-on-shared-tasks (new delta), referencing
approval-workflow.
- Affected code: `lib/Service/ApprovalService.php`, test stubs for the
OpenRegister task service.
- Depends on: openregister `task-expiry-and-outcomes` (runtime only; tests
stub the shared service).
Loading
Loading