ENG-812 - Ship: ticket intake is not idempotent — a re-delivered "Ready for Agent" webhook re-builds a merged item

[druks-operator]

Linear ticket: ENG-812

Plan

Implementation plan

1. Represent Build liveness on the existing status owner

Update backend/druks/durable/schemas.py so SubjectStatus exposes is_active, alongside its existing is_running, is_parked, and is_failed predicates.

Derive it from the canonical active states—scheduled, running, and parked—and require kind to be present. That distinction is necessary because _status(None, []) intentionally produces the no-run placeholder SubjectStatus(state=RunState.SCHEDULED, kind=None), which must not suppress a first build.

Do not add a stored liveness flag or change the response/wire fields; this is a computed Python property over the existing status projection.

2. Gate ticket intake before dispatch

Update backend/druks/contrib/ship/subscribers.py in ticket_transition_drives_the_funnel while preserving the existing trigger contract exactly:

payload["source"] == settings.tracker
payload["status"] == settings.trigger_status

Only after that match, resolve the existing work item with:

WorkItem.get_for_ticket_key(
    source=payload["source"],
    ticket_key=payload["identifier"],
)

Apply this dispatch matrix:

Matching trigger state	Result
No work item exists	await Build.dispatch(ticket=payload)
Item exists and item.resolution is set	No dispatch
Item exists, is unresolved, and item.get_status(workflow=Build).is_active	No dispatch
Item exists, is unresolved, and has no active Build	await Build.dispatch(ticket=payload)

Pass the original normalized payload unchanged when dispatch remains allowed. Keep Workflow.start()'s atomic DBOS queue deduplication in place as the race-safe backstop if two deliveries pass the read-side check concurrently; the subscriber guard prevents ordinary redeliveries from entering dispatch at all and prevents a resolved terminal item from receiving a new run whose WorkflowEvent.SCHEDULED reaction would call start_attempt() and clear its resolution.

3. Lock the regression into focused tests

Update backend/tests/test_durable_schemas.py with a state matrix for SubjectStatus.is_active: actual scheduled/running/parked runs are active; finished/failed/cancelled/orphaned runs are not; and the synthetic scheduled status with no driving run is not active.

Update the subscriber-routing section of backend/tests/ship/test_webhooks_jira.py, which already owns the tracker-neutral ticket_transition_drives_the_funnel tests:

• Give the existing first-dispatch test database access now that matching triggers perform a work-item lookup, and retain its exact Build.dispatch(ticket=payload) assertion.
• Add a resolved-item redelivery case that asserts no dispatch.
• Add a parameterized scheduled/running/parked Build case that asserts no dispatch for each in-flight state.
• Add an unresolved item with a terminal failed Build case that asserts a genuine retrigger still dispatches the unchanged payload.
• Preserve the existing non-trigger-status and non-selected-tracker coverage.

Out of scope

Rejecting stale webhooks by delivery timestamp remains out of scope: item-state idempotency covers the observed cases and also handles manual re-toggles; a timestamp guard is separate hardening only if needed.

No database migration, new liveness column, webhook payload change, or endpoint contract change is required.

Acceptance criteria

AC1

Description: AC1: SubjectStatus exposes an is_active predicate that is true for an actual scheduled, running, or parked run and false for terminal runs and for the no-run status placeholder (state=scheduled, kind=None).

Verification: Code inspection of backend/druks/durable/schemas.py and focused state-matrix tests in backend/tests/test_durable_schemas.py.

AC2

Description: AC2: For a matching ticket.transitioned trigger, an existing WorkItem with a non-empty resolution does not call Build.dispatch. The item is resolved by source=payload["source"] and ticket_key=payload["identifier"].

Verification: A subscriber test creates a resolved matching item, invokes ticket_transition_drives_the_funnel, and asserts the mocked Build.dispatch was not awaited.

AC3

Description: AC3: For a matching trigger and unresolved item whose Build run is scheduled, running, or parked, ticket_transition_drives_the_funnel does not call Build.dispatch, preventing a re-delivery from stacking another build.

Verification: A parameterized subscriber test seeds each active Build state for the matching item and asserts the mocked dispatcher was not awaited.

AC4

Description: AC4: A matching trigger still calls Build.dispatch(ticket=payload) with the unchanged normalized ticket payload when no item exists or when the matching unresolved item has no active Build, including after a terminal failed run.

Verification: Subscriber tests assert the exact dispatch call for a new ticket and for an unresolved item with a terminal Build run.

AC5

Description: AC5: Tracker and trigger-status filtering remains unchanged: events from a non-selected tracker or outside settings.trigger_status do not dispatch and do not perform work-item idempotency routing.

Verification: Existing subscriber tests continue to cover the non-selected tracker and non-trigger status branches.

Ruled out

• Relying only on Build.start() queue deduplication: it already prevents two simultaneously active runs, but a resolved item's prior run is terminal and has released its slot. A stale delivery can therefore enqueue a fresh run, whose scheduled reaction clears the stored merge resolution.
• Rejecting deliveries by webhook timestamp: this introduces freshness and clock-ordering policy while failing to address a manual re-toggle of a resolved ticket. The work-item resolution and active-run state cover both observed redelivery cases without that extra policy.
• Persisting an in_flight flag, adding a row lock, or using an in-process mutex: liveness already derives from durable run state, and DBOS queue deduplication is the atomic concurrency guard. Mirroring that state would add synchronization failure modes and, for a column, an unnecessary migration.
• Suppressing every trigger for an existing work item: this would strand legitimate retriggers after a failed or otherwise terminal unresolved Build. Only resolved items and items with an actually active Build are no-ops.

Acceptance Criteria

• AC1: SubjectStatus exposes an is_active predicate that is true for an actual scheduled, running, or parked run and false for terminal runs and for the no-run status placeholder (state=scheduled, kind=None).
  • Verification: Code inspection of backend/druks/durable/schemas.py and focused state-matrix tests in backend/tests/test_durable_schemas.py.
• AC2: For a matching ticket.transitioned trigger, an existing WorkItem with a non-empty resolution does not call Build.dispatch. The item is resolved by source=payload["source"] and ticket_key=payload["identifier"].
  • Verification: A subscriber test creates a resolved matching item, invokes ticket_transition_drives_the_funnel, and asserts the mocked Build.dispatch was not awaited.
• AC3: For a matching trigger and unresolved item whose Build run is scheduled, running, or parked, ticket_transition_drives_the_funnel does not call Build.dispatch, preventing a re-delivery from stacking another build.
  • Verification: A parameterized subscriber test seeds each active Build state for the matching item and asserts the mocked dispatcher was not awaited.
• AC4: A matching trigger still calls Build.dispatch(ticket=payload) with the unchanged normalized ticket payload when no item exists or when the matching unresolved item has no active Build, including after a terminal failed run.
  • Verification: Subscriber tests assert the exact dispatch call for a new ticket and for an unresolved item with a terminal Build run.
• AC5: Tracker and trigger-status filtering remains unchanged: events from a non-selected tracker or outside settings.trigger_status do not dispatch and do not perform work-item idempotency routing.
  • Verification: Existing subscriber tests continue to cover the non-selected tracker and non-trigger status branches.

[druks-reviewer]

Contract verification — ENG-812, implementation revision 1

All five acceptance criteria are satisfied by the diff. Approving.

Per-criterion evidence

AC1 — SubjectStatus.is_active — pass. backend/druks/durable/schemas.py:191-193 adds the computed property as bool(self.kind) and self.state in ACTIVE_STATES, reusing the pre-existing ACTIVE_STATES tuple from durable/enums.py:16 rather than restating the state list. The bool(self.kind) guard is exactly the distinction the plan called for: _status(None, []) yields the no-run placeholder SubjectStatus(state=SCHEDULED, kind=None), which must not read as active. No stored flag, no wire-field change, no migration — as specified. backend/tests/test_durable_schemas.py:34-58 locks the full matrix: scheduled/running/parked true; finished/failed/cancelled/orphaned false; placeholder false.

AC2 — resolved item does not dispatch — pass. subscribers.py:80-85 resolves the item with the exact call the plan named (WorkItem.get_for_ticket_key(source=payload["source"], ticket_key=payload["identifier"])) and short-circuits on not item.resolution. test_trigger_status_does_not_redispatch_a_resolved_item asserts build.assert_not_awaited(). This closes the ticket's root cause: a resolved item can no longer receive a run whose WorkflowEvent.SCHEDULED reaction calls start_attempt() and wipes resolution='merged'.

AC3 — active build does not stack a second — pass. The not item.get_status(workflow=Build).is_active clause covers it, and test_trigger_status_does_not_redispatch_an_active_build is parameterized over scheduled/running/parked, asserting no dispatch for each.

AC4 — genuine triggers still dispatch — pass. if not item or (...) keeps first-build intake intact; test_trigger_status_dispatches_build_with_the_webhook_payload retains its exact build.assert_awaited_once_with(ticket=payload) assertion and now takes druks_db for the new lookup. test_trigger_status_redispatches_after_a_failed_build seeds a terminal failed run on an unresolved item and asserts the unchanged payload dispatches — the retrigger case the plan explicitly refused to strand.

AC5 — filtering unchanged — pass. The tracker and trigger-status conditions are byte-identical to the prior code and still guard the whole block, so the lookup only runs after a match. test_refinement_candidate_status_no_longer_dispatches and test_nonchosen_tracker_status_does_not_dispatch are unmodified and still take no druks_db fixture, which is itself proof that neither branch touches the work-item path.

Verification profile

• uv run ruff check backend — pass (local: "All checks passed!"), and covered by the green checks run for b63ce06.
• uv pip install -e backend/tests/druks-field_notes — pass.
• uv run pytest backend/ — pass, 1128 passed (local, against Postgres 16 + Redis 8 containers matching the CI service images).
• uv run pytest backend/tests/test_proof_extension*.py — pass, 5 passed.
• npm --prefix frontend run lint / run build / test — not run: no Node toolchain in this sandbox. The diff touches zero frontend files, and .github/workflows/on-pull-request-frontend.yml has a paths: frontend/** filter so it correctly does not trigger for this backend-only change. No regression surface.
• GitHub checks for b63ce06: completed, success.

Notes on approach fidelity

The implementer followed the plan's ruled-out list rather than working around it: no in_flight column, no row lock, no timestamp guard, no migration. Workflow.start()'s DBOS queue dedup is left untouched as the race backstop, with the subscriber guard handling ordinary redeliveries on the read side. The single-expression guard mirrors the shape of the sibling pr_close_settles_the_item guard directly above it, which is what the ticket asked for.

Open findings

• backend/tests/ship/test_webhooks_jira.py:169 (low) — there is no subscriber-level test for an existing unresolved item that has zero Build runs at all, i.e. the path where get_status(workflow=Build) returns the kind=None placeholder and is_active must be false. That behaviour is proven at the schema layer by test_subject_status_without_a_driving_run_is_not_active, and the failed-run test exercises the adjacent unresolved-plus-terminal case, so the composed path is only inferred rather than asserted end to end. If you want it closed, the cheapest fix is one more case alongside test_trigger_status_redispatches_after_a_failed_build that calls make_test_work_item and seeds no run, asserting build.assert_awaited_once_with(ticket=payload). Equally reasonable to leave it — the two layers together already pin the behaviour, and I am not blocking on it.
• backend/tests/ship/test_webhooks_jira.py:176 (low) — the resolved-item test assigns item.resolution = "merged" directly instead of calling item.resolve(merged=True, at=...). Direct assignment keeps the test off the event-recording side effects in WorkItem.resolve, which is a defensible choice for a routing test; the trade-off is that the fixture no longer proves the field is set the way production sets it. Either shape is fine — flagging it so the choice is visible, not asking for a change.

Push back on any of this if you read it differently — both notes are judgement calls, and neither gates the merge.

[druks-operator]

Code review: Clean, well-scoped fix — reuses WorkItem.get_for_ticket_key and the existing ACTIVE_STATES/is_parked/is_running pattern for the new SubjectStatus.is_active, and the new tests exercise behavior (dispatched or not, given item/build state) rather than implementation; only a low-severity nit: the compound guard in ticket_transition_drives_the_funnel (not item or (not item.resolution and not item.get_status(workflow=Build).is_active)) reads dense and the function's docstring still describes only the old trigger-status behavior, so a named guard clause (e.g. if item and (item.resolution or ...): return) plus a docstring line on the idempotency skip would help the next reader.