fix(agents): report session binding candidates, not the last one walked - #5066
huangruiteng merged 4 commits into
Conversation
|
我的 codex review token 用完了,明天 review |
huangruiteng
left a comment
There was a problem hiding this comment.
Exact head: 3c1379f3530ec812402b917123743fe3741508ad; comparison base: 27f0fc93b806925cb151df4ffac26e3aebdbff2c.
[P2, blocking] 请先按完整绑定身份去重,再生成显示摘要。 在 management_projection.py:600–611,thread ID 先经过 120 字符的显示压缩,host surface 先经过 60 字符压缩,再用压缩后的字典判断是否重复。绑定 owner 实际接受最长 128 字符的 thread ID 和 64 字符的 host surface。我用两个合法、同前缀但不同后缀的 121 字符 ID,通过真实 File registry → loopx status --goal-id probe 复现:正向 resolver 分别能解析两条绑定,新的 session_binding_count 却是 1;两个 61 字符 host surface 也会发生同样问题。这直接违反 #5065 的 distinct/full-count 验收,不是无关 CI 失败。
最小修复是把去重/计数键与展示文本分开:先对完整、有效且可公开的绑定身份计数,再对显示列表执行大小限制与必要的文本处理;保留当前公开信息过滤,不能为了避免截断而把原始敏感值直接投影。添加两条“owner 接受、显示前缀相同”的长 ID/长 host 回归,要求 count 为 2,并验证 cap 不影响真实总数。
动机
#5039 指出了历史绑定被误当成唯一执行路由的问题,当前命名任务只解决其中的展示缺口。旧代码每走过一条绑定就覆盖同一 Agent 的前一条记录,让遍历顺序看起来像选择结果。改成候选列表和完整数量是有实际价值的独立切片,但完整身份的计数还没有达到任务验收;不能用普通短 ID 测试通过代替这个承诺。
改动思路
真实入口是 status 的 Agent 管理投影,权威输入仍是 registry 经 run-history 投影的既有绑定,候选字段不负责选择执行 Session。源码确认 run-history 只压缩其它 coordination 元数据,没有先裁剪绑定数组,因此这里可以看见完整候选。当前实现把覆盖改成列表收集、首见去重和三项显示上限,没有新增写操作或 Session store,也没有复制正向路由解析器。只改变展示是合理边界,宿主可用性、接收方采纳和结果返回继续由父任务的现有 owner 负责。
具体改动
关键代码讲解
build_agent_management_projection 的绑定收集分支把每个 Agent 对应的单个字典替换成列表;普通两条绑定现在都会留下,重复发表的同一条绑定计一次,五条候选显示前三条并报告五条。问题就在收集时沿用了只适合展示的压缩函数作为身份键,合法不同值会变成相同省略文本,新增数量随之失真。
_agent_state 仍只接收是否存在绑定,而不是候选数量;我对单条、多条、无绑定、乱序和重复绑定做了对照,生命周期保持原样,也没有引入自动接管权。发射分支移除旧的单数字段,并同时发布新列表与计数。协议文档和原有生命周期断言随之更新,新测试覆盖普通候选场景,但缺少合法标识长度边界。
仓库搜索覆盖基线、当前头及最新主干的相关 owner 和消费者。Dashboard 的 Agent 行 schema 是 passthrough,视图不读取旧单数字段,因此没有发现必须保留这个“被选中路由”别名的内部消费者。外部脚本可能需要迁移,协议已经说明新形状;这不是要求增加兼容分支的理由。
对主干的风险
最强风险不是生命周期变化,而是展示仍会把真实多绑定报告成唯一候选。原有测试和新加的普通短值测试全部可以通过,同时这个反例仍然成立。我用独立的合成 registry,实跑 status CLI 和正向 resolver,没有用 mock 填入期望的候选数;多次 readback 前后 registry 字节不变,未使用活动 Goal 或真实 Session 做验证。
验证由我直接执行:作者新增/邻近测试 36 项通过,扩展的绑定、目录、生命周期、material gate、status/history 定向套件 81 项通过;Ruff 通过,配置的 Mypy 23 个文件通过,协议 smoke 通过,风险预合并的 5 项直接检查及 13 项选择检查通过。环境使用当前 checkout 的 Python 3.13,准备了项目 Python/Node 依赖;没有沿用作者的借用环境报告。按本次 capability 的 wait_for_ci=false,未查询或等待远端 CI,完整平台矩阵不冒充本地已执行。当前问题由相同基线/头的独立反例归因,不因任何远端红灯要求改代码。
语义与 CI 对齐
本次是现有只读投影词汇的扩展,不是新的路由 authority。三项上限是展示预算,不能改变“完整 distinct 数量”的含义;需要修复身份去重,而不是增加预算或放宽 oracle。普通两条候选的独立断言在旧实现失败、在当前头通过;长标识断言在当前头仍失败。
我的整体评价
设计方向和规模合适:四个文件、运行时小改动,测试与协议围绕同一遗漏,没有要求完整父任务一次交付。持续工作方面,短值多绑定的可见性确实改善,但 coordinator 仍不能信任全部合法绑定的数量,用户仍可能被迫手工纠正“只有一个候选”的判断。相关未来改进应在这个 owner 内区分完整身份与显示摘要,不需要新 framework,也不需要把宿主验证混进显示层。修复上述回归后,再对新的 exact head 重跑这些路径;本轮没有合并或改变原 PR。
REQUEST_CHANGES
English verdict: REQUEST_CHANGES — The bounded display fix is useful, but deduplicating truncated display values misreports distinct valid bindings. Preserve full identity for counting and add long-ID/host regressions before approval.
Amendment — identity and display are now separate at head
|
huangruiteng
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES — 上轮的长标识去重问题已修好,但这个 head 引入了一个本地必需质量检查失败,需要在本 PR 内做一次保留语义的局部整理。
Reviewed head: 1e3760828b4caf3f60682d1b13c966f99128e4fb
Compared base: 27f0fc93b806925cb151df4ffac26e3aebdbff2c
[P2] 将候选收集从已超预算的主投影函数中拆出。新增收集分支位于 management_projection.py:591–623。用相同的 uv run --extra test python examples/control_plane/control-plane-maintainability-ratchet-smoke.py --format json 比较,base 通过,head 报 oversized_decision_function:…:build_agent_management_projection:AST 语句数 81 → 92,决策点 41 → 45,当前门限仍为 90/60。标准 premerge canary 也执行并报告了同一失败;不是别处的红 CI。
最小修复建议是在同一 owning module 提取有明确输入/输出的私有 binding 收集 helper,主投影只消费候选摘要。必须保留完整三元组去重、现有公开安全过滤、完整计数后再截展示上限,以及生命周期不变;不要删合法分支、缩扫描范围或仅抬门限消掉失败。随后重跑原 ratchet、候选测试和真实 status/bind-unbind 回读。
动机
我核对了 #5065 及父任务 #5039。这里要解决的是“一个 Agent 的多条历史绑定,被最后遍历的一条覆盖成唯一会话”造成的诊断误导,不是授权 Agent 控制某个宿主或自动替换 peer。这个只读展示切片本身有独立价值,但不能宣称已完成父任务的宿主可达性、接收与结果返回。
改动思路
沿用 run_history.coordination.thread_agent_bindings 这一来源,把旧 singular 字段换成有界候选列表及完整数量。此次修订把 identity 与 display 分开:原始、去两端空白后的 Agent/thread/host 三元组负责身份;公开压缩只负责展示。因此同一可见前缀不再被误判成同一真实绑定,也没有另建需要人工同步的 locator 状态。
具体改动
4 个文件,268 additions / 14 deletions:生产投影 34/10、协议文档 12/3、生命周期测试 1/1,以及 221 行候选回归测试。文档明确列表最多展示 3 条,缺席不能否定其他绑定;不把候选等同于唯一、最新或可控制的 route。
关键代码讲解
build_agent_management_projection(563,收集分支 591–623):先按完整三元组去重,再存展示值;同一绑定重复出现只计一次,合法长 thread/host 的同前缀仍分别计数。691–694 先保留全集长度,再切展示上限。_agent_state(471):仍只接收“是否有可展示的 binding”这一既有事实;本次没有改 membership、claim/lease、任务资格或宿主执行权限。_compact(47):继续复用现有公开安全与展示压缩规则;没有把凭证样式标识直接露到公共投影。
我在隔离 File registry 上通过真实 loopx status 和 canonical bind/unbind/resolver 跑了 14 组相同 base/head 夹具。普通两绑定旧版只留一条,head 保留两条;7 条输入计数 7、显示 3。长 thread/host 各自的 3 条合法输入,上一评审 head 会少算为 2,本 head 正确为 3。重复、其他 Agent/Goal、顺序、无关 annotation、安全过滤及解绑/重绑都验证了;每次 status 回读保持 registry 字节不变。
对主干的风险
这不是默认关闭的新能力:现有管理 JSON 的字段形状确实发生变化,协议文档已有披露。检查了 dashboard 的 passthrough schema 和当前消费者,未找到读取旧 singular 字段的代码;本 PR 不新增视觉入口,也不承担宿主控制 UI。外部 JSON 消费者仍须按新文档升级,不能把候选数组当作 routing authority。
语义与 CI 对齐
按当前 review capability,仅使用 repository-native 本地验证,未获取、轮询或等待 GitHub CI。66 个聚焦 Python 测试及 747 个 projection/lifecycle/binding/status/material 相关测试、Ruff、配置内 Mypy、投影契约/live-status/公开安全 smoke 通过;标准 canary 的 5 个 direct checks 通过,13 个选中检查中只有上述 maintainability ratchet 失败。公开边界扫描 4 个改动文件无命中。失败已在不可变 base/head 上归因,属于本 PR 新增回归。
未来维护边界也审视过:需要的是候选收集与大投影函数的有界分离,不是新 provider、第二套绑定权威或整个 TypeScript 迁移。整理后的验收应保留所有正确候选结果及既有生命周期语义。
我的整体评价
候选展示和上轮 full-identity 修复的方向正确,实际反例也已通过;本轮不重复要求作者修已经解决的问题。但当前交付还没有通过它自身新增的必需质量门,所以先 request changes。局部提取收集逻辑后,按原工作负载回归即可重新评审,无需顺带修父任务或其他 PR。
English verdict: REQUEST_CHANGES.
1e37608 to
e9fa97e
Compare
huangruiteng
left a comment
There was a problem hiding this comment.
详细中文评审
结论:APPROVE。Exact head:e9fa97eb9c4578276fb7c903f42544c6de4a948b;base:6b4d59c20e8f8aa060f75572becafc3197644205。上一轮的两个问题都已关闭:长标识按完整身份去重/计数(作者上一版修复)和本轮提出的必需质量门失败(build_agent_management_projection 超预算),后者已通过在同一个 owning module 内提取候选收集 helper 修好,语义逐字节不变。
动机
一个 Agent 可以同时持有若干条历史会话绑定。旧实现把 run_history.coordination.thread_agent_bindings 折叠成单个 session_binding 字段,最后遍历到的那条会覆盖前面的,读表的人很容易把它当成到该 peer 的唯一 route——这正是父任务 #5039 里“真实 peer 评审请求被临时宿主子代理替代”那类误判的展示侧来源。本 head 把它换成有界候选列表加完整计数,_agent_state() 仍只消费“有没有绑定”这一事实,因此 claim/lease、任务资格与生命周期语义不变;这是一个可独立验收、可回退的只读展示切片。
改动思路
来源仍是既有的 thread_agent_bindings,没有新增绑定权威,也没有把候选当成 routing 决定。作者上一版把“身份”和“显示”分开:原始(仅去两端空白)的 Agent/thread/host 三元组负责身份与去重,_compact 只负责渲染,因此两条只在末尾不同的 121 字符 thread ID 仍分别计数。本轮只做结构性整理:把这段收集逻辑从主投影函数里提取为同模块的私有 helper _collect_session_binding_candidates,主函数只消费返回的摘要。提取没有改动任何判定顺序、过滤规则、计数时机或展示上限。
具体改动
关键代码讲解
loopx/control_plane/agents/management_projection.py:563 _collect_session_binding_candidates(本 head 新增):遍历run_history.goals[].coordination.thread_agent_bindings,用未压缩三元组(agent_id, thread_id, host_surface)做去重键,_compact结果只写进展示值;空 agent/thread 与非 dict 条目跳过,缺失 host 记为unknown。management_projection.py:608 build_agent_management_projection:改为在:636调用该 helper,行发射处保留session_binding_candidates(截到MAX_SESSION_BINDING_CANDIDATES = 3)与:707的session_binding_count = len(binding_candidates),即先保留全集长度再切展示上限。management_projection.py:471 _agent_state:仍只接收has_session_binding这一事实,绑定是一条还是多条都不改变addressable/bound判定,也不改变 claim/lease 或任务资格。management_projection.py:47 _compact:继续复用既有公开安全压缩规则,没有把凭证样式标识直接写进公共投影;协议文档docs/reference/protocols/agent-management-projection-v0.md同步说明列表最多展示 3 条、缺席不能否定其他绑定,也不把候选等同于唯一或可控制的 route。
对主干的风险
上一轮 review 的 P2 是必需质量门失败:同一台机器上用同一命令比较,base 通过,而上一个 head 报 oversized_decision_function:…:build_agent_management_projection(AST 语句 81 → 92,决策点 41 → 45,门限 90/60)。本 head 提取 helper 后 ratchet 恢复 ok=True,unreviewed_count=0、stale_exception_count=0,没有删合法分支、缩小扫描范围或抬门限。
语义保持的证据:我用同一份混合夹具(重复身份、只在末尾不同的长 thread/host、前后空格、非 dict 条目、另一个 Agent)在提取前后各跑一次 build_agent_management_projection,输出逐字节相同,session_binding_count=5、渲染 3 条。作者新增的 5 个候选回归与 26 个生命周期测试在整理后仍全绿。字段形状确实是可见变化(旧的 singular 字段被移除),协议文档已披露,仓库内旧字段的两个读者(文档与生命周期测试断言)都在本 PR 内同步更新;dashboard passthrough 是转发 schema,未发现按旧字段取值。候选数组仍不是 routing authority,本 PR 不新增写入口或宿主控制 UI。
归因说明:canary 中唯一失败项仍是 examples/control_plane/peer-agent-runtime-v1-smoke.py,它在 base 6b4d59c20 与本 head 上以同一行、同一断言失败(其嵌套的 agent-onboard-host-loop-activation-smoke.py 仍在断言旧的 heartbeat 提示词措辞),需要在 main 上另修,与本 diff 无关。按 goal 策略本轮不查询、不等待远端 CI。
我的整体评价
APPROVE。problem_context 的 long_horizon 与 user_experience 均为 improved:读表的人不再把一条历史绑定误当唯一 route,且完整计数让“还有别的绑定”可被看见。observable_semantics 为 intentional_change_validated:base 只有 singular 字段,head 输出候选数组与完整计数,而本轮的提取相对上一 head 是逐字节等价的纯结构整理。repository_reuse、scope_fit、code_volume 与 change_proportionality 通过:没有新模块、新状态或第二套绑定权威,_compact 与生命周期判定都复用既有 owner;typed_state_rule、authority_semantics、default_off_isolation 无新协议或新写权限。验证:ratchet ok=True,6 个相关测试文件 72 项通过,提取前后逐字节等价,ruff 与 diff check 干净,exact-diff change-quality receipt cqr_c6943b62ba5cb7aeaec6 有效。合并授权来自维护者,本评审只说明该 exact head 的批准条件已经满足。
English verdict: APPROVE - exact head e9fa97e on base 6b4d59c. The prior full-identity dedupe/count fix stays closed, and the required quality gate that this PR had broken is repaired: the candidate collection moved into a private helper, so build_agent_management_projection is back inside the maintainability ratchet (ok=true) with byte-identical projection output on a mixed fixture. Validation: ratchet, 72 tests across the six related files, the before/after equivalence probe, ruff, diff check and a valid exact-diff quality receipt. Unrelated and non-blocking: the peer-agent canary fails identically on base through the agent-onboard smoke's stale heartbeat-prompt assertion. Merge authority comes from the maintainer, not from this review.
Superseded: the requested change and the P2 quality-gate failure were repaired and re-reviewed on the exact head e9fa97e (review 5326418217).
`session_bindings` was keyed by Agent and reassigned for every `thread_agent_bindings` entry the goal walk touched, so a peer with two historical bindings published one object under the singular name `session_binding` and dropped the rest without saying so. loopx-project#5039 records what that cost: a coordinator read the surviving row as the only route and substituted a temporary host sub-agent for an existing peer. The row now carries a first-seen, deduplicated candidate list capped by a named budget plus the distinct count, so a short list reads as a cap and not as a disproved remainder, and no single object remains for a consumer to mistake for the selected route. Membership still drives the lifecycle state, so an Agent's state is unchanged whether it holds one binding or several. The protocol document is updated in the same commit, including the explicit statement that neither field selects an execution route or transfers any authority; `thread_agent_binding.py` remains the resolver owner and this module imports nothing from it. Signed-off-by: JunZ-Leo <100498253+JunZ-Leo@users.noreply.github.com>
Deduplication keyed on the already-rendered candidate: the thread identifier was compacted to 120 characters and the host surface to 60 before the pair was compared, while the binding owner accepts 128 and 64. Two valid bindings that differ only past the visible edge therefore rendered to the same text and were counted once, so the field the slice exists to make trustworthy still under-reported the number of routes a peer holds. Identity is now the untouched value; compaction only renders it. `strip` matches the owner's own normalisation, which is what keeps a padded identifier from being counted as a second binding. The rendered values stay inside the display budget, and the protocol document states that distinctness is decided on the full identity, so two counted bindings may legitimately render alike. Adds the long thread identifier and long host surface regressions, a padding case, and asserts the rendered length so a fix cannot pass by publishing raw values instead. Signed-off-by: JunZ-Leo <100498253+JunZ-Leo@users.noreply.github.com>
…ector The session-binding candidate branch pushed build_agent_management_projection past the maintainability ratchet: 92 statements and 45 decision points against the 90/60 thresholds, so `control-plane-maintainability-ratchet-smoke` failed on this head while base passed. Collecting the bindings now lives in a private _collect_session_binding_candidates helper in the same module; the projection function only consumes the returned summary. Identity, public-safe compaction, full count before the display cap and lifecycle membership are unchanged. Behaviour evidence: the same mixed fixture (duplicate identity, long thread and host values that share a display prefix, whitespace padding, non-dict entries, a second Agent) produces byte-identical agent rows before and after the extraction, with session_binding_count = 5 and three rendered candidates. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Main now publishes peer routes from `collect_accepted_bindings` in the binding owner module, and that walk normalises the agent lane through `normalize_todo_claimed_by`. The management projection had its own walk that keyed candidates by the raw (strip-only) agent string, so a binding written as `Peer` was grouped under a key no management row ever looks up: the candidate was silently dropped, and a later `peer` row could under-report its routes. The private walk also counted bindings the owner refuses to name (a thread id past 128 characters, a host surface with inner whitespace, a missing host, an unnameable lane), which rendered a route no resolver would confirm. Delegate to the owner's collector instead: one normalisation rule, candidates keyed by the same agent identity the rows use, and `session_binding_count` that matches the `peer_route.candidate_count` the peer directory publishes for the same bindings. Display keeps its own 3-entry cap and 120/60 character budget, so identity still stays whole and `_compact` still only renders it. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
e9fa97e to
725b8fe
Compare
huangruiteng
left a comment
There was a problem hiding this comment.
详细中文评审
结论:APPROVE。Exact head:725b8fe8750cbf4185a6e867c0e857349b31d3be;base:59804c78222a5f0e4e0c158965de28c03fa23757。上一轮的必需质量门失败已关闭,本轮又在 rebase 到最新 main 后补了一处语义对齐:当同一份 bindings 已经由绑定所有者统一收口时,管理投影不再自己走第二遍。
动机
一个 Agent 可以同时持有若干条历史会话绑定。旧实现把 run_history.coordination.thread_agent_bindings 折叠成单个 session_binding 字段,最后遍历到的那条会覆盖前面的,读表的人很容易把它当成到该 peer 的唯一 route——这正是父任务 #5039 里“真实 peer 评审请求被临时宿主子代理替代”那类误判的展示侧来源。本 head 把它换成有界候选列表加完整计数,_agent_state() 仍只消费“有没有绑定”这一事实,因此 claim/lease、任务资格与生命周期语义不变。
改动思路
作者原方案把“身份”和“显示”分开:原始三元组负责身份与去重,_compact 只负责渲染,因此两条只在末尾不同的长 thread ID 仍分别计数。维护者修复分两步:先按必需质量门把收集逻辑从主投影函数里提取为同模块私有 helper,使 build_agent_management_projection 回到 ratchet 之内;随后在 rebase 到最新 main 时发现 #5068 已在绑定所有者模块导出 collect_accepted_bindings,于是不再保留第二套 walk,直接改用它,投影自己只保留展示预算。
具体改动
关键代码讲解
loopx/control_plane/agents/management_projection.py:563 _collect_session_binding_candidates:现在只做两件事——调用collect_accepted_bindings(run_history.goals)取回所有者已接受、已去重的绑定,再按binding["agent_id"]分桶、把 thread/host 经_compact压到 120/60 字符。此前那段 33 行的私有遍历(strip 后自建三元组身份、缺失 host 记unknown)已删除。management_projection.py:617 build_agent_management_projection:仍在:636附近消费该摘要,行上保留session_binding_candidates(截到MAX_SESSION_BINDING_CANDIDATES = 3)与session_binding_count(先取全集长度再切展示上限),agent_id in session_binding_candidates继续供_agent_state判定 addressable/bound。- 导入
from ...thread_agent_binding import collect_accepted_bindings:与同包directory.py的summarize_agent_binding_routes共用同一 owner,没有新模块、新状态或第二套绑定权威;_compact、_as_dict、_as_list继续复用。
语义修复
旧私有遍历用 str(...).strip() 自己判定身份,而所有者用 normalize_todo_claimed_by/normalize_thread_id/_normalized_host_surface。这在两处产生了真实偏差:一是把 Peer 这类大小写不同的 lane 放进一个任何管理行都不会读取的 key,候选被静默丢弃;二是把所有者拒绝命名的绑定(thread ID 超过 128 字符、host 含内部空白、缺 host、非法 lane)也算成 route,并渲染成看不出问题的一行。现在管理行的 session_binding_count 与 peer directory 的 peer_route.candidate_count 对同一份 bindings 一致,两个公共投影不再各报一个数。
对主干的风险
字段形状确实是可见变化(旧 singular 字段被移除),协议文档已披露,仓库内旧字段的两个读者(文档与生命周期测试断言)都在本 PR 内同步更新;dashboard passthrough 是转发 schema,未发现按旧字段取值。_agent_state 仍只取“有/无绑定”,因此生命周期、claim/lease 与任务资格不受影响,也没有新增写入口或宿主控制 UI。残余风险是语义上的:所有者拒绝命名的绑定现在被省略而不是渲染成 unknown,读表者看到的候选可能少于原始 coordination 块,这是单一 owner 规则的预期结果,但相对上一版 head 是一处可见 delta,应被明确记录。验证:ratchet ok=True,8 个相关文件 117 项通过,owner-accepted 绑定上新旧遍历逐字节等价,差分探针显示两面计数一致,ruff 与 diff check 干净,exact-diff change-quality receipt 有效。归因说明:canary 唯一失败项仍是 examples/control_plane/peer-agent-runtime-v1-smoke.py,其嵌套断言依赖的 "not a heartbeat automation" 在 base 的 loopx/** 中已不存在,属 main 侧陈旧断言,与本 diff 无关。
我的整体评价
APPROVE。problem_context 的 long_horizon 与 user_experience 均为 improved:读表的人不再把一条历史绑定误当唯一 route,完整计数让“还有别的绑定”可被看见,而且该计数与 peer directory 公布的路由数一致,不会再出现同一状态两处口径不同。observable_semantics 为 intentional_change_validated,并且我把本轮相对上一 head 的真实差异单独列出(所有者拒绝命名的绑定被省略、lane 大小写归一到同一行)。repository_reuse、scope_fit、code_volume 与 change_proportionality 通过:没有新模块、新状态或第二套绑定权威,_compact 与生命周期判定都复用既有 owner;typed_state_rule、authority_semantics、default_off_isolation 无新协议或新写权限。合并授权来自维护者,本评审只说明该 exact head 的批准条件已经满足。
English verdict: APPROVE - exact head 725b8fe on base 59804c7. The prior quality-gate failure is closed (the candidate collector was extracted, so build_agent_management_projection is back inside the maintainability ratchet), and the rebase exposed one more real defect that this head fixes: the projection ran its own normalisation of the same bindings that loopx/thread_agent_binding.py already owns, so a case-variant lane landed under a key no row reads and the row count could disagree with the peer directory it is supposed to describe. Delegating to collect_accepted_bindings removes the duplicate state rule, deletes 33 lines, and makes session_binding_count match peer_route.candidate_count for the same payload. Validation: ratchet, 117 tests across the eight related files, byte-identical parity on owner-accepted bindings, a differential probe for the intended delta, ruff, diff check and a valid exact-diff quality receipt. Unrelated and non-blocking: the peer-agent canary fails on base through the agent-onboard smoke's stale heartbeat-prompt assertion. Merge authority comes from the maintainer, not from this review.
|
Merge decision (maintainer) Reviewed exact head: Changed surfaces: Checks named in the review comment: maintainability ratchet on base and head ( Failures and skips: remote CI was not consulted ( Why the coverage is enough: the diff is one collection helper plus one row-emission site in a single read-only projection, with no new state, protocol or write path. The ratchet, the two probes and the 117 tests close the specific risks this diff introduced (quality-gate regression, duplicate binding-authority knowledge, and divergence from the peer directory's published route count). Gate note: Authority note: merge authority came from the maintainer, not from the review. |
Goal And Delivered Outcome
Goal/source and gap: Resolve existing peer Agent delivery routes before substituting host sub-agents #5039's "Confirmed follow-up: the display projection is lossy" section, taken as one scoped slice.
management_projection.pykeyed adict[str, dict[str, str]]by Agent and reassigned it for everythread_agent_bindingsentry the goal walk touched, so a peer with two historical bindings published one object under the singular namesession_bindingand dropped the rest with no signal that anything had been dropped. That is precisely the inference Resolve existing peer Agent delivery routes before substituting host sub-agents #5039 says must not be reachable: array order and recency do not establish which binding should receive new work, yet the row offered exactly one.Observable before → after, with the validation row that proves it: before, two bindings for one Agent produced
session_binding = {thread_id: "thread-new", ...}and the earlier binding was unrecoverable from the row; after, the same payload yieldssession_binding_candidateswith both entries in first-seen order andsession_binding_count: 2. Theunitandregression_parityrows below are the proof; mutation A is the before-shape reinstalled and it fails exactly the three tests that describe survival.Issue/task and intended base: Closes [Task]: keep a peer session-binding candidates in the Agent row, not the last one walked #5065. Base
mainat27f0fc93b.Scope And Continuation
Completed scope and remaining work: what an Agent row reports about bindings the status payload already contains.
_agent_state()consumed this data through membership (has_session_binding) and still does, so lifecycle states are unchanged whether an Agent holds one binding or several — that is asserted, not assumed.The singular
session_bindingkey is removed rather than kept alongside the list. Tree-wide search on27f0fc93bfound exactly two readers of that key:docs/reference/protocols/agent-management-projection-v0.md:115, which documents it, andtests/control_plane/test_agent_lifecycle_state.py:54, which asserts only its presence. Neither is left to a follow-up — the doc bullet and that assertion are changed in this commit, and the rewritten doc statement is explicit that neither new field selects a route or transfers authority. A singular alias kept beside the list would have preserved the ambiguity being removed, and picking which candidate it should show is the decision this projection does not own.Not in this slice, and left to the owners Resolve existing peer Agent delivery routes before substituting host sub-agents #5039 names: the bounded reverse lookup / route preview over
loopx/thread_agent_binding.py, verifying a selected route through the owning host, and the delegation identity work separating delivery from receiver adoption and result return. This module imports nothing new from the resolver and does not choose among candidates.Slice boundary / successor: complete within this scope for the display-projection half. Resolve existing peer Agent delivery routes before substituting host sub-agents #5039 stays open for the routing half.
Validation
1e3760828(amended after the first review — dedupe and count now key on the full binding identity while_compactonly renders it; see the amendment comment for the reproduced counterexample and the four added regressions). The rows below were measured at the first head3c1379f35; the identity fix adds 4 test cases and the 747-case neighbourhood re-run at the new head is recorded in the amendment comment.staticpassedpython3 -m ruff checkon all changed files;python3 -m mypywithout arguments as CI runs it —Success: no issues found in 23 source files;loopx check --scan-pathover the three touched source/doc/test paths plus the new test —public boundary scan clean: 3 files;git diff --checkclean.unitpassedtests/control_plane/test_agent_session_binding_candidates.py(new, 5 tests): two bindings survive in first-seen order and the singular key is absent; a binding republished across two goals counts once; five bindings yield a capped list of three withsession_binding_count: 5; a second Agent's binding does not enter this Agent's candidates; and the lifecycle state isaddressablefor one binding and for several while an unbound Agent gains neither field.tests/control_plane/test_agent_lifecycle_state.py26 passed after updating the presence assertion.integrationpassedtests/control_plane -k "management_projection or agent_lifecycle or projection"— 449 passed, 0 failed, which is the neighbourhood that renders this row.real_entrypointpassedexamples/control_plane/agent-management-projection-contract-smoke.py,agent-management-observability-mvp-smoke.py,agent-management-live-status-smoke.pyandpeer-supervisor-smoke.pyeach exit 0.regression_paritypassedmanualpassedpytest -q tests/run recorded below.Coverage and gaps: the change is one collection site and one row-emission site in a single projection, and the tests address both the surviving shape and the three ways it could silently degrade (dup inflation, cap pretending to be completeness, cross-Agent leakage). The removed key's only two readers are updated here, so no stale consumer remains in-tree.
Full-suite attribution, measured rather than assumed. An earlier aggregate
pytest -q tests/run on this branch is not quoted, because I was concurrently running other selections and lint on the same machine and it returned 152 failed / 11950 passed — a number contaminated by that load, including 20 failures intests/test_handoff_receiver.pythat pass 25/25 when that file is run alone. After freeing the machine, I ran the 15 non-architecture files that had failed as one fixed selection on two isolated worktrees at the same base: pristinemain27f0fc93bproduced 7 failures, this head produced 5, and the head set is a subset of the base set, so the change adds no failure. The two differences are the known order-sensitivetests/test_external_scheduler_worker.pypair.tests/capabilities/test_repository_change_window.pyfails 9/17 identically on both trees for an environment reason unrelated to this change — the borrowed interpreter'sloopxconsole script resolves to a different local worktree, and that suite spawns it. The remaining bulk istests/architecture/test_semantic_*(~87), which fails here becausenpm ci --ignore-scriptswas not run in this worktree.Environment disclosure: validation used a borrowed interpreter from an existing local virtualenv rather than a fresh
uv sync --extra test, with Node 22.23.2 on PATH. Neitherloopx/control_plane/agents/management_projection.pynortests/control_plane/test_agent_lifecycle_state.pysatisfiesruff format --checkonmain; they were not reformatted, and the deviation count was measured before and after this change on the production file (76 → 76). The new test file is format-clean at 0 deviations.Maintainer Repair At The Merged Head
The reviewed and merged head is
725b8fe8750cbf4185a6e867c0e857349b31d3beon base59804c78222a5f0e4e0c158965de28c03fa23757, merged as9849366c6e9f09f06872b697d2ad86126b536ceawith an identical tree. Two maintainer commits were added on top of the author's two:fbc5ba812— the candidate collector was extracted into the private helper_collect_session_binding_candidates. Without itbuild_agent_management_projectionexceeded the maintainability ratchet (oversized_decision_function, 92 statements / 45 decisions against a 90 / 60 ceiling) that passes on base; the extraction adds no mechanism and leaves the projection output unchanged.725b8fe87— the rebase onto latestmainexposed a second, real defect.#5068had already moved the authoritative walk intoloopx/thread_agent_binding.py(collect_accepted_bindings), and the peer directory already publishes route candidates through it. This projection still ran its own strip-only walk, which (a) keyed candidates by the raw agent string, so a lane writtenPeerlanded under a key no management row ever reads and the candidate was silently dropped, and (b) counted bindings the owner refuses to name — a thread id past 128 characters, a host surface with inner whitespace, a missing host, an unnameable lane — and rendered some of them as anunknownhost. The row now reads the owner's collector, sosession_binding_countequals thepeer_route.candidate_countthe peer directory publishes for the same bindings. That deleted 33 lines and removed the second normalisation rule; the display cap (3) and the 120 / 60 character render budget are unchanged.Behaviour disclosure: bindings the owner refuses to name are no longer rendered with an
unknownhost, and lane identity is normalised before bucketing. Both are intended single-owner results and both are visible deltas against the previously reviewed head; on bindings the owner accepts, the previous walk and the shared collector return identical candidate groups.Validation at the merged head: maintainability ratchet
ok=truewithunreviewed_count=0; 117 tests across the eight related files (including the author's candidate suite and the#5068peer-directory suite); a differential probe showing the two public surfaces now agree; a parity probe showing byte-identical output on owner-accepted bindings;ruffandgit diff --checkclean; exact-diff change-quality receipt valid. One canary failure remains and is unrelated:examples/control_plane/peer-agent-runtime-v1-smoke.pyfails identically on base because its nestedagent-onboard-host-loop-activation-smoke.pystill asserts"not a heartbeat automation", a string that no longer exists underloopx/**. That is amain-side stale assertion for its own owner, not this diff.