Skip to content

fix: conductor 双态窗口收敛——标签触发转移可达(ADR-0085) - #353

Merged
randypanding merged 1 commit into
mainfrom
fix-conductor-dual-state
Aug 24, 2026
Merged

fix: conductor 双态窗口收敛——标签触发转移可达(ADR-0085)#353
randypanding merged 1 commit into
mainfrom
fix-conductor-dual-state

Conversation

@randypanding

@randypanding randypanding commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Card: #350

T8 首次实走(#349 置 state:done)实测暴露:actor 置新态时旧态并存,labeled 事件携带 [in-progress,done] 双态,conductor 多状态守卫先于转移匹配 abort——标签触发的转移全族(T5/T7/T8/T9)在该写序下不可达。收敛:事件标签=新态、另一态=转移前态;swap_state 移旧置新后终态单态。守卫对非 label:state: 事件与真并存维持 abort。AST+YAML 双验证通过。

Summary by CodeRabbit

  • Bug Fixes
    • 优化状态标签事件处理,支持识别新旧状态标签并继续执行状态切换。
    • 对多状态或未知状态组合进行安全中止,避免错误的状态转换。

Copilot AI lite review requested due to automatic review settings August 24, 2026 17:34

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 64a3330c-a922-48ba-a317-c0de07bf7f36

📥 Commits

Reviewing files that changed from the base of the PR and between 9dd5e98 and ceff35d.

📒 Files selected for processing (1)
  • .github/workflows/conductor.yml

Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

Conductor 工作流更新状态标签解析逻辑。labeled 事件可识别新旧双标签窗口,并在状态组合不明确时继续 fail-closed 中止。

Changes

状态标签转移

Layer / File(s) Summary
解析 labeled 事件的双状态窗口
.github/workflows/conductor.yml
当事件标签是唯一新状态且存在一个非事件标签时,工作流使用非事件标签作为旧状态并记录审计。其他多状态或未知状态组合继续中止。

Suggested labels: security, bug

Merge Risk: ⚪ Minimal · up to ceff3

This localized workflow change is merge-ready after normal checks and review; no actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题使用了必需的 fix: 前缀,长度为 41 个字符,并准确描述了 conductor 标签状态转移修复。
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-conductor-dual-state

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot added bug Something isn't working security labels Aug 24, 2026
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix Conductor dual-state label window so label-triggered transitions remain reachable

🐞 Bug fix ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• 在多 state 标签并存时对 label:state:* 事件进行“写序语义”收敛
• 允许 T5/T7/T8/T9 等标签触发转移在双态窗口内正常匹配
• 对非 state 标签事件或真并存场景维持 abort 以避免歧义
Diagram

graph TD
  GH{{"GitHub labeled event"}} --> WF["conductor.yml job"] --> RES["Resolve state labels"] --> D{"Multiple state labels?"}
  D -->|"label:state:* + 1 other"| CONV["Converge to from_state"] --> MATCH["Transition match"] --> SWAP["swap_state old→new"]
  D -->|"otherwise"| ABORT["Abort (multi-state)"]

  subgraph Legend
    direction LR
    _ext{{"External event"}} ~~~ _proc["Workflow step"] ~~~ _dec{"Decision"}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Make state label updates atomic at the actor side
  • ➕ 从源头消除双态窗口,conductor 无需推断写序语义
  • ➕ 减少 workflow 内状态机边界条件与审计分支
  • ➖ 需要改动执行 actor/调用方的标签写入策略,可能牵涉更多仓库与权限
  • ➖ 无法覆盖 GitHub 平台/并发导致的短暂双态可见性
2. Persist state outside labels (issue field / file / API-backed store)
  • ➕ 避免标签系统的最终一致性/写序歧义
  • ➕ 状态读取更稳定,可扩展为更复杂的状态机元数据
  • ➖ 引入新的存储与同步路径,显著增加系统复杂度
  • ➖ 需要迁移现有基于标签的工作流与工具链

Recommendation: 当前 PR 选择在 workflow 内对“label:state:* 双态窗口”做最小语义收敛,同时对非预期场景继续 abort,能在不改动外部 actor 的前提下恢复 T5/T7/T8/T9 转移可达性且保持保守性。若后续仍频繁出现多态歧义或并发问题,再考虑推动 actor 侧原子化写入作为长期治理。

Files changed (1) +12 / -1

Bug fix (1) +12 / -1
conductor.ymlConverge dual-state window for label:state events instead of aborting +12/-1

Converge dual-state window for label:state events instead of aborting

• 调整多 state 标签并存时的处理:当事件为 label:state:* 且仅存在一个“另一态”时,将 current 收敛为转移前态以继续匹配转移。对非 state 标签事件或无法判定写序语义的多态场景仍直接 abort,并补充 ADR-0085 背景审计日志。

.github/workflows/conductor.yml

@randypanding
randypanding merged commit 9f88095 into main Aug 24, 2026
18 checks passed
@randypanding
randypanding deleted the fix-conductor-dual-state branch August 24, 2026 17:38
@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📜 Skill insights (0)

Grey Divider


Action required

1. Dual-state invalid jump no-op 🐞 Bug ≡ Correctness
Description
当同时存在两个 state 标签且事件为 label:state:* 时,代码会把 current 收敛为“另一态”并继续;若该组合在 transitions.yaml
中没有匹配转移,则会走到“无匹配转移→noop”分支退出 0,导致双 state 继续并存且不会 fail-closed。该行为回退了原先“多 state 直接
abort”的一致性保护,错误贴标/跳态会被静默吞掉并污染状态机。
Code

.github/workflows/conductor.yml[R217-220]

+              ev_label_name = ev[len("label:"):] if ev.startswith("label:") else ""
+              others = [s for s in states if f"state:{s}" != ev_label_name]
+              if ev.startswith("label:state:") and len(others) == 1:
+                  current = others[0]
Relevance

●● Moderate

Recent conductor precedent accepts fail-closed state-handling fixes, but no close precedent covers
this dual-state transition-window case.

PR-#208

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
证据链:1) 本 PR 在多 state 时不再直接 abort,而是在 label:state:* 双态窗口下将 current 设为“另一态”;2) 转移表匹配若找不到
(from_state=current,event=ev) 会直接退出 0;3) transitions.yaml 对同一事件(如 label:state:done)只定义了特定
from_state(如 in-progress / wave-planned / fixed),因此当“另一态”不是这些 from_state(例如 spec)时就会触发静默 noop,留下双
state 并存。

.github/workflows/conductor.yml[204-223]
.github/workflows/conductor.yml[233-241]
governance/transitions.yaml[91-116]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
当前对 `len(states) > 1` 的“收敛”策略只检查事件是否为 `label:state:*` 且双态窗口中“另一态”数量为 1,就会将 `current` 设为该“另一态”。但如果该 `(from_state=current, event=ev)` 在 `governance/transitions.yaml` 里不存在,后续会走到“无匹配转移→noop→exit 0”,从而让 issue 继续保持双 state 并存,且不会 fail-closed。

这会把原本的“多 state = abort(保护一致性)”弱化为“多 state 也可能静默 noop”,让错误贴标/跳态更难被发现和修复。

### Issue Context
- `issues.labeled` 事件只会给出“新增的 label”,无法保证旧 state label 已被移除。
- 本 PR 的目标是让合法转移在双态窗口可达,但仍应对“无合法转移的双态”保持 fail-closed(或至少回滚事件标签后再退出)。

### Fix Focus Areas
- .github/workflows/conductor.yml[208-241]
- .github/workflows/conductor.yml[233-241]
- governance/transitions.yaml[91-116]

### Suggested fix approach
1. 在双态收敛分支中引入“收敛模式”标记(例如 `dual_state_window = True`),并保留 `event_state`(从 `ev` 提取的 state 名)。
2. 在加载 `transitions.yaml` 并执行转移匹配后:
  - 若 `dual_state_window` 为真且 `t is None`(没有匹配转移),则应 **fail-closed**(`verdict=abort` 并退出非 0),避免把双态不一致静默吞掉。
  - 可选(更强一致性):在 abort 前尝试回滚本次事件新增的 state 标签(DELETE `state:<event_state>`),使 issue 回到单态旧状态;回滚失败则仍应 abort。
3. 保持本 PR 既有的“合法双态窗口可达”逻辑不变:当存在匹配转移时正常执行 `swap_state`,最终写出单态。

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

2. PR body missing ADR token 📘 Rule violation § Compliance
Description
This PR modifies a file under .github/, but the PR description body does not contain any
ADR-NNNN reference as required. This can break governance traceability for standards-related
changes.
Code

.github/workflows/conductor.yml[R212-213]

+              # 标签写序现实(ADR-0085,T8 首次实走实测 2026-08-25):actor 置新态时
+              # 旧态仍在——labeled 事件天然携带 [旧,新] 双态窗口,先 abort 则标签触发
Relevance

● Weak

The PR title and body explicitly contain ADR-0085, satisfying the stated regex requirement; this
finding is inapplicable.

PR-#19

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The rule applies because a modified file is under .github/. Based on the provided PR description
body, there is no substring matching \bADR-[0-9]{4}\b, so the requirement is not met.

Rule 2778538: Require ADR reference in PR description when governance or standards files change
.github/workflows/conductor.yml[212-213]


Grey Divider

Context sources
✅ Compliance rules (platform): 18 rules
Review mode: ⚖️ Balanced: This is a behavioral change to a CI workflow's state-transition guard, affecting event reachability and workflow correctness; although localized, it is not clearly trivial enough for lite and is not dense enough for extended.
ⓘ  1 issues published inline · 2 in summary

Grey Divider

Tip of the day
💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment on lines +217 to +220
ev_label_name = ev[len("label:"):] if ev.startswith("label:") else ""
others = [s for s in states if f"state:{s}" != ev_label_name]
if ev.startswith("label:state:") and len(others) == 1:
current = others[0]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. Dual-state invalid jump no-op 🐞 Bug ≡ Correctness

当同时存在两个 state 标签且事件为 label:state:* 时,代码会把 current 收敛为“另一态”并继续;若该组合在 transitions.yaml
中没有匹配转移,则会走到“无匹配转移→noop”分支退出 0,导致双 state 继续并存且不会 fail-closed。该行为回退了原先“多 state 直接
abort”的一致性保护,错误贴标/跳态会被静默吞掉并污染状态机。
Agent Prompt
### Issue description
当前对 `len(states) > 1` 的“收敛”策略只检查事件是否为 `label:state:*` 且双态窗口中“另一态”数量为 1,就会将 `current` 设为该“另一态”。但如果该 `(from_state=current, event=ev)` 在 `governance/transitions.yaml` 里不存在,后续会走到“无匹配转移→noop→exit 0”,从而让 issue 继续保持双 state 并存,且不会 fail-closed。

这会把原本的“多 state = abort(保护一致性)”弱化为“多 state 也可能静默 noop”,让错误贴标/跳态更难被发现和修复。

### Issue Context
- `issues.labeled` 事件只会给出“新增的 label”,无法保证旧 state label 已被移除。
- 本 PR 的目标是让合法转移在双态窗口可达,但仍应对“无合法转移的双态”保持 fail-closed(或至少回滚事件标签后再退出)。

### Fix Focus Areas
- .github/workflows/conductor.yml[208-241]
- .github/workflows/conductor.yml[233-241]
- governance/transitions.yaml[91-116]

### Suggested fix approach
1. 在双态收敛分支中引入“收敛模式”标记(例如 `dual_state_window = True`),并保留 `event_state`(从 `ev` 提取的 state 名)。
2. 在加载 `transitions.yaml` 并执行转移匹配后:
   - 若 `dual_state_window` 为真且 `t is None`(没有匹配转移),则应 **fail-closed**(`verdict=abort` 并退出非 0),避免把双态不一致静默吞掉。
   - 可选(更强一致性):在 abort 前尝试回滚本次事件新增的 state 标签(DELETE `state:<event_state>`),使 issue 回到单态旧状态;回滚失败则仍应 abort。
3. 保持本 PR 既有的“合法双态窗口可达”逻辑不变:当存在匹配转移时正常执行 `swap_state`,最终写出单态。

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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

Labels

bug Something isn't working security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants