# Decision Idempotency Analysis (repository-based)

> Read-only analysis. No implementation. The repository is the source of truth.
> Verified against `feat/agent-knowledge-cache` (post `8e892f9`).

**Scope:** `POST /api/v1/ai/agent-runs/{run}/decision` → `AgentDecisionService::decide()`
and the objects it touches (`AgentRun`, `AgentApprovalRequest`, `ExecutionRequest`).

---

## 1. Current state

`decide()` (`app/Services/Agents/AgentDecisionService.php`) runs, on **every** call:
`recordAnalysisResult` (skipped if analysis already terminal) → pause/disable gate →
`ActionPolicyService::resolve` → branch on the resolved policy. There is **no
"already-decided" short-circuit** — the branch executes on each submission.

| Path | Retry-safe today? | Why |
|---|---|---|
| **always_allow → execute** | ✅ **Yes** (per `run + action_slug`) | `ExecutionRequestService::claim` is atomic on the unique key `agent-run-{run.id}:{slug}`; a duplicate claim returns the stored result and the **native action + notifications never re-run** (`executeNow` returns early on `claim['duplicate']`). Proven by `ManagedAgentsInvariantsTest::execution_request_claim_is_idempotent`. |
| **blocked** | ✅ **Yes** | `recordExecutionResult(blocked)` overwrites `execution_status`; no external effect, no new object. |
| **needs_approval** | ❌ **No** | `ApprovalService::createFromRun` **unconditionally** inserts a new `AgentApprovalRequest`; re-submission creates a **duplicate pending approval**. |
| **Analysis freeze** | ✅ immutable | `recordAnalysisResult` throws once `isAnalysisLocked()`; `proposed_action`/`decision_payload` are frozen after the first terminal write. |

**Which objects can be duplicated:**
- `AgentApprovalRequest` — **YES** (no dedup, no unique constraint; only `index('agent_run_id')`).
- `ExecutionRequest` — **NO** (unique `idempotency_key`).
- `AgentRun` — **NO** (unique `idempotency_key`; `AgentRunService::create` returns the existing run).

**Which side effects can occur twice:**
- Native action execution + notifications (inside `AgentActionExecutor::executeForRun`): **NOT** twice for the same `(run, slug)` — gated by the execution claim. The only way to execute twice is to submit a **different** `action_slug` for the same run (a different execution key), which the analysis-freeze does not prevent at the branch level.
- **Duplicate pending approvals**: **YES** — the concrete, realistic double-effect under at-least-once delivery.

**Net:** the real idempotency gap is (a) **duplicate approvals** on the `needs_approval` branch, and (b) the absence of a **one-decision-per-run** guard, which also permits a re-submit with a different slug to branch again.

---

## 2. Existing identifiers

- **Natural idempotency key?** Yes. `agent_runs.idempotency_key` is unique (run creation is idempotent). For the *decision*, the run itself is the boundary.
- **Can `agent_run_id` be the decision uniqueness boundary?** **Yes — it is the natural business key.** A run carries exactly **one** frozen `proposed_action` (analysis lock), so "one decision per run" is the intended invariant. Execution is already keyed per `(run, slug)`; approval should be **one pending per run**.
- **Is there already a "decide once" invariant?** **Partially.** `isAnalysisLocked()` freezes the decision **data** (`DECISION_FIELDS`) after the first terminal write — but `decide()` does **not** short-circuit the **action** (policy resolution + branch) for an already-decided run. So the invariant holds for the recorded decision, not for repeated side effects.

---

## 3. Approval duplication

**Exactly how it happens today:**
```
decide() → resolve = needs_approval → requireApproval()
        → ApprovalService::createFromRun($run, …)
        → AgentApprovalRequest::create([... status = 'pending' ...])   // unconditional INSERT
```
There is no pre-insert lookup and no DB uniqueness on `agent_run_id` (schema has only
`index('agent_run_id')`). Each re-submission of the same decision inserts another
`pending` approval for the same run.

**Can it be made idempotent without changing the architecture? Yes — additively:**
- **App-level:** `requireApproval` becomes a `firstOrCreate` on the active pending approval for the run — return the existing `pending` approval instead of inserting.
- **DB backstop (mirrors the handoff/policy/G8 pattern):** an active-scoped unique guard = a STORED generated column `active_approval_key` = `agent_run_id` **only while `status = 'pending'`**, else NULL, plus a unique index. NULLs are distinct → only **pending** rows are constrained (settled approvals — approved/rejected/cancelled/expired — are unconstrained history), so at most **one pending approval per run**. A concurrent double-decide's second insert violates the guard → caught → resolved to the existing approval.

Both are additive; neither changes the approval lifecycle, the one-shot `decide()`
semantics, or the human-decision flow.

---

## 4. Execution duplication

- **Is `always_allow` truly idempotent?** **Yes**, per `(run, action_slug)`. `ExecutionRequestService::claim` (unique `idempotency_key = agent-run-{run.id}:{slug}`) returns the original + `duplicate=true` on any repeat; `executeNow` then returns the stored outcome **without re-running** `AgentActionExecutor::executeForRun`. So the native action and its notifications execute **exactly once** for a given `(run, slug)`.
- **Can notifications / actions / external calls execute twice?** **No** for the same `(run, slug)`. The only residual path is a re-submit with a **different** `action_slug` (a different execution key) — which a **one-decision-per-run** guard (§5) closes by refusing to re-branch an already-decided run.

Execution therefore needs **no change**; the claim key already makes it idempotent.

---

## 5. Recommended architecture

| Option | Assessment |
|---|---|
| **A — caller retry discipline only** | ❌ Rejected. At-least-once orchestration should not depend on "check-before-retry" alone. |
| **B — `agent_run_id` as the decision business key** | ✅ **Recommended (primary).** `decide()` short-circuits: if the run is **already decided** (analysis terminal from a prior call **and** an execution result or pending approval exists), reconstruct and return the **prior** outcome instead of re-branching. Makes the whole endpoint idempotent per run and blocks different-slug re-branching. Uses the existing analysis-lock invariant + `agent_run_id`; a guard clause + a small "reconstruct prior result" helper — additive, no pipeline change. |
| **C — explicit decision idempotency keys** | ➖ Unnecessary. `agent_run_id` is already the natural one-decision-per-run boundary; a separate key adds a concept without benefit and duplicates what the run already guarantees. |
| **D — active-scoped unique guard on approvals** | ✅ **Recommended (backstop).** One `pending` approval per `agent_run_id`, enforced by a generated column + unique index (as in §3). Closes the **concurrent** double-decide race that B (application-level) alone cannot. Execution already has its own claim guard, so no execution change. |

**Recommendation: B + D.**
- **B** (primary) makes `decide()` idempotent for the common **sequential** retry: a replay returns the existing outcome, never re-branches, never re-approves.
- **D** (backstop) closes the **concurrent** race: two simultaneous `needs_approval` decides can't create two pending approvals; the loser catches the unique violation and resolves to the existing approval (the same pattern already proven for handoffs and policies).
- **Execution:** unchanged — the `(run, slug)` claim key already guarantees single execution.

Result: `POST /agent-runs/{run}/decision` becomes idempotent per run under both sequential and concurrent at-least-once delivery, with **no** reliance on caller retry discipline.

---

## 6. Constraints — compliance of the recommendation

| Constraint | B + D compliance |
|---|---|
| Preserve the decision pipeline | ✅ `decide()` gains a guard clause + reconstruct; the resolve→branch logic is unchanged for first-time decisions. |
| Preserve the approval architecture | ✅ Lifecycle, one-shot decide, and human flow untouched; only a firstOrCreate + a DB guard on the pending row are added. |
| Preserve the execution architecture | ✅ No change — already claim-idempotent. |
| Additive only | ✅ A guard clause + one generated column + one unique index; no removals, no signature breaks. |
| Backward compatible | ✅ First-time decisions behave identically; existing rows already satisfy "≤1 pending per run" (to be verified pre-migration, like G8/handoff). |
| Numu remains sole business authority | ✅ All logic stays inside `AgentDecisionService`/`ApprovalService`/the DB; Make gains no authority. |

---

## 7. Deliverable status

This is the repository-verified Decision Idempotency Analysis. **No code was changed.**
Recommended next step (on approval): implement **B + D** as an additive change on
`feat/agent-knowledge-cache` — a `decide()` idempotency guard keyed on `agent_run_id`
plus an active-scoped unique guard on pending approvals — with the same
verify-before-migrate discipline used for the G8 and Handoff guards (prove zero
existing violations, MariaDB + sqlite parity, rolled-back real-DB proof, full Agents
suite). Make orchestration remains **not implemented** until this idempotency work is
approved.

**Open item for pre-implementation verification:** confirm the production/dev
`agent_approval_requests` data currently has **≤1 pending approval per `agent_run_id`**
(0 violations) so the active-approval unique guard can be added cleanly; if any run has
multiple pending approvals, those must be settled/cancelled first.