Skip to content

feat(steward): give the steward Turn the facts a team preview needs - #4533

Merged
huangruiteng merged 1 commit into
mainfrom
codex/team-plan-admission-facts
Sep 16, 2026
Merged

huangruiteng merged 1 commit into
mainfrom
codex/team-plan-admission-facts

Conversation

@huangruiteng

Copy link
Copy Markdown
Collaborator

Problem

A team preview is admitted only when the host can say which Agents exist for the Goal the plan names,
and nothing in production supplied that. A model-authored preview was therefore always dropped, so
the intake shipped in #4519 / #4522 / #4524 could not surface at all.

What changed

The Turn owner now supplies the admission facts, per Goal.

  • The manager channel is not bound to one Goal, so admission receives a lookup rather than one
    Goal's facts. The owner's own channel resolves any registered Goal; an external manager channel
    resolves only the Goals its scope resolver authorizes; and a Goal the registry does not know — or
    one outside that channel's scope — resolves to "cannot describe this Goal", which drops the preview
    instead of validating it against another Goal's Agents.
  • The lookup runs once per proposal, for the Goal the proposal names, and re-resolves the channel
    scope each time rather than caching an authorization decision.
  • The contract now distinguishes two answers that looked alike before: an empty Agent list is a
    fact, so the plan's lanes become typed agent_not_registered gaps, while an unresolvable Goal
    surfaces nothing.
  • parse_agent_response carries the context to all four segments that parse an answer (managed dsh,
    Codex agent, ACP, provider HTTP), so one admission rule applies to every transport rather than only
    the transport the steward happens to run on.
  • The RFC records the new item and rewrites the "still inert" paragraph: what remains missing is the
    effect (a Chat-side apply path for a confirmed plan, a frontend confirmation surface, the intent
    revision binding on materialized lane Todos), not the preview.

Changed surfaces

  • loopx/chat.py, loopx/chat_runtime.py, loopx/chat_dsh.py, loopx/chat_agent.py,
    loopx/chat_acp.py, loopx/chat_providers.py.
  • tests/test_steward_team_plan_preview.py; the RFC in both editions.

Validation

  • tests/test_steward_team_plan_preview.py, tests/test_steward_team_plan_apply.py,
    tests/test_chat_manager_context.py, tests/test_attached_session_broker.py: 63 passed. New
    coverage: a preview for a Goal outside the channel scope is dropped; the per-Goal lookup is asked
    only about the named Goal; a Goal with no registered Agents becomes a typed gap; the owner channel
    resolves any registered Goal; an unknown Goal resolves to nothing.
  • examples/docs-governance-smoke.py: ok.
  • examples/loopx-steward-managed-chat-smoke.py: ok (real dsh segment still completes with its
    proposal).
  • loopx canary premerge --from-git-diff: merge_gate_passed=false for two pre-existing,
    unrelated
    failures, both reproduced on clean origin/main (7bf8104c9) with no part of this
    diff applied:
    • examples/cli-project-lifecycle-command-modularization-smoke.py →
      AssertionError: project lifecycle module missing retry it before delivery, a stale marker
      expectation left by the refresh-state module move (refactor(cli): own the refresh-state command in its own module #4521), in files this diff does not touch;
    • examples/control_plane/control-plane-maintainability-ratchet-smoke.py → two unreviewed
      findings in loopx/extensions/lark/goal_topic_runtime.py and
      loopx/control_plane/quota/should_run_prepare.py, with magnitude_regressions: 0; earlier runs
      classified it advisory when the diff did not select the control-plane surface.
      All other selected canaries, risk-profile smokes and the public-boundary scan pass, and
      manual_holds=0. Merged with explicit maintainer authorization; both baseline reds are tracked
      separately rather than worked around here.

Boundaries

The contract grants nothing new: the preview still creates no Todo, registers no Agent, sets no quota
and spends nothing, and the apply still re-validates with the host's own facts. The lookup is
read-only over the registry and re-uses the channel authorization the Turn owner already resolved; it
does not widen any scope.

A team preview is admitted only when the host can say which Agents exist for the
Goal the plan names, and nothing in production supplied that. A model-authored
preview was therefore always dropped, so the intake shipped in #4519/#4522/#4524
could not surface at all. The Turn owner now supplies the facts, per Goal.

The manager channel is not bound to one Goal, so admission receives a lookup
rather than one Goal's facts: the owner's own channel resolves any registered
Goal, an external manager channel resolves only the Goals its scope resolver
authorizes, and a Goal the registry does not know - or one outside that channel's
scope - resolves to "cannot describe this Goal", which drops the preview instead
of validating it against another Goal's Agents. The lookup runs once per
proposal, for the Goal the proposal names, and re-resolves the channel scope each
time rather than caching an authorization decision.

The contract now distinguishes two answers that used to look alike: an empty
Agent list is a fact, so the plan's lanes become typed `agent_not_registered`
gaps, while an unresolvable Goal surfaces nothing at all. `parse_agent_response`
carries the context through to the four segments that parse an answer (managed
dsh, Codex agent, ACP, provider HTTP), so the same admission rule applies to
every transport instead of only the one the steward happens to run on.

Verified: tests/test_steward_team_plan_preview.py, tests/test_steward_team_plan_apply.py,
tests/test_chat_manager_context.py and tests/test_attached_session_broker.py 63 passed,
including a preview for a Goal outside the channel scope that is dropped, a
per-Goal lookup that is asked only about the named Goal, a Goal with no
registered Agents that becomes a typed gap, and the owner channel resolving any
registered Goal. examples/docs-governance-smoke.py and
examples/loopx-steward-managed-chat-smoke.py ok.

Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Approval conclusion (author-owned PR; GitHub blocks formal self-approval)

一、变更内容

  • loopx/chat_runtime.py:新增 _team_plan_admission_context(session),为管家通道按 Goal 解析地提供准入事实;在 start_turn 前挂到解析答案的那一段上。业主自己的通道可解析任意已注册 Goal;外部管家通道只解析其 scope resolver 授权的 Goal;registry 不认识的 Goal(或超范围)解析为"无法描述该 Goal",从而丢弃预览而不是拿别的 Goal 的 Agent 去校验。每次提案解析一次、每次重新解析通道范围(不缓存授权决定)。
  • loopx/chat.py:_validated_team_plan_preview 支持两种上下文形态——registered_agents_by_goal 映射,或 resolve_registered_agents 回调;parse_agent_response 透传 team_plan_context。
  • 四个解析答案的 segment(managed dsh、Codex agent、ACP、provider HTTP)都透传该上下文,因此同一条准入规则覆盖所有 transport。
  • 语义收紧:空 Agent 列表是事实(lane 变成 typed agent_not_registered gap),不可解析的 Goal 不是事实(什么都不浮现)。这两者此前无法区分。
  • RFC 双语更新:新增"准入事实"条目,并把"仍然惰性"改写成"缺的是效果而非预览"(缺 Chat 侧落地路径、前端确认面、lane Todo 的意图修订绑定)。

二、依据与一致性

  • 依据是你指出的那处缺口本身:预览只有在宿主能说出"计划点名的那个 Goal 有哪些 Agent"时才能被准入;此前生产上没有任何调用方传 team_plan_context,所以 #4519/#4522/#4524 的能力实际上无法浮现。
  • 与既有授权模型一致:解析回调复用 Turn owner 已经解析过的通道授权(manager_scope_resolver),只做只读 registry 查询,不扩大任何范围;不新增身份、不新增写入路径。
  • 与"非法状态难以表达"一致:None(不可描述)与 [](有 Goal 但无注册 Agent)被显式区分,前者丢弃、后者产生 typed gap。
  • 计划自身仍然 applies: false,落地仍用宿主自己重新推导的事实复校;本次没有改动任何 effect 权限。

三、验证

  • tests/test_steward_team_plan_preview.py、tests/test_steward_team_plan_apply.py、tests/test_chat_manager_context.py、tests/test_attached_session_broker.py:63 passed。新增覆盖:超范围 Goal 的预览被丢弃、按 Goal 的查询只被问及点名的 Goal、无注册 Agent 的 Goal 产生 typed gap、业主通道可解析任意已注册 Goal、未知 Goal 解析为空。
  • examples/docs-governance-smoke.py:ok;examples/loopx-steward-managed-chat-smoke.py:ok(真实 dsh segment 仍完整跑完并带出提案)。
  • loopx canary premerge --from-git-diff:merge_gate_passed=false,原因是两条与本次 diff 无关的既有红,均在干净 origin/main(7bf8104c9)原名复现且涉及本次未修改的文件:cli-project-lifecycle-command-modularization-smoke.py(#4521 移动 refresh-state 模块后遗留的过期 marker 断言)与 control-plane-maintainability-ratchet-smoke.py(两处 unreviewed finding,magnitude_regressions: 0;此前 diff 未选中 control-plane 面时它被归类为 advisory)。其余 canary、risk-profile smoke 与 public boundary 全部通过,manual_holds=0。

四、风险与残余缺口

  • 兼容性:_validated_team_plan_preview 的上下文形态由"扁平列表"改为"按 Goal 映射或回调"。生产上此前没有任何调用方传该上下文,因此没有迁移风险;测试同步更新。
  • 权限面:解析回调是只读的,且在外部通道上每次重新解析 scope;不缓存授权决定,代价是每提案一次 scope 解析(每次回答 ≤5 个提案),我认为这个代价换正确性是值得的。
  • 仍未做:Chat 侧落地路径(确认无处落地)、多 lane 前端确认面、lane Todo 的意图修订绑定;以及上面两条既有门禁红(需要各自的 owner 处理,本次不越权改他人文件)。

五、结论

批准以 admin squash 合并(需维护者授权,因门禁为既有基线红而非本 diff)。改动单一目的:让管家轮次按 Goal 提供准入事实,使已合并的团队入端口径第一次真正能浮现;无权限扩大、可回滚、覆盖面明确。

English verdict: Approved for an admin squash merge with maintainer authorization. The steward Turn now supplies per-Goal admission facts, so a validated team preview can finally surface: the owner channel resolves any registered Goal, an external manager channel only its authorized Goals, and an unresolvable Goal drops the preview instead of validating it against another Goal's Agents — with an empty Agent list kept distinct as a typed-gap fact. 63 focused tests pass, both smokes pass, and the two canary failures are pre-existing baseline reds reproduced on clean main in files this diff does not touch.

@huangruiteng
huangruiteng merged commit 9714c96 into main Sep 16, 2026
5 checks passed
@huangruiteng
huangruiteng deleted the codex/team-plan-admission-facts branch September 16, 2026 10:43
huangruiteng added a commit that referenced this pull request Sep 16, 2026
`examples/control_plane/control-plane-maintainability-ratchet-smoke.py` fails on
main: `loopx/chat_runtime.py` is 1560 lines against its checked-in ceiling of
1502, and 35 `Any` against 33. The growth is exactly #4533's team-preview
admission facts - one 51-line rule plus its 7-line wiring - and nothing else in
the module moved, so this is reviewed growth rather than drift.

The ceiling is the remedy the ratchet itself provides for that case. A module
metric budget is settled in the checked-in ledger, not through
`REVIEWED_MAINTAINABILITY_EXCEPTIONS`: an evaluated finding still counts in
`category_counts` even when an exception covers it, so the ledger edit is the
reviewer-visible act. The repository already works this way - `loopx/todos.py`
was refreshed 2165 -> 2190 -> 2229 -> 2249 -> 2285, `#2896` is a baseline
refresh, and `#2953` grandfathers a module. Relocating the rule was considered
first and is not enough on its own: moving the whole method and its call site out
of the module leaves 1504 lines, still above the frozen 1502, so the ledger would
have to be edited either way and the extra churn would not restore the ceiling.

The failure output also now names the ledger to refresh next to the finding. Main
sat red here because nothing in the CI text pointed at
`loopx/canary/module_metric_baseline.json`, and both the new line and the
negative case are locked by focused tests.

Verified: pytest tests/canary/test_maintainability_ratchet.py tests/control_plane/test_m6_quality_gates.py -q -> 12 passed; examples/control_plane/control-plane-maintainability-ratchet-smoke.py -> ok, unreviewed: 0.

Follow-up (deferred, recorded rather than bundled): `loopx/chat_runtime.py` stays a hot module with no headroom, so the next change that touches the manager turn prologue - the turn context assembly and the `ManagerInspection` wiring around it - should extract that bounded context builder instead of growing the controller again.

Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Co-authored-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Approval conclusion (author-owned PR; GitHub blocks formal self-approval)

动机

缺口的描述准确:团队预览只有在"宿主能说出计划点名的那个 Goal 有哪些 Agent"时才被准入,而生产里没有任何东西提供这个事实——所以模型产出的预览总是被丢弃,#4519/#4522/#4524 落地的契约在用户侧完全不可见。我在父提交上确认了这一点:没有任何生产调用方传 team_plan_context。

这个"契约、校验器、落地都在、但业主永远看不到"的状态代价很高:它和"没实现"无法区分,后续切片也没有真实旅程可验,所以补上准入事实是有价值的一步,而不是为了好看。

改动思路

三个设计判断我认为都对:

  • 给"查询"而不是"一个 Goal 的事实"。 管家通道本来就不绑定单一 Goal,所以 _team_plan_admission_context 返回的是 resolve_registered_agents(goal_id) 加上 supported_action_kinds;解析答案的那一段按预览自己点名的 Goal 去查。这正好补上了我在 #4532 里记下的残余(准入侧无法比较 Goal)。
  • 授权复用既有边界,且不缓存授权决定。 resolve 内部每次都重新调用 manager_scope_resolver(session):业主自己的 manager 通道可解析任意已注册 Goal,manager.external.* 只解析它被绑定的 Goal,范围外的直接返回"无法描述这个 Goal"。这比"准入时算一次然后缓存"更稳。
  • 把两种"查不到"分开。 空 Agent 列表是事实(lane 变成类型化 agent_not_registered gap),无法解析的 Goal 则让预览消失——None 与 [] 的语义区分写进了测试注释和断言。

另外把上下文接到了全部四处解析答案的段落(managed dsh、Codex agent、ACP、provider HTTP),所以准入规则只有一份;parse_agent_response 的新参数默认 None,没接的调用方保持旧的丢弃行为。我 grep 过全部 5 个 parse_agent_response 调用点,没有漏接的地方。

具体改动

六处 runtime(chat_runtime.py 新增约 50 行 builder、chat.py 约 25 行按 Goal 查询 + 透传参数、四个 adapter 各一行)、tests/test_steward_team_plan_preview.py +111 行、RFC 中英两版把"仍然惰性"段落改写成"第 4 项已落地:准入事实;仍缺的是效果(Chat 侧落地路径、前端确认面、lane Todo 的意图修订绑定)"。

我跑的验证:PR 点名的四个测试文件 63 passed(含范围拒绝、只查询点名的 Goal、空 Agent 列表→typed gap、业主通道解析任意已注册 Goal、未知 Goal 解析为空、非管家通道拿不到上下文);examples/docs-governance-smoke.py ok;examples/loopx-steward-managed-chat-smoke.py 返回 ok=true、turn_status=completed、proposal_count=1;git diff --check 干净。

一个 P3(非阻塞,已记入 findings):adapter.team_plan_context 只在非 None 时赋值、且从不清理,而 adapter 是按 session_id 缓存的;只要某个 adapter 实例在某次"没有事实"的 turn 里被复用,它就会带着上一次管家 turn 的查询。我没能证明存在活的触发路径(session 按通道划分,通道决定是否给上下文),所以这是防御性建议而不是已证缺陷:把赋值改成无条件(包含 None)或显式清理一行即可,让"准入事实按 turn、按 Goal"成为对象属性而不是调用顺序的巧合。

另外记录但不作为 findings:Goal 无法描述时预览是静默丢弃(业主只看到没有 proposal 的答案),这是 #4522 就有的"丢弃即静默"设计,本 PR 只是把"空列表 vs 无法描述"两件事分开了。后续若要给业主一个 typed 原因,可以在不改准入规则的前提下加。

对主干的风险

这次改动的性质是激活:管家通道从"预览永远被丢弃"变成"按 Goal 校验后可以浮现"。风险面因此有几处需要点明:

  • 激活范围由既有谓词 is_manager_channel 界定,非管家通道行为完全不变(测试断言 builder 对 lark:topic 返回 None);parse_agent_response 的新参数默认 None,未来调用方不接也不会意外放宽。
  • 授权面没有放宽:外部通道仍只解析它被绑定的 Goal,且每次查询都重新解析范围,而不是缓存一次授权决定。
  • 效果面仍未落地(Chat 侧落地路径、前端确认面、意图修订绑定),RFC 已明确写出"缺的是效果而不是预览",所以业主现在能确认一份暂时无处落地的计划——这一点作者已在文档中声明,属可接受的分片边界。
  • 解析签名变宽、四个 transport 都被触碰,漏接一处就会静默丢弃预览;我逐个核对过调用点,并确认默认值保持旧行为。

我的整体评价

这是一次目标明确、归属正确的激活切片:把缺失的准入事实补在 Turn owner 一侧,用"按 Goal 的查询"替掉了原先的扁平 Agent 列表,复用既有授权边界而非新造一套,并把上下文接到所有 transport 使准入规则只有一份。我复跑了作者点名的测试与两个 smoke,结果一致(63 passed、managed-chat ok=true、docs smoke ok),RFC 的两版改写也与实现一致地说明"预览已上线、效果尚未落地"。

建议顺手做两件小事:把 adapter 属性无条件赋值/清理(P3),以及后续给"无法描述该 Goal"的丢弃加上业主可见的原因;两者都不构成合并阻塞。

English verdict: APPROVE (exact head f9ae886)

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.

1 participant