Redesign the webhook host to be an explicit unambiguous state machine #1

New Issue

2026-07-21T14:10:11+02:00

StanPonomarev commented

2026-07-21 14:10:11 +02:00

With the next_state function and everything. webhooks that come are transformed to events, the state machine state is fetched from the db and evolved, and state machine listeners perform asynchronous side effects.

StanPonomarev commented

2026-07-21 14:18:52 +02:00

/agent plan

agentci commented

2026-07-21 14:18:53 +02:00

Implementation Plan: Explicit Webhook State Machine

Objective

Replace the current implicit lifecycle spread across src/agentci/api/webhook.py, src/agentci/worker.py, workflow methods, and arbitrary JobStore.update_job() calls with one explicit, persisted command state machine. Every accepted Gitea command webhook becomes a typed domain event; every event is applied through a pure next_state() function; the new state and durable listener work are committed atomically; asynchronous listeners own permission checks, Gitea status comments, workflow execution, and recovery side effects.

Preserve the existing command set, FIFO single-job execution, one-comment UX, completed workflow artifacts, OpenCode readiness gating, and conservative restart policy: queued jobs survive, while a job that was running when the service stopped is aborted and marked failed rather than replayed.

Chosen Design

Use one aggregate per command/job, not one aggregate per issue or pull request. The aggregate ID is a deterministic UUIDv5 derived from the non-empty Gitea delivery ID, so webhook retries address the same state before any database lookup.
Keep current relational jobs rows as the persisted state projection rather than introducing a second JSON state table. Add an append-only event inbox and a durable listener-task outbox for deduplication and asynchronous delivery.
Implement the state machine in project code with frozen dataclasses and Pydantic event serialization; do not add a state-machine dependency.
Keep workflows as durable artifacts referenced by job state. Workflow persistence is not a second job lifecycle; job status, stage, workflow/session links, comments, and terminal outcomes may only change through state-machine events.
Provide two durable listener queues. control handles permission, comments, and recovery work; jobs executes workflows with concurrency one. This prevents a long OpenCode turn from blocking webhook authorization and comment reconciliation while retaining current FIFO job execution.
Guarantee atomic state/outbox persistence and at-least-once listener invocation. Do not claim exactly-once external effects. Make comment reconciliation idempotent with a deterministic hidden job marker, and never replay an interrupted workflow execution.

Domain Contract

Create src/agentci/domain/state_machine.py with these public types:

def next_state(state: JobState | None, event: JobEvent) -> Transition:
    ...

JobState is immutable and contains the existing job identity/target fields plus delivery_id, raw command_body, optional parsed kind, parsed message, lifecycle status, human-readable stage, error, workflow_id, runtime_session_id, accepted_comment_id, final comment_body, and monotonically increasing version.

Transition contains the complete replacement state and a tuple of typed listener notifications. It contains no coroutines, clients, database handles, clocks, random generation, or filesystem paths generated at transition time.

Use these lifecycle states:

State	Meaning
`received`	The command webhook is durable and awaits permission evaluation.
`queued`	Permission, syntax, and issue/PR placement are valid; execution is pending.
`running`	The single job listener has started workflow execution.
`succeeded`	Workflow execution and artifact persistence completed.
`rejected`	An expected user-facing permission, command, or workflow precondition failed.
`failed`	An unexpected execution/infrastructure failure or service interruption occurred.

Define a discriminated JobEvent union in src/agentci/domain/events.py:

Event	Legal source state	State change and notifications
`CommandReceived`	no state	Create `received` state; enqueue `authorize` on `control`.
`PermissionGranted`	`received`	Run existing pure `parse_command()` and `resolve_job_kind()`. Valid input becomes `queued` and enqueues `execute` on `jobs` plus `reconcile_comment` on `control`; a `CommandError` becomes `rejected` and only reconciles the comment.
`PermissionDenied`	`received`	Become `rejected` with the existing write-permission message; reconcile the comment.
`JobStarted`	`queued`	Become `running` with stage `starting`; reconcile the comment.
`JobProgress`	`running`	Replace the stage only.
`WorkflowLinked`	`running`	Persist `workflow_id` and the supplied stage.
`RuntimeSessionLinked`	`running`	Persist `runtime_session_id`.
`JobCompleted`	`running`	Become `succeeded`, stage `completed`, retain the workflow-produced final comment body, and reconcile the comment.
`JobRejected`	`running`	Become `rejected`, persist the safe reason, and reconcile the comment.
`JobFailed`	`running`	Become `failed`, persist the stage and sanitized/truncated error, enqueue `fail_workflow`, and reconcile the comment.
`ServiceRestarted`	`running`	Become `failed`/`interrupted`, enqueue session abort, active-workflow failure, and comment reconciliation. It is a no-op for every other lifecycle state.
`CommentLinked`	any existing state	Persist the discovered/created Gitea comment ID without changing lifecycle status. Repeating the same ID is idempotent.

Reject every other state/event pair with InvalidTransition; do not silently coerce invalid transitions. Duplicate event IDs are filtered by the store before next_state() and return the already persisted state without notifications. Terminal states remain terminal apart from CommentLinked metadata.

Keep rendering pure in render_job_comment(state). Prefix every operational comment with , then render queued, started, rejected, failed, or the workflow-provided successful body. Continue embedding existing plan/implementation markers inside successful bodies.

Persistence and Atomic Evolution

Add 003_state_machine.sql and update src/agentci/adapters/database.py, job_store.py, and storage.py as follows:

Rebuild jobs so kind may be null while permission is pending, and add delivery_id, command_body, comment_body, and version. Preserve all existing IDs, statuses, stages, errors, workflow/session links, timestamps, accepted_comment_id, and legacy started_comment_id. Backfill delivery_id by joining deliveries.comment_id to jobs.comment_id, using legacy:<job-id> only if old data has no matching delivery.
Add a unique index on jobs.delivery_id and retain the existing queue/target indexes. Continue returning both accepted and legacy started comment IDs from operational_comment_ids() so old bot comments remain excluded from issue context.
Add job_events(event_id PRIMARY KEY, job_id, event_type, payload_json, created_at). Backfill one synthetic event per existing deliveries row so historical deliveries remain deduplicated after the redesign.
Add listener_tasks(id, job_id, state_version, listener, queue, status, attempts, available_at, error, created_at, started_at, finished_at) with a uniqueness constraint on (job_id, state_version, listener) and a claim index on (queue, status, available_at, id).
Backfill execute and comment-reconciliation tasks for legacy queued jobs. Do not enqueue execution for terminal jobs. Legacy running jobs are handled by startup recovery.
Replace public mutation methods record_delivery(), enqueue(), claim_next(), update_job(), set_job_comment(), and recover_running() with evolve(event_id, event), get_job_state(), claim_listener_task(queue), complete_listener_task(), retry_listener_task(), fail_listener_task(), and running_job_states().
In evolve(), use BEGIN IMMEDIATE, insert the event inbox row, load the current job state, call next_state(), persist the full state with an incremented version, insert all listener tasks, and commit. Any failure rolls back the event, state, and tasks together.

Keep SQLite WAL, foreign keys, and the 30-second busy timeout. The explicit transaction plus one version increment per event makes concurrent webhook retries and internal events deterministic even if listener concurrency is increased later.

Webhook Boundary

Refactor src/agentci/api/webhook.py into transport validation and event conversion only:

Preserve HMAC verification, supported event names, action == "created", issue/PR extraction, owner fallback, bot-user filtering, and non-command 204 behavior.
Require a non-empty X-Gitea-Delivery for a supported /agent comment; return 400 when absent because unkeyed delivery deduplication is ambiguous.
Convert any body beginning with /agent into CommandReceived without calling Gitea or parsing command syntax. This preserves the current permission-first behavior: an unauthorized malformed command still receives the permission rejection rather than syntax details.
Call the state-machine host once. Return 202 when the event was newly persisted, 200 for a duplicate delivery, 204 for ignored payloads, 400 for malformed payloads, 401 for invalid signatures, and 500 if durable evolution fails so Gitea can redeliver.
Remove all direct permission checks, job construction, storage mutation, and comment creation from the route.

Listener Runtime

Introduce src/agentci/state_machine.py as the application service around storage and src/agentci/listeners.py for listener dispatch. Rewrite src/agentci/worker.py to run the two task queues and startup recovery.

authorize: load current state, call GiteaClient.has_write_permission(), then emit a uniquely identified PermissionGranted or PermissionDenied event.
execute: wait until OpenCodeClient.ready() before claiming job work. Emit JobStarted, invoke the dispatcher, emit JobCompleted with its final comment body, map JobRejected to the corresponding event, and map every other exception to JobFailed using the latest persisted stage and the existing 1,000-character safe error format.
reconcile_comment: reload current state so stale queued/running tasks always publish the newest status. If no comment ID is stored, search issue comments for the hidden job marker before creating one; emit CommentLinked after discovery or creation. If an ID exists, update that comment. This closes the crash window between Gitea creation and local ID persistence and preserves one comment per job.
fail_workflow: retain WorkflowStore.fail_job_workflow(), which only changes an active workflow and therefore does not invalidate a previously completed workflow used by a failed follow-up job.
abort_sessions: reuse the current workflow/session lookup and one-shot fix workspace convention, then call OpenCodeClient.abort() for each known session.
Retry idempotent control tasks up to three times with 2 ** attempts seconds of backoff. Mark exhausted comment tasks failed without changing a successful job outcome. The Gitea client’s own request retries remain in place.
Do not retry execute after it has emitted JobStarted. Its listener catches normal workflow errors and emits a terminal event. If the process dies, startup recovery emits ServiceRestarted instead of replaying the task.
On startup, reset abandoned running control tasks to pending, mark abandoned jobs tasks failed, and emit ServiceRestarted for every persisted running job. Queued jobs and pending tasks remain intact.

Update src/agentci/app.py and container.py to construct the state-machine host/listener registry, start the worker loops, stop them through the existing lifespan event/cancellation path, and close clients as today. Keep readiness behavior unchanged: the service can ingest commands while OpenCode is unavailable, but the jobs queue does not start them.

Workflow Refactor

Make workflow code a side effect behind the execute listener while removing its ability to mutate job lifecycle directly:

Change Dispatcher.dispatch() and each public workflow entry point in workflows/plan.py, implement.py, and pull_request.py to return the final comment body instead of updating Gitea.
Inject a small JobReporter interface with progress(stage), link_workflow(workflow_id, stage), and link_runtime_session(session_id). Its implementation emits state-machine events. Replace every storage.update_job() call in workflows and ChangeSet/CodeReviewLoop with the corresponding reporter call.
Keep workflow artifact operations (create_workflow(), update_workflow(), lookups, completed/failed status), Git, development setup, Gitea repository/PR queries and PR creation, and OpenCode calls inside workflow listeners as asynchronous side effects.
Remove update_job_comment() from workflows/common.py. Have plan completion, discussion, implementation, iteration, and fix paths build and return the same existing result text and review findings. The state-machine comment listener publishes it.
Stop mutating the in-memory Job object. Replace workflow parameters with immutable JobState snapshots and use reporter events to persist workflow/session links immediately after those external resources are created.
Preserve all existing rejection guards: missing/non-OpenCode plans, missing sessions/artifacts, existing open or merged agent PRs, mismatched/closed PRs, and no generated changes.

Edge Cases and Operational Rules

Multiple distinct commands on the same issue/PR remain separate aggregates and execute globally FIFO on the single jobs queue.
Duplicate delivery IDs never rerun permission checks, comments, or workflows, even when the original HTTP response was lost.
Unsupported events, edited/deleted comments, bot comments, and ordinary discussion create no state or listener tasks.
Permission and syntax/location rejections are terminal persisted jobs and receive one reconciled comment; they never enter the execution queue.
A status-comment webhook from the bot is ignored, preventing feedback loops.
A queued-comment failure does not block execution. A later started or terminal reconciliation searches by marker and creates/updates the one operational comment.
A crash after comment creation but before CommentLinked is repaired by marker discovery. Other non-idempotent workflow side effects are not replayed after a crash; the job is failed as interrupted, matching current safety policy.
Keep started_comment_id only as legacy read data. New jobs use accepted_comment_id exclusively.
Add event_id, state_version, listener, and queue to structured logging fields while retaining job/workflow/stage context.

Tests

Add or revise tests while keeping every Python file under the repository’s 250-line limit:

tests/test_state_machine.py: table-test every legal transition, emitted notification, stage/link update, terminal behavior, idempotent CommentLinked, restart no-op outside running, command parsing after permission, and representative invalid transitions. Assert the input state is not mutated and repeated calls are deterministic.
tests/test_storage.py plus a focused state-store test file: verify atomic state/event/task writes, duplicate event suppression, monotonic versions, FIFO task claiming by queue, concurrent duplicate evolution, retry scheduling, queued/running migration backfills, historical delivery dedupe, and preservation of workflows/session/comment IDs.
tests/test_webhook.py: replace direct Gitea/storage fakes with a fake state-machine host; cover issue and PR conversion, multiline messages retained in raw input, status codes, missing delivery ID, bot/non-command/non-created/unsupported events, malformed JSON/payloads, and duplicate delivery responses.
tests/test_listeners.py: cover permission outcomes and transport failures, execution success/rejection/failure events, latest-stage error reporting, OpenCode readiness gating, comment create/update/marker recovery, stale reconciliation publishing current state, control retries, and no execution retry after start.
tests/test_worker.py: preserve session-abort coverage and add startup handling for abandoned control tasks, interrupted executions, active workflow failure, queued task survival, and one-comment recovery.
Adapt workflow tests to assert reporter events and returned final bodies instead of update_job()/direct comment calls. Retain setup ordering, code-review loop, workflow artifact, and completed-workflow protection tests.
Update README.md state/recovery documentation to describe durable event evolution, asynchronous listener processing, eventual status comments, at-least-once control effects, single FIFO execution, and interruption semantics.

Verification and Acceptance Criteria

Run:

uv sync
uv run ruff check .
uv run pyright
uv run pytest
docker compose config

The implementation is complete when all job lifecycle mutations flow through next_state() and evolve(), webhook handlers and workflows no longer call arbitrary job-update/comment methods, every state change and listener task is atomically durable, duplicate deliveries produce no duplicate side effects, one operational comment is maintained through retries and crashes, queued jobs wait for OpenCode and survive restart, running jobs are failed/aborted rather than replayed, legacy SQLite data migrates without losing workflows or resumable session metadata, and the existing command behavior and workflow outputs remain intact.

Remaining review findings

The plan has five material correctness gaps. No matching duplicate issues were found in the required issue searches.

MAJOR: A crash before JobStarted can permanently strand a queued job — `Listener Runtime: execute handling and startup recovery`

The jobs listener claims an execute task before emitting JobStarted, while startup recovery marks every abandoned jobs task failed and emits ServiceRestarted only for jobs already in running. A process exit after claim but before JobStarted therefore leaves the aggregate queued with its sole execute task failed. This contradicts the stated rule that queued jobs survive restart and are eventually executed.

Recommendation: On recovery, reset an abandoned execute task to pending when its persisted job is still queued; only fail the task and emit ServiceRestarted when the job is running. Add a test for termination in the claim-to-JobStarted window.

MAJOR: Exhausted control tasks have no consistent recovery outcome — `Listener Runtime: control retries; Verification and Acceptance Criteria`

All control tasks stop after three attempts, but the plan defines no state transition or durable repair path when authorization exhausts retries. Such a job remains received forever. Exhausted comment, abort-session, and fail-workflow tasks can likewise permanently violate the acceptance claims that one operational comment is maintained and interrupted work is aborted/failed.

Recommendation: Define task-specific exhaustion semantics. Authorization infrastructure failure should produce an explicit terminal event or remain durably retriable. Required reconciliation and recovery work should remain retryable, enter a replayable dead-letter state, or have the acceptance guarantees narrowed. Test each exhausted task type and its resulting aggregate/external state.

MAJOR: JobRejected can leave a linked workflow active — `Domain Contract: JobRejected; Workflow Refactor: rejection guards`

JobRejected is legal from running and only enqueues comment reconciliation. The plan also permits workflows to link an active workflow before later rejection guards such as no generated changes are evaluated. Unlike JobFailed and ServiceRestarted, that path has no fail_workflow notification, so the job can be terminally rejected while its durable workflow remains active.

Recommendation: Either enqueue fail_workflow for JobRejected when a workflow is linked, or make the workflow contract explicitly finalize its artifact before emitting JobRejected. Add a test that rejects after WorkflowLinked and verifies no active workflow remains.

MAJOR: Listener-task claiming is not specified or tested as atomic — `Persistence and Atomic Evolution: listener_tasks and claim_listener_task; Tests: storage`

The plan defines claim_listener_task and FIFO tests but does not require a single transaction that selects and marks a task running, nor a concurrency test proving one task cannot be claimed twice. Duplicate claims are especially unsafe for execute because workflow effects are intentionally non-idempotent and not replayable.

Recommendation: Specify an atomic SQLite claim operation, including its transaction and eligibility predicate, and add concurrent-claimer coverage asserting that only one worker receives each execute task.

MAJOR: Marker recovery trusts an unverified issue comment — `Listener Runtime: reconcile_comment; Edge Cases: one-comment recovery`

Crash recovery searches issue comments for the hidden job marker and treats any match as the operational comment. Hidden markers are correlation identifiers, not authentication; users can view and reproduce them. Without checking the comment author and handling multiple matches deterministically, recovery can link a user-authored spoof, after which updates can fail or target the wrong comment.

Recommendation: Only recover a marker from a comment authored by the configured agent/bot identity, define deterministic handling for zero or multiple valid matches, and test spoofed user comments alongside the real bot comment.

# Implementation Plan: Explicit Webhook State Machine ## Objective Replace the current implicit lifecycle spread across `src/agentci/api/webhook.py`, `src/agentci/worker.py`, workflow methods, and arbitrary `JobStore.update_job()` calls with one explicit, persisted command state machine. Every accepted Gitea command webhook becomes a typed domain event; every event is applied through a pure `next_state()` function; the new state and durable listener work are committed atomically; asynchronous listeners own permission checks, Gitea status comments, workflow execution, and recovery side effects. Preserve the existing command set, FIFO single-job execution, one-comment UX, completed workflow artifacts, OpenCode readiness gating, and conservative restart policy: queued jobs survive, while a job that was running when the service stopped is aborted and marked failed rather than replayed. ## Chosen Design - Use one aggregate per command/job, not one aggregate per issue or pull request. The aggregate ID is a deterministic UUIDv5 derived from the non-empty Gitea delivery ID, so webhook retries address the same state before any database lookup. - Keep current relational `jobs` rows as the persisted state projection rather than introducing a second JSON state table. Add an append-only event inbox and a durable listener-task outbox for deduplication and asynchronous delivery. - Implement the state machine in project code with frozen dataclasses and Pydantic event serialization; do not add a state-machine dependency. - Keep `workflows` as durable artifacts referenced by job state. Workflow persistence is not a second job lifecycle; job status, stage, workflow/session links, comments, and terminal outcomes may only change through state-machine events. - Provide two durable listener queues. `control` handles permission, comments, and recovery work; `jobs` executes workflows with concurrency one. This prevents a long OpenCode turn from blocking webhook authorization and comment reconciliation while retaining current FIFO job execution. - Guarantee atomic state/outbox persistence and at-least-once listener invocation. Do not claim exactly-once external effects. Make comment reconciliation idempotent with a deterministic hidden job marker, and never replay an interrupted workflow execution. ## Domain Contract Create `src/agentci/domain/state_machine.py` with these public types: ```python def next_state(state: JobState | None, event: JobEvent) -> Transition: ... ``` `JobState` is immutable and contains the existing job identity/target fields plus `delivery_id`, raw `command_body`, optional parsed `kind`, parsed `message`, lifecycle `status`, human-readable `stage`, `error`, `workflow_id`, `runtime_session_id`, `accepted_comment_id`, final `comment_body`, and monotonically increasing `version`. `Transition` contains the complete replacement state and a tuple of typed listener notifications. It contains no coroutines, clients, database handles, clocks, random generation, or filesystem paths generated at transition time. Use these lifecycle states: | State | Meaning | | --- | --- | | `received` | The command webhook is durable and awaits permission evaluation. | | `queued` | Permission, syntax, and issue/PR placement are valid; execution is pending. | | `running` | The single job listener has started workflow execution. | | `succeeded` | Workflow execution and artifact persistence completed. | | `rejected` | An expected user-facing permission, command, or workflow precondition failed. | | `failed` | An unexpected execution/infrastructure failure or service interruption occurred. | Define a discriminated `JobEvent` union in `src/agentci/domain/events.py`: | Event | Legal source state | State change and notifications | | --- | --- | --- | | `CommandReceived` | no state | Create `received` state; enqueue `authorize` on `control`. | | `PermissionGranted` | `received` | Run existing pure `parse_command()` and `resolve_job_kind()`. Valid input becomes `queued` and enqueues `execute` on `jobs` plus `reconcile_comment` on `control`; a `CommandError` becomes `rejected` and only reconciles the comment. | | `PermissionDenied` | `received` | Become `rejected` with the existing write-permission message; reconcile the comment. | | `JobStarted` | `queued` | Become `running` with stage `starting`; reconcile the comment. | | `JobProgress` | `running` | Replace the stage only. | | `WorkflowLinked` | `running` | Persist `workflow_id` and the supplied stage. | | `RuntimeSessionLinked` | `running` | Persist `runtime_session_id`. | | `JobCompleted` | `running` | Become `succeeded`, stage `completed`, retain the workflow-produced final comment body, and reconcile the comment. | | `JobRejected` | `running` | Become `rejected`, persist the safe reason, and reconcile the comment. | | `JobFailed` | `running` | Become `failed`, persist the stage and sanitized/truncated error, enqueue `fail_workflow`, and reconcile the comment. | | `ServiceRestarted` | `running` | Become `failed`/`interrupted`, enqueue session abort, active-workflow failure, and comment reconciliation. It is a no-op for every other lifecycle state. | | `CommentLinked` | any existing state | Persist the discovered/created Gitea comment ID without changing lifecycle status. Repeating the same ID is idempotent. | Reject every other state/event pair with `InvalidTransition`; do not silently coerce invalid transitions. Duplicate event IDs are filtered by the store before `next_state()` and return the already persisted state without notifications. Terminal states remain terminal apart from `CommentLinked` metadata. Keep rendering pure in `render_job_comment(state)`. Prefix every operational comment with ``, then render queued, started, rejected, failed, or the workflow-provided successful body. Continue embedding existing plan/implementation markers inside successful bodies. ## Persistence and Atomic Evolution Add `003_state_machine.sql` and update `src/agentci/adapters/database.py`, `job_store.py`, and `storage.py` as follows: 1. Rebuild `jobs` so `kind` may be null while permission is pending, and add `delivery_id`, `command_body`, `comment_body`, and `version`. Preserve all existing IDs, statuses, stages, errors, workflow/session links, timestamps, `accepted_comment_id`, and legacy `started_comment_id`. Backfill `delivery_id` by joining `deliveries.comment_id` to `jobs.comment_id`, using `legacy:<job-id>` only if old data has no matching delivery. 2. Add a unique index on `jobs.delivery_id` and retain the existing queue/target indexes. Continue returning both accepted and legacy started comment IDs from `operational_comment_ids()` so old bot comments remain excluded from issue context. 3. Add `job_events(event_id PRIMARY KEY, job_id, event_type, payload_json, created_at)`. Backfill one synthetic event per existing `deliveries` row so historical deliveries remain deduplicated after the redesign. 4. Add `listener_tasks(id, job_id, state_version, listener, queue, status, attempts, available_at, error, created_at, started_at, finished_at)` with a uniqueness constraint on `(job_id, state_version, listener)` and a claim index on `(queue, status, available_at, id)`. 5. Backfill `execute` and comment-reconciliation tasks for legacy queued jobs. Do not enqueue execution for terminal jobs. Legacy running jobs are handled by startup recovery. 6. Replace public mutation methods `record_delivery()`, `enqueue()`, `claim_next()`, `update_job()`, `set_job_comment()`, and `recover_running()` with `evolve(event_id, event)`, `get_job_state()`, `claim_listener_task(queue)`, `complete_listener_task()`, `retry_listener_task()`, `fail_listener_task()`, and `running_job_states()`. 7. In `evolve()`, use `BEGIN IMMEDIATE`, insert the event inbox row, load the current job state, call `next_state()`, persist the full state with an incremented version, insert all listener tasks, and commit. Any failure rolls back the event, state, and tasks together. Keep SQLite WAL, foreign keys, and the 30-second busy timeout. The explicit transaction plus one version increment per event makes concurrent webhook retries and internal events deterministic even if listener concurrency is increased later. ## Webhook Boundary Refactor `src/agentci/api/webhook.py` into transport validation and event conversion only: - Preserve HMAC verification, supported event names, `action == "created"`, issue/PR extraction, owner fallback, bot-user filtering, and non-command `204` behavior. - Require a non-empty `X-Gitea-Delivery` for a supported `/agent` comment; return `400` when absent because unkeyed delivery deduplication is ambiguous. - Convert any body beginning with `/agent` into `CommandReceived` without calling Gitea or parsing command syntax. This preserves the current permission-first behavior: an unauthorized malformed command still receives the permission rejection rather than syntax details. - Call the state-machine host once. Return `202` when the event was newly persisted, `200` for a duplicate delivery, `204` for ignored payloads, `400` for malformed payloads, `401` for invalid signatures, and `500` if durable evolution fails so Gitea can redeliver. - Remove all direct permission checks, job construction, storage mutation, and comment creation from the route. ## Listener Runtime Introduce `src/agentci/state_machine.py` as the application service around storage and `src/agentci/listeners.py` for listener dispatch. Rewrite `src/agentci/worker.py` to run the two task queues and startup recovery. - `authorize`: load current state, call `GiteaClient.has_write_permission()`, then emit a uniquely identified `PermissionGranted` or `PermissionDenied` event. - `execute`: wait until `OpenCodeClient.ready()` before claiming job work. Emit `JobStarted`, invoke the dispatcher, emit `JobCompleted` with its final comment body, map `JobRejected` to the corresponding event, and map every other exception to `JobFailed` using the latest persisted stage and the existing 1,000-character safe error format. - `reconcile_comment`: reload current state so stale queued/running tasks always publish the newest status. If no comment ID is stored, search issue comments for the hidden job marker before creating one; emit `CommentLinked` after discovery or creation. If an ID exists, update that comment. This closes the crash window between Gitea creation and local ID persistence and preserves one comment per job. - `fail_workflow`: retain `WorkflowStore.fail_job_workflow()`, which only changes an active workflow and therefore does not invalidate a previously completed workflow used by a failed follow-up job. - `abort_sessions`: reuse the current workflow/session lookup and one-shot fix workspace convention, then call `OpenCodeClient.abort()` for each known session. - Retry idempotent `control` tasks up to three times with `2 ** attempts` seconds of backoff. Mark exhausted comment tasks failed without changing a successful job outcome. The Gitea client’s own request retries remain in place. - Do not retry `execute` after it has emitted `JobStarted`. Its listener catches normal workflow errors and emits a terminal event. If the process dies, startup recovery emits `ServiceRestarted` instead of replaying the task. - On startup, reset abandoned running `control` tasks to pending, mark abandoned `jobs` tasks failed, and emit `ServiceRestarted` for every persisted `running` job. Queued jobs and pending tasks remain intact. Update `src/agentci/app.py` and `container.py` to construct the state-machine host/listener registry, start the worker loops, stop them through the existing lifespan event/cancellation path, and close clients as today. Keep readiness behavior unchanged: the service can ingest commands while OpenCode is unavailable, but the `jobs` queue does not start them. ## Workflow Refactor Make workflow code a side effect behind the `execute` listener while removing its ability to mutate job lifecycle directly: - Change `Dispatcher.dispatch()` and each public workflow entry point in `workflows/plan.py`, `implement.py`, and `pull_request.py` to return the final comment body instead of updating Gitea. - Inject a small `JobReporter` interface with `progress(stage)`, `link_workflow(workflow_id, stage)`, and `link_runtime_session(session_id)`. Its implementation emits state-machine events. Replace every `storage.update_job()` call in workflows and `ChangeSet`/`CodeReviewLoop` with the corresponding reporter call. - Keep workflow artifact operations (`create_workflow()`, `update_workflow()`, lookups, completed/failed status), Git, development setup, Gitea repository/PR queries and PR creation, and OpenCode calls inside workflow listeners as asynchronous side effects. - Remove `update_job_comment()` from `workflows/common.py`. Have plan completion, discussion, implementation, iteration, and fix paths build and return the same existing result text and review findings. The state-machine comment listener publishes it. - Stop mutating the in-memory `Job` object. Replace workflow parameters with immutable `JobState` snapshots and use reporter events to persist workflow/session links immediately after those external resources are created. - Preserve all existing rejection guards: missing/non-OpenCode plans, missing sessions/artifacts, existing open or merged agent PRs, mismatched/closed PRs, and no generated changes. ## Edge Cases and Operational Rules - Multiple distinct commands on the same issue/PR remain separate aggregates and execute globally FIFO on the single `jobs` queue. - Duplicate delivery IDs never rerun permission checks, comments, or workflows, even when the original HTTP response was lost. - Unsupported events, edited/deleted comments, bot comments, and ordinary discussion create no state or listener tasks. - Permission and syntax/location rejections are terminal persisted jobs and receive one reconciled comment; they never enter the execution queue. - A status-comment webhook from the bot is ignored, preventing feedback loops. - A queued-comment failure does not block execution. A later started or terminal reconciliation searches by marker and creates/updates the one operational comment. - A crash after comment creation but before `CommentLinked` is repaired by marker discovery. Other non-idempotent workflow side effects are not replayed after a crash; the job is failed as interrupted, matching current safety policy. - Keep `started_comment_id` only as legacy read data. New jobs use `accepted_comment_id` exclusively. - Add `event_id`, `state_version`, `listener`, and `queue` to structured logging fields while retaining job/workflow/stage context. ## Tests Add or revise tests while keeping every Python file under the repository’s 250-line limit: 1. `tests/test_state_machine.py`: table-test every legal transition, emitted notification, stage/link update, terminal behavior, idempotent `CommentLinked`, restart no-op outside `running`, command parsing after permission, and representative invalid transitions. Assert the input state is not mutated and repeated calls are deterministic. 2. `tests/test_storage.py` plus a focused state-store test file: verify atomic state/event/task writes, duplicate event suppression, monotonic versions, FIFO task claiming by queue, concurrent duplicate evolution, retry scheduling, queued/running migration backfills, historical delivery dedupe, and preservation of workflows/session/comment IDs. 3. `tests/test_webhook.py`: replace direct Gitea/storage fakes with a fake state-machine host; cover issue and PR conversion, multiline messages retained in raw input, status codes, missing delivery ID, bot/non-command/non-created/unsupported events, malformed JSON/payloads, and duplicate delivery responses. 4. `tests/test_listeners.py`: cover permission outcomes and transport failures, execution success/rejection/failure events, latest-stage error reporting, OpenCode readiness gating, comment create/update/marker recovery, stale reconciliation publishing current state, control retries, and no execution retry after start. 5. `tests/test_worker.py`: preserve session-abort coverage and add startup handling for abandoned control tasks, interrupted executions, active workflow failure, queued task survival, and one-comment recovery. 6. Adapt workflow tests to assert reporter events and returned final bodies instead of `update_job()`/direct comment calls. Retain setup ordering, code-review loop, workflow artifact, and completed-workflow protection tests. 7. Update `README.md` state/recovery documentation to describe durable event evolution, asynchronous listener processing, eventual status comments, at-least-once control effects, single FIFO execution, and interruption semantics. ## Verification and Acceptance Criteria Run: ```sh uv sync uv run ruff check . uv run pyright uv run pytest docker compose config ``` The implementation is complete when all job lifecycle mutations flow through `next_state()` and `evolve()`, webhook handlers and workflows no longer call arbitrary job-update/comment methods, every state change and listener task is atomically durable, duplicate deliveries produce no duplicate side effects, one operational comment is maintained through retries and crashes, queued jobs wait for OpenCode and survive restart, running jobs are failed/aborted rather than replayed, legacy SQLite data migrates without losing workflows or resumable session metadata, and the existing command behavior and workflow outputs remain intact. ## Remaining review findings The plan has five material correctness gaps. No matching duplicate issues were found in the required issue searches. ### MAJOR: A crash before JobStarted can permanently strand a queued job — `Listener Runtime: execute handling and startup recovery` The jobs listener claims an execute task before emitting JobStarted, while startup recovery marks every abandoned jobs task failed and emits ServiceRestarted only for jobs already in running. A process exit after claim but before JobStarted therefore leaves the aggregate queued with its sole execute task failed. This contradicts the stated rule that queued jobs survive restart and are eventually executed. Recommendation: On recovery, reset an abandoned execute task to pending when its persisted job is still queued; only fail the task and emit ServiceRestarted when the job is running. Add a test for termination in the claim-to-JobStarted window. ### MAJOR: Exhausted control tasks have no consistent recovery outcome — `Listener Runtime: control retries; Verification and Acceptance Criteria` All control tasks stop after three attempts, but the plan defines no state transition or durable repair path when authorization exhausts retries. Such a job remains received forever. Exhausted comment, abort-session, and fail-workflow tasks can likewise permanently violate the acceptance claims that one operational comment is maintained and interrupted work is aborted/failed. Recommendation: Define task-specific exhaustion semantics. Authorization infrastructure failure should produce an explicit terminal event or remain durably retriable. Required reconciliation and recovery work should remain retryable, enter a replayable dead-letter state, or have the acceptance guarantees narrowed. Test each exhausted task type and its resulting aggregate/external state. ### MAJOR: JobRejected can leave a linked workflow active — `Domain Contract: JobRejected; Workflow Refactor: rejection guards` JobRejected is legal from running and only enqueues comment reconciliation. The plan also permits workflows to link an active workflow before later rejection guards such as no generated changes are evaluated. Unlike JobFailed and ServiceRestarted, that path has no fail_workflow notification, so the job can be terminally rejected while its durable workflow remains active. Recommendation: Either enqueue fail_workflow for JobRejected when a workflow is linked, or make the workflow contract explicitly finalize its artifact before emitting JobRejected. Add a test that rejects after WorkflowLinked and verifies no active workflow remains. ### MAJOR: Listener-task claiming is not specified or tested as atomic — `Persistence and Atomic Evolution: listener_tasks and claim_listener_task; Tests: storage` The plan defines claim_listener_task and FIFO tests but does not require a single transaction that selects and marks a task running, nor a concurrency test proving one task cannot be claimed twice. Duplicate claims are especially unsafe for execute because workflow effects are intentionally non-idempotent and not replayable. Recommendation: Specify an atomic SQLite claim operation, including its transaction and eligibility predicate, and add concurrent-claimer coverage asserting that only one worker receives each execute task. ### MAJOR: Marker recovery trusts an unverified issue comment — `Listener Runtime: reconcile_comment; Edge Cases: one-comment recovery` Crash recovery searches issue comments for the hidden job marker and treats any match as the operational comment. Hidden markers are correlation identifiers, not authentication; users can view and reproduce them. Without checking the comment author and handling multiple matches deterministically, recovery can link a user-authored spoof, after which updates can fail or target the wrong comment. Recommendation: Only recover a marker from a comment authored by the configured agent/bot identity, define deterministic handling for zero or multiple valid matches, and test spoofed user comments alongside the real bot comment.

StanPonomarev commented

2026-07-21 14:37:13 +02:00

/agent iterate

agentci commented

2026-07-21 14:37:15 +02:00

Implementation Plan: Explicit Webhook State Machine

Objective

Replace the implicit job lifecycle currently distributed across src/agentci/api/webhook.py, src/agentci/worker.py, workflow methods, and unrestricted JobStore.update_job() calls with one explicit persisted state machine.

Every accepted Gitea command webhook becomes a typed event. The host loads the command state from SQLite, applies a pure next_state() function, and atomically persists both the replacement state and durable listener work. Asynchronous listeners own permission checks, Gitea status comments, workflow execution, and recovery effects.

Preserve the existing command set, permission-first rejection behavior, FIFO single-job execution, one-comment UX, workflow artifacts, OpenCode readiness gating, and conservative restart policy: queued jobs survive; a job that had entered running is aborted and failed rather than replayed.

Architecture Decisions

Use one state-machine aggregate per command/job, not per issue or pull request. Distinct commands on the same target remain independent jobs.
Derive the aggregate/job ID as UUIDv5 from a fixed application namespace and the non-empty Gitea delivery ID. Webhook retries therefore address the same aggregate without generating a second ID.
Keep the relational jobs row as the authoritative current state. Add an append-only event inbox for deduplication and a durable listener-task outbox; do not add a parallel JSON state table.
Keep workflows as durable plan/implementation artifacts referenced by job state. Workflow artifact updates remain specialized storage operations, but job status, stage, links, comments, and terminal outcomes may only change through state-machine events.
Implement the reducer with frozen dataclasses and the existing Pydantic dependency. Do not add a third-party state-machine package.
Run two durable listener queues. control processes permission, comment, workflow-cleanup, and session-abort effects. jobs executes workflows with concurrency one, preserving global FIFO execution without delaying control work during a long OpenCode turn.
Provide at-least-once execution for idempotent control listeners. Never claim exactly-once external effects. Do not retry non-idempotent workflow execution after the state has entered running.

State-Machine Contract

Create src/agentci/domain/events.py for event models and src/agentci/domain/state_machine.py for state, transitions, validation, and comment rendering.

Expose this pure interface:

def next_state(state: JobState | None, event: JobEvent) -> Transition:
    ...

JobState is immutable and contains:

Existing job identity and target fields: id, target_key, repository, issue/PR number, requester, and source comment ID.
delivery_id and raw command_body.
Optional parsed kind and parsed message; both remain unset while permission is pending.
Lifecycle status, human-readable stage, error, workflow_id, runtime_session_id, accepted_comment_id, final comment_body, timestamps, and monotonically increasing version.

Transition contains the complete replacement state and typed listener notifications. It must contain no clients, database handles, coroutines, clocks, random generation, or direct side effects.

Use these lifecycle states:

State	Meaning
`received`	The command webhook is durable and awaits permission evaluation.
`queued`	Permission, syntax, and issue/PR placement are valid; execution is pending.
`running`	The execution listener has durably emitted `JobStarted`; workflow effects may have begun.
`succeeded`	Workflow execution and artifact persistence completed.
`rejected`	An expected user-facing permission, syntax, placement, or workflow precondition failed.
`failed`	An unexpected execution/infrastructure failure or service interruption occurred.

Define a discriminated JobEvent union with these transitions:

Event	Legal source	Result
`CommandReceived`	no state	Create `received`; enqueue `authorize` on `control`.
`PermissionGranted`	`received`	Run existing pure `parse_command()` and `resolve_job_kind()`. Valid input becomes `queued` and enqueues `execute` plus comment reconciliation. A `CommandError` becomes `rejected` and only reconciles the comment.
`PermissionDenied`	`received`	Become `rejected` with the current write-permission message; reconcile the comment.
`JobStarted`	`queued`	Become `running`, stage `starting`; reconcile the comment. This transition must complete before any workflow side effect starts.
`JobProgress`	`running`	Replace the stage only.
`WorkflowLinked`	`running`	Persist `workflow_id` and the supplied stage.
`RuntimeSessionLinked`	`running`	Persist `runtime_session_id`.
`JobCompleted`	`running`	Become `succeeded`, stage `completed`, persist the workflow-produced final comment body, and reconcile the comment.
`JobRejected`	`running`	Become `rejected`, persist the safe reason, reconcile the comment, and enqueue `fail_workflow` when `workflow_id` is set. The cleanup remains safe for follow-up jobs because it only changes an `active` workflow, never a previously `completed` one.
`JobFailed`	`running`	Become `failed`, persist the failed stage and sanitized error, reconcile the comment, and enqueue `fail_workflow` when linked.
`ServiceRestarted`	`running`	Become `failed`/`interrupted`; enqueue session abort, active-workflow failure, and comment reconciliation. It is an explicit no-op in every other lifecycle state.
`CommentLinked`	any existing state	Persist the canonical Gitea comment ID without changing lifecycle status. Repeating the same ID is idempotent.

Reject every other state/event pair with InvalidTransition; do not silently coerce it. Terminal states remain terminal apart from CommentLinked metadata. Duplicate event IDs are rejected by storage before reduction and return the already persisted state without creating notifications.

Use deterministic internal event IDs based on the listener task ID and outcome, such as task:<task-id>:permission-granted and task:<task-id>:comment:<comment-id>. JobReporter progress events use the execute task ID plus a local sequence number and retry the same event ID if the database response is uncertain. This prevents listener retries from applying the same transition twice.

Keep render_job_comment(state) pure. Prefix every operational comment with , then render queued, started, rejected, failed, or the workflow-provided successful body. Preserve the existing plan/implementation markers inside successful bodies.

Persistence and Migration

Add src/agentci/migrations/003_state_machine.sql and refactor database.py, job_store.py, and storage.py.

Rebuild jobs so kind may be null while permission is pending. Add delivery_id, command_body, comment_body, and version. Preserve existing IDs, statuses, kinds, messages, target fields, workflow/session links, errors, timestamps, accepted_comment_id, and legacy started_comment_id.
Backfill delivery_id by joining deliveries.comment_id to jobs.comment_id; use legacy:<job-id> only for inconsistent historical rows with no delivery. Add a unique index on jobs.delivery_id and retain target/status indexes.
Keep started_comment_id as legacy read-only data. operational_comment_ids() must continue returning accepted and legacy started IDs so historical bot comments remain excluded from issue context.
Add job_events(event_id PRIMARY KEY, job_id, event_type, payload_json, created_at). Backfill one synthetic event per existing deliveries row so historical deliveries remain deduplicated.
Add listener_tasks(id, job_id, state_version, listener, queue, status, attempts, available_at, error, created_at, started_at, finished_at) with uniqueness on (job_id, state_version, listener) and an eligibility index on (queue, status, available_at, id).
Backfill one execute task and one comment-reconciliation task for each legacy queued job. Do not enqueue execution for terminal jobs. Legacy running jobs are handled by startup recovery.
Replace record_delivery(), enqueue(), claim_next(), update_job(), set_job_comment(), and recover_running() with evolve(), get_job_state(), claim_listener_task(), complete_listener_task(), retry_listener_task(), fail_listener_task(), and running_job_states().

Implement evolve(event_id, event) as one BEGIN IMMEDIATE transaction:

Insert the event inbox row.
If the event ID already exists, roll back the attempted insert and return duplicate with the current state.
Load the current state by job ID.
Call next_state().
Persist the complete replacement state with version + 1 and lifecycle timestamps.
Insert every listener notification as an outbox task.
Commit all three records together.

Implement task claiming atomically, not as separate select/update calls:

Open BEGIN IMMEDIATE on a fresh SQLite connection.
Select the oldest task where queue = ?, status = 'pending', and available_at <= now, ordered by id.
Update that exact row to running, set started_at, and increment attempts with a conditional WHERE id = ? AND status = 'pending'.
Require rowcount == 1; otherwise retry selection inside a new transaction.
Commit before returning the claimed task.

This serializes competing claimers and guarantees that an execute task cannot be handed to two workers. Keep WAL, foreign keys, and the existing 30-second SQLite timeout.

Webhook Boundary

Refactor src/agentci/api/webhook.py into transport validation and event conversion only.

Preserve HMAC verification, supported event names, action == "created", issue/PR extraction, owner fallback, bot-user filtering, and non-command 204 behavior.
Require non-empty X-Gitea-Delivery for a supported /agent comment; return 400 when absent because deduplication would be ambiguous.
Convert every body beginning with /agent into CommandReceived without calling Gitea or parsing syntax. Permission remains first, so an unauthorized malformed command receives the permission rejection rather than syntax details.
Call the state-machine host once. Return 202 when newly persisted, 200 for a duplicate delivery, 204 for ignored payloads, 400 for malformed payloads, 401 for an invalid signature, and 500 if durable evolution fails so Gitea can redeliver.
Remove direct permission checks, job construction, storage mutation, and comment creation from the route.

Unsupported events, non-created comments, bot comments, and ordinary discussion create no state, event inbox row, or listener task.

Listener Runtime

Introduce src/agentci/state_machine.py as the application service, src/agentci/listeners.py as the listener registry, and rewrite src/agentci/worker.py around the control and jobs queues.

Authorization

authorize reloads the current received state, calls GiteaClient.has_write_permission(), and emits PermissionGranted or PermissionDenied with a deterministic task-derived event ID.

Authorization is idempotent and must remain durably retryable until it succeeds; do not abandon it after a fixed number of attempts and leave a job permanently received. On transport/infrastructure failure, return the task to pending with capped exponential backoff:

min(2 ** min(attempts, 8), 300) seconds

Persist the latest error and log every retry. available_at ordering lets later control work proceed while a failing task waits.

Workflow Execution

The jobs worker checks OpenCodeClient.ready() before claiming an execute task. Once claimed, execute must:

Reload the state and require queued.
Emit and confirm JobStarted before performing any Gitea, Git, filesystem, development setup, OpenCode, or workflow persistence effect.
Invoke the dispatcher.
Emit JobCompleted, JobRejected, or JobFailed as appropriate.

Use the existing 1,000-character safe error formatting and read the latest persisted stage for failures. The listener catches expected and unexpected workflow exceptions and emits terminal events rather than allowing the task itself to be retried.

Never replay execution once JobStarted is persisted. If dispatching JobStarted itself fails while the state remains queued, return the execute task to pending because no workflow effect is permitted to have started yet.

Comment Reconciliation

reconcile_comment reloads the latest state so stale queued/running tasks always publish the newest status.

If accepted_comment_id exists, update that comment.
Otherwise fetch issue comments and search for the exact hidden job marker.
Only accept marker matches authored by settings.bot_username, comparing normalized/case-folded usernames. A user-authored copied marker is never linked or updated.
If one bot-authored match exists, emit CommentLinked and update it.
If multiple bot-authored matches exist, choose the lowest numeric comment ID as canonical, log the duplicate IDs, emit CommentLinked for the canonical comment, and update only it. Do not create another comment.
If no valid bot-authored match exists, create one and emit CommentLinked.

Because task claiming is atomic and there is one control worker, new duplicate bot comments are not created concurrently. Marker recovery closes the crash window between remote creation and local ID persistence.

Comment reconciliation is idempotent and remains durably retryable with capped backoff until Gitea recovers. Exhaustion must not silently discard the task or alter an already successful job outcome.

Workflow and Session Cleanup

fail_workflow keeps the current conditional update that changes only a linked active workflow to failed. It is emitted for JobRejected, JobFailed, and ServiceRestarted when a workflow is linked. This prevents a rejection after WorkflowLinked from leaving a new workflow active while preserving completed workflows used by follow-up commands.

abort_sessions reuses the linked workflow sessions and one-shot fix workspace convention. Change OpenCodeClient.abort() to support a strict mode that raises on transport/HTTP failure for recovery listeners while retaining best-effort behavior during ordinary client shutdown. Session-abort and active-workflow cleanup tasks are idempotent and remain durably retryable with capped backoff rather than being discarded after a fixed attempt count.

A terminal job is not reverted if cleanup is temporarily unavailable. Outstanding recovery tasks and their last errors remain visible in SQLite and structured logs until completed.

Startup Recovery

Perform startup task recovery before normal queue polling. Handle each abandoned running listener task by joining it to current job state in one transaction:

Task/job state	Recovery action
`execute` task + job `queued`	Reset the task to `pending`. Since `JobStarted` was not persisted and execution is forbidden before that event, no workflow effect can have begun.
`execute` task + job `running`	Mark the task failed and emit deterministic `ServiceRestarted`; never replay it.
`execute` task + terminal job	Mark the stale task completed/failed without changing state.
idempotent `control` task	Reset to `pending` with its previous attempt/error metadata retained.

After task recovery, emit ServiceRestarted for every remaining persisted running job, using a deterministic recovery event ID so repeated startup attempts are harmless. Queued jobs and pending tasks remain intact.

This explicitly covers the claim-to-JobStarted crash window: a process exit after task claim but before the start event cannot strand the job, while a process exit after the start event cannot replay non-idempotent workflow work.

Workflow Refactor

Make workflow orchestration an asynchronous side effect behind execute, while removing direct job lifecycle mutation.

Change Dispatcher.dispatch() and public entry points in workflows/plan.py, implement.py, and pull_request.py to return the final comment body instead of updating Gitea.
Introduce a small JobReporter interface with progress(stage), link_workflow(workflow_id, stage), and link_runtime_session(session_id). Its implementation emits deterministic state-machine events.
Replace every storage.update_job() call in workflows, ChangeSet, and CodeReviewLoop with reporter calls.
Keep workflow artifact operations, Git operations, development setup, Gitea repository/PR reads and PR creation, and OpenCode calls inside the execution listener as asynchronous effects.
Remove update_job_comment() from workflows/common.py. Have plan completion, discussion, implementation, iteration, and fix build and return the same result/review text; the comment listener publishes it.
Replace mutable Job parameters with immutable JobState snapshots. Persist workflow and runtime-session links immediately after those resources are created.
Preserve all current rejection guards: missing or legacy plans, missing artifacts/sessions, existing open or merged agent PRs, closed/mismatched PRs, and no generated changes.

Application Wiring

Update src/agentci/container.py and app.py to construct the state-machine host, reporter/listener dependencies, and both worker loops. Continue using the existing lifespan stop/cancellation path and client shutdown.

Keep readiness behavior unchanged: webhooks and control listeners continue while OpenCode is unavailable, but the jobs worker does not claim execute tasks. Queued jobs therefore remain queued and recoverable.

Add event_id, state_version, listener, task_id, and queue to structured logging fields while retaining job/workflow/stage context.

Edge Cases and Guarantees

Distinct deliveries on the same issue/PR produce distinct aggregates and execute in global FIFO order.
Duplicate delivery IDs do not repeat permission checks, comments, or workflow execution, even if the original HTTP response was lost.
Permission, syntax, and command-location rejections are persisted terminal jobs, receive one reconciled comment, and never enter execution.
A status-comment webhook from the bot is ignored, preventing feedback loops.
A queued-comment outage does not block execution. Reconciliation later renders the newest state rather than replaying stale queued text.
A crash after bot comment creation but before CommentLinked is repaired by author-verified marker discovery. A spoofed user comment cannot become canonical.
A crash before JobStarted returns execution to the queue. A crash after JobStarted fails and aborts the job without replaying Git/OpenCode effects.
JobRejected after a new workflow link cannot leave that workflow active. Rejections linked to previously completed workflows do not invalidate them.
Idempotent control work is retried indefinitely with bounded delay; it is never silently dead-lettered. Guarantees involving external systems are eventual and depend on those systems becoming available again.
New jobs use accepted_comment_id only; legacy started_comment_id remains queryable for context filtering.

Test Plan

Keep each Python file below the repository’s 250-line limit and split focused suites where needed.

Add tests/test_state_machine.py with table-driven coverage for every legal transition, emitted listener notification, command parsing after permission, workflow/session links, rejection cleanup, terminal behavior, restart no-ops, idempotent CommentLinked, and representative invalid transitions. Assert immutability and deterministic repeated calls.
Extend tests/test_storage.py and add a focused listener-store suite for atomic event/state/task commits, duplicate event suppression, monotonic versions, migration preservation, historical delivery dedupe, task retry metadata, and queued-task backfills.
Add concurrent claimer coverage using separate SQLite connections: two workers racing for one eligible execute task must yield exactly one claim, one running row, and no duplicate execution task delivery.
Add startup recovery tests for all joined task/job combinations, especially process termination after execute claim but before JobStarted; verify that case resets to pending and later executes exactly once.
Revise tests/test_webhook.py around a fake state-machine host. Cover issue/PR conversion, raw multiline command preservation, duplicate status codes, missing delivery IDs, bot/non-command/non-created/unsupported events, malformed payloads, and no direct Gitea calls.
Add listener tests for permission outcomes, indefinite retry scheduling, deterministic outcome event IDs, OpenCode readiness gating, execution success/rejection/failure, latest-stage errors, and no retry after JobStarted.
Add comment tests for create/update, crash recovery, stale-task rendering, a spoofed user marker, one valid bot marker, multiple bot markers choosing the lowest ID, and retries preserving a successful aggregate.
Add cleanup tests proving JobRejected after WorkflowLinked eventually marks an active workflow failed, while a failed/rejected follow-up does not alter a completed workflow. Test strict abort retries and idempotent workflow cleanup.
Adapt workflow tests to assert reporter events and returned final bodies instead of arbitrary update_job() or direct comment calls. Retain setup ordering, review loops, artifact persistence, and rejection guards.
Update README.md to document event evolution, asynchronous listeners, eventual comments/cleanup, bounded retry delay, single FIFO execution, author-verified marker recovery, and the precise before/after-JobStarted restart semantics.

Verification and Acceptance Criteria

Run:

uv sync
uv run ruff check .
uv run pyright
uv run pytest
docker compose config

The redesign is complete when:

All job lifecycle changes flow through next_state() and transactional evolve().
Webhook handlers and workflows no longer call arbitrary job-update or status-comment methods.
Event deduplication, state replacement, and listener scheduling are atomic.
Listener claims are atomic under concurrent workers.
A crash before JobStarted cannot strand a queued job, and a crash after JobStarted cannot replay workflow effects.
Idempotent control tasks remain durably retriable and expose their latest failure rather than silently exhausting.
Rejected, failed, and interrupted jobs cannot leave a newly linked active workflow behind.
Comment recovery only trusts bot-authored markers and deterministically handles duplicate bot matches.
Duplicate deliveries produce no duplicate effects, and the canonical operational comment is eventually created or updated when Gitea is available.
Legacy SQLite data migrates without losing jobs, workflows, session IDs, comment IDs, artifacts, or delivery deduplication.
Existing command behavior, workflow outputs, OpenCode readiness gating, and completed-workflow protection remain intact.

Remaining review findings

The revised plan resolves the prior recovery, cleanup, atomic-claim, and marker-authentication concerns, but six material correctness or compatibility decisions remain. The required duplicate searches found no matching issues.

MAJOR: Timestamp ownership contradicts the pure replacement-state contract — `State-Machine Contract; Persistence and Migration: evolve steps 5-6`

JobState includes lifecycle timestamps and Transition is defined as the complete replacement state, while next_state cannot use a clock. Persistence then independently adds version + 1 and lifecycle timestamps after reduction. That makes the persisted state differ from the reducer output and leaves timestamp behavior outside the explicit state machine the issue requests.

Recommendation: Choose one model explicitly: carry occurrence timestamps in events and have next_state set them deterministically, or classify timestamps/version as storage metadata outside JobState and Transition. Add assertions for exact timestamp/version behavior and deterministic replay.

MAJOR: A recovered authorization task can become permanently stale — `Listener Runtime: Authorization; Startup Recovery: idempotent control tasks`

If PermissionGranted or PermissionDenied commits but the process exits before the authorize task is completed, startup resets that control task to pending. On retry the aggregate is no longer received, yet authorization is specified to reload a received state and emit an outcome. Rechecking permission can produce an invalid transition or a different outcome event ID, causing indefinite retries despite the original outcome already being durable.

Recommendation: Specify that authorize completes successfully without another permission call when the job is no longer received, optionally confirming its deterministic outcome event already exists. Test crashes after each permission outcome commits but before task completion.

MAJOR: Comment deletion cannot self-heal once an ID is linked — `Listener Runtime: Comment Reconciliation`

When accepted_comment_id is present, reconciliation only updates that ID. If the bot comment was deleted, a definitive not-found response is retried forever; marker search and creation are never reached, even though Gitea is healthy. This violates eventual canonical-comment recovery.

Recommendation: Treat a definitive missing-comment response as a relink case: search for an author-verified marker, create a replacement if needed, and emit CommentLinked with the new canonical ID. Add deletion tests for queued, running, and terminal jobs.

MAJOR: Workflow creation still has an unhandled pre-link crash window — `Workflow Refactor: persist links immediately; Workflow and Session Cleanup; Startup Recovery`

Workflow persistence is an external step followed by a separate WorkflowLinked event. A crash after creating an active workflow but before linking it leaves the running job without workflow_id. ServiceRestarted therefore cannot enqueue the linked fail_workflow cleanup, leaving an orphan active workflow despite interruption recovery.

Recommendation: Atomically create and link workflows in the same SQLite transaction where feasible, or make restart cleanup locate active workflows by job ID even when workflow_id was never linked. Add a crash test between workflow creation and WorkflowLinked.

MAJOR: The stated global FIFO order is not defined across authorization retries — `Architecture Decisions: queue behavior; Authorization retry policy; Edge Cases and Guarantees`

Execute tasks are created only when permission succeeds, while a failed authorization is delayed and later control work proceeds. A later delivery can therefore enqueue and execute before an earlier delivery whose authorization is retrying. This conflicts with the unqualified global FIFO guarantee.

Recommendation: Define whether FIFO begins at webhook receipt or at transition to queued. If receipt order must be preserved, persist a monotonic receive sequence and prevent later eligible jobs from passing an unresolved earlier command. Otherwise narrow the guarantee and test the chosen ordering under authorization backoff and migration backfills.

MAJOR: Strict session abort is not yet defined as idempotent — `Listener Runtime: Workflow and Session Cleanup`

The plan calls strict OpenCode abort idempotent but says it raises on HTTP failure. If abort succeeds remotely and the listener crashes before task completion, a retry may receive an already-stopped or not-found response and then retry forever. At-least-once recovery requires explicit success semantics for those responses.

Recommendation: Define which already-terminated/not-found responses count as successful abort completion, while retaining retries for genuine transport/server failures. Test a crash after remote abort succeeds but before the task is completed.

# Implementation Plan: Explicit Webhook State Machine ## Objective Replace the implicit job lifecycle currently distributed across `src/agentci/api/webhook.py`, `src/agentci/worker.py`, workflow methods, and unrestricted `JobStore.update_job()` calls with one explicit persisted state machine. Every accepted Gitea command webhook becomes a typed event. The host loads the command state from SQLite, applies a pure `next_state()` function, and atomically persists both the replacement state and durable listener work. Asynchronous listeners own permission checks, Gitea status comments, workflow execution, and recovery effects. Preserve the existing command set, permission-first rejection behavior, FIFO single-job execution, one-comment UX, workflow artifacts, OpenCode readiness gating, and conservative restart policy: queued jobs survive; a job that had entered `running` is aborted and failed rather than replayed. ## Architecture Decisions - Use one state-machine aggregate per command/job, not per issue or pull request. Distinct commands on the same target remain independent jobs. - Derive the aggregate/job ID as UUIDv5 from a fixed application namespace and the non-empty Gitea delivery ID. Webhook retries therefore address the same aggregate without generating a second ID. - Keep the relational `jobs` row as the authoritative current state. Add an append-only event inbox for deduplication and a durable listener-task outbox; do not add a parallel JSON state table. - Keep `workflows` as durable plan/implementation artifacts referenced by job state. Workflow artifact updates remain specialized storage operations, but job status, stage, links, comments, and terminal outcomes may only change through state-machine events. - Implement the reducer with frozen dataclasses and the existing Pydantic dependency. Do not add a third-party state-machine package. - Run two durable listener queues. `control` processes permission, comment, workflow-cleanup, and session-abort effects. `jobs` executes workflows with concurrency one, preserving global FIFO execution without delaying control work during a long OpenCode turn. - Provide at-least-once execution for idempotent control listeners. Never claim exactly-once external effects. Do not retry non-idempotent workflow execution after the state has entered `running`. ## State-Machine Contract Create `src/agentci/domain/events.py` for event models and `src/agentci/domain/state_machine.py` for state, transitions, validation, and comment rendering. Expose this pure interface: ```python def next_state(state: JobState | None, event: JobEvent) -> Transition: ... ``` `JobState` is immutable and contains: - Existing job identity and target fields: `id`, `target_key`, repository, issue/PR number, requester, and source comment ID. - `delivery_id` and raw `command_body`. - Optional parsed `kind` and parsed `message`; both remain unset while permission is pending. - Lifecycle `status`, human-readable `stage`, `error`, `workflow_id`, `runtime_session_id`, `accepted_comment_id`, final `comment_body`, timestamps, and monotonically increasing `version`. `Transition` contains the complete replacement state and typed listener notifications. It must contain no clients, database handles, coroutines, clocks, random generation, or direct side effects. Use these lifecycle states: | State | Meaning | | --- | --- | | `received` | The command webhook is durable and awaits permission evaluation. | | `queued` | Permission, syntax, and issue/PR placement are valid; execution is pending. | | `running` | The execution listener has durably emitted `JobStarted`; workflow effects may have begun. | | `succeeded` | Workflow execution and artifact persistence completed. | | `rejected` | An expected user-facing permission, syntax, placement, or workflow precondition failed. | | `failed` | An unexpected execution/infrastructure failure or service interruption occurred. | Define a discriminated `JobEvent` union with these transitions: | Event | Legal source | Result | | --- | --- | --- | | `CommandReceived` | no state | Create `received`; enqueue `authorize` on `control`. | | `PermissionGranted` | `received` | Run existing pure `parse_command()` and `resolve_job_kind()`. Valid input becomes `queued` and enqueues `execute` plus comment reconciliation. A `CommandError` becomes `rejected` and only reconciles the comment. | | `PermissionDenied` | `received` | Become `rejected` with the current write-permission message; reconcile the comment. | | `JobStarted` | `queued` | Become `running`, stage `starting`; reconcile the comment. This transition must complete before any workflow side effect starts. | | `JobProgress` | `running` | Replace the stage only. | | `WorkflowLinked` | `running` | Persist `workflow_id` and the supplied stage. | | `RuntimeSessionLinked` | `running` | Persist `runtime_session_id`. | | `JobCompleted` | `running` | Become `succeeded`, stage `completed`, persist the workflow-produced final comment body, and reconcile the comment. | | `JobRejected` | `running` | Become `rejected`, persist the safe reason, reconcile the comment, and enqueue `fail_workflow` when `workflow_id` is set. The cleanup remains safe for follow-up jobs because it only changes an `active` workflow, never a previously `completed` one. | | `JobFailed` | `running` | Become `failed`, persist the failed stage and sanitized error, reconcile the comment, and enqueue `fail_workflow` when linked. | | `ServiceRestarted` | `running` | Become `failed`/`interrupted`; enqueue session abort, active-workflow failure, and comment reconciliation. It is an explicit no-op in every other lifecycle state. | | `CommentLinked` | any existing state | Persist the canonical Gitea comment ID without changing lifecycle status. Repeating the same ID is idempotent. | Reject every other state/event pair with `InvalidTransition`; do not silently coerce it. Terminal states remain terminal apart from `CommentLinked` metadata. Duplicate event IDs are rejected by storage before reduction and return the already persisted state without creating notifications. Use deterministic internal event IDs based on the listener task ID and outcome, such as `task:<task-id>:permission-granted` and `task:<task-id>:comment:<comment-id>`. `JobReporter` progress events use the execute task ID plus a local sequence number and retry the same event ID if the database response is uncertain. This prevents listener retries from applying the same transition twice. Keep `render_job_comment(state)` pure. Prefix every operational comment with ``, then render queued, started, rejected, failed, or the workflow-provided successful body. Preserve the existing plan/implementation markers inside successful bodies. ## Persistence and Migration Add `src/agentci/migrations/003_state_machine.sql` and refactor `database.py`, `job_store.py`, and `storage.py`. 1. Rebuild `jobs` so `kind` may be null while permission is pending. Add `delivery_id`, `command_body`, `comment_body`, and `version`. Preserve existing IDs, statuses, kinds, messages, target fields, workflow/session links, errors, timestamps, `accepted_comment_id`, and legacy `started_comment_id`. 2. Backfill `delivery_id` by joining `deliveries.comment_id` to `jobs.comment_id`; use `legacy:<job-id>` only for inconsistent historical rows with no delivery. Add a unique index on `jobs.delivery_id` and retain target/status indexes. 3. Keep `started_comment_id` as legacy read-only data. `operational_comment_ids()` must continue returning accepted and legacy started IDs so historical bot comments remain excluded from issue context. 4. Add `job_events(event_id PRIMARY KEY, job_id, event_type, payload_json, created_at)`. Backfill one synthetic event per existing `deliveries` row so historical deliveries remain deduplicated. 5. Add `listener_tasks(id, job_id, state_version, listener, queue, status, attempts, available_at, error, created_at, started_at, finished_at)` with uniqueness on `(job_id, state_version, listener)` and an eligibility index on `(queue, status, available_at, id)`. 6. Backfill one `execute` task and one comment-reconciliation task for each legacy queued job. Do not enqueue execution for terminal jobs. Legacy running jobs are handled by startup recovery. 7. Replace `record_delivery()`, `enqueue()`, `claim_next()`, `update_job()`, `set_job_comment()`, and `recover_running()` with `evolve()`, `get_job_state()`, `claim_listener_task()`, `complete_listener_task()`, `retry_listener_task()`, `fail_listener_task()`, and `running_job_states()`. Implement `evolve(event_id, event)` as one `BEGIN IMMEDIATE` transaction: 1. Insert the event inbox row. 2. If the event ID already exists, roll back the attempted insert and return `duplicate` with the current state. 3. Load the current state by job ID. 4. Call `next_state()`. 5. Persist the complete replacement state with `version + 1` and lifecycle timestamps. 6. Insert every listener notification as an outbox task. 7. Commit all three records together. Implement task claiming atomically, not as separate select/update calls: 1. Open `BEGIN IMMEDIATE` on a fresh SQLite connection. 2. Select the oldest task where `queue = ?`, `status = 'pending'`, and `available_at <= now`, ordered by `id`. 3. Update that exact row to `running`, set `started_at`, and increment `attempts` with a conditional `WHERE id = ? AND status = 'pending'`. 4. Require `rowcount == 1`; otherwise retry selection inside a new transaction. 5. Commit before returning the claimed task. This serializes competing claimers and guarantees that an execute task cannot be handed to two workers. Keep WAL, foreign keys, and the existing 30-second SQLite timeout. ## Webhook Boundary Refactor `src/agentci/api/webhook.py` into transport validation and event conversion only. - Preserve HMAC verification, supported event names, `action == "created"`, issue/PR extraction, owner fallback, bot-user filtering, and non-command `204` behavior. - Require non-empty `X-Gitea-Delivery` for a supported `/agent` comment; return `400` when absent because deduplication would be ambiguous. - Convert every body beginning with `/agent` into `CommandReceived` without calling Gitea or parsing syntax. Permission remains first, so an unauthorized malformed command receives the permission rejection rather than syntax details. - Call the state-machine host once. Return `202` when newly persisted, `200` for a duplicate delivery, `204` for ignored payloads, `400` for malformed payloads, `401` for an invalid signature, and `500` if durable evolution fails so Gitea can redeliver. - Remove direct permission checks, job construction, storage mutation, and comment creation from the route. Unsupported events, non-created comments, bot comments, and ordinary discussion create no state, event inbox row, or listener task. ## Listener Runtime Introduce `src/agentci/state_machine.py` as the application service, `src/agentci/listeners.py` as the listener registry, and rewrite `src/agentci/worker.py` around the `control` and `jobs` queues. ### Authorization `authorize` reloads the current `received` state, calls `GiteaClient.has_write_permission()`, and emits `PermissionGranted` or `PermissionDenied` with a deterministic task-derived event ID. Authorization is idempotent and must remain durably retryable until it succeeds; do not abandon it after a fixed number of attempts and leave a job permanently `received`. On transport/infrastructure failure, return the task to `pending` with capped exponential backoff: ```text min(2 ** min(attempts, 8), 300) seconds ``` Persist the latest error and log every retry. `available_at` ordering lets later control work proceed while a failing task waits. ### Workflow Execution The `jobs` worker checks `OpenCodeClient.ready()` before claiming an execute task. Once claimed, `execute` must: 1. Reload the state and require `queued`. 2. Emit and confirm `JobStarted` before performing any Gitea, Git, filesystem, development setup, OpenCode, or workflow persistence effect. 3. Invoke the dispatcher. 4. Emit `JobCompleted`, `JobRejected`, or `JobFailed` as appropriate. Use the existing 1,000-character safe error formatting and read the latest persisted stage for failures. The listener catches expected and unexpected workflow exceptions and emits terminal events rather than allowing the task itself to be retried. Never replay execution once `JobStarted` is persisted. If dispatching `JobStarted` itself fails while the state remains `queued`, return the execute task to pending because no workflow effect is permitted to have started yet. ### Comment Reconciliation `reconcile_comment` reloads the latest state so stale queued/running tasks always publish the newest status. - If `accepted_comment_id` exists, update that comment. - Otherwise fetch issue comments and search for the exact hidden job marker. - Only accept marker matches authored by `settings.bot_username`, comparing normalized/case-folded usernames. A user-authored copied marker is never linked or updated. - If one bot-authored match exists, emit `CommentLinked` and update it. - If multiple bot-authored matches exist, choose the lowest numeric comment ID as canonical, log the duplicate IDs, emit `CommentLinked` for the canonical comment, and update only it. Do not create another comment. - If no valid bot-authored match exists, create one and emit `CommentLinked`. Because task claiming is atomic and there is one control worker, new duplicate bot comments are not created concurrently. Marker recovery closes the crash window between remote creation and local ID persistence. Comment reconciliation is idempotent and remains durably retryable with capped backoff until Gitea recovers. Exhaustion must not silently discard the task or alter an already successful job outcome. ### Workflow and Session Cleanup `fail_workflow` keeps the current conditional update that changes only a linked `active` workflow to `failed`. It is emitted for `JobRejected`, `JobFailed`, and `ServiceRestarted` when a workflow is linked. This prevents a rejection after `WorkflowLinked` from leaving a new workflow active while preserving completed workflows used by follow-up commands. `abort_sessions` reuses the linked workflow sessions and one-shot fix workspace convention. Change `OpenCodeClient.abort()` to support a strict mode that raises on transport/HTTP failure for recovery listeners while retaining best-effort behavior during ordinary client shutdown. Session-abort and active-workflow cleanup tasks are idempotent and remain durably retryable with capped backoff rather than being discarded after a fixed attempt count. A terminal job is not reverted if cleanup is temporarily unavailable. Outstanding recovery tasks and their last errors remain visible in SQLite and structured logs until completed. ## Startup Recovery Perform startup task recovery before normal queue polling. Handle each abandoned `running` listener task by joining it to current job state in one transaction: | Task/job state | Recovery action | | --- | --- | | `execute` task + job `queued` | Reset the task to `pending`. Since `JobStarted` was not persisted and execution is forbidden before that event, no workflow effect can have begun. | | `execute` task + job `running` | Mark the task failed and emit deterministic `ServiceRestarted`; never replay it. | | `execute` task + terminal job | Mark the stale task completed/failed without changing state. | | idempotent `control` task | Reset to `pending` with its previous attempt/error metadata retained. | After task recovery, emit `ServiceRestarted` for every remaining persisted `running` job, using a deterministic recovery event ID so repeated startup attempts are harmless. Queued jobs and pending tasks remain intact. This explicitly covers the claim-to-`JobStarted` crash window: a process exit after task claim but before the start event cannot strand the job, while a process exit after the start event cannot replay non-idempotent workflow work. ## Workflow Refactor Make workflow orchestration an asynchronous side effect behind `execute`, while removing direct job lifecycle mutation. - Change `Dispatcher.dispatch()` and public entry points in `workflows/plan.py`, `implement.py`, and `pull_request.py` to return the final comment body instead of updating Gitea. - Introduce a small `JobReporter` interface with `progress(stage)`, `link_workflow(workflow_id, stage)`, and `link_runtime_session(session_id)`. Its implementation emits deterministic state-machine events. - Replace every `storage.update_job()` call in workflows, `ChangeSet`, and `CodeReviewLoop` with reporter calls. - Keep workflow artifact operations, Git operations, development setup, Gitea repository/PR reads and PR creation, and OpenCode calls inside the execution listener as asynchronous effects. - Remove `update_job_comment()` from `workflows/common.py`. Have plan completion, discussion, implementation, iteration, and fix build and return the same result/review text; the comment listener publishes it. - Replace mutable `Job` parameters with immutable `JobState` snapshots. Persist workflow and runtime-session links immediately after those resources are created. - Preserve all current rejection guards: missing or legacy plans, missing artifacts/sessions, existing open or merged agent PRs, closed/mismatched PRs, and no generated changes. ## Application Wiring Update `src/agentci/container.py` and `app.py` to construct the state-machine host, reporter/listener dependencies, and both worker loops. Continue using the existing lifespan stop/cancellation path and client shutdown. Keep readiness behavior unchanged: webhooks and control listeners continue while OpenCode is unavailable, but the jobs worker does not claim execute tasks. Queued jobs therefore remain queued and recoverable. Add `event_id`, `state_version`, `listener`, `task_id`, and `queue` to structured logging fields while retaining job/workflow/stage context. ## Edge Cases and Guarantees - Distinct deliveries on the same issue/PR produce distinct aggregates and execute in global FIFO order. - Duplicate delivery IDs do not repeat permission checks, comments, or workflow execution, even if the original HTTP response was lost. - Permission, syntax, and command-location rejections are persisted terminal jobs, receive one reconciled comment, and never enter execution. - A status-comment webhook from the bot is ignored, preventing feedback loops. - A queued-comment outage does not block execution. Reconciliation later renders the newest state rather than replaying stale queued text. - A crash after bot comment creation but before `CommentLinked` is repaired by author-verified marker discovery. A spoofed user comment cannot become canonical. - A crash before `JobStarted` returns execution to the queue. A crash after `JobStarted` fails and aborts the job without replaying Git/OpenCode effects. - `JobRejected` after a new workflow link cannot leave that workflow active. Rejections linked to previously completed workflows do not invalidate them. - Idempotent control work is retried indefinitely with bounded delay; it is never silently dead-lettered. Guarantees involving external systems are eventual and depend on those systems becoming available again. - New jobs use `accepted_comment_id` only; legacy `started_comment_id` remains queryable for context filtering. ## Test Plan Keep each Python file below the repository’s 250-line limit and split focused suites where needed. 1. Add `tests/test_state_machine.py` with table-driven coverage for every legal transition, emitted listener notification, command parsing after permission, workflow/session links, rejection cleanup, terminal behavior, restart no-ops, idempotent `CommentLinked`, and representative invalid transitions. Assert immutability and deterministic repeated calls. 2. Extend `tests/test_storage.py` and add a focused listener-store suite for atomic event/state/task commits, duplicate event suppression, monotonic versions, migration preservation, historical delivery dedupe, task retry metadata, and queued-task backfills. 3. Add concurrent claimer coverage using separate SQLite connections: two workers racing for one eligible execute task must yield exactly one claim, one `running` row, and no duplicate execution task delivery. 4. Add startup recovery tests for all joined task/job combinations, especially process termination after execute claim but before `JobStarted`; verify that case resets to pending and later executes exactly once. 5. Revise `tests/test_webhook.py` around a fake state-machine host. Cover issue/PR conversion, raw multiline command preservation, duplicate status codes, missing delivery IDs, bot/non-command/non-created/unsupported events, malformed payloads, and no direct Gitea calls. 6. Add listener tests for permission outcomes, indefinite retry scheduling, deterministic outcome event IDs, OpenCode readiness gating, execution success/rejection/failure, latest-stage errors, and no retry after `JobStarted`. 7. Add comment tests for create/update, crash recovery, stale-task rendering, a spoofed user marker, one valid bot marker, multiple bot markers choosing the lowest ID, and retries preserving a successful aggregate. 8. Add cleanup tests proving `JobRejected` after `WorkflowLinked` eventually marks an active workflow failed, while a failed/rejected follow-up does not alter a completed workflow. Test strict abort retries and idempotent workflow cleanup. 9. Adapt workflow tests to assert reporter events and returned final bodies instead of arbitrary `update_job()` or direct comment calls. Retain setup ordering, review loops, artifact persistence, and rejection guards. 10. Update `README.md` to document event evolution, asynchronous listeners, eventual comments/cleanup, bounded retry delay, single FIFO execution, author-verified marker recovery, and the precise before/after-`JobStarted` restart semantics. ## Verification and Acceptance Criteria Run: ```sh uv sync uv run ruff check . uv run pyright uv run pytest docker compose config ``` The redesign is complete when: - All job lifecycle changes flow through `next_state()` and transactional `evolve()`. - Webhook handlers and workflows no longer call arbitrary job-update or status-comment methods. - Event deduplication, state replacement, and listener scheduling are atomic. - Listener claims are atomic under concurrent workers. - A crash before `JobStarted` cannot strand a queued job, and a crash after `JobStarted` cannot replay workflow effects. - Idempotent control tasks remain durably retriable and expose their latest failure rather than silently exhausting. - Rejected, failed, and interrupted jobs cannot leave a newly linked active workflow behind. - Comment recovery only trusts bot-authored markers and deterministically handles duplicate bot matches. - Duplicate deliveries produce no duplicate effects, and the canonical operational comment is eventually created or updated when Gitea is available. - Legacy SQLite data migrates without losing jobs, workflows, session IDs, comment IDs, artifacts, or delivery deduplication. - Existing command behavior, workflow outputs, OpenCode readiness gating, and completed-workflow protection remain intact. ## Remaining review findings The revised plan resolves the prior recovery, cleanup, atomic-claim, and marker-authentication concerns, but six material correctness or compatibility decisions remain. The required duplicate searches found no matching issues. ### MAJOR: Timestamp ownership contradicts the pure replacement-state contract — `State-Machine Contract; Persistence and Migration: evolve steps 5-6` JobState includes lifecycle timestamps and Transition is defined as the complete replacement state, while next_state cannot use a clock. Persistence then independently adds `version + 1` and lifecycle timestamps after reduction. That makes the persisted state differ from the reducer output and leaves timestamp behavior outside the explicit state machine the issue requests. Recommendation: Choose one model explicitly: carry occurrence timestamps in events and have next_state set them deterministically, or classify timestamps/version as storage metadata outside JobState and Transition. Add assertions for exact timestamp/version behavior and deterministic replay. ### MAJOR: A recovered authorization task can become permanently stale — `Listener Runtime: Authorization; Startup Recovery: idempotent control tasks` If PermissionGranted or PermissionDenied commits but the process exits before the authorize task is completed, startup resets that control task to pending. On retry the aggregate is no longer received, yet authorization is specified to reload a received state and emit an outcome. Rechecking permission can produce an invalid transition or a different outcome event ID, causing indefinite retries despite the original outcome already being durable. Recommendation: Specify that authorize completes successfully without another permission call when the job is no longer received, optionally confirming its deterministic outcome event already exists. Test crashes after each permission outcome commits but before task completion. ### MAJOR: Comment deletion cannot self-heal once an ID is linked — `Listener Runtime: Comment Reconciliation` When accepted_comment_id is present, reconciliation only updates that ID. If the bot comment was deleted, a definitive not-found response is retried forever; marker search and creation are never reached, even though Gitea is healthy. This violates eventual canonical-comment recovery. Recommendation: Treat a definitive missing-comment response as a relink case: search for an author-verified marker, create a replacement if needed, and emit CommentLinked with the new canonical ID. Add deletion tests for queued, running, and terminal jobs. ### MAJOR: Workflow creation still has an unhandled pre-link crash window — `Workflow Refactor: persist links immediately; Workflow and Session Cleanup; Startup Recovery` Workflow persistence is an external step followed by a separate WorkflowLinked event. A crash after creating an active workflow but before linking it leaves the running job without workflow_id. ServiceRestarted therefore cannot enqueue the linked fail_workflow cleanup, leaving an orphan active workflow despite interruption recovery. Recommendation: Atomically create and link workflows in the same SQLite transaction where feasible, or make restart cleanup locate active workflows by job ID even when workflow_id was never linked. Add a crash test between workflow creation and WorkflowLinked. ### MAJOR: The stated global FIFO order is not defined across authorization retries — `Architecture Decisions: queue behavior; Authorization retry policy; Edge Cases and Guarantees` Execute tasks are created only when permission succeeds, while a failed authorization is delayed and later control work proceeds. A later delivery can therefore enqueue and execute before an earlier delivery whose authorization is retrying. This conflicts with the unqualified global FIFO guarantee. Recommendation: Define whether FIFO begins at webhook receipt or at transition to queued. If receipt order must be preserved, persist a monotonic receive sequence and prevent later eligible jobs from passing an unresolved earlier command. Otherwise narrow the guarantee and test the chosen ordering under authorization backoff and migration backfills. ### MAJOR: Strict session abort is not yet defined as idempotent — `Listener Runtime: Workflow and Session Cleanup` The plan calls strict OpenCode abort idempotent but says it raises on HTTP failure. If abort succeeds remotely and the listener crashes before task completion, a retry may receive an already-stopped or not-found response and then retry forever. At-least-once recovery requires explicit success semantics for those responses. Recommendation: Define which already-terminated/not-found responses count as successful abort completion, while retaining retries for genuine transport/server failures. Test a crash after remote abort succeeds but before the task is completed.

StanPonomarev commented

2026-07-21 14:50:28 +02:00

/agent discuss

Why are versions needed in the job state?

/agent discuss Why are versions needed in the job state?

agentci commented

2026-07-21 14:50:28 +02:00

They are not strictly needed under the proposed SQLite design.

The version was intended to:

identify the state revision that produced a listener task;
prevent duplicate listener-task insertion;
leave room for optimistic concurrency if state writes later stop using BEGIN IMMEDIATE.

However, evolve() already serializes each read/reduce/write transaction with BEGIN IMMEDIATE, and job_events.event_id already provides idempotency. Listeners also reload the latest state rather than acting on a versioned snapshot. In that model, the version adds bookkeeping without enforcing an additional invariant.

I propose removing version from JobState and the jobs migration. Instead, each listener task should reference its source_event_id, with uniqueness on (source_event_id, listener, ordinal). The ordinal only matters if one event intentionally emits the same listener more than once. Logs can use event_id and task_id rather than state_version.

Concurrency remains unambiguous:

BEGIN IMMEDIATE serializes state evolution.
Unique event IDs suppress retries.
next_state() always evaluates against the latest committed state.
Atomic task claiming prevents duplicate listener execution.
Comment listeners deliberately reload current state, so they do not need revision matching.

A version should only be reintroduced if the implementation adopts optimistic writes such as UPDATE jobs ... WHERE version = ?, supports multiple database writers without serialized evolution, or exposes revision-based APIs. I would revise the canonical plan to omit it.

They are not strictly needed under the proposed SQLite design. The version was intended to: - identify the state revision that produced a listener task; - prevent duplicate listener-task insertion; - leave room for optimistic concurrency if state writes later stop using `BEGIN IMMEDIATE`. However, `evolve()` already serializes each read/reduce/write transaction with `BEGIN IMMEDIATE`, and `job_events.event_id` already provides idempotency. Listeners also reload the latest state rather than acting on a versioned snapshot. In that model, the version adds bookkeeping without enforcing an additional invariant. I propose removing `version` from `JobState` and the `jobs` migration. Instead, each listener task should reference its `source_event_id`, with uniqueness on `(source_event_id, listener, ordinal)`. The ordinal only matters if one event intentionally emits the same listener more than once. Logs can use `event_id` and `task_id` rather than `state_version`. Concurrency remains unambiguous: - `BEGIN IMMEDIATE` serializes state evolution. - Unique event IDs suppress retries. - `next_state()` always evaluates against the latest committed state. - Atomic task claiming prevents duplicate listener execution. - Comment listeners deliberately reload current state, so they do not need revision matching. A version should only be reintroduced if the implementation adopts optimistic writes such as `UPDATE jobs ... WHERE version = ?`, supports multiple database writers without serialized evolution, or exposes revision-based APIs. I would revise the canonical plan to omit it.

StanPonomarev commented

2026-07-21 15:23:12 +02:00

/agent iterate

Remove the version column, persist a receive sequence. Rest - follow recommendations in reviews.

/agent iterate Remove the version column, persist a receive sequence. Rest - follow recommendations in reviews.

agentci commented

2026-07-21 15:23:14 +02:00

Implementation Plan: Explicit Webhook State Machine

Objective

Replace the implicit job lifecycle currently distributed across src/agentci/api/webhook.py, src/agentci/worker.py, workflow methods, and unrestricted JobStore.update_job() calls with one explicit persisted state machine.

Each accepted Gitea command webhook is transformed into a typed incoming event. The host assigns it a durable receive sequence, loads the command state from SQLite, applies a pure next_state() function, and atomically persists the replacement state, event record, and asynchronous listener work. Listeners own permission checks, Gitea status comments, workflow execution, and recovery effects.

Preserve the existing command set, permission-first rejection behavior, one-comment UX, workflow artifacts, OpenCode readiness gating, and restart policy. Commands execute strictly in webhook receive order. Queued jobs survive restart; jobs that reached running are aborted and failed rather than replayed.

Architecture Decisions

Use one state-machine aggregate per command/job, not per issue or pull request. Distinct commands on the same target remain independent aggregates.
Derive the aggregate/job ID as UUIDv5 from a fixed application namespace and the non-empty Gitea delivery ID. Retries address the same aggregate without creating another ID.
Persist an immutable, globally monotonic receive_sequence for every command. This sequence, not task creation time or authorization completion time, defines execution order.
Do not persist a job-state version. SQLite BEGIN IMMEDIATE serializes state evolution, event IDs provide idempotency, and listeners reload current state instead of relying on revision snapshots.
Keep the relational jobs row as the authoritative current state. Add an append-only event inbox and durable listener-task outbox; do not introduce a parallel JSON state table.
Keep workflows as durable plan/implementation artifacts referenced by job state. Workflow artifact updates remain specialized storage operations, but job status, stage, links, comments, and terminal outcomes may only change through state-machine events.
Implement the reducer with frozen dataclasses and the existing Pydantic dependency. Do not add a third-party state-machine package.
Run two durable listener queues. control processes permission, comment, cleanup, and abort effects. jobs executes workflows with concurrency one.
Provide at-least-once execution for idempotent control listeners. Never claim exactly-once external effects. Never retry non-idempotent workflow execution after JobStarted is durable.

State-Machine Contract

Create src/agentci/domain/events.py for event models and src/agentci/domain/state_machine.py for state, transitions, validation, and comment rendering.

Expose this pure interface:

def next_state(state: JobState | None, event: JobEvent) -> Transition:
    ...

JobState is immutable and contains:

Existing job identity and target fields: ID, target key, repository, issue/PR number, requester, and source comment ID.
delivery_id, immutable receive_sequence, and raw command_body.
Optional parsed kind and message; both remain unset while permission is pending.
Lifecycle status, human-readable stage, error, workflow_id, runtime_session_id, accepted_comment_id, and final comment_body.

Do not place state versions or persistence timestamps in JobState. created_at, started_at, and finished_at remain storage metadata with explicit persistence rules. Transition is the complete replacement domain state plus typed listener notifications and contains no clients, database handles, clocks, random generation, or direct effects.

Use these lifecycle states:

State	Meaning
`received`	The command is durable and awaits permission evaluation.
`queued`	Permission, syntax, and issue/PR placement are valid; execution is pending.
`running`	`JobStarted` is durable; workflow effects may have begun.
`succeeded`	Workflow execution and artifact persistence completed.
`rejected`	An expected permission, syntax, placement, or workflow precondition failed.
`failed`	An unexpected execution/infrastructure failure or service interruption occurred.

Define a discriminated JobEvent union with these transitions:

Event	Legal source	Result
`CommandReceived`	no state	Create `received` with the assigned receive sequence; enqueue `authorize`.
`PermissionGranted`	`received`	Run `parse_command()` and `resolve_job_kind()`. Valid input becomes `queued` and enqueues `execute` plus comment reconciliation. `CommandError` becomes `rejected` and only reconciles the comment.
`PermissionDenied`	`received`	Become `rejected` with the existing write-permission message; reconcile the comment.
`JobStarted`	`queued`	Become `running`, stage `starting`; reconcile the comment. This must commit before any workflow effect.
`JobProgress`	`running`	Replace the stage only.
`WorkflowCreated`	`running`	Link a newly created workflow and set its stage. The event includes all workflow fields needed for atomic workflow insertion and job linking.
`WorkflowLinked`	`running`	Link an already persisted workflow, such as a completed plan or implementation resumed by a follow-up command.
`RuntimeSessionLinked`	`running`	Persist `runtime_session_id`.
`JobCompleted`	`running`	Become `succeeded`, stage `completed`, persist the final comment body, and reconcile it.
`JobRejected`	`running`	Become `rejected`, persist the safe reason, reconcile the comment, and enqueue `fail_workflow` when linked.
`JobFailed`	`running`	Become `failed`, persist failed stage/error, reconcile the comment, and enqueue `fail_workflow` when linked.
`ServiceRestarted`	`running`	Become `failed`/`interrupted`; enqueue session abort, active-workflow failure, and comment reconciliation. It is an explicit no-op in every other lifecycle state.
`CommentLinked`	any existing state	Replace the canonical Gitea comment ID without changing lifecycle status. Repeating the same ID is idempotent.

Reject every other state/event pair with InvalidTransition. Terminal states remain terminal apart from CommentLinked. Duplicate event IDs are rejected by storage before reduction and create no notifications.

Use deterministic internal event IDs based on listener task ID and outcome, such as task:<task-id>:permission-granted and task:<task-id>:comment:<comment-id>. JobReporter events use the execute task ID plus a local sequence number and retry the same ID if the database response is uncertain.

Keep render_job_comment(state) pure. Prefix operational comments with , then render queued, started, rejected, failed, or the successful workflow body. Preserve existing plan/implementation markers inside successful bodies.

Receive Sequencing and FIFO Semantics

The webhook adapter produces an IncomingCommand transport event without a sequence. StateMachine.receive(event_id, incoming) performs one BEGIN IMMEDIATE transaction:

Check job_events for the delivery-derived event ID and return the existing job for a duplicate.
Allocate receive_sequence = COALESCE(MAX(receive_sequence), 0) + 1 while holding the write transaction.
Construct the persisted CommandReceived event containing that sequence.
Call next_state(None, event) and atomically insert the event, job state, and listener tasks.

Rolled-back or duplicate receives do not consume a sequence. Existing jobs are backfilled in stable (created_at, id) order.

Strict FIFO starts at webhook receipt, not at authorization completion. The jobs queue may claim a queued execute task only when no lower receive_sequence job remains in received, queued, or running. A delayed authorization therefore intentionally blocks later execution, although later control tasks may continue. Rejected, failed, and succeeded earlier jobs no longer block the queue.

The execute-task claim query must join jobs and order by jobs.receive_sequence, not listener-task ID.

Persistence and Migration

Add src/agentci/migrations/003_state_machine.sql and refactor database.py, job_store.py, and storage.py.

Rebuild jobs so kind may be null while permission is pending. Add delivery_id, unique receive_sequence, command_body, and comment_body. Do not add version.
Preserve existing IDs, statuses, kinds, messages, targets, errors, workflow/session links, timestamps, accepted_comment_id, and legacy started_comment_id.
Backfill delivery_id through deliveries.comment_id; use legacy:<job-id> only for inconsistent historical rows. Backfill receive sequences in (created_at, id) order and retain target/status indexes.
Keep started_comment_id as legacy read-only data. operational_comment_ids() continues returning accepted and legacy started IDs.
Add job_events(event_id PRIMARY KEY, job_id, event_type, payload_json, created_at). Backfill one synthetic event per historical delivery so old deliveries remain deduplicated.
Add listener_tasks(id, job_id, source_event_id, ordinal, listener, queue, status, attempts, available_at, error, created_at, started_at, finished_at) with a foreign key to job_events and uniqueness on (source_event_id, listener, ordinal).
Add an eligibility index on (queue, status, available_at, id) and a job-order index on jobs(status, receive_sequence).
Backfill execute and comment-reconciliation tasks for legacy queued jobs. Do not enqueue terminal jobs; recover legacy running jobs at startup.
Replace record_delivery(), enqueue(), claim_next(), update_job(), set_job_comment(), and recover_running() with receive(), evolve(), get_job_state(), atomic task APIs, and running_job_states().

Implement evolve(event_id, event) as one BEGIN IMMEDIATE transaction:

Insert the event inbox row, returning duplicate with current state on unique conflict.
Load current state by job ID.
Call next_state().
Persist the complete replacement domain state.
Apply event-specific relational projections in the same transaction. For WorkflowCreated, insert the workflow row before linking its ID in the job replacement.
Insert listener tasks referencing source_event_id and ordinal.
Update storage timestamps and commit.

Timestamp ownership is explicitly outside the reducer:

Initial CommandReceived sets jobs.created_at and job_events.created_at to the store’s current UTC time.
The first queued -> running transition sets started_at; later events never replace it.
The first transition into succeeded, rejected, or failed sets finished_at; CommentLinked never changes it.
Every event receives its own job_events.created_at metadata timestamp.

Reducer tests ignore timestamps; storage tests assert these exact rules. Persisted domain fields must otherwise equal Transition.state exactly.

Atomic Listener Claims

Claim tasks in a single SQLite transaction:

Open BEGIN IMMEDIATE on a fresh connection.
For control, select the oldest eligible pending task by task ID.
For jobs, select the eligible execute task with the lowest job receive sequence and require that no lower-sequence job is received, queued, or running.
Update the selected task to running, set started_at, and increment attempts with WHERE id = ? AND status = 'pending'.
Require rowcount == 1; otherwise restart selection in a new transaction.
Commit before returning the task.

This prevents two workers from receiving one execute task and makes FIFO independent of authorization timing. Keep SQLite WAL, foreign keys, and the existing 30-second timeout.

Webhook Boundary

Refactor src/agentci/api/webhook.py into transport validation and incoming-event conversion only.

Preserve HMAC verification, supported event names, action == "created", issue/PR extraction, owner fallback, bot-user filtering, and non-command 204 behavior.
Require non-empty X-Gitea-Delivery for a supported /agent comment; return 400 when absent.
Convert each body beginning with /agent into IncomingCommand without calling Gitea or parsing syntax. Permission remains first, preserving current unauthorized-malformed-command behavior.
Call StateMachine.receive() once. Return 202 when newly persisted, 200 for a duplicate, 204 for ignored payloads, 400 for malformed payloads, 401 for invalid signature, and 500 on persistence failure so Gitea can redeliver.
Remove direct permission checks, job construction, job mutation, and comment creation from the route.

Unsupported, non-created, bot, and ordinary comments create no state, event, sequence, or listener task.

Listener Runtime

Introduce src/agentci/state_machine.py as the application service, src/agentci/listeners.py as the listener registry, and rewrite src/agentci/worker.py around control and jobs queues.

Authorization

authorize reloads current state before any external call:

If state is received, check GiteaClient.has_write_permission() and emit the deterministic granted/denied event.
If state is no longer received, treat the task as already applied and complete it without another Gitea request or event. This handles a crash after the permission outcome commits but before task completion.

Authorization remains durably retryable on transport/infrastructure failure with capped exponential delay:

min(2 ** min(attempts, 8), 300) seconds

Persist the latest task error. Delayed control tasks do not prevent later control work, but the receive-sequence barrier prevents later workflow execution from passing an unresolved earlier command.

Workflow Execution

The jobs worker checks OpenCodeClient.ready() before claiming work. A claimed execute task must:

Reload state and require queued.
Emit and confirm JobStarted before any Gitea, Git, filesystem, workflow-row, development, or OpenCode effect.
Invoke the dispatcher.
Emit JobCompleted, JobRejected, or JobFailed.

Use the current 1,000-character safe error formatting and latest persisted stage. Workflow exceptions become terminal events instead of task retries.

If JobStarted cannot be persisted and state remains queued, return the execute task to pending because no workflow effect may have started. Never replay execution after JobStarted commits.

Comment Reconciliation

reconcile_comment reloads latest state so stale tasks publish current text.

If accepted_comment_id exists, call an updated GiteaClient.update_comment() that distinguishes 404 from retryable failures.
If update succeeds, complete the task.
If the comment is definitively missing, perform relink recovery instead of retrying that stale ID forever.
Fetch issue comments and find exact hidden-marker matches authored by settings.bot_username, using normalized/case-folded usernames.
Ignore user-authored marker copies.
If one valid bot match exists, emit CommentLinked and update it.
If multiple valid bot matches exist, choose the lowest numeric ID, log all duplicates, emit CommentLinked for the canonical ID, and update only it.
If none exists, create a replacement and emit CommentLinked.

CommentLinked may replace a deleted comment’s old ID. Reconciliation is indefinitely retryable with capped backoff for transport, rate-limit, and server failures. A definitive 404 is recovery input, not a retryable task failure.

Workflow Creation and Cleanup

For a new plan or implementation, replace separate create_workflow() plus WorkflowLinked calls with one JobReporter.create_workflow(workflow, stage) operation. It emits WorkflowCreated; evolve() inserts the workflow and links the job in the same SQLite transaction. A process cannot leave a persisted active workflow without its owning job link.

Follow-up commands that use an existing completed workflow emit WorkflowLinked; no new active row is created.

fail_workflow retains the conditional update that changes only a linked active workflow to failed. Emit it for JobRejected, JobFailed, and ServiceRestarted when linked. This cleans up newly created workflows without invalidating completed workflows used by follow-ups.

Session Abort

abort_sessions uses linked workflow sessions and the one-shot fix workspace convention. Refactor OpenCodeClient.abort() into strict and best-effort modes:

2xx means success.
404 means the session no longer exists and is idempotent success.
409 means already inactive/conflicting with an abort and is idempotent success.
Transport failures, timeouts, 429, and 5xx raise and return the listener task to pending.
Other 4xx errors remain visible and retry with capped backoff; they are not silently treated as success.
Normal client shutdown uses best-effort mode and logs failures without blocking shutdown.

Thus a crash after remote abort but before task completion can safely retry and complete when OpenCode reports the session absent/inactive.

Cleanup tasks are indefinitely retryable. A terminal job is not reverted while cleanup is unavailable; outstanding tasks and latest errors remain visible in SQLite and logs.

Startup Recovery

Recover abandoned running tasks before normal polling, joining each task to current job state in one transaction:

Task/job state	Recovery action
`execute` + `queued`	Reset task to `pending`; `JobStarted` was not persisted, so no workflow effect was allowed.
`execute` + `running`	Mark task failed and emit deterministic `ServiceRestarted`; never replay it.
`execute` + terminal	Mark stale task completed/failed without changing job state.
`authorize` + state no longer `received`	Mark completed; its outcome is already represented in state.
other idempotent `control` task	Reset to `pending` with attempt/error metadata retained.

After task recovery, emit deterministic ServiceRestarted for every remaining running job. Duplicate recovery events are harmless. Queued jobs and pending tasks remain intact.

This covers both critical crash windows: before JobStarted, execution safely returns to the queue; after JobStarted, it fails without replaying workflow effects.

Workflow Refactor

Make workflow orchestration an asynchronous effect behind execute, while removing direct lifecycle mutation.

Change Dispatcher.dispatch() and public entry points in workflows/plan.py, implement.py, and pull_request.py to return the final comment body instead of updating Gitea.
Introduce JobReporter.progress(stage), create_workflow(workflow, stage), link_workflow(workflow_id, stage), and link_runtime_session(session_id). Each emits a deterministic event.
Replace every storage.update_job() call in workflows, ChangeSet, and CodeReviewLoop with reporter calls.
For new workflows, clone/create branches first, then use atomic create_workflow; continue updating artifacts through update_workflow() afterward.
Keep Git, development setup, Gitea repository/PR reads and PR creation, and OpenCode calls inside workflow listeners.
Remove update_job_comment() from workflows/common.py. Workflow paths return the same result/review body; comment reconciliation publishes it.
Replace mutable Job parameters with immutable JobState snapshots.
Preserve all current guards: missing/legacy plans, missing artifacts/sessions, existing open or merged agent PRs, closed/mismatched PRs, and no generated changes.

Application Wiring and Logging

Update src/agentci/container.py and app.py to construct the state-machine host, reporter/listener dependencies, and both worker loops. Retain the lifespan stop/cancellation path and client shutdown.

Webhooks and control listeners continue while OpenCode is unavailable. The jobs worker does not claim execute tasks until readiness succeeds.

Add event_id, receive_sequence, listener, task_id, and queue to structured logging fields. Do not add state_version.

Edge Cases and Guarantees

Commands execute strictly by persisted webhook receive sequence, including when an earlier authorization is retrying.
Duplicate deliveries do not consume another sequence or repeat permission, comments, or execution.
Permission, syntax, and location rejections are terminal persisted jobs and unblock later receive sequences.
A recovered authorization task whose outcome already committed completes without rechecking permission.
A deleted canonical Gitea comment is rediscovered or recreated and relinked for queued, running, and terminal jobs.
Marker recovery trusts only bot-authored comments; multiple valid matches choose the lowest ID deterministically.
A crash before JobStarted cannot strand a queued job. A crash after JobStarted cannot replay Git/OpenCode effects.
New workflow creation and job linking are atomic, eliminating orphan active workflows in the pre-link crash window.
Rejection/failure cleanup only fails active workflows and preserves completed workflows.
Session abort retries regard absent/already-inactive sessions as success.
Idempotent control work retries indefinitely with bounded delay. External guarantees are eventual and depend on services recovering.
New jobs use accepted_comment_id; legacy started_comment_id remains queryable for context filtering.

Test Plan

Keep each Python file below the repository’s 250-line limit and split suites where needed.

Add tests/test_state_machine.py with table-driven coverage for every transition, emitted listener, command parsing after permission, workflow/session links, cleanup notifications, terminal behavior, comment relinking, restart no-ops, invalid transitions, immutability, and deterministic replay.
Extend storage tests for atomic event/state/task commits, event dedupe, no version column, receive-sequence allocation, duplicate receives not consuming a sequence, timestamp metadata rules, migration preservation, and historical delivery dedupe.
Test migration receive-sequence backfill order and execute-task backfills for legacy queued jobs.
Add concurrent task-claimer tests with separate SQLite connections. Assert one claim per task and strict receive-order execution even when a later command authorizes first.
Test the intentional FIFO barrier: an earlier received job blocks later queued execution; its rejection or authorization allows the next sequence to proceed.
Revise webhook tests around a fake state-machine host, covering conversion, raw multiline bodies, status codes, missing delivery IDs, duplicate receives, ignored payloads, and absence of direct Gitea calls.
Add authorization tests for success/denial, indefinite retry scheduling, and crashes after outcome commit but before task completion; retries must not recheck permission or emit an invalid transition.
Add execution/startup tests for claim-before-JobStarted, after-JobStarted interruption, deterministic recovery IDs, readiness gating, success/rejection/failure, and latest-stage errors.
Add comment tests for create/update, author-verified marker recovery, spoofed user markers, multiple bot matches, crash-after-create, and deletion/relink behavior in queued, running, and terminal states.
Add workflow tests proving WorkflowCreated inserts and links atomically, including simulated failure between reduction and commit; no active orphan may remain. Verify rejected active workflows fail while completed follow-up workflows remain completed.
Add strict-abort tests for 2xx, 404, 409, transport/timeout/server retries, and crash-after-remote-success behavior.
Adapt existing workflow tests to reporter events and returned comment bodies while retaining setup order, review loops, artifact persistence, and rejection guards.
Update README.md with receive-order FIFO, asynchronous listeners, retry semantics, timestamp metadata ownership, comment deletion recovery, atomic workflow linking, and before/after-JobStarted restart behavior.

Verification and Acceptance Criteria

Run:

uv sync
uv run ruff check .
uv run pyright
uv run pytest
docker compose config

The redesign is complete when:

All lifecycle changes flow through pure next_state() and transactional receive()/evolve().
No job version column or state-version logic remains.
Every command has a durable unique receive sequence, and execution follows that sequence even across authorization retries.
Webhook handlers and workflows no longer call arbitrary job-update or status-comment methods.
Event deduplication, state replacement, workflow creation/linking, and listener scheduling are atomic where specified.
Listener claims are atomic under concurrent workers.
Timestamp metadata follows the documented storage rules without changing reducer output.
Recovered authorization tasks cannot become stale retry loops.
Deleted comments self-heal through author-verified relinking.
No running rejection, failure, or interruption leaves a newly created active workflow orphaned.
A crash before JobStarted requeues execution; a crash after it never replays workflow effects.
Strict session abort is idempotent for absent/inactive sessions and retryable for transient failures.
Legacy SQLite data migrates without losing jobs, workflows, sessions, comments, artifacts, delivery dedupe, or queue order.
Existing command behavior, workflow outputs, readiness gating, and completed-workflow protection remain intact.

Remaining review findings

The plan resolves the previously identified version, FIFO, stale-authorization, comment-404, workflow-linking, and abort-response issues. Four material recovery and migration gaps remain. The required duplicate searches found no matching issues.

MAJOR: Deleted terminal comments have no reconciliation trigger — `Webhook Boundary; Comment Reconciliation; Edge Cases and Guarantees`

Reconciliation can recover from a 404 only while a reconciliation task is already running. After a terminal job's final task completes, deleting its bot comment creates no new task because non-created comments are explicitly ignored. The stated guarantee that deleted comments self-heal for terminal jobs therefore cannot hold.

Recommendation: Either handle deletion events for known canonical bot comment IDs by enqueueing reconciliation, add a periodic reconciliation sweep, or narrow the guarantee to deletions encountered during pending work. Add an end-to-end test that deletes a comment after all terminal tasks have completed.

MAJOR: Concurrent reconciliation tasks can still create two comments — `Architecture Decisions; Atomic Listener Claims; Comment Reconciliation`

Atomic task claiming prevents two workers from claiming the same task, but queued and JobStarted transitions can create distinct reconciliation tasks for the same job. The plan does not constrain control concurrency. Two control workers can claim those tasks, both observe no accepted_comment_id or marker, and both create a bot comment before either CommentLinked event commits. Later canonical selection does not remove the duplicate, breaking the one-comment UX.

Recommendation: Declare and enforce globally single control-worker execution, serialize reconciliation per job, or coalesce/lock comment creation so only one task may perform the search-create-link critical section. Add a concurrent test using two distinct reconciliation tasks for one job.

MAJOR: Runtime sessions retain a pre-link crash window — `State-Machine Contract: RuntimeSessionLinked; Session Abort; Workflow Refactor`

OpenCode session creation is an external effect followed by RuntimeSessionLinked. A crash after the remote session is created but before that event commits leaves no runtime_session_id. Recovery is described as using linked workflow sessions, but the plan does not guarantee that the newly created session is discoverable there before the link, so ServiceRestarted may be unable to abort it.

Recommendation: Define a durable session-intent or discovery mechanism that exists before remote creation, or prove recovery can locate every unlinked session by stable job/workspace metadata. Add a crash test between remote session creation and RuntimeSessionLinked for each session-creating workflow path.

MAJOR: Legacy queued jobs without deliveries cannot satisfy the task foreign key — `Persistence and Migration steps 3, 5, 6, and 8`

The migration explicitly supports jobs with no matching delivery by assigning legacy:<job-id>, but creates synthetic job_events only for historical delivery rows. It then backfills listener tasks for every legacy queued job, and those tasks require source_event_id to reference job_events. An inconsistent queued job therefore has no valid source event and its task backfill cannot satisfy the foreign key.

Recommendation: Create a synthetic migration event for every legacy job lacking a delivery and reference it from backfilled tasks, or define an explicit terminal/quarantine policy for such jobs. Add a migration fixture containing a queued job with no delivery and verify migration plus FIFO recovery.

# Implementation Plan: Explicit Webhook State Machine ## Objective Replace the implicit job lifecycle currently distributed across `src/agentci/api/webhook.py`, `src/agentci/worker.py`, workflow methods, and unrestricted `JobStore.update_job()` calls with one explicit persisted state machine. Each accepted Gitea command webhook is transformed into a typed incoming event. The host assigns it a durable receive sequence, loads the command state from SQLite, applies a pure `next_state()` function, and atomically persists the replacement state, event record, and asynchronous listener work. Listeners own permission checks, Gitea status comments, workflow execution, and recovery effects. Preserve the existing command set, permission-first rejection behavior, one-comment UX, workflow artifacts, OpenCode readiness gating, and restart policy. Commands execute strictly in webhook receive order. Queued jobs survive restart; jobs that reached `running` are aborted and failed rather than replayed. ## Architecture Decisions - Use one state-machine aggregate per command/job, not per issue or pull request. Distinct commands on the same target remain independent aggregates. - Derive the aggregate/job ID as UUIDv5 from a fixed application namespace and the non-empty Gitea delivery ID. Retries address the same aggregate without creating another ID. - Persist an immutable, globally monotonic `receive_sequence` for every command. This sequence, not task creation time or authorization completion time, defines execution order. - Do not persist a job-state version. SQLite `BEGIN IMMEDIATE` serializes state evolution, event IDs provide idempotency, and listeners reload current state instead of relying on revision snapshots. - Keep the relational `jobs` row as the authoritative current state. Add an append-only event inbox and durable listener-task outbox; do not introduce a parallel JSON state table. - Keep `workflows` as durable plan/implementation artifacts referenced by job state. Workflow artifact updates remain specialized storage operations, but job status, stage, links, comments, and terminal outcomes may only change through state-machine events. - Implement the reducer with frozen dataclasses and the existing Pydantic dependency. Do not add a third-party state-machine package. - Run two durable listener queues. `control` processes permission, comment, cleanup, and abort effects. `jobs` executes workflows with concurrency one. - Provide at-least-once execution for idempotent control listeners. Never claim exactly-once external effects. Never retry non-idempotent workflow execution after `JobStarted` is durable. ## State-Machine Contract Create `src/agentci/domain/events.py` for event models and `src/agentci/domain/state_machine.py` for state, transitions, validation, and comment rendering. Expose this pure interface: ```python def next_state(state: JobState | None, event: JobEvent) -> Transition: ... ``` `JobState` is immutable and contains: - Existing job identity and target fields: ID, target key, repository, issue/PR number, requester, and source comment ID. - `delivery_id`, immutable `receive_sequence`, and raw `command_body`. - Optional parsed `kind` and `message`; both remain unset while permission is pending. - Lifecycle `status`, human-readable `stage`, `error`, `workflow_id`, `runtime_session_id`, `accepted_comment_id`, and final `comment_body`. Do not place state versions or persistence timestamps in `JobState`. `created_at`, `started_at`, and `finished_at` remain storage metadata with explicit persistence rules. `Transition` is the complete replacement domain state plus typed listener notifications and contains no clients, database handles, clocks, random generation, or direct effects. Use these lifecycle states: | State | Meaning | | --- | --- | | `received` | The command is durable and awaits permission evaluation. | | `queued` | Permission, syntax, and issue/PR placement are valid; execution is pending. | | `running` | `JobStarted` is durable; workflow effects may have begun. | | `succeeded` | Workflow execution and artifact persistence completed. | | `rejected` | An expected permission, syntax, placement, or workflow precondition failed. | | `failed` | An unexpected execution/infrastructure failure or service interruption occurred. | Define a discriminated `JobEvent` union with these transitions: | Event | Legal source | Result | | --- | --- | --- | | `CommandReceived` | no state | Create `received` with the assigned receive sequence; enqueue `authorize`. | | `PermissionGranted` | `received` | Run `parse_command()` and `resolve_job_kind()`. Valid input becomes `queued` and enqueues `execute` plus comment reconciliation. `CommandError` becomes `rejected` and only reconciles the comment. | | `PermissionDenied` | `received` | Become `rejected` with the existing write-permission message; reconcile the comment. | | `JobStarted` | `queued` | Become `running`, stage `starting`; reconcile the comment. This must commit before any workflow effect. | | `JobProgress` | `running` | Replace the stage only. | | `WorkflowCreated` | `running` | Link a newly created workflow and set its stage. The event includes all workflow fields needed for atomic workflow insertion and job linking. | | `WorkflowLinked` | `running` | Link an already persisted workflow, such as a completed plan or implementation resumed by a follow-up command. | | `RuntimeSessionLinked` | `running` | Persist `runtime_session_id`. | | `JobCompleted` | `running` | Become `succeeded`, stage `completed`, persist the final comment body, and reconcile it. | | `JobRejected` | `running` | Become `rejected`, persist the safe reason, reconcile the comment, and enqueue `fail_workflow` when linked. | | `JobFailed` | `running` | Become `failed`, persist failed stage/error, reconcile the comment, and enqueue `fail_workflow` when linked. | | `ServiceRestarted` | `running` | Become `failed`/`interrupted`; enqueue session abort, active-workflow failure, and comment reconciliation. It is an explicit no-op in every other lifecycle state. | | `CommentLinked` | any existing state | Replace the canonical Gitea comment ID without changing lifecycle status. Repeating the same ID is idempotent. | Reject every other state/event pair with `InvalidTransition`. Terminal states remain terminal apart from `CommentLinked`. Duplicate event IDs are rejected by storage before reduction and create no notifications. Use deterministic internal event IDs based on listener task ID and outcome, such as `task:<task-id>:permission-granted` and `task:<task-id>:comment:<comment-id>`. `JobReporter` events use the execute task ID plus a local sequence number and retry the same ID if the database response is uncertain. Keep `render_job_comment(state)` pure. Prefix operational comments with ``, then render queued, started, rejected, failed, or the successful workflow body. Preserve existing plan/implementation markers inside successful bodies. ## Receive Sequencing and FIFO Semantics The webhook adapter produces an `IncomingCommand` transport event without a sequence. `StateMachine.receive(event_id, incoming)` performs one `BEGIN IMMEDIATE` transaction: 1. Check `job_events` for the delivery-derived event ID and return the existing job for a duplicate. 2. Allocate `receive_sequence = COALESCE(MAX(receive_sequence), 0) + 1` while holding the write transaction. 3. Construct the persisted `CommandReceived` event containing that sequence. 4. Call `next_state(None, event)` and atomically insert the event, job state, and listener tasks. Rolled-back or duplicate receives do not consume a sequence. Existing jobs are backfilled in stable `(created_at, id)` order. Strict FIFO starts at webhook receipt, not at authorization completion. The jobs queue may claim a queued execute task only when no lower `receive_sequence` job remains in `received`, `queued`, or `running`. A delayed authorization therefore intentionally blocks later execution, although later control tasks may continue. Rejected, failed, and succeeded earlier jobs no longer block the queue. The execute-task claim query must join `jobs` and order by `jobs.receive_sequence`, not listener-task ID. ## Persistence and Migration Add `src/agentci/migrations/003_state_machine.sql` and refactor `database.py`, `job_store.py`, and `storage.py`. 1. Rebuild `jobs` so `kind` may be null while permission is pending. Add `delivery_id`, unique `receive_sequence`, `command_body`, and `comment_body`. Do not add `version`. 2. Preserve existing IDs, statuses, kinds, messages, targets, errors, workflow/session links, timestamps, `accepted_comment_id`, and legacy `started_comment_id`. 3. Backfill `delivery_id` through `deliveries.comment_id`; use `legacy:<job-id>` only for inconsistent historical rows. Backfill receive sequences in `(created_at, id)` order and retain target/status indexes. 4. Keep `started_comment_id` as legacy read-only data. `operational_comment_ids()` continues returning accepted and legacy started IDs. 5. Add `job_events(event_id PRIMARY KEY, job_id, event_type, payload_json, created_at)`. Backfill one synthetic event per historical delivery so old deliveries remain deduplicated. 6. Add `listener_tasks(id, job_id, source_event_id, ordinal, listener, queue, status, attempts, available_at, error, created_at, started_at, finished_at)` with a foreign key to `job_events` and uniqueness on `(source_event_id, listener, ordinal)`. 7. Add an eligibility index on `(queue, status, available_at, id)` and a job-order index on `jobs(status, receive_sequence)`. 8. Backfill execute and comment-reconciliation tasks for legacy queued jobs. Do not enqueue terminal jobs; recover legacy running jobs at startup. 9. Replace `record_delivery()`, `enqueue()`, `claim_next()`, `update_job()`, `set_job_comment()`, and `recover_running()` with `receive()`, `evolve()`, `get_job_state()`, atomic task APIs, and `running_job_states()`. Implement `evolve(event_id, event)` as one `BEGIN IMMEDIATE` transaction: 1. Insert the event inbox row, returning `duplicate` with current state on unique conflict. 2. Load current state by job ID. 3. Call `next_state()`. 4. Persist the complete replacement domain state. 5. Apply event-specific relational projections in the same transaction. For `WorkflowCreated`, insert the workflow row before linking its ID in the job replacement. 6. Insert listener tasks referencing `source_event_id` and ordinal. 7. Update storage timestamps and commit. Timestamp ownership is explicitly outside the reducer: - Initial `CommandReceived` sets `jobs.created_at` and `job_events.created_at` to the store’s current UTC time. - The first `queued -> running` transition sets `started_at`; later events never replace it. - The first transition into `succeeded`, `rejected`, or `failed` sets `finished_at`; `CommentLinked` never changes it. - Every event receives its own `job_events.created_at` metadata timestamp. Reducer tests ignore timestamps; storage tests assert these exact rules. Persisted domain fields must otherwise equal `Transition.state` exactly. ## Atomic Listener Claims Claim tasks in a single SQLite transaction: 1. Open `BEGIN IMMEDIATE` on a fresh connection. 2. For `control`, select the oldest eligible pending task by task ID. 3. For `jobs`, select the eligible execute task with the lowest job receive sequence and require that no lower-sequence job is `received`, `queued`, or `running`. 4. Update the selected task to `running`, set `started_at`, and increment `attempts` with `WHERE id = ? AND status = 'pending'`. 5. Require `rowcount == 1`; otherwise restart selection in a new transaction. 6. Commit before returning the task. This prevents two workers from receiving one execute task and makes FIFO independent of authorization timing. Keep SQLite WAL, foreign keys, and the existing 30-second timeout. ## Webhook Boundary Refactor `src/agentci/api/webhook.py` into transport validation and incoming-event conversion only. - Preserve HMAC verification, supported event names, `action == "created"`, issue/PR extraction, owner fallback, bot-user filtering, and non-command `204` behavior. - Require non-empty `X-Gitea-Delivery` for a supported `/agent` comment; return `400` when absent. - Convert each body beginning with `/agent` into `IncomingCommand` without calling Gitea or parsing syntax. Permission remains first, preserving current unauthorized-malformed-command behavior. - Call `StateMachine.receive()` once. Return `202` when newly persisted, `200` for a duplicate, `204` for ignored payloads, `400` for malformed payloads, `401` for invalid signature, and `500` on persistence failure so Gitea can redeliver. - Remove direct permission checks, job construction, job mutation, and comment creation from the route. Unsupported, non-created, bot, and ordinary comments create no state, event, sequence, or listener task. ## Listener Runtime Introduce `src/agentci/state_machine.py` as the application service, `src/agentci/listeners.py` as the listener registry, and rewrite `src/agentci/worker.py` around `control` and `jobs` queues. ### Authorization `authorize` reloads current state before any external call: - If state is `received`, check `GiteaClient.has_write_permission()` and emit the deterministic granted/denied event. - If state is no longer `received`, treat the task as already applied and complete it without another Gitea request or event. This handles a crash after the permission outcome commits but before task completion. Authorization remains durably retryable on transport/infrastructure failure with capped exponential delay: ```text min(2 ** min(attempts, 8), 300) seconds ``` Persist the latest task error. Delayed control tasks do not prevent later control work, but the receive-sequence barrier prevents later workflow execution from passing an unresolved earlier command. ### Workflow Execution The jobs worker checks `OpenCodeClient.ready()` before claiming work. A claimed execute task must: 1. Reload state and require `queued`. 2. Emit and confirm `JobStarted` before any Gitea, Git, filesystem, workflow-row, development, or OpenCode effect. 3. Invoke the dispatcher. 4. Emit `JobCompleted`, `JobRejected`, or `JobFailed`. Use the current 1,000-character safe error formatting and latest persisted stage. Workflow exceptions become terminal events instead of task retries. If `JobStarted` cannot be persisted and state remains `queued`, return the execute task to pending because no workflow effect may have started. Never replay execution after `JobStarted` commits. ### Comment Reconciliation `reconcile_comment` reloads latest state so stale tasks publish current text. 1. If `accepted_comment_id` exists, call an updated `GiteaClient.update_comment()` that distinguishes `404` from retryable failures. 2. If update succeeds, complete the task. 3. If the comment is definitively missing, perform relink recovery instead of retrying that stale ID forever. 4. Fetch issue comments and find exact hidden-marker matches authored by `settings.bot_username`, using normalized/case-folded usernames. 5. Ignore user-authored marker copies. 6. If one valid bot match exists, emit `CommentLinked` and update it. 7. If multiple valid bot matches exist, choose the lowest numeric ID, log all duplicates, emit `CommentLinked` for the canonical ID, and update only it. 8. If none exists, create a replacement and emit `CommentLinked`. `CommentLinked` may replace a deleted comment’s old ID. Reconciliation is indefinitely retryable with capped backoff for transport, rate-limit, and server failures. A definitive `404` is recovery input, not a retryable task failure. ### Workflow Creation and Cleanup For a new plan or implementation, replace separate `create_workflow()` plus `WorkflowLinked` calls with one `JobReporter.create_workflow(workflow, stage)` operation. It emits `WorkflowCreated`; `evolve()` inserts the workflow and links the job in the same SQLite transaction. A process cannot leave a persisted active workflow without its owning job link. Follow-up commands that use an existing completed workflow emit `WorkflowLinked`; no new active row is created. `fail_workflow` retains the conditional update that changes only a linked `active` workflow to `failed`. Emit it for `JobRejected`, `JobFailed`, and `ServiceRestarted` when linked. This cleans up newly created workflows without invalidating completed workflows used by follow-ups. ### Session Abort `abort_sessions` uses linked workflow sessions and the one-shot fix workspace convention. Refactor `OpenCodeClient.abort()` into strict and best-effort modes: - `2xx` means success. - `404` means the session no longer exists and is idempotent success. - `409` means already inactive/conflicting with an abort and is idempotent success. - Transport failures, timeouts, `429`, and `5xx` raise and return the listener task to pending. - Other `4xx` errors remain visible and retry with capped backoff; they are not silently treated as success. - Normal client shutdown uses best-effort mode and logs failures without blocking shutdown. Thus a crash after remote abort but before task completion can safely retry and complete when OpenCode reports the session absent/inactive. Cleanup tasks are indefinitely retryable. A terminal job is not reverted while cleanup is unavailable; outstanding tasks and latest errors remain visible in SQLite and logs. ## Startup Recovery Recover abandoned `running` tasks before normal polling, joining each task to current job state in one transaction: | Task/job state | Recovery action | | --- | --- | | `execute` + `queued` | Reset task to `pending`; `JobStarted` was not persisted, so no workflow effect was allowed. | | `execute` + `running` | Mark task failed and emit deterministic `ServiceRestarted`; never replay it. | | `execute` + terminal | Mark stale task completed/failed without changing job state. | | `authorize` + state no longer `received` | Mark completed; its outcome is already represented in state. | | other idempotent `control` task | Reset to `pending` with attempt/error metadata retained. | After task recovery, emit deterministic `ServiceRestarted` for every remaining `running` job. Duplicate recovery events are harmless. Queued jobs and pending tasks remain intact. This covers both critical crash windows: before `JobStarted`, execution safely returns to the queue; after `JobStarted`, it fails without replaying workflow effects. ## Workflow Refactor Make workflow orchestration an asynchronous effect behind `execute`, while removing direct lifecycle mutation. - Change `Dispatcher.dispatch()` and public entry points in `workflows/plan.py`, `implement.py`, and `pull_request.py` to return the final comment body instead of updating Gitea. - Introduce `JobReporter.progress(stage)`, `create_workflow(workflow, stage)`, `link_workflow(workflow_id, stage)`, and `link_runtime_session(session_id)`. Each emits a deterministic event. - Replace every `storage.update_job()` call in workflows, `ChangeSet`, and `CodeReviewLoop` with reporter calls. - For new workflows, clone/create branches first, then use atomic `create_workflow`; continue updating artifacts through `update_workflow()` afterward. - Keep Git, development setup, Gitea repository/PR reads and PR creation, and OpenCode calls inside workflow listeners. - Remove `update_job_comment()` from `workflows/common.py`. Workflow paths return the same result/review body; comment reconciliation publishes it. - Replace mutable `Job` parameters with immutable `JobState` snapshots. - Preserve all current guards: missing/legacy plans, missing artifacts/sessions, existing open or merged agent PRs, closed/mismatched PRs, and no generated changes. ## Application Wiring and Logging Update `src/agentci/container.py` and `app.py` to construct the state-machine host, reporter/listener dependencies, and both worker loops. Retain the lifespan stop/cancellation path and client shutdown. Webhooks and control listeners continue while OpenCode is unavailable. The jobs worker does not claim execute tasks until readiness succeeds. Add `event_id`, `receive_sequence`, `listener`, `task_id`, and `queue` to structured logging fields. Do not add `state_version`. ## Edge Cases and Guarantees - Commands execute strictly by persisted webhook receive sequence, including when an earlier authorization is retrying. - Duplicate deliveries do not consume another sequence or repeat permission, comments, or execution. - Permission, syntax, and location rejections are terminal persisted jobs and unblock later receive sequences. - A recovered authorization task whose outcome already committed completes without rechecking permission. - A deleted canonical Gitea comment is rediscovered or recreated and relinked for queued, running, and terminal jobs. - Marker recovery trusts only bot-authored comments; multiple valid matches choose the lowest ID deterministically. - A crash before `JobStarted` cannot strand a queued job. A crash after `JobStarted` cannot replay Git/OpenCode effects. - New workflow creation and job linking are atomic, eliminating orphan active workflows in the pre-link crash window. - Rejection/failure cleanup only fails active workflows and preserves completed workflows. - Session abort retries regard absent/already-inactive sessions as success. - Idempotent control work retries indefinitely with bounded delay. External guarantees are eventual and depend on services recovering. - New jobs use `accepted_comment_id`; legacy `started_comment_id` remains queryable for context filtering. ## Test Plan Keep each Python file below the repository’s 250-line limit and split suites where needed. 1. Add `tests/test_state_machine.py` with table-driven coverage for every transition, emitted listener, command parsing after permission, workflow/session links, cleanup notifications, terminal behavior, comment relinking, restart no-ops, invalid transitions, immutability, and deterministic replay. 2. Extend storage tests for atomic event/state/task commits, event dedupe, no version column, receive-sequence allocation, duplicate receives not consuming a sequence, timestamp metadata rules, migration preservation, and historical delivery dedupe. 3. Test migration receive-sequence backfill order and execute-task backfills for legacy queued jobs. 4. Add concurrent task-claimer tests with separate SQLite connections. Assert one claim per task and strict receive-order execution even when a later command authorizes first. 5. Test the intentional FIFO barrier: an earlier `received` job blocks later queued execution; its rejection or authorization allows the next sequence to proceed. 6. Revise webhook tests around a fake state-machine host, covering conversion, raw multiline bodies, status codes, missing delivery IDs, duplicate receives, ignored payloads, and absence of direct Gitea calls. 7. Add authorization tests for success/denial, indefinite retry scheduling, and crashes after outcome commit but before task completion; retries must not recheck permission or emit an invalid transition. 8. Add execution/startup tests for claim-before-`JobStarted`, after-`JobStarted` interruption, deterministic recovery IDs, readiness gating, success/rejection/failure, and latest-stage errors. 9. Add comment tests for create/update, author-verified marker recovery, spoofed user markers, multiple bot matches, crash-after-create, and deletion/relink behavior in queued, running, and terminal states. 10. Add workflow tests proving `WorkflowCreated` inserts and links atomically, including simulated failure between reduction and commit; no active orphan may remain. Verify rejected active workflows fail while completed follow-up workflows remain completed. 11. Add strict-abort tests for `2xx`, `404`, `409`, transport/timeout/server retries, and crash-after-remote-success behavior. 12. Adapt existing workflow tests to reporter events and returned comment bodies while retaining setup order, review loops, artifact persistence, and rejection guards. 13. Update `README.md` with receive-order FIFO, asynchronous listeners, retry semantics, timestamp metadata ownership, comment deletion recovery, atomic workflow linking, and before/after-`JobStarted` restart behavior. ## Verification and Acceptance Criteria Run: ```sh uv sync uv run ruff check . uv run pyright uv run pytest docker compose config ``` The redesign is complete when: - All lifecycle changes flow through pure `next_state()` and transactional `receive()`/`evolve()`. - No job version column or state-version logic remains. - Every command has a durable unique receive sequence, and execution follows that sequence even across authorization retries. - Webhook handlers and workflows no longer call arbitrary job-update or status-comment methods. - Event deduplication, state replacement, workflow creation/linking, and listener scheduling are atomic where specified. - Listener claims are atomic under concurrent workers. - Timestamp metadata follows the documented storage rules without changing reducer output. - Recovered authorization tasks cannot become stale retry loops. - Deleted comments self-heal through author-verified relinking. - No running rejection, failure, or interruption leaves a newly created active workflow orphaned. - A crash before `JobStarted` requeues execution; a crash after it never replays workflow effects. - Strict session abort is idempotent for absent/inactive sessions and retryable for transient failures. - Legacy SQLite data migrates without losing jobs, workflows, sessions, comments, artifacts, delivery dedupe, or queue order. - Existing command behavior, workflow outputs, readiness gating, and completed-workflow protection remain intact. ## Remaining review findings The plan resolves the previously identified version, FIFO, stale-authorization, comment-404, workflow-linking, and abort-response issues. Four material recovery and migration gaps remain. The required duplicate searches found no matching issues. ### MAJOR: Deleted terminal comments have no reconciliation trigger — `Webhook Boundary; Comment Reconciliation; Edge Cases and Guarantees` Reconciliation can recover from a 404 only while a reconciliation task is already running. After a terminal job's final task completes, deleting its bot comment creates no new task because non-created comments are explicitly ignored. The stated guarantee that deleted comments self-heal for terminal jobs therefore cannot hold. Recommendation: Either handle deletion events for known canonical bot comment IDs by enqueueing reconciliation, add a periodic reconciliation sweep, or narrow the guarantee to deletions encountered during pending work. Add an end-to-end test that deletes a comment after all terminal tasks have completed. ### MAJOR: Concurrent reconciliation tasks can still create two comments — `Architecture Decisions; Atomic Listener Claims; Comment Reconciliation` Atomic task claiming prevents two workers from claiming the same task, but queued and JobStarted transitions can create distinct reconciliation tasks for the same job. The plan does not constrain control concurrency. Two control workers can claim those tasks, both observe no accepted_comment_id or marker, and both create a bot comment before either CommentLinked event commits. Later canonical selection does not remove the duplicate, breaking the one-comment UX. Recommendation: Declare and enforce globally single control-worker execution, serialize reconciliation per job, or coalesce/lock comment creation so only one task may perform the search-create-link critical section. Add a concurrent test using two distinct reconciliation tasks for one job. ### MAJOR: Runtime sessions retain a pre-link crash window — `State-Machine Contract: RuntimeSessionLinked; Session Abort; Workflow Refactor` OpenCode session creation is an external effect followed by RuntimeSessionLinked. A crash after the remote session is created but before that event commits leaves no runtime_session_id. Recovery is described as using linked workflow sessions, but the plan does not guarantee that the newly created session is discoverable there before the link, so ServiceRestarted may be unable to abort it. Recommendation: Define a durable session-intent or discovery mechanism that exists before remote creation, or prove recovery can locate every unlinked session by stable job/workspace metadata. Add a crash test between remote session creation and RuntimeSessionLinked for each session-creating workflow path. ### MAJOR: Legacy queued jobs without deliveries cannot satisfy the task foreign key — `Persistence and Migration steps 3, 5, 6, and 8` The migration explicitly supports jobs with no matching delivery by assigning `legacy:<job-id>`, but creates synthetic job_events only for historical delivery rows. It then backfills listener tasks for every legacy queued job, and those tasks require source_event_id to reference job_events. An inconsistent queued job therefore has no valid source event and its task backfill cannot satisfy the foreign key. Recommendation: Create a synthetic migration event for every legacy job lacking a delivery and reference it from backfilled tasks, or define an explicit terminal/quarantine policy for such jobs. Add a migration fixture containing a queued job with no delivery and verify migration plus FIFO recovery.

StanPonomarev commented

2026-07-21 15:56:33 +02:00

/agent implement

agentci commented

2026-07-21 15:56:35 +02:00

Pull request created: http://gitea:3000/StanPonomarev/agentci/pulls/2

Agent result

Implemented the explicit persisted webhook state machine.

Added immutable typed events, JobState, pure next_state(), transition validation, and pure comment rendering.
Added migration 003_state_machine.sql with durable receive sequencing, event inbox, listener outbox, no job version column, and legacy backfills.
Added transactional receive()/evolve(), duplicate delivery handling, atomic workflow creation/linking, timestamp ownership, atomic listener claims, and receive-order FIFO blocking.
Reduced the webhook route to validation and incoming-event persistence; permission checks and command parsing now run asynchronously in receive order.
Reworked the worker into control and single-concurrency jobs queues, persisting JobStarted before workflow effects and preventing replay after interruption.
Routed workflow progress, workflow/session links, and final bodies through JobReporter events instead of unrestricted job updates.
Added comment marker recovery, durable cleanup retries, strict idempotent OpenCode abort handling, restart recovery, tests, and README documentation.

No commit or Git metadata changes were made.

Validation

uv sync — succeeded, but the host-provided UV_NO_DEV=1 removed development tools.
env -u UV_NO_DEV uv sync — succeeded; installed the 10 development packages required for validation.
uv run ruff check . — passed: All checks passed!
uv run pyright — passed: 0 errors, 0 warnings, 0 informations
uv run pytest — passed: 91 passed in 1.99s
git diff --check — passed with no output.
docker compose config — not run successfully: Docker is unavailable on this host (docker: command not found).

Commit: 378e372a4bd842772cff9b7fb69656adea0f01ec

Remaining review findings

One major migration defect found. Focused tests, Ruff, Pyright, and the full pytest suite passed; docker compose config could not run because Docker is unavailable. The focused duplicate-issue search found no existing issue.

MAJOR: Legacy delivery-only dedupe records are discarded — `src/agentci/migrations/003_state_machine.sql:59-60`

The migration backfills job_events exclusively from the rebuilt jobs table. Historically, permission, syntax, and placement rejections called record_delivery() without creating a job, so those rows exist only in deliveries. After migration they have no delivery:<id> inbox record, and receive() checks only job_events; a Gitea redelivery can therefore be accepted as a new command, consume a sequence, repeat authorization/comment effects, and potentially execute if conditions changed. This violates the canonical requirement that every historical delivery remain deduplicated. No matching existing issue was found by tea issues list --repo StanPonomarev/agentci --state all --keyword "legacy delivery deduplication migration".

Recommendation: Preserve every row from deliveries, including rows without jobs, in a durable dedupe representation consulted by receive() (or migrate suitable tombstone/inbox records), and add a migration test proving a legacy delivery-only row remains a duplicate and does not consume a receive sequence.

Pull request created: http://gitea:3000/StanPonomarev/agentci/pulls/2 ## Agent result Implemented the explicit persisted webhook state machine. - Added immutable typed events, `JobState`, pure `next_state()`, transition validation, and pure comment rendering. - Added migration `003_state_machine.sql` with durable receive sequencing, event inbox, listener outbox, no job version column, and legacy backfills. - Added transactional `receive()`/`evolve()`, duplicate delivery handling, atomic workflow creation/linking, timestamp ownership, atomic listener claims, and receive-order FIFO blocking. - Reduced the webhook route to validation and incoming-event persistence; permission checks and command parsing now run asynchronously in receive order. - Reworked the worker into `control` and single-concurrency `jobs` queues, persisting `JobStarted` before workflow effects and preventing replay after interruption. - Routed workflow progress, workflow/session links, and final bodies through `JobReporter` events instead of unrestricted job updates. - Added comment marker recovery, durable cleanup retries, strict idempotent OpenCode abort handling, restart recovery, tests, and README documentation. No commit or Git metadata changes were made. ## Validation - `uv sync` — succeeded, but the host-provided `UV_NO_DEV=1` removed development tools. - `env -u UV_NO_DEV uv sync` — succeeded; installed the 10 development packages required for validation. - `uv run ruff check .` — passed: `All checks passed!` - `uv run pyright` — passed: `0 errors, 0 warnings, 0 informations` - `uv run pytest` — passed: `91 passed in 1.99s` - `git diff --check` — passed with no output. - `docker compose config` — not run successfully: Docker is unavailable on this host (`docker: command not found`). Commit: `378e372a4bd842772cff9b7fb69656adea0f01ec` ## Remaining review findings One major migration defect found. Focused tests, Ruff, Pyright, and the full pytest suite passed; `docker compose config` could not run because Docker is unavailable. The focused duplicate-issue search found no existing issue. ### MAJOR: Legacy delivery-only dedupe records are discarded — `src/agentci/migrations/003_state_machine.sql:59-60` The migration backfills `job_events` exclusively from the rebuilt `jobs` table. Historically, permission, syntax, and placement rejections called `record_delivery()` without creating a job, so those rows exist only in `deliveries`. After migration they have no `delivery:<id>` inbox record, and `receive()` checks only `job_events`; a Gitea redelivery can therefore be accepted as a new command, consume a sequence, repeat authorization/comment effects, and potentially execute if conditions changed. This violates the canonical requirement that every historical delivery remain deduplicated. No matching existing issue was found by `tea issues list --repo StanPonomarev/agentci --state all --keyword "legacy delivery deduplication migration"`. Recommendation: Preserve every row from `deliveries`, including rows without jobs, in a durable dedupe representation consulted by `receive()` (or migrate suitable tombstone/inbox records), and add a migration test proving a legacy delivery-only row remains a duplicate and does not consume a receive sequence.

agentci referenced a pull request that will close this issue

2026-07-21 16:13:59 +02:00

Agent: Redesign the webhook host to be an explicit unambiguous state machine #2

StanPonomarev closed this issue

2026-07-21 22:03:45 +02:00

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: StanPonomarev/agentci#1

Redesign the webhook host to be an explicit unambiguous state machine #1

Implementation Plan: Explicit Webhook State Machine

Objective

Chosen Design

Domain Contract

Persistence and Atomic Evolution

Webhook Boundary

Listener Runtime

Workflow Refactor

Edge Cases and Operational Rules

Tests

Verification and Acceptance Criteria

Remaining review findings

MAJOR: A crash before JobStarted can permanently strand a queued job — Listener Runtime: execute handling and startup recovery

MAJOR: Exhausted control tasks have no consistent recovery outcome — Listener Runtime: control retries; Verification and Acceptance Criteria

MAJOR: JobRejected can leave a linked workflow active — Domain Contract: JobRejected; Workflow Refactor: rejection guards

MAJOR: Listener-task claiming is not specified or tested as atomic — Persistence and Atomic Evolution: listener_tasks and claim_listener_task; Tests: storage

MAJOR: Marker recovery trusts an unverified issue comment — Listener Runtime: reconcile_comment; Edge Cases: one-comment recovery

Implementation Plan: Explicit Webhook State Machine

Objective

Architecture Decisions

State-Machine Contract

Persistence and Migration

Webhook Boundary

Listener Runtime

Authorization

Workflow Execution

Comment Reconciliation

Workflow and Session Cleanup

Startup Recovery

Workflow Refactor

Application Wiring

Edge Cases and Guarantees

Test Plan

Verification and Acceptance Criteria

Remaining review findings

MAJOR: Timestamp ownership contradicts the pure replacement-state contract — State-Machine Contract; Persistence and Migration: evolve steps 5-6

MAJOR: A recovered authorization task can become permanently stale — Listener Runtime: Authorization; Startup Recovery: idempotent control tasks

MAJOR: Comment deletion cannot self-heal once an ID is linked — Listener Runtime: Comment Reconciliation

MAJOR: Workflow creation still has an unhandled pre-link crash window — Workflow Refactor: persist links immediately; Workflow and Session Cleanup; Startup Recovery

MAJOR: The stated global FIFO order is not defined across authorization retries — Architecture Decisions: queue behavior; Authorization retry policy; Edge Cases and Guarantees

MAJOR: Strict session abort is not yet defined as idempotent — Listener Runtime: Workflow and Session Cleanup

Implementation Plan: Explicit Webhook State Machine

Objective

Architecture Decisions

State-Machine Contract

Receive Sequencing and FIFO Semantics

Persistence and Migration

Atomic Listener Claims

Webhook Boundary

Listener Runtime

Authorization

Workflow Execution

Comment Reconciliation

Workflow Creation and Cleanup

Session Abort

Startup Recovery

Workflow Refactor

Application Wiring and Logging

Edge Cases and Guarantees

Test Plan

Verification and Acceptance Criteria

Remaining review findings

MAJOR: Deleted terminal comments have no reconciliation trigger — Webhook Boundary; Comment Reconciliation; Edge Cases and Guarantees

MAJOR: Concurrent reconciliation tasks can still create two comments — Architecture Decisions; Atomic Listener Claims; Comment Reconciliation

MAJOR: Runtime sessions retain a pre-link crash window — State-Machine Contract: RuntimeSessionLinked; Session Abort; Workflow Refactor

MAJOR: Legacy queued jobs without deliveries cannot satisfy the task foreign key — Persistence and Migration steps 3, 5, 6, and 8

Agent result

Validation

Remaining review findings

MAJOR: Legacy delivery-only dedupe records are discarded — src/agentci/migrations/003_state_machine.sql:59-60

MAJOR: A crash before JobStarted can permanently strand a queued job — `Listener Runtime: execute handling and startup recovery`

MAJOR: Exhausted control tasks have no consistent recovery outcome — `Listener Runtime: control retries; Verification and Acceptance Criteria`

MAJOR: JobRejected can leave a linked workflow active — `Domain Contract: JobRejected; Workflow Refactor: rejection guards`

MAJOR: Listener-task claiming is not specified or tested as atomic — `Persistence and Atomic Evolution: listener_tasks and claim_listener_task; Tests: storage`

MAJOR: Marker recovery trusts an unverified issue comment — `Listener Runtime: reconcile_comment; Edge Cases: one-comment recovery`

MAJOR: Timestamp ownership contradicts the pure replacement-state contract — `State-Machine Contract; Persistence and Migration: evolve steps 5-6`

MAJOR: A recovered authorization task can become permanently stale — `Listener Runtime: Authorization; Startup Recovery: idempotent control tasks`

MAJOR: Comment deletion cannot self-heal once an ID is linked — `Listener Runtime: Comment Reconciliation`

MAJOR: Workflow creation still has an unhandled pre-link crash window — `Workflow Refactor: persist links immediately; Workflow and Session Cleanup; Startup Recovery`

MAJOR: The stated global FIFO order is not defined across authorization retries — `Architecture Decisions: queue behavior; Authorization retry policy; Edge Cases and Guarantees`

MAJOR: Strict session abort is not yet defined as idempotent — `Listener Runtime: Workflow and Session Cleanup`

MAJOR: Deleted terminal comments have no reconciliation trigger — `Webhook Boundary; Comment Reconciliation; Edge Cases and Guarantees`

MAJOR: Concurrent reconciliation tasks can still create two comments — `Architecture Decisions; Atomic Listener Claims; Comment Reconciliation`

MAJOR: Runtime sessions retain a pre-link crash window — `State-Machine Contract: RuntimeSessionLinked; Session Abort; Workflow Refactor`

MAJOR: Legacy queued jobs without deliveries cannot satisfy the task foreign key — `Persistence and Migration steps 3, 5, 6, and 8`

MAJOR: Legacy delivery-only dedupe records are discarded — `src/agentci/migrations/003_state_machine.sql:59-60`