fix(manager): read repository evidence before handoff - #4306
huangruiteng wants to merge 4 commits into
Conversation
|
Request changes conclusion (author-owned PR; GitHub blocks formal self-review) 动机本 PR 为 managed manager Chat 增加 改动思路
具体改动
关键代码讲解
对主干的风险当前精确 head 发现一个明确的 P1 blocker:当前 exact head 的 SonarCloud Quality Gate 未通过。 另有一项非阻塞 typed-state 建议: 我的整体评价这是一项范围合适、边界意识较强的 manager evidence-gap 变更:它复用了现有 identity/Todo/profile/handoff owner,没有新增网络或仓库读权限;但当前新代码覆盖率/复杂度质量门失败,不能合并。当前结论为 REQUEST_CHANGES。修复 Sonar blocker、补充分支覆盖并重新读取同一精确 head 的完整远端证据后再复审;typed invalid-profile 建议可一并加固。 English verdict: REQUEST_CHANGES — exact head 593832a fails the SonarCloud Quality Gate (77.1% new-code coverage vs 80% required, plus complexity 16/18 over 15). Add coverage/refactoring, rerun Sonar and the full exact-head evidence pass; malformed-profile diagnostics are a non-blocking hardening suggestion. |
huangruiteng
left a comment
There was a problem hiding this comment.
结论:APPROVE。审阅 exact head 593832a07d871797ead923b21a75ec688585302a。没有 blocking finding;GitHub 对 PR 作者不允许正式 self-approval,因此本条以 comment 形式发布。
动机
Manager 现有 context read 能读 Goal、Todo、delivery 和 handoff projection,但无法安全处理具体 PR 问题:缺少 artifact evidence 时,模型容易猜事实、挑选唯一可见 Agent,或越过 scope 去读 checkout/network。PR 新增一个只读、Goal 绑定、明确 unknown 的 repository-artifact evidence gap,并把缺口只路由给 capability-matched 的已授权 Agent。
改动思路
ManagerInspection.read 新增 view=repository_artifact,先用严格 grammar 解析 #NUMBER、NUMBER 或 credential-free HTTPS PR URL;再从 Goal repo 与 Core Todo task_repository 解析 canonical git identity;只有在 Goal scope 内才继续。Core 不提供 artifact 内容时,结果明确为 unknown/unavailable,不做事实推断。
路由只看当前 Goal 的 context_delegation.targets 与已通过 normalize_agent_profile 的 preferred_action_kinds,avoid_action_kinds 优先,恰好一个匹配才返回 recommended_handoff。零个或多个匹配都保持 typed gap,绝不使用“唯一可见 Agent” fallback。整个路径不执行 shell、checkout、网络读取或 LoopX 写入。
具体改动
- 新增
repository_evidence.py,集中负责 PR ref parsing、Goal/Todo repository binding、unknown projection 和 exact profile routing。 inspection.py扩展 tool schema/argument validation,禁止 repository-artifact 的 SSH/remote read,并在 scope revocation 后抑制结果。authority()投影已验证的 routing profiles;manager/chat-agent prompt、README/SKILL 与 context version 11 同步说明“先读 evidence、只用 exact recommended_handoff”。- 新增 manager inspection 与 handoff tests,覆盖动态 subprocess tool 安装、scope、profile projection 和 typed handoff。
关键代码讲解
_artifact_identity 通过严格 URL/number grammar 和既有 normalize_repository_identity 统一身份,拒绝 credentials、query、fragment、malformed path 及显式 repository conflict。_repository_bindings 只收 Goal repo 和 Core Todo 中的 git: identity,Todo authority warning/error 不会变成授权。_routing 将 profile 与 exact Goal target 求交集,匹配数量不是 1 就返回 no_capability_matched_agent 或 ambiguous_capability_match,并固定 sole_candidate_fallback_used=false。ManagerInspection.read 负责工具边界、local-only 和前后 scope 检查;repository inspector 只产生派生 evidence projection,不读取外部 artifact。
对主干的风险
我重跑了 focused manager suite:
uv run --with pytest pytest -q tests/test_chat_manager_inspection.py tests/test_manager_context_handoff.py
22 个测试全部通过,包含真实 manager subprocess 的 dynamic tool protocol。额外核对了 invalid/credentialed URL、identity conflict、Goal 外 repository、remote source、无匹配/多匹配 profile 和 scope revocation 分支;它们都会返回 typed error/gap,不会推断 PR 内容或选择未匹配 Agent。现有 portfolio/todos/deliveries/handoffs 分支保持原 enum 与边界。远端 required checks 和 merge-gate 均通过,唯一失败的是明确标记为 non-blocking 的 SonarCloud Code Analysis。
残余风险是未来若添加真正的 artifact provider,必须继续返回同一 unknown/no-inference contract,并由 provider-owned change 单独验证网络、凭据和权限边界;本 PR 没有偷偷扩大这些权限。
我的整体评价
这是一个边界清晰、scope 合理的 manager capability:它复用了既有 Goal/Todo identity、manager authority 和 agent-profile owners,把“缺证据”变成可观察的 typed result,而不是第二套事实或授权存储。新增 prompt 是生产行为面的一部分,已与工具 schema、context version 和文档同步;read-only、no-network、no-guess 和 ambiguity semantics 也有测试固定。基于 exact-head diff、caller trace、负向路径和 focused validation,我对该 PR 给出 APPROVE。
English verdict: APPROVE on exact head 593832a07d871797ead923b21a75ec688585302a. The additive repository_artifact manager view strictly parses and Goal-binds PR references, returns unknown evidence without inference, and routes only through one exact authorized profile match; ambiguity never falls back to the sole visible agent. Existing manager views remain unchanged, focused 22-test coverage passes, required checks/merge-gate are green, and the only failed check is explicitly non-blocking SonarCloud. No shell, network, checkout, write, review, or merge authority is introduced.
593832a to
5911286
Compare
|
Rework complete on exact head
Local evidence: 129 relevant tests passed, all changed Python files passed Ruff, the new provider/evidence modules passed targeted mypy, compile/diff checks passed, and a live exact-head GitHub read verified overview, pagination, diff and commit-pinned source. Standard canary and remote checks are still running and are not represented as complete. 中文:当前精确 head 已从“缺证即交接”改为“授权范围内先读原件”。深读强制固定并复核 PR head,失败与截断有明确类型;普通读取失败不会自动交接,工程交接还必须同时匹配 profile 与同仓库 Todo 职责。129 项相关回归及真实 provider 读取通过,远端 CI / canary 完成后再作合并判断。已请求独立 reviewer 复核,当前不是 merge approval。 |
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
5911286 to
bee8861
Compare
|
Superseded by the product/architecture direction in #4330. The manager should use normal authorized host tools for repository investigation, with general semantic work handoff rather than a mandatory manager-specific GitHub evidence provider. This PR contains useful revision/coverage/error/routing regression requirements; retain those where they exercise the replacement path. Closing this implementation proposal does not declare #4305 fixed. Track direct investigation under M1 and responsibility/handoff under M2 of the bilingual RFC. The branch is retained for reference. 由 #4330 的强能力管家与通用语义交接方案接替,不继续扩建专用 GitHub provider。保留有价值的回归要求,#4305 的真实用户问题继续按 M1/M2 验收,不能因关闭 PR 宣称已修复。 |
Summary
摘要
Validation / 验证
129 passed: manager repository provider/inspection, context, handoff, roundtrip, SSH isolation, portfolio, Chat agent and manager report suitesgit diff --checkpassed59112862f9a9101504378e976517177667dd364aProduct entry points / 产品入口
loopx_manager_read view=repository_artifactand records the same audience-scoped evidence event as other manager reads.goal-portfolio --manager-view repository_artifactwith the same head guard, pagination and typed failures.Boundary / 边界
The provider accepts only a Goal-bound credential-free repository identity, a numeric PR, fixed semantic sections, and repository-relative source paths at the PR head or base. It does not expose shell, arbitrary
gharguments, local files, writes, merge/review authority, credentials, raw provider payloads, private state, or generated logs.Provider source text is data, not instructions. Scope is checked before and after the read. A head change invalidates the result instead of mixing revisions. Large pages and unavailable patches remain explicitly incomplete.
Closes #4305