Skip to content

feat(devloop): dispatch WorkItem execution requests with canonical execution identity #461

Closed

Description

@mashkovd

Parent roadmap: mctlhq/.github#21 (surfaces)

Problem

Nothing turns "a surface created or opened a WorkItem and asked for it to run" into an execution. mctl-api neither dispatches on create nor signals a DevLoop on resume. The investigator and DevLoop run only when triggered by an issue label, a cron or an operator. The acceptance path of mctl-telegram#443 ("investigator execution launched from Telegram is correlated to WorkItem + execution identity + ContextSnapshot") and of mctl-api#341 has no platform component behind it.

Owner decision (2026-09-23): the bridge belongs to the execution platform, meaning the existing mctl-agents/Temporal orchestration boundary:

  • not the Telegram bot;
  • surface:telegram gets no Temporal or admin privileges.

Goal

A dispatcher in mctl-agents that consumes mctl-api execution requests (mctl-api#368) and runs them through the existing DevLoop / investigator path, owning the canonical execution identity.

execution request (mctl-api, pending)
      ↓ claim (lease, single holder)
dispatcher (mctl-agents, service principal)
      ↓ start DevLoop / investigator, bound to exact WorkItem + request
      ↓ fulfil request: attach canonical execution (engine, engine_ref) → we_ id
investigator run uses that we_ as its execution identity (mctl-agents#455 case a)
      ↓
ContextSnapshot sealed for that execution (#431 / #456)

Required properties

  • Idempotent and fenced:

    • claim through mctl-api's lease CAS;
    • the Temporal workflow id is derived from the request id, with a reuse policy that refuses a second start;
    • fulfil is idempotent on (engine, engine_ref).

    A crash between start and fulfil re-claims after the lease and converges on the same workflow and execution, never a second run.

  • Bound to the exact WorkItem and request. The workflow carries work_item_id, execution_request_id and the fulfilled we_ id. The investigator refuses a mismatch; the existing work-item-mismatch check covers it.

  • Canonical execution identity. The dispatcher creates it at fulfilment, and nothing downstream invents one. This is consistent with mctl-agents#460 (store-issued we_).

  • Scope for v1: investigator runs for WorkItems bound to a GitHub issue (issue_url on the item). A request for an item without one is rejected with a typed reason (no_runnable_target), never guessed.

  • Auditable. Claim, start, fulfil and reject are each logged with request, work item, workflow and execution ids. mctl-api records its own events.

  • Resume. A kind=resume request delivers the resume to the item's live DevLoop through the existing resume signal, or starts a continuation where none is live. The canonical resume path from feat(work-context): resume investigator from canonical WorkItem across surfaces #267 applies. Approval and policy are re-evaluated, not inherited.

  • Governance. The dispatcher's mctl-api mutations go through the feat(governance): runtime policy checkpoints for agent tool actions #197 policy checkpoint, like the other platform writes.

Acceptance

Depends on mctl-api#368. Consumed by mctl-telegram#443 and mctl-api#341.

Activity

  1. mashkovd commented on Sep 23, 2026

    @mashkovd
    MemberAuthor

    Notes from preparing the remaining gaps (not in #468):

    • Order for gap 2 (the loop id) and gap 3 (execution_request_id): gitops CWFT params first (prepared locally), then mctl-api registry declarations (the issue just filed; StripUndeclared drops anything undeclared), then the investigator CLI flags. The CLI must also use the passed workflow id where it now calls workflow_id_for(issue_url) (run_issue_investigator.py post_proposal_comment, context_assembly.py).

    • Risk to fix together with gap 2: once the human-input producer ships, a dispatched loop would reject its own clarification request as belonging to another run. The investigator stamps the issue-keyed id, but the loop compares against dev-loop-xr_*.

    • Gap 1 design (resume onto a live loop). Recommended:

      • the dispatcher delivers via a Temporal Update, accept_execution_request (update id = request id), before fulfilling;
      • it then fulfils with engine_ref = <loop id>#<xr id>;
      • the loop binds the we_ itself through the existing bind activity;
      • duplicates are dropped by the update id plus an accepted-ids set carried across continue-as-new;
      • it goes behind workflow.patched("execution-request-resume"), and reconciliation learns #xr_ refs.

      No mctl-api change is needed (Temporal server/SDK 1.31.0 support Update).

    • Gap 4 (the live-loop race): only a single workflow id per issue, with update-with-start, closes it. That is a larger change, and it also removes gap 2.

  2. mashkovd commented on Sep 23, 2026

    @mashkovd
    MemberAuthor

    #468 merged: the dispatcher for start requests, crash convergence, typed rejects, reconciliation of dead dispatched loops, and the advance landing before every human-input or approval park. It is off by default (EXECUTION_REQUEST_DISPATCHER). This issue stays open for the remaining gaps: resume onto a live loop (design above), dev-loop-xr_* visibility and execution_request_id to the investigator (gitops CWFT params → mctl-api#372 → investigator CLI), the live-loop race, and live e2e.

    P3s carried from the last review round:

    • Advance before park. _await_reapproval and _watch_pr get no pending-advance re-attempt. Either add the call before the merge watch or narrow the 'before every park' wording in ADR-011 §8.
    • The before parameter. It is a free-form string whose only checked value is approval. Use a Literal[...] or pass the RetryPolicy.
  3. mashkovd commented on Sep 23, 2026

    @mashkovd
    MemberAuthor

    #475 merged. The investigator takes --temporal-workflow-id, --temporal-run-id and --execution-request-id from the CWFT (mctl-gitops#1345), and the DevLoop passes them. It stays inert until mctl-api declares the params (mctl-api#372), which must happen after the registry-pinned investigator image contains #475.

    P3 follow-ups from the #475 review:

    • Empty values. --temporal-workflow-id "" hard-exits; the sibling id flags treat an empty value as absent. Normalize "" to None, so a missing Argo when guard cannot break every investigate pod.
    • Silent fallback. Warn when a dispatched run (an --execution-request-id or we_) arrives with no --temporal-workflow-id; the fallback to the derived id is silent.
    • Error message. The 256 cap message cites the snapshot id ceiling, which does not bound these fields. Reword it.
    • Tracing. Put workflow_id/workflow_run_id on the investigator root span (tracing.annotate catalog keys).
    • Self-enforcing rollout. Gate the two DevLoop params on investigator_release.version, so the rollout order does not depend on process alone.
    • Continuation. Assert that the human-input continuation keeps the three ids, and decide whether execution_request_id should ride along after the identity is dropped.

    Still open for this issue: #474 (dispatched-loop visibility, required before enabling the dispatcher), resume onto a live loop, the live-loop race, and live e2e.

  4. mashkovd commented on Sep 23, 2026

    @mashkovd
    MemberAuthor

    Progress: #480 merged (4eceb3a) — a resume request is now delivered onto a live DevLoop via the accept_execution_request Update (gated by execution-request-resume), with minted-but-refused executions ended Failed and terminal advances carried until they land. Still open for this issue: the start race for a live loop (one workflow id per issue + update-with-start), a live end-to-end run, mctl-api#380 (shepherd alias), and the mctl-api#372 loop-id declaration after an agents release that contains #475/#480. Deferred P3s: #482.

  5. mashkovd commented on Sep 24, 2026

    @mashkovd
    MemberAuthor

    Remaining code gap: the start race needs a decision first

    State on main (4eceb3a, after #480): two starters create DevLoops for the same issue under different workflow ids:

    starter id guard against the other
    agents:intake poller (start_dev_loop_workflow) dev-loop-<owner>-<repo>-<n> none: it never looks for a dispatched loop
    dispatcher (start_dispatched_dev_loop) dev-loop-xr_<request id> check-then-start: _live_loop_candidates → loop_state → start

    Temporal gives no atomicity across two ids, so a label added while a request is dispatched (or the reverse) can run two loops on one issue: two investigators, two proposals racing on one slug. Today it can't happen in production, because EXECUTION_REQUEST_DISPATCHER is off.

    Options:

    A. One workflow id per issue, requests delivered by update-with-start (recommended; this is the design sketched in the 2026-09-23 note above).

    • The dispatcher starts or attaches to dev-loop-<issue> with USE_EXISTING, delivering accept_execution_request in the same call, so the race disappears by construction.
    • It also retires dev-loop-xr_*, the devloop: make dispatched dev-loop-xr_* loops visible to shepherd, implement sweep, orphans and lifecycle reconcile #474 memo alias, and the loop-id declaration planned for mctl-api#372 (gap 2).
    • Cost:
      • the per-request "run exactly once" rule (REJECT_DUPLICATE on the xr id) becomes the issue-keyed ALLOW_DUPLICATE_FAILED_ONLY plus the request-id dedupe the loop already carries (accepted_request_ids);
      • engine_ref for a start becomes <issue loop>#<xr id>, as a resume's already is;
      • a replay-safe workflow.patched path;
      • reconciliation updates.
    • Invasive, but it is the only fix that is atomic.

    B. Keep both ids and narrow the window.

    C. Accept the race while the dispatcher is off: document it as a precondition for enabling the dispatcher.

    This choice also decides what mctl-api#372 declares, so it should be made before #372 and before an agents release is cut for it. The other open items are live or release-gated: live e2e, mctl-api#380, and #372 after an agents release containing #475/#480.

  6. mashkovd commented on Sep 24, 2026

    @mashkovd
    MemberAuthor

    Decision (owner, 2026-09-24): option A for the start race

    One workflow id per issue: dev-loop-<owner>-<repo>-<n>.

    • The dispatcher starts or attaches to that loop with USE_EXISTING and delivers the request in the same call (update-with-start, accept_execution_request, update id = request id).
    • dev-loop-xr_* is retired, along with the devloop: make dispatched dev-loop-xr_* loops visible to shepherd, implement sweep, orphans and lifecycle reconcile #474 memo alias. The loop-id declaration planned for mctl-api#372 is dropped.
    • The per-request "run once" rule becomes the issue-keyed reuse policy plus the loop's request-id dedupe (accepted_request_ids).
    • A start's engine_ref is <issue loop>#<xr id>, like a resume's.
    • Replay-safe behind workflow.patched.

    mctl-api#372 waits for this implementation. 1.55.0 (with #475/#480) was released and rolled out 2026-09-24 06:44Z. The dispatcher is still off.

  7. mashkovd commented on Sep 24, 2026

    @mashkovd
    MemberAuthor

    Option A is implemented in #487. There is one refinement of the detail written here, flagged for owner confirmation:

    • The dispatcher's start uses ALLOW_DUPLICATE, not the intake's ALLOW_DUPLICATE_FAILED_ONLY. A resume of a loop that completed has to become a new run of the same issue loop. Under FAILED_ONLY, Update-with-Start refuses that with WorkflowAlreadyStartedError (verified against a real Temporal server on temporalio 1.31.0), so every resume of a finished loop would be stuck.
    • A request still runs once. It is fulfilled once, and a repeated update id is answered from Temporal's registry or from the run's accepted set.
    • The intake poller keeps FAILED_ONLY. A label re-added on a completed issue is still "already handled".
    • The split is pinned by test_after_a_completed_loop_the_dispatcher_starts_a_new_run_and_the_intake_does_not.

    #461 will be closed by hand after #487 merges. Enabling EXECUTION_REQUEST_DISPATCHER in prod, and any live evidence from it, stays a separate human gate.

  8. added a commit that references this issue on Sep 24, 2026
  9. mashkovd commented on Sep 24, 2026

    @mashkovd
    MemberAuthor

    Closing on the code acceptance. Option A landed in #487 (merge b68291c, reviewed head 79d5876):

    • the dispatcher reaches the issue's one DevLoop with Update-with-Start;
    • engine_ref is <loop>#<xr>;
    • replay-safe behind issue-keyed-dispatch and dispatched-not-run-fails, with the pre-A fixtures kept;
    • all 10 acceptance scenarios, the offline end-to-end and 12 mutation checks are in the PR description and review threads.

    The reuse-policy refinement described above was reviewed in the PR: the dispatcher uses ALLOW_DUPLICATE, the intake keeps FAILED_ONLY, and a dispatched run that ran nothing ends FAILED so the intake label is never shadowed.

    The live end-to-end line of the acceptance is not claimed here. It is gated on releases and on enabling EXECUTION_REQUEST_DISPATCHER, both human decisions, and is tracked in #490. The residual P3s are in #488.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions