Skip to content

Track final delivery and safely retry pull request publication - #537

Open
JWThewes wants to merge 24 commits into
fix/blocked-unit-completionfrom
fix/intent-delivery-status-retry
Open

JWThewes wants to merge 24 commits into
fix/blocked-unit-completionfrom
fix/intent-delivery-status-retry

Conversation

@JWThewes

@JWThewes JWThewes commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

A successfully executed intent can still fail to publish one or more final pull requests. Persist final delivery separately from execution success, show repository-level results, and let an authorized project owner/admin preview and retry publication without rerunning stages, gates, models or Git integration.

The shared publisher handles first publication and retry. It uses #534's canonical completion owner, verifies current plan/evidence again before provider creation, and fences claims/writes with the run, plan and delivery attempt. Missing required work, changed or unavailable completion evidence withholds publication. Pre-deploy MERGED rows without integrationResults remain integrated; explicitly empty, invalid or failed evidence retains strict checks. A legacy serial intent with no unit diagnostics still needs verified workflow scope before publication. Approved partial work remains eligible. Existing repository successes survive retry; uncertain create responses are reconciled against provider state before another create. Provider/head/base checks, a bounded delivery lease, ordinary rewind/run exclusion and tracker handoff remain.

Depends directly on the narrowed #534 preventive completion contracts. It has no #536 compatibility or #539 audit dependency. #539 is an independent sibling on #534; terminal-preservation recovery and #536 are deferred together.

Original correction validation at c35dc98 on Node 24 (one backend worker):

  • Affected API/orchestrator/provider/real local DynamoDB and replay suites: 452 tests passed, including actual initial/retry publication of pre-deploy multi-repository MERGED rows without rebuilding unit work.
  • Frozen marker-absent histories and completion-policy-1/no-delivery histories preserve publication ordering. The unreleased intermediate completion version is removed with Prevent incomplete intents from reaching success and publication #534.
  • Rendered delivery/intent/unit-lane tests: 92 passed. Lint, formatting, secretlint, dependency/SDK checks and docs build passed. Normal commit hooks: 1,098 tests passed.
  • The corrected Prevent incomplete intents from reaching success and publication #534 base passed its full backend suite (5,302 passed, 3 existing Bun-dependent skips). The delivery feature's earlier full-suite/build/release/Terraform validation predates this base update; affected checks above were rerun on the current tree. This correction changes no infrastructure or dependencies.

Cancellation amendment at bc26dd8 inherits #534's corrected aborted classification through an additive merge. Cancellation/failed-abort states cannot offer retry or publish: real DynamoDB delivery suite 40 passed; the other affected shared/API/final-completion/frozen-replay files 72 passed. Normal commit hooks 1,107 passed. Lint, formatting, secretlint, dependency/SDK checks passed. The full-suite and rendered UI counts above are historical for the preceding correction heads; no production delivery behavior or frontend source is changed by this amendment.

Deploy source-control first, then the coordinated orchestrator/API route and frontend. New histories capture delivery policy 1 in the existing load-meta step; old execution checkpoints and runtime/methodology pins are preserved. No provider/model E2E, production retry, migration or deployment was performed. Provider-side concurrent edits remain external to DynamoDB ownership and are checked immediately before creation.

@JWThewes
JWThewes removed this pull request from stack #551 October 15, 2026 15:36
@JWThewes
JWThewes force-pushed the fix/intent-delivery-status-retry branch from 65163f4 to 7fc0bd9 Compare October 15, 2026 15:46
@JWThewes JWThewes changed the title Track final PR delivery and retry publication without rerunning work Track final delivery and safely retry pull request publication Oct 15, 2026
@JWThewes
JWThewes changed the base branch from fix/recovery-compatibility-preflight to fix/blocked-unit-completion October 15, 2026 15:46
@JWThewes

Copy link
Copy Markdown
Contributor Author

@jeromevdl This is now a delivery-only follow-up at 7fc0bd9, based directly on the narrowed #534. It no longer depends on #536 or #539.

It still needs #534's shared completion/evidence contracts, so those are the explicit prerequisite instead of the recovery stack. Both initial publication and retry use that completion owner. Delivery-specific checks previously in #539 now live here: fresh eligibility before provider creation, plan/attempt fencing, reconciliation of uncertain create responses, and preservation of successful repository results. Retry does not rerun models, stages, gates or Git integration.

Validation: 5,343 backend tests passed (3 existing skips), 840 frontend tests and production build, 97 release tests, Terraform validation and 19 mock tests. The five replacement CI checks pass on this head; superseded runs were cancelled, and CodeQL is not reported for this head. The independent audit sibling was also combined and tested locally. No deployment or live provider retry was performed.

Could you re-review the delivery scope and its reduced dependency on #534?

@jeromevdl jeromevdl left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 — A rewound or re-iterated intent can't publish when its branch already has a closed or merged PR

Where: lambda/shared/delivery-provider.js:14 (lookup over ['all'], or every terminal state on Bitbucket), lambda/shared/final-delivery.js:201 (refuses a PR that isn't open), docs/using-the-platform/git-integration.md:75.

What happens

  1. The intent branch is fixed when the intent is created (lambda/intents/index.js:5964). Rewind and relaunch keep it.
  2. A new run has no DELIVERY#<runId> row, so inspect searches for PRs by source branch alone, in every state. It finds the PR from the earlier run.
  3. If that PR was closed (a reviewer rejected it and the user rewound to redo the work) or merged (the user iterates on the intent after a merge), the repository is recorded as failed with retryable: false: "retry will not recreate closed work".
  4. The automatic publication and Retry delivery both refuse. The new run's work can never be published from the platform.
  5. Code also shows the old closed PR's link as if it were the current PR, because deliveryView returns pr for failed repositories.

Before vs. after: before, GitHub POSTs a new PR (createPullRequest only reuses an open PR on a 422), and CodeCommit's per-run attemptKey exists for this exact case. The docs still say "a new execution can open a replacement". The closed/merged tests (final-delivery.test.js, it.each(['closed', 'merged'])) only cover a lost response within the same run.

Fix: accept a terminal PR as this run's own only if this run actually tried to create it. For example, persist a create-requested timestamp or the attempt key on the repository row before create-pr, and match on it or on the PR's creation time. Otherwise reconcile open PRs only. Add a test: a new run after a closed or merged PR publishes a new PR.


P2 — Older serial intents without a release pin can't be checked or retried

Where: lambda/shared/final-delivery.js:115, together with #534's lambda/shared/unit-completion.js:339.

What happens

  1. #534 now returns null from assessExecutionCompletion for an intent without a pin that has no unit plan and no unit rows. GET treats such an intent as fine, and the page no longer says Completion unverified.
  2. assertDeliveryComplete maps that null back to unverifiedCompletion('legacy_workflow_unpinned').
  3. Check delivery on such an intent therefore returns 409 delivery_unverified, with "0/0 units verified integrated, 0 explicitly omitted… Completion evidence unavailable". This is the message #534 just removed from the intent page.
  4. Older completed runs are one of the cases this PR advertises ("Older completed runs initially show Delivery not yet verified"). Serial intents without a pin are a supported creation path, and they can't get past the check.
  5. The existing test (does not assess an unpinned legacy workflow as verified) uses the parallel workflowPlan. The serial case has no test.

Fix: when there is no UNITPLAN row, the resolved plan has no parallel section, and every serial stage row is SUCCEEDED/SKIPPED (already checked below), accept the intent as complete. If keeping the refusal is deliberate, show a message that explains it instead of "0/0 units verified". Either way, add a serial no-pin preview test.


P2 (maintainability) — Completion is re-assessed three times, and create-time failures are misclassified

Where: lambda/shared/final-delivery.js:343, :360, :417 (verifyCompletion before the claim, after the claim, and inside openPr), :460 (completionFailure), and lambda/shared/intent-pr.js:249-252.

What happens

  1. The publisher re-runs the full assessment each time: getCompletionRecords, plan reload and assessExecutionCompletion. Claim and save also fence the unit plan.
  2. When the recheck inside openPr fails, or saveDelivery({ ifUnitPlan }) is cancelled, the error is caught by openIntentPrs's per-repository try/catch. It becomes v2.pr.failed "PR open failed for …: ".
  3. The repository is therefore recorded as retryable: true, and its reason can be a raw DynamoDB "Transaction cancelled…" string. The completionFailure → retryable: false branch never runs for this phase.
  4. No create happens and a later preview refuses, so this is safe. But the UI offers a retry that is guaranteed to fail. The test withholds a provider mutation when evidence changes during provider inspection only checks status: 'failed'.

Why it matters: for a run whose completionValidation receipt matches the current run, #534 has already made the completion decision. After that, a rewind changes the run ID, which is checked, and recovery has to write META, which the delivery lease blocks. Re-assessing during publication duplicates #534's decision path, and every future change to completion rules has to keep both callers aligned.

Fix: when the receipt matches the current run, trust it, checked once in the claim transaction together with ifUnitPlan. Use assessExecutionCompletion once, at preview/claim time, only for older runs that have no receipt. Drop the recheck inside openPr. If the recheck stays, rethrow delivery_* and transaction errors out of openPr so they're recorded as non-retryable with a readable reason.


Notes (non-blocking)

  • Tracker handoff: waiting for complete publication is now documented (git-integration.md:238; orchestrator index.js:2331, :441). But if one repository is refused permanently (retryable: false), the tracker never receives the PRs that did open, and no event says so. Consider emitting a v2.tracker.deferred event.
  • Read limit: delivery reads completion records with includeStages: true under the default maxRecords = 2000, while #534's finalize reads without stage rows. A very large intent could pass finalization but have delivery withheld as completion_read_limit. I haven't confirmed realistic row counts; worth a quick check.

@JWThewes
JWThewes force-pushed the fix/blocked-unit-completion branch from 9288d89 to b6883ab Compare October 26, 2026 13:06
@jeromevdl

Copy link
Copy Markdown
Collaborator

@JWThewes, what's the status of that one?

@jeromevdl jeromevdl added area/frontend React application in frontend/ area/agents AgentCore runtime: CLI drivers, MCP tools, workspace and Git operations area/backend Lambda APIs, lambda/shared, v2-orchestrator area/infra Terraform, installer and deployment size/XL 3000+ changed lines (lockfiles excluded), consider splitting labels Oct 30, 2026

This branch has not been deployed

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

Labels

area/agents AgentCore runtime: CLI drivers, MCP tools, workspace and Git operations area/backend Lambda APIs, lambda/shared, v2-orchestrator area/frontend React application in frontend/ area/infra Terraform, installer and deployment size/XL 3000+ changed lines (lockfiles excluded), consider splitting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants