Skip to content

[feat] Deliver workflow branches as pull requests #1090

Closed

Description

@hsuchifeng

Parent: #818

Goal

Implement deliver: pr through the existing provider abstraction while ensuring TeamAI never merges workflow changes.

Scope

  • Add deliver: pr.
  • Resolve the target to the nearest earlier delivered branch along first needs, otherwise the run base.
  • Accept a driver-provided pr_url.
  • When no URL is reported, create the PR through the existing GitProvider.
  • Represent post-agent delivery as a distinct state.
  • Preserve the branch and workspace when push or PR creation fails.
  • Validate a reported PR URL against the expected repository and branches when provider data permits.
  • Reject protected-path changes against the run base, including CI, agent, MCP, rules, and equivalent tool configuration.
  • Keep the execution-time half in [feat] Add workflow workspaces, branches, and session continuation #1080: later steps run with agent configuration from the run’s base before they execute.
  • Reuse existing provider authentication and generic-git fallback behavior.
  • Never call provider merge APIs.

Out of scope

Git-provider approval, automatic merge, review-comment automation, triggers, and waits for CI or PR merge events.

Dependencies

Acceptance criteria

  • A delivered branch opens one PR against the correct base.
  • Stacked delivery targets the nearest earlier delivered branch.
  • A remote driver may return its own valid PR URL.
  • Missing credentials produce an actionable failure without losing work.
  • Protected-path failures name files before PR creation.
  • No workflow path can merge a PR.

Verification

Focused target-resolution, provider, remote-URL, protected-path, and recovery tests; build/typecheck/lint; one real-CLI branch-delivery E2E, with one provider-specific path where available. Update bilingual delivery documentation.

