You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

Greptile Summary

This PR adds contract fixture tests for the advanced workflow plugin seams (approval workflows, policy gates, background monitors, artifact replies, and retry control) introduced in #72383, along with the runtime infrastructure they exercise (host-hook-runtime.ts, host-hook-state.ts, host-hook-workflow.ts, host-hook-cleanup.ts, trusted-tool-policy.ts).

• Retry budget not cleaned up: The FINALIZE_RETRY_BUDGET_KEY global singleton Map in lifecycle-hook-helpers.ts grows without bound — entries are added on every retry but never removed. The test's afterEach calls clearPluginHostRuntimeState(), which does not touch this Map, so re-running the retry-control test in the same process (vitest watch mode) will produce wrong results on the second pass.

Confidence Score: 3/5

Safe to merge with caution — the retry budget leak is a real defect that will surface in watch-mode test runs and gradually in production.

One P1 (retry budget never pruned, causing test isolation failure in watch mode and unbounded memory growth in production) and one P2 (inbound_claim fixture unconditionally claims all messages). The P1 is isolated to a new subsystem but the bug is definite, not speculative, capping the score at 4; the combination with a P2 pulls it to 3.

src/agents/harness/lifecycle-hook-helpers.ts (retry budget Map has no cleanup path) and src/plugins/contracts/advanced-workflow-fixtures.ts (inbound_claim guard)

This is a comment left during a code review.
Path: src/agents/harness/lifecycle-hook-helpers.ts
Line: 99-115

Comment:
**Retry budget Map never pruned — test isolation bug and production leak**

The `FINALIZE_RETRY_BUDGET_KEY` global singleton `Map` accumulates one entry per unique `runId:idempotencyKey` pair and is never cleared. There is no exported helper to reset it, and `clearPluginHostRuntimeState()` only touches `runContextByRunId` and `schedulerJobsByPlugin` — it does not reach this Map.

**Test impact:** the `afterEach` in `advanced-workflow-fixtures.contract.test.ts` calls `clearPluginHostRuntimeState()` but that does nothing here. In vitest watch mode (or any scenario where the same process re-runs the test), the `"retry-run:retry-control-fixture"` key already has `count=2` from the previous run, so the first assertion (`{ action: "revise" }`) will fail and return `{ action: "continue" }` instead.

**Production impact:** every agent run that triggers a retry adds a permanent `Map` entry keyed by `runId`, which is typically a unique ID — the Map grows without bound over the lifetime of the process.

A minimal fix is to expose a reset function (e.g. `clearFinalizeRetryBudgetForTest`) and call it from both `clearPluginHostRuntimeState` (or an equivalent test-reset utility) and the `afterEach` block in the test. Alternatively, treat terminal lifecycle events as the cleanup trigger, the same way `clearPluginRunContext` is called from `dispatchPluginAgentEventSubscriptions`.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/plugins/contracts/advanced-workflow-fixtures.ts
Line: 90-93

Comment:
**`inbound_claim` guard ignores session approval state**

The handler unconditionally returns `handled: true` regardless of whether an approval is actually in `pending` state in the session store. Any message arriving after the plugin is registered — including ones that arrive after the approval resolves — will be silently swallowed with the "approval is pending" reply.

Since these are contract fixtures intended to guide plugin authors, a more accurate fixture would read the approval extension state (which the event context provides via `sessionKey`) and only claim the message when `status === "pending"`. As written, the fixture demonstrates the hook signature correctly but models incorrect conditional-guard behavior that could mislead implementors.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(plugin-sdk): align inbound guard fix..." | Re-trigger Greptile}

Codex automated review: keeping this open.

Keep this PR open. Current main has the merged foundation host-hook contracts, but it does not contain this PR’s unique advanced workflow fixture files or the workflow action/outbound/scheduler/finalize-retry SDK seams those fixtures exercise. It is also stacked on same-author open #72383, so it remains an active implementation candidate rather than conservative cleanup material.

Best possible solution:

Keep this PR open as the advanced workflow contract-fixture layer. Review or resolve #72383 first, then rebase this PR and review the unique fixture files against the accepted workflow APIs with focused contract and security validation for session actions, outbound attachments, scheduled turns, priority next-turn injections, trusted policy state reads, cleanup, and bounded finalize retry behavior.

What I checked:

• current_main_clean: The checkout is on current main SHA fb40ed9 and git status --short produced no output after review. (fb40ed99a7f0)
• advanced_fixture_files_absent: The contract fixture directory contains host-hook-fixture.ts and host-hooks.contract.test.ts, but no advanced-workflow-fixtures.ts or advanced-workflow-fixtures.contract.test.ts. (src/plugins/contracts, fb40ed99a7f0)
• sdk_surface_lacks_workflow_methods: OpenClawPluginApi currently exposes foundation hooks such as session extensions, next-turn injections, trusted policy, UI descriptors, event subscriptions, run context, and scheduler job registration, but not registerSessionAction, sendSessionAttachment, scheduleSessionTurn, or plugin-attributed emitAgentEvent. (src/plugins/types.ts:2337, fb40ed99a7f0)
• gateway_session_action_absent: The plugin host-hook Gateway handlers implement only plugins.uiDescriptors, and the server method list includes plugins.uiDescriptors but not plugins.sessionAction. (src/gateway/server-methods/plugin-host-hooks.ts:10, fb40ed99a7f0)
• protocol_lacks_workflow_schema: The current plugin protocol schema still models PluginJsonValueSchema as Type.Unknown() and defines only UI descriptor params/result; it lacks session-action params/result and the descriptor fields renderer, stateNamespace, and actionIds shown in the PR stack. (src/gateway/protocol/schema/plugins.ts:4, fb40ed99a7f0)
• foundation_fixture_only: The existing fixture registers foundation seams: session extension, tool metadata, Control UI descriptor, lifecycle cleanup, event subscription, scheduler job, command continuation, agent_turn_prepare, heartbeat contribution, and trusted policy. It does not model session actions, outbound attachments, priority resume injection, or bounded finalize retry. (src/plugins/contracts/host-hook-fixture.ts:3, fb40ed99a7f0)

Remaining risk / open question:

• The unmerged diff touches security-sensitive host workflow paths: plugin session actions, outbound attachment delivery, scheduled wakeups, plugin-attributed events, operator scopes, generated protocol clients, and finalize retry budgets. It needs exact-head security review before merge, not closure as obsolete.
• The branch is stacked on [plugin sdk] Add workflow action, outbound, scheduler, and retry host seams #72383. If [plugin sdk] Add workflow action, outbound, scheduler, and retry host seams #72383 changes, lands, or is rejected, this PR should be rebased or retargeted before fixture review.
• The provided review discussion includes real hardening concerns and author follow-ups. Those are useful review material, but they are not evidence that current main already implements this PR’s intended change.

Codex Review notes: model gpt-5.5, reasoning high; reviewed against fb40ed99a7f0.

Superseded by #73384 now that #72287 has merged.\n\nThe advanced workflow fixture proofs from this PR are included in the consolidated follow-up after the workflow seam commits. The fixture commits are still separate in #73384 so reviewers can inspect the approval, policy gate, background monitor, artifact reply, and finalize-retry examples without chasing inherited stack noise here.