Skip to content

Commit b5180ee

Browse files
committed
fix(workflow): 交付前复查分支并等待活跃写入报告
- 在查询 PR 后重新读取远端分支,在创建门禁内核对已记录提交;分支变化时交给后续轮询重新检查受保护文件。 - 共享分支仍有可写租约时继续等待报告,清除无人报告轮询计数,避免三次轮询误判交付失败。 - 新增查询期间已报告与未报告分支变化、连续六次轮询回归,修改前均失败;225 项相关单测、4 项 CLI 交付 E2E、构建、类型检查和 lint 通过。 - 保留维护者 efdaf68 的交付重构,仅作为独立交接提交提供,不更新维护者接管的 PR 分支。
1 parent efdaf68 commit b5180ee

6 files changed

Lines changed: 56 additions & 9 deletions

File tree

‎docs/guide/workflow.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,8 @@ A `deliver: pr` step that would run read-only fails validation, and so does a de
236236

237237
When the step succeeds, its driver has already pushed the branch, as for any writable step, and the step is `delivering`. Steps that need it wait. The next `teamai workflow next`, from whichever driver calls it, looks for an open pull request from that branch into its target, and opens one only when there is none. A run that a crash stopped finds the pull request it opened before and opens no other. So does a run another machine took over, unless the machine that handed it over is still opening one at that moment. GitHub and GitLab reject such a duplicate. TeamAI opens pull requests on GitHub, GitLab, TGit, and GitCode origins, with the login the provider's CLI or token already has, and never starts a login prompt. On any other origin, such as a generic git host or CNB, it opens none, and the step fails naming its branch.
238238

239+
If a sibling advances the shared branch while lookup is pending, TeamAI checks its new recorded tip on a later poll before opening a pull request. A pushed commit still awaiting an active writer's report keeps delivery pending.
240+
239241
A pull request goes into the nearest branch its own branch grew from that already delivered one, or else into the branch the run started from. A branch grows first from its `for_each` item's `after` branches, then from its `needs` in order. The first step of a sub-workflow grows from the needs of the step that runs it. A skipped step wrote no branch, so it is passed over. Each pull request then shows only the work done since the branch it goes into.
240242

241243
Each pass of a looped `workspace: new` step has its own `/p<pass>` branch, so it opens its own pull request. The earlier ones stay open, by design. A step that loops on one `continue` branch has one pull request.

‎docs/guide/zh-CN/workflow.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,8 @@ steps:
236236

237237
步骤成功时,驱动已经像对任何可写步骤一样推送了分支,步骤进入 `delivering` 状态,需要它的步骤会等待。下一次 `teamai workflow next`(无论哪个驱动调用)会查找从这条分支合入目标分支的已打开 pull request,没有时才新开一个。因崩溃而中断的运行,会找到之前开的 pull request,不会再开第二个。被另一台机器接管的运行也是如此,除非交出运行的机器此刻仍在开 pull request;GitHub 和 GitLab 会拒绝这样的重复请求。TeamAI 在 GitHub、GitLab、TGit 和 GitCode 的 origin 上开 pull request,使用 provider 的 CLI 或 token 已有的登录,从不弹出登录提示。在其他 origin 上(例如通用 Git 主机或 CNB),它不开 pull request,步骤失败并写明分支。
238238

239+
如果同一分支上的另一步骤在查询期间推进分支,TeamAI 会在后续轮询中检查新记录的提交后再创建 pull request。已推送但仍在等待活跃写入步骤报告的提交会保持待交付状态。
240+
239241
pull request 合入这条分支来源中最近一条已经交付过的分支,没有时合入运行开始时的分支。分支首先来自其 `for_each` item 的 `after` 分支,然后依次来自 `needs`。子工作流的第一个步骤来自运行它的步骤的 needs。跳过的步骤没有写分支,会被越过。因此每个 pull request 只显示它所合入的分支之后的工作。
240242

241243
循环中 `workspace: new` 步骤的每个 pass 都有自己的 `/p<pass>` 分支,因此各开一个 pull request。之前的 pull request 保持打开,这是有意为之。在同一条 `continue` 分支上循环的步骤只有一个 pull request。

‎src/__tests__/workflow-delivery.test.ts‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1054,6 +1054,31 @@ describe('retrying and cancelling a delivery', () => {
10541054
expect(creates(gh)).toEqual([]);
10551055
});
10561056

1057+
it.each([true, false])('rechecks a shared branch changed during lookup before creation (reported: %s)', async (reported) => {
1058+
const { config, root, runId } = await fixture(IMPL_AND_DOCS);
1059+
const impl = await take(config, runId);
1060+
await succeed(config, runId, impl, { to: pushWork(root, impl) });
1061+
const lock = deliveryLockPath(config, runId);
1062+
expect(await acquireLock(lock)).toBe(true);
1063+
const docs = await take(config, runId);
1064+
await releaseLock(lock);
1065+
let changed = false;
1066+
const create = providerDouble(async () => {
1067+
if (!changed) {
1068+
changed = true;
1069+
const to = pushOnto(root, docs.workspace!.branch, { '.github/workflows/ci.yml': 'on: push' });
1070+
if (reported) await succeed(config, runId, docs, { to });
1071+
}
1072+
return undefined;
1073+
}, async () => 'https://github.com/fixture/app/pull/1');
1074+
await leaseNext(config, runId, remote, { ...writable, running: [docs.attempt] });
1075+
expect(create).not.toHaveBeenCalled();
1076+
if (reported) {
1077+
await leaseNext(config, runId, remote, writable);
1078+
expect((await state(config, runId)).steps.find(step => step.id === 'work.impl')).toMatchObject({ status: 'failed', failure: { message: expect.stringContaining('protected paths') } });
1079+
}
1080+
});
1081+
10571082
it('keeps a step delivering while origin\'s branch is ahead of what the run recorded, and delivers it once a report records that commit', async () => {
10581083
const { config, root, runId } = await fixture(IMPL_AND_DOCS);
10591084
const gh = fakeGh(path.join(root, 'gh'));
@@ -1066,7 +1091,9 @@ describe('retrying and cancelling a delivery', () => {
10661091
const branch = docs.workspace?.branch ?? '';
10671092
const ahead = pushOnto(root, branch, { 'docs.txt': 'docs' });
10681093

1069-
expect(await leaseNext(config, runId, remote, { ...writable, running: [docs.attempt] })).toMatchObject({ data: { wait: { delivering: ['work.impl'] } } });
1094+
for (let poll = 0; poll < 6; poll += 1) {
1095+
expect(await leaseNext(config, runId, remote, { ...writable, running: [docs.attempt] })).toMatchObject({ data: { wait: { delivering: ['work.impl'] } } });
1096+
}
10701097
expect(creates(gh)).toEqual([]);
10711098
await succeed(config, runId, docs, { to: ahead });
10721099
expect(await leaseNext(config, runId, remote, writable)).toMatchObject({ data: { done: { outcome: 'succeeded' } } });

‎src/workflow/delivery-ledger.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,26 +81,30 @@ function recordReceipt(run: RunState, key: DeliveryKey, target: string, url: PrU
8181
* while origin's branch is ahead of the recorded commit and a report of it may still come.
8282
* `requeueEnd` is set when the receipt reached a run that already ended, whose end record must go out again to list it.
8383
*/
84-
export function completeDelivery(run: RunState, job: DeliveryJob, outcome: DeliveryOutcome, reconcile: (run: RunState) => unknown): { requeueEnd: boolean } {
84+
export function completeDelivery(run: RunState, job: DeliveryJob, outcome: DeliveryOutcome, reconcile: (run: RunState) => unknown, hasActiveWriter = false): { requeueEnd: boolean } {
8585
const receipted = outcome.ok && recordReceipt(run, job.key, job.target, outcome.pr);
8686
if (outcome.ok) forgetCreate(run, { ...job.key, target: job.target });
8787
if (run.status === 'running') reconcile(run);
8888
const step = deliveringStep(run, job.key);
8989
if (step && outcome.ok) delivered(step);
90-
else if (step && !outcome.ok && !(outcome.fault.kind === 'branch_ahead' && awaitsReport(step, outcome.fault.tip))) {
90+
else if (step && !outcome.ok && !(outcome.fault.kind === 'branch_ahead' && awaitsReport(step, outcome.fault.tip, hasActiveWriter))) {
9191
deliveryFailed(step, deliveryFailure(job, outcome.fault));
9292
}
9393
return { requeueEnd: receipted && run.status !== 'running' };
9494
}
9595

96-
/** How many drains in a row may find origin's branch ahead of the recorded commit, at one commit, before delivery fails. */
96+
/** How many drains without an active writer in a row may find origin's branch ahead of the recorded commit, at one commit, before delivery fails. */
9797
const AHEAD_DRAINS = 3;
9898

9999
/**
100100
* Whether `step` still waits for a report of `tip`, the commit origin's branch is ahead at: one may be on its way, from a
101-
* step of the same branch that pushed before it reported. The drain that finds it there a third time fails the step.
101+
* step of the same branch that pushed before it reported. Without an active writer, the drain that finds it there a third time fails the step.
102102
*/
103-
function awaitsReport(step: DeliveringStep, tip: string): boolean {
103+
function awaitsReport(step: DeliveringStep, tip: string, hasActiveWriter: boolean): boolean {
104+
if (hasActiveWriter) {
105+
delete step.unrecordedTip;
106+
return true;
107+
}
104108
const drains = step.unrecordedTip?.commit === tip ? step.unrecordedTip.drains + 1 : 1;
105109
if (drains >= AHEAD_DRAINS) return false;
106110
step.unrecordedTip = { commit: tip, drains };

‎src/workflow/engine.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -889,11 +889,13 @@ export interface NextOptions {
889889
* is looked up, whatever becomes of the step, rather than lost.
890890
*/
891891
async function markCreating(config: LocalConfig, runId: string, job: DeliveryJob): Promise<'open' | 'refused' | 'busy'> {
892-
const marked = await updateRun(config, runId, async (run): Promise<{ value: 'open' | 'refused'; write?: RunState }> => {
892+
const marked = await updateRun(config, runId, async (run): Promise<{ value: 'open' | 'refused' | 'busy'; write?: RunState }> => {
893893
if (run === 'missing' || run.status !== 'running') return { value: 'refused' };
894894
const settledRun = reconcileRun(run);
895895
const step = 'unreadable' in settledRun || 'outcome' in settledRun ? undefined : deliveringStep(run, job.key);
896896
if (!step) return { value: 'refused' };
897+
// A report may have advanced this shared branch while the provider lookup was pending.
898+
if (run.copies?.[job.key.branch]?.commit !== job.recordedHead) return { value: 'busy' };
897899
const create = { ...job.key, target: job.target };
898900
const listed = run.creating?.find((item) => sameCreate(item, create));
899901
// A request listed before is asked again now, so its lookups count from this create.
@@ -938,7 +940,13 @@ async function drainDeliveries(config: LocalConfig, runId: string): Promise<void
938940
// As when the pick is busy: the step still delivers, and a later call delivers it.
939941
if (outcome === 'busy') return requeued;
940942
if (!outcome.ok && outcome.fault.kind === 'branch_ahead') waiting.push(job.key);
941-
apply = (run) => completeDelivery(run, job, outcome, reconcileRun);
943+
apply = (run) => completeDelivery(run, job, outcome, reconcileRun, run.steps.some((step) => {
944+
if (step.status !== 'leased') return false;
945+
const located = locateStep(run, step.id);
946+
if (!located) return false;
947+
const plan = planWorkspace(run, located.scope, step, located.step, { driver: step.lease.driver, base: run.base }, true);
948+
return plan.mode !== 'read-only' && plan.contract?.branch === job.key.branch;
949+
}));
942950
} else {
943951
const { create, checkout } = picked.value;
944952
tried.push(create);

‎src/workflow/pr-delivery.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,13 @@ export async function deliverPr(job: DeliveryJob, gate: CreateGate): Promise<Del
106106
const { provider, repo } = origin;
107107
const ref: PullRequestRef = { repo, source: job.key.branch, target: job.target, timeoutMs: DELIVERY_TIMEOUT_MS };
108108
const found = await findOpen(origin, ref);
109-
if (found.kind === 'found') return { ok: true, pr: found.pr };
110109
if (found.kind === 'failed') return failed({ kind: 'lookup_failed', provider: provider.name });
111110
if (found.kind === 'unsafe') return failed({ kind: 'unsafe_url', provider: provider.name });
111+
// Lookup can take long enough for a sibling to push. Re-pick and check the new tip on a later drain.
112+
const latest = await originTip(job.checkout, job.key.branch);
113+
if (latest.kind === 'unreadable') return failed({ kind: 'branch_unreadable' });
114+
if (latest.commit !== tip.commit) return 'busy';
115+
if (found.kind === 'found') return { ok: true, pr: found.pr };
112116
const gated = await gate();
113117
if (gated === 'busy') return 'busy';
114118
if (gated === 'refused') return failed({ kind: 'withdrawn' });

0 commit comments

Comments
 (0)