Activity

  1. SaulMoro commented on Oct 13, 2026

    @SaulMoro
    Collaborator

    #818 had a second half to protected paths: a step that starts from a branch an agent wrote runs the tool with the agent config from the run's base, so a hook one agent committed doesn't run in a later step that holds a token. The checks here cover the delivered diff. Maybe that half could go here too, since it's the same policy?

  2. hsuchifeng commented on Oct 14, 2026

    @hsuchifeng
    CollaboratorAuthor

    The protected-path policy stands, and the work is split.

    This issue checks the delivered diff and rejects protected-path changes before PR creation.

    The other half belongs to #1080: a later step runs with the agent config from the run’s base. That must happen before the step runs; checking only at delivery is too late.

  3. SaulMoro commented on Oct 14, 2026

    @SaulMoro
    Collaborator

    Notes from #1078 (#1129) on "the nearest earlier delivered branch along first needs":

    • needs is required after the first step, and needs: [] declares another root. A root has no first need, so it targets the run base.
    • Inside a sub-workflow, the child's first step has no needs of its own; walking "first needs" has to continue through the parent's workflow step's needs. Child steps are addressed by path (ship.check.classify).
    • Skipped steps never ran, so they never delivered a branch; the walk should pass over them rather than stop.
    • A post-agent delivery state is a new step state. The engine's rules key off stopped() (failed today) and settled() (succeeded, failed, skipped): a delivery failure that should skip dependents or fail the run must count as stopped, and a step waiting on delivery must not count as settled.

    The engine rules that span files are in docs/dev/workflow-engine.md, added by #1129.

  4. SaulMoro commented on Oct 15, 2026

    @SaulMoro
    Collaborator

    Run records (#1083) already publish to teamai-reports, and the PR a step delivers belongs there too. #1088 shows it, and it is the only place that ties a PR to the run that opened it. Things that are easy to miss:

    Input Published
    https://github.com/acme/app/pull/101 unchanged
    https://bitbucket.acme.internal/projects/APP/repos/web/pull-requests/7/overview unchanged: any host and path
    https://review.acme.internal/changes/4821?session=abc#c3 https://review.acme.internal/changes/4821
    https://user:token@github.com/acme/app/pull/1 dropped: userinfo means credentials
    http://github.com/acme/app/pull/1 dropped: not https:
    not a url, or anything over 512 characters dropped

    Matching provider URL shapes would drop private Bitbucket Server, Azure DevOps, Gitea and other self-hosted servers. That is why any host and path are accepted.

    • A dropped URL is not silent. A driver-reported pr_url that fails validation or sanitizing does not fail the report. The report envelope names it as dropped, with a reason and without the value, since the value may hold a token. The envelope gets the same treatment as the ignored list in [feat] Tell a driver which payload fields next and report ignored #1164.
    • Audience. prs follows the usage report opt-out like the rest of the record. On a public team repo it is public, so the "Run records" docs should say so.
    • Do not add fields to done. workflow-inline and workflow-subworkflow compare the whole envelope with toEqual.

    Acceptance criteria to add:

    • A for_each over three packages with deliver: pr publishes three prs, in item order. gh pr view opens each one.
    • A loop that delivers to the same PR on three passes publishes it once. A pass that opens a new PR adds it, in order.
    • Every row of the table above holds. fast-check properties (already a devDependency) cover the sanitizer: no userinfo, query or fragment survives, sanitizing twice changes nothing, and the length cap holds.
    • A driver-reported https://user:token@… is dropped, the report is accepted, and the token appears neither in the envelope nor in the record.
    • jq -r 'select((.prs // []) | index("<url>")) | .run' workflows/runs/*.json on teamai-reports returns the run that delivered <url>.
    • A record with prs parses with today's WorkflowRunRecord reader, and a record from before this change parses with the new one.
    • A conformance scenario covers a reported pr_url, accepted and dropped. Drivers see it.
  5. SaulMoro commented on Oct 20, 2026

    @SaulMoro
    Collaborator

    Notes from #1175 for delivery, plus one gap in how runs start:

    • Cleanup before delivery loses nothing. feat(workflow): add workspace branches #1175 returns cleanup: true on the step's own report (engine.ts:1217), and the local driver removes the worktree then (local-driver.ts:736-737). By then the driver has committed every path that is not ignored (worktree.ts:287) and pushed when the checkout has an origin; a failed push already fails the step with push_failed and keeps the worktree, and the branch outlives the worktree. So "Preserve the branch and workspace when push or PR creation fails" can read "Preserve the branch", and delivery needs no hold on cleanup. Criterion: a delivery that fails on missing credentials leaves the branch on the remote, and the failure names it.

    • pr_url is this issue's. feat(workflow): add workspace branches #1175's changes is { branch, base, from, to, commits }, and parseReport drops any other key inside changes without listing it in ignored, which covers only top-level keys (engine.ts:1063-1079). Add changes.pr_url to ChangesSchema and parseReport, and list unknown changes.* keys in ignored. [feat] Add workflow workspaces, branches, and session continuation #1080's scope line can then drop pr_url?.

    • Resolve the target from where the branch started. In feat(workflow): add workspace branches #1175 a new step starts from its item's after branches first, then its needs (workspace.ts:130-137). Walking first needs gives an after item a PR whose diff includes the item it waited for. Resolve the target from the recorded origin of the delivered branch, after before other needs and following inherited workflow dependencies, to the nearest branch that was delivered, otherwise the run base. start.branch gives that origin for a new leaf copy; a continued copy and a delivered workflow step, such as Discussion: Workflow Runtime Integration #818's change, need the same lineage kept. Reuse an open PR for the same head branch. Criteria: in a for_each with after, item 1's PR targets item 0's delivered branch and holds only item 1's commits; Discussion: Workflow Runtime Integration #818's composite change delivers one PR.

    • A looped new step opens a PR per pass. Branch names end in /p<pass> (workspace.ts:55), so each pass of a new step with deliver: pr opens a new PR and leaves the earlier one open. State whether that is intended, or whether delivery comments on or closes the previous pass's PR. Closing is not merging.

    • Document starting a run from CI. Discussion: Workflow Runtime Integration #818's v1 trigger is a GitHub Actions job with the local driver, and neither workflow guide shows it. Discussion: Workflow Runtime Integration #818's own example also needs teamai pull after init, and its fix-ci call uses workflow start --base, which the CLI does not have. This is the first issue where an unattended run produces something a person reviews, so the recipe belongs here:

      # .github/workflows/ship-issue.yml, excerpt
      on: { issues: { types: [labeled] } }
      concurrency: ship-issue-${{ github.event.issue.number }}
      jobs:
        ship:
          runs-on: ubuntu-latest
          steps:
            - uses: actions/checkout@v5
              with: { ref: main }              # a branch, so the run base has one (F2)
            - run: npx teamai-cli init "$TEAM_REPO" --scope workspace
            - run: npx teamai-cli pull
            - run: npx teamai-cli workflow start ship-issue --driver local --auto-approve --inputs '{"issue":${{ github.event.issue.number }}}'

      Add this to both workflow guides, with a token that can push to the product repo. --auto-approve answers only optional approve and choose requests, so the guide says what a required gate does in an unattended job. To start from a failing branch, the job checks that branch out by name: the default pull_request checkout is a detached merge commit, which gives the run base no branch (F2 in [feat] Close the #1080 gaps that #1175 leaves open #1246). One real-CLI E2E runs the documented commands against a fixture repo and a bare remote, with a fake provider checking the PR request and the returned URL. The generic git provider opens no PR (src/providers/git/index.ts:112-116), so its case checks that the failure names the pushed branch. Triggers stay in Later, as Discussion: Workflow Runtime Integration #818 decided; the documented CI job is the v1 path.

    The protected-path list shared with execution (F1) and the run base as a branch (F2) are in #1246, which this issue now waits for.

  6. added a commit that references this issue on Oct 25, 2026
    f3cd042
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

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions