Skip to content

feat(workflow): add durable pull request delivery - #1306

Open
nateEc wants to merge 3 commits into
Tencent:mainfrom
nateEc:feat/1090-workflow-pr-delivery
Open

nateEc wants to merge 3 commits into
Tencent:mainfrom
nateEc:feat/1090-workflow-pr-delivery

Conversation

@nateEc

@nateEc nateEc commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes #1090.

A successful writable workflow step can now set deliver: pr. Its completed agent work enters a durable delivering state while the existing GitProvider opens or reuses the review request. Dependents wait for delivery; failures retain the pushed branch, and retry resumes delivery without another agent attempt. Composite workflows deliver their shared branch once, fan-out items deliver independently, and each new loop pass keeps its own branch/PR. TeamAI never invokes a provider merge API.

Stacked targets follow the recorded branch origin, with for_each.after before other needs and inherited workflow dependencies. Delivery checks the exact commit against the run base and rejects protected CI, agent, MCP, rules and hook paths before provider creation. Existing provider authentication is reused without interactive login; generic Git remains transport-only. Open-request lookup and recognized-URL metadata validation cover GitHub, GitLab, TGit, GitCode and CNB.

Driver changes.pr_url accepts HTTPS review services on any host, strips queries/fragments, rejects userinfo/invalid/oversized URLs, and returns value-free dropped reasons without rejecting the report. Unknown nested changes fields appear by name in ignored. End records publish only validated changes links as ordered, deduplicated prs, honoring reporting opt-out; no fields are added to done. Cancellation retains a safe receipt if an in-flight provider request completes, without reviving the step. Execution bounds survive delivery, and limits reached by a report reconcile before provider I/O.

Type of Change

  • New workflow delivery feature and recovery behavior
  • Bilingual guide/CI recipe, dashboard delivery status and generated help reference

Test Plan

  • npm run typecheck, npm run lint, git diff --check
  • npx vitest run workflow github-provider gitlab-provider gitcode-provider cnb-provider --minWorkers=1 --maxWorkers=2: 26 files, 1,311 tests passed. Final provider/transport/recovery review was followed by 36 passing focused tests; run controls, records and dashboard coverage also passed (157 tests).
  • Sanitizer table cases and 300 fast-check runs verify no userinfo/query/fragment survives, idempotence and the 512-character cap. Provider tests cover existing requests, wrong branches/repositories/forks, auth failure, pagination, configured GitLab subpaths and CNB refs/heads metadata. Credentialed HTTPS and SSH transport URLs are normalized without passing credentials to provider options.
  • Real Git coverage checks pushed content, origin/after/inherited/skip stacking, one composite PR, per-pass PRs, same-branch reuse, protected inherited changes, delivery-only retry, expired delivery bounds, cancellation during provider I/O and ordered record links.
  • npm run test:e2e -- workflow-pr-delivery-1090 workflow-conformance workflow-workspaces workflow-inline workflow-subworkflow workflow-loop-fan-out: 6 files, 40 CLI E2E cases passed, including accepted/dropped driver review links.
  • Real CLI at commit 16afa01, npm run test:e2e -- workflow-pr-delivery-1090 (build first): isolated HOME, init <team> --scope workspace, pull, and workflow start --driver local --auto-approve --inputs. Real local bare remotes verify committed/pushed content; a fake authenticated gh checks the actual PR request and returned URL. Restarting persisted delivery reuses the open PR. Generic Git failure names and preserves the remote branch. Required unattended approval fails with hitl_required; no merge or login operation is accepted by the fake provider.
  • Combined locally with feat(workflow): enforce scoped requirements and execution limits #1303 and feat(workflow): add session discovery and managed public leases #1304: typecheck; 90 focused tests, 28 delivery/capability/budget integration tests, and all three real-CLI feature round trips passed. The budget integration confirms a report that reaches the token limit cancels delivery before any PR is created. This PR independently targets main and includes neither branch's commits.

Notes for Reviewers

  • No live authenticated provider account or model was used. GitHub's CLI path is exercised through the fake executable and real Git transport; other provider lookups/validation use fixtures, with live provider coverage left to CI/maintainers.
  • Provider I/O is outside the run lock under a reclaimable delivery lock. A cancelled in-flight creation cannot be revoked remotely; its safe receipt is recorded if it completes, and the step stays cancelled. Failed push/creation never deletes the branch. A composite's worktree may already be cleaned after its last child, as discussed on the issue.
  • New loop workspaces intentionally leave earlier PRs open. There are no merge, CI-wait, review-comment or trigger automation paths. Open-request scans on providers without a head filter fail safely after 20 pages rather than assuming no match.
  • Provider API contracts checked against GitHub pull request REST docs, GitLab merge request API, GitCode PR API and the CNB OpenAPI schema.

Checklist

  • Changes are scoped to delivery and its protocol/state consumers
  • Tests and real-CLI verification recorded above
  • English/Chinese user guides updated together
  • Conventional Commit with a non-empty detailed English body, following the repository's predominant recent history

Related Issues

Closes #1090.

Review follow-up (6056c15)

Retain both nested workflow and enclosing step timeout bounds when entering delivery, consistent with the requirements follow-up in #1303. All 29 delivery tests, typecheck, lint and the build-first real CLI delivery/reuse/recovery/required-gate round trip passed.

Cancellation/validation follow-up (f6490bc)

Addressed the automated review's cancellation race: after a pending open-request lookup, the engine revalidates that the same attempt/pass is still delivering before issuing a create call. A cancellation completed during lookup now creates no PR; an already in-flight creation still records its safe receipt without reviving the cancelled step. The regression failed before the fix with one unwanted create call and now passes.

Commands with no explicit workspace are always read-only, so deliver: pr is now rejected for those commands during parsing, including inside a writable workflow; valid inherited writable prompt workspaces are retained. Both language guides explain the distinction. 118 delivery/parser/provider/guide tests passed, with typecheck, lint and the build-first real CLI delivery/reuse/recovery/required-gate round trip.

- Add deliver: pr for writable leaves, composite branches, fan-out items and
  loop passes. Persist post-agent delivery separately, serialize provider I/O
  outside the run lock, retain cancellation receipts and retry failed delivery
  without rerunning the agent or losing the pushed branch.
- Record copy origins and resolve stacked targets through after dependencies,
  skipped/read-only steps and inherited workflow needs. Reuse open requests
  through existing GitHub, GitLab, TGit, GitCode and CNB authentication; retain
  the generic Git fallback and never invoke a provider merge API.
- Inspect the exact delivered commit against the run base before PR creation,
  reject protected CI/agent/MCP/rules/hook paths, and preserve execution bounds
  across delivery. Reconcile limits reached by a report before provider I/O.
- Validate and sanitize driver review URLs without exposing rejected values,
  report unknown changes fields by name, and publish ordered, deduplicated prs
  from validated changes only. Keep usage-report opt-out and done envelopes.
- Document delivery and the unattended CI path in both languages, update help
  and dashboard state, and verify 1,311 related unit tests, 40 CLI E2E cases,
  focused provider/recovery/security tests, typecheck, lint and combined
  requirements/session/delivery integration against local fixture remotes.
@github-actions

Copy link
Copy Markdown
  • [P1 blocking] Recheck cancellation before creating the PR — src/workflow/delivery.ts:116. If an open-request lookup is pending when workflow cancel cancels the step/run, a lookup returning no match still proceeds to createPullRequest. The later state check preserves cancellation but cannot undo the unwanted PR. Revalidate the delivery state after lookup and before creation; cancellation during lookup is not an already in-flight creation.

  • [P2 non-blocking] Reject delivery on implicitly read-only steps — src/workflow/parse.ts:496. Validation checks only explicit workspace: read-only. A top-level command with deliver: pr and no workspace passes validation, executes read-only, then fails delivery because it has no writable branch. Validate the effective workspace mode, while retaining valid inherited writable workspaces.

The PR description supplies sufficient testing, including a build-first real-CLI record at 16afa01. Deferred live-provider coverage is not blocking. No earlier findings were supplied. No PR code was run, built, or installed.

- Carry both a workflow step timeout and its nested workflow timeout into the
  delivery deadline, keeping the earliest bound rather than preferring one.
- Keep the root and lease deadlines in the same minimum so delivery cannot
  relax the constraints established by the requirements implementation.
- Verify all 29 delivery regressions, typecheck, lint and the build-first real
  CLI PR creation/reuse, generic-host recovery and required-approval cases.
@github-actions

Copy link
Copy Markdown
  • [P1 blocking] Recheck cancellation before creating the PR — src/workflow/delivery.ts:116. If workflow cancel cancels the step/run while open-request lookup is pending, a lookup returning no match still calls createPullRequest. The later state check cannot undo that unwanted PR. Revalidate cancellation after lookup and before creation. Earlier finding remains unresolved.

  • [P2 non-blocking] Reject delivery on implicitly read-only steps — src/workflow/parse.ts:496. Validation rejects only explicit workspace: read-only. A top-level command with deliver: pr and no workspace passes validation, executes read-only, then fails delivery without a writable branch. Validate the effective workspace mode while preserving valid inherited writable workspaces. Earlier finding remains unresolved.

The description documents sufficient testing, including build-first real-CLI verification at 16afa01. Its older commit reference and deferred live-provider coverage are not blocking.

No additional confident findings. Only the scoped diff was read; no PR code was run, built, or installed.

- Reload the durable run and verify the same delivering pass/attempt after an
  open-request lookup returns no match, before starting any PR creation request.
  Cancellation during lookup now prevents creation; already in-flight creation
  still retains its safe receipt without reviving cancelled work.
- Reject deliver: pr on commands with an implicitly read-only workspace, while
  retaining prompts that inherit a writable enclosing workflow. Clarify the
  command requirement in both language guides.
- Add a regression that cancels during lookup and previously observed an
  unwanted create call, plus parser coverage for implicit read-only commands
  and valid inherited prompt workspaces.
- Verify 118 delivery/parser/provider/guide tests, typecheck, lint and a
  build-first real CLI delivery, reuse, recovery and approval round trip.
@github-actions

Copy link
Copy Markdown
  • [P2 non-blocking] Reject delivery on top-level prompts without a writable workspace — src/workflow/parse.ts:497. The new check handles commands, but a top-level prompt with deliver: pr and no workspace still passes validation. It has no writable branch or changes, so successful agent execution ends in delivery_failed. Validate the effective prompt workspace while preserving inherited writable workspaces.
  • [P2 non-blocking] Add a runner to the CI recipe — docs/guide/workflow.md:234. jobs.ship has neither runs-on nor a reusable-workflow uses, so GitHub Actions rejects the example before any CLI command runs. Add runs-on in both language versions.

Both earlier findings are resolved for their reported scenarios: cancellation is rechecked after lookup, and commands without an explicit writable workspace are rejected.

The description documents sufficient build-first real-CLI testing. The older commit reference and deferred live-provider coverage are not blocking.

Only the scoped diff was read; no PR code was run, built, or installed.

@SaulMoro SaulMoro 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.

This was generated by AI during triage.

@nateEc Thanks. This branch conflicts with main in src/workflow/types.ts and src/__tests__/workflow-records.test.ts. Main caused it: #1234 (2401d047) added workflow_namespace_conflict and prompt_namespace_conflict to the WorkflowDiagnostic['code'] union and to the diagnostics list in the records test, on the same lines this PR extends. Keeping both sets of codes should resolve it. We'll review the rebased head. #1303, #1304 and #1306 also share most of their workflow files (types.ts, parse.ts, local-driver.ts, src/index.ts, the workflow guides), so whichever lands first will need the other two rebased again.

@SaulMoro

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

@nateEc an update to the request for changes: the conflict with main is now wider than the two files it named. #1280 (publish and resume workflow runs across machines) landed and touches the same engine. This head now conflicts in src/workflow/engine.ts, src/workflow/step-state.ts, src/workflow/conformance.ts, src/workflow/types.ts, the workflow guides, docs/dev/workflow-engine.md and the conformance and records tests. Please rebase onto current main. Keep #1234's workflow_namespace_conflict and prompt_namespace_conflict codes and #1280's publish/resume state, then rerun the real-CLI record on the result.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feat] Deliver workflow branches as pull requests

3 participants