Skip to content

fix(workflows): deprecate background launch alias - #139

Merged
tt-a1i merged 7 commits into
openpi-dev:mainfrom
sunsunsun-java:codex/issue-132-wait-only
Aug 30, 2026
Merged

fix(workflows): deprecate background launch alias#139
tt-a1i merged 7 commits into
openpi-dev:mainfrom
sunsunsun-java:codex/issue-132-wait-only

Conversation

@sunsunsun-java

@sunsunsun-java sunsunsun-java commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Relates to #132

Summary

  • make wait the only recommended Workflow launch policy for new calls
  • preserve the published background input as a deprecated inverse alias during the migration window
  • map background at one coordinator boundary while keeping internal inline/detached runtime modes
  • reject conflicting dual inputs and detached requests on hosts without later delivery before side effects
  • provide an actionable migration in the schema, Workflow reference, and TUI call rendering without changing successful tool receipts
  • preserve WorkflowDetails.background and historical artifacts as actual detached-state facts
  • keep wait interruption, workflow_stop, terminal delivery, retry, and cancellation semantics unchanged
  • leave deprecation(workflows): 移除 background 反向别名,统一使用 wait #132 open for removal in a separately announced breaking release

Compatibility mapping

  • background: true becomes wait: false
  • background: false becomes wait: true
  • consistent dual inputs remain accepted; conflicting inputs fail closed

Validation

  • independent compatibility and design reviews: no merge blockers
  • focused Workflow tests: 35/35
  • bun run check
  • bun run test: Node 1065 passed, 1 Windows-only skip, Vitest 30/30
  • fresh GitHub CI required on the final pushed head before merge

@tt-a1i tt-a1i 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.

实现本身对 wait lifecycle、host fail-closed 和历史 artifact 的边界处理是正确的;当前 main 合并态下 focused tests、bun run check 和完整测试也都通过。

但这里有一个合并阻断的兼容性问题:Issue #132 明确要求先让旧调用在 deprecation window 获得清晰、可操作的迁移提示,再在 breaking release 删除 alias。当前改动在 package 仍为 0.4.0 时直接从 schema 删除 background 并设置 additionalProperties: false,旧模型或自动化的 background: true/false 只会得到通用 validation failure,而不是 true → wait:falsefalse → wait:true 的迁移指引。

建议二选一:

  1. 本版本保留 deprecated alias,并在调用时给出 actionable warning,下一 breaking release 再删除;或
  2. 明确将本 PR 纳入已经公告的 breaking release/version bump,并把迁移映射写进 release plan。

在这个发布决策明确前,我建议先不要合并。另:PR 中的 Pi smoke 目前只有文字结论,没有 revision、pi list 单一源码证明和可复核日志,暂不能作为 runtime smoke receipt。

@sunsunsun-java
sunsunsun-java force-pushed the codex/issue-132-wait-only branch from 3816529 to b9e437d Compare August 25, 2026 11:17
@sunsunsun-java sunsunsun-java changed the title fix(workflows): remove background launch alias deprecate(workflows): guide background callers to wait Aug 25, 2026
@sunsunsun-java

sunsunsun-java commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the compatibility blocker in the latest update:

Validation:

  • focused Workflow tests: 23/23
  • bun run check
  • full test suite: Node 887/887, Vitest 30/30

Please re-review compatibility blocker 1. The reproducible local Pi runtime receipt requested in blocker 2 will be supplied separately.

@tt-a1i tt-a1i 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.

上次提出的兼容性阻断已经解决:legacy background 在 deprecation window 内仍可执行,所有关键返回路径都有明确的 true → wait:false / false → wait:true 迁移提示,警告保持 call-local,Issue #132 也继续保留给下一次 breaking release。这里的实现和覆盖方向都没问题。

当前还有一个合并阻断:分支与最新 main 在 extensions/workflows/execute.e2e.test.ts 冲突。main 的 da2836f 新增了 oversized workflow args 的 fail-closed E2E,本 PR 在同一位置新增 print-host 和 legacy detached alias 测试。同步 main 时需要保留这三组测试,不能选择任意单侧版本。

我在临时 worktree 中按上述方式合并验证过:bun run check 通过,完整测试为 Node 880/880、Vitest 30/30。请同步最新 main、保留双方测试并推送冲突解决;之后可以直接复审。作者提到的 Pi runtime receipt 本轮没有提供,因此我没有把它计入验证证据,但它不是这次代码集成失败的原因。

@tt-a1i tt-a1i 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.

感谢你把上一轮提出的兼容性问题处理得这么完整。我们重新讨论了这个接口的长期形态,维护者决定不再保留 deprecation window,而是在这次变更中直接把模型输入统一为正向语义的 wait。

请按以下边界调整:

  • 从 workflow tool input schema 删除 background,只保留 wait
  • 从 WorkflowLaunchPolicyInput 和 coordinator 删除 background 的 inverse/conflict/migration-warning 分支
  • 新文档、Skill、示例和 fixtures 全部使用 wait
  • 继续保留 WorkflowDetails.background 以及历史 artifact 的读取兼容;这里的 background 是实际 detached 状态,不是模型输入 alias
  • 未知输入字段继续 fail closed;模型误传 background 时由 schema validation 暴露错误并自行改正,不需要运行时兼容映射
  • 同步最新 main,并解决 extensions/workflows/execute.e2e.test.ts 的冲突;请保留 main 中 oversized workflow args 的 fail-closed 测试

完成后请重新运行 bun run check 和 bun run test。这个方向会让模型可见接口和 runtime 分支都更小、更直接。

@tt-a1i

tt-a1i commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

辛苦了,也谢谢你认真补齐了上一轮要求的兼容提示和测试。我们内部重新把这个接口讨论了一遍,最后决定直接统一使用 wait,不再为 background 输入保留过渡兼容。

这属于维护方向调整,不是你当前实现质量有问题,抱歉让你多做了一轮。修改时请只删除模型输入侧的 background;artifact/details 里的 background 仍然是实际运行状态,需要继续保留历史读取兼容。另外当前分支与最新 main 的 E2E 测试有冲突,同步时请把双方测试都留下。

改好后 @ 我一下,我会尽快复审。

@sunsunsun-java
sunsunsun-java force-pushed the codex/issue-132-wait-only branch from b9e437d to ff4f3ee Compare August 26, 2026 12:37
@sunsunsun-java sunsunsun-java changed the title deprecate(workflows): guide background callers to wait fix(workflows): remove background launch alias Aug 26, 2026
@sunsunsun-java

Copy link
Copy Markdown
Contributor Author

@tt-a1i 已按最新维护方向完成调整:

  • 模型输入侧只保留 wait,已删除 schema、prompt、coordinator 中的 background alias、inverse/conflict 和 migration-warning 分支
  • WorkflowDetails.background 与历史 artifact 读取兼容保持不变
  • 已 rebase 到 upstream/main@dd82010
  • execute.e2e.test.ts 冲突已解决:保留 main 的 oversized args fail-closed E2E,并保留本分支的 print-host default/fail-closed E2E;调用输入均改为 wait
  • focused Workflow tests 22/22,bun run check 通过,完整测试 Node 876/876、Vitest 30/30

已推送 ff4f3ee,烦请复审。

@sunsunsun-java

sunsunsun-java commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

@tt-a1i 在最新 wait-only 版本上继续做了简化审计,并已同步 upstream/main@813f58e(当前 HEAD c0f401e):

Validation:

  • focused Workflow tests: 34/34
  • bun run check
  • full suite: Node 878/878, Vitest 30/30

另外 Issue #132 正文仍保留旧的 deprecation-window 方案;当前实现和 PR 描述已按您 8 月 26 日的 wait-only 决定执行,Issue 文本可在方便时标记为 superseded。烦请复审。

@github-actions github-actions Bot added documentation Improvements or additions to documentation area:workflows Workflow engine, capability, skills, or tests labels Aug 30, 2026
@tt-a1i tt-a1i changed the title fix(workflows): remove background launch alias fix(workflows): deprecate background launch alias Aug 30, 2026

@tt-a1i tt-a1i 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.

Reviewed exact head 2390d04. The compatibility blocker is resolved: new calls use wait, published background calls retain exact inverse behavior during the migration window, conflicting inputs and unsupported detached hosts fail closed before side effects, and successful receipts/artifacts remain unchanged. Two independent final reviews found no reproducible blocker. Local validation: Workflow 35/35, bun run check, full Node 1065 passed with one Windows-only skip, Vitest 30/30. Fresh exact-head GitHub CI passed on Node 22.19.0, Node 24, and Windows background terminals.

@tt-a1i
tt-a1i merged commit 82a2c1b into openpi-dev:main Aug 30, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:workflows Workflow engine, capability, skills, or tests documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants