Conversation
Duang777
left a comment
Duang777
left a comment
There was a problem hiding this comment.
Exact-head self-review
Reviewed commit: f7b48f7e4a9417a41f7f1cae58b4c0c10ea33d00
Verdict: no blocking findings; ready for maintainer review.
The review checked Goal A/B isolation, stale enqueue/claim/resume rejection, historical completion admission, missing-registry fail-closed behavior after exact history exists, legacy byte compatibility, public payload omission, and lifetime-lock ordering. A resume TOCTOU found during review was fixed before this head and covered by a concurrent recreation regression test.
Validation on this exact head:
- 84 Chat/attached and architecture tests passed.
- TypeScript control-plane suite: 3154 tests, 3124 passed, 30 environment-gated skips, 0 failures.
- Ruff, TypeScript typecheck, registry I/O manifest validation, and diff checks passed.
- Standard premerge gate passed 19/19 checks with no manual holds.
Scope remains limited to attached_host_chat_session; managed provider startup and the remaining M3 owner rows stay blocked. This review does not grant merge authority.
huangruiteng
left a comment
huangruiteng
left a comment
There was a problem hiding this comment.
动机
目标是把 attached-host chat 的 Session 与真正的 Goal 实例绑定起来，避免同一 Goal key 被重建以后，旧会话继续获得新实例的执行权。这个 M3 增量有真实调用路径，也复用了现有身份词汇；但不能仅凭 stamped Session 的成功路径就宣称整个 attached family 已 qualified。保留的未绑定 Session 同样是启用后必须安全处理的对象。
改动思路
TS lifecycle 决策复用已有 source-session lifetime/GoalRef，Python 负责传递事实、持锁和写入，方向合理。Session 的实例绑定与 Turn 的 admitted binding 是历史权威事实，不是需要用户额外手工维护的展示字段；当前 GoalRef 不能代替旧 Turn 的已接受身份。
普通非 source 项目仍走既有格式；已经接受的历史 A Turn 可以完成或重放自身结果，而不能给 B 实例新的执行权。关键问题是兼容路径必须由权威 execution profile 决定，不能由某条记录“有没有 stamp”决定。
具体改动
全 PR 为 17 个文件、1785 行新增/62 行删除，包含生产接线、两个内部身份字段、typed decision/RPC、针对性测试、registry IO manifest 和双语 M3 状态说明。API/UI 继续使用 opaque Session id，没有把私有实例身份变成客户端输入；现有 controller 与 worker broker 确实接入，不是只写 serializer。源码路径改变了 attached-host 行为，没有实现或开放整个 firstparty_runtime family，RFC 中的其他 hold 应继续保留。
关键代码讲解
decideChatSessionLifecycle（TS 第 260 行）：负责当前 Session admission 和历史 Turn return 的类型化判断，source profile 下缺少实例本应拒绝。_current_attached_session_guard（Python 第 516 行）：有 stamp 时持 lifetime 锁并重新读取；第 529 行却在没有 stamp 时直接 yield，resume 因而跳过 registry/typed gate。enqueue_attached_agent_turn（第 593 行）：第 608–614 行在没有 stamp 时直接创建 queued Turn；submit 和 enqueue 都到这条分支。_claim_attached_turn_once（第 646 行）：第 662 行同样直接 legacy claim，没有先确认 source profile 是否已经启用。
对主干的风险
[P1] 已启用的严格 profile 可被未绑定 Session 绕过（loopx/attached_session.py:608–614，同根因还有 529、662 行）。
复现条件：用现有测试的 _register(tmp_path) 创建真实 fresh source-session registry，再通过现有 store API 创建同 Goal/agent/host、但没有 goal_instance_id 的保留 Session。分别调用 controller submit、enqueue、resume 和 broker claim。四个独立反例全部接受并修改 Session，没有抛出契约要求的 goal_instance_id_missing。它们运行真实文件 backend，没有 mock admission 判定或写入结果。
最小 enqueue 复现（在 PR 根目录保存为一个 pytest 文件运行）：
import runpy
import pytest
from loopx.chat_runtime import ChatRuntimeController
from loopx.chat_store import CHAT_SESSION_MODE_ATTACHED
helpers = runpy.run_path("tests/test_attached_session_goal_instance.py")
def test_strict_profile_rejects_unstamped_session(tmp_path):
registry, _, store, _ = helpers["_register"](tmp_path)
session = store.create_session(
goal_id=helpers["GOAL_ID"],
agent_id=helpers["AGENT_ID"],
adapter_kind="attached_host_session",
upstream_thread_id=helpers["HOST_SESSION_ID"],
session_mode=CHAT_SESSION_MODE_ATTACHED,
host_surface=helpers["HOST_SURFACE"],
)
runtime = ChatRuntimeController(
store=store, codex_bin="missing-codex", registry_path=registry
)
with pytest.raises(ValueError, match="goal_instance_id_missing"):
runtime.enqueue_turn(
session_id=session["session_id"], client_turn_id="new",
message="must reject", work_dir=tmp_path, objective="fixture"
)
assert store.turn_for_client(session["session_id"], "new") is None运行 uv run --extra test python -m pytest -q <reproducer.py>，当前 head 会失败于 DID NOT RAISE。
最小修复是先读取权威 profile/current lifetime，再把未绑定 Session 也交给现有 typed decision；仅确认 profile 未启用后才允许 legacy 路径。不要通过自动给历史记录补 stamp 来消除反例。submit/enqueue/resume/claim/completion 都应验证“拒绝发生在任何 Session、Turn、ingress/event 写入之前”。
语义与 CI 对齐
这复用的是已有 GoalRef 语义，不是未来 RFC 属性的新门槛：现有 RFC §4 identity matrix 对当前实例缺少绑定要求 execution reject，§11 明确启用项目不能 legacy fallback。当前 Python dispatch 与这个义务冲突；inventory 的 m3_qualified/exact_goal_ref_enforced 需要在修复后才能成立。
我独立完成 91 个 Python 测试、18 个 TS 测试、control-plane typecheck、registry IO manifest（250 sites）、改动 Python 文件的 Ruff 与 diff check，均通过。起初本地缺 TypeScript 依赖造成一次架构测试环境错误，安装声明依赖后同项通过；它不是本 PR 的缺陷。未查询或等待远端 CI。阻塞来自额外四个真实负例，不来自无关红 CI。
我的整体评价
请求修改。long_horizon 与 user_experience 的严格身份承诺仍存在 regression：旧记录可以继续执行，却没有明确的缺绑定错误/恢复入口。与此同时，普通关闭路径的独立 base/head harness 覆盖 bind、submit、enqueue、claim、complete、resume 以及单独存储/公开 readback；仅规范化 UUID 和时间，完整结果一致，未发现默认关闭漂移。
范围本身是一个可审阅的 attached-host 增量，不要求把整个 RFC 或 TS 重构一并做完。相邻的 bounded refactor 建议与本修复一起完成：共享权威 profile/lifetime context，消除几处“null 就等于 legacy”的并行判断，继续让 TS 拥有语义。保留未启用项目的 persisted compatibility，但不能因此跳过已启用的约束。修复后请重跑当前实例/替换/历史完成正负例、上面的未绑定入口矩阵及同一 legacy base/head 对照，再重新判断完整 PR。
English verdict: REQUEST_CHANGES - exact head f7b48f7; active source-session admission is bypassed by unstamped attached Sessions in four independently reproduced public paths.
| if session is None or session.get("status") == "closed": | ||
| raise KeyError("chat session was not found") | ||
| goal_instance_id = session.get("goal_instance_id") | ||
| if goal_instance_id is None: |
There was a problem hiding this comment.
[P1] Check the authoritative execution profile before taking the unstamped compatibility path. On an active source_session_v1 Goal, a retained attached Session without goal_instance_id reaches this branch and creates a queued Turn without registry/lifetime validation. Independent public controller submit/enqueue, resume and broker claim cases all accept and mutate state instead of raising goal_instance_id_missing. The existing TS lifecycle decision already rejects this state, but this branch never calls it. Select legacy only after verifying a non-source profile; otherwise reject before effects, and cover all public entrypoints plus no-effect readback while retaining inactive-profile parity.
|
This pull request has merge conflicts with Choose the remote for the base repository, not an out-of-date fork. git fetch upstream
git rebase upstream/main
# Resolve each conflict, git add the resolved files, then git rebase --continue.
git push --force-with-lease origin HEADFor a same-repository clone whose Keep the DCO |
mergify
Bot
added
the
needs-rebase
f7b48f7 to
b0b5552
Compare
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
b0b5552 to
61d2b08
Compare
huangruiteng
left a comment
huangruiteng
left a comment
There was a problem hiding this comment.
结论：REQUEST_CHANGES — 一个 P1 阻塞项。精确审查 head：61d2b08a05fdb4062734e843c9969e2c66fdd09a，immutable base：6643f367064b9921c864db75a042979fddc4b8c3。发布前检测到 rebase 后重新建 checkout、重跑验证；PR 最终 diff 字节未变，没有沿用旧 head 的验证结论。
[P1] registry 缺失不能把已知 strict context 中的旧 Session 降级为 legacy。 _attached_session_lifetime 的缺失分支 只检查当前 Session 是否有 goal_instance_id。只要选择的是未绑定的保留 Session，它就返回 legacy context；即使同一 Goal/Agent/channel 已有 exact-bound Session 历史，也不会读取或拒绝该上下文。submit、enqueue、resume、claim、complete 随即绕过 TS admission 并写入真实状态。selection 路径 对同样情况却会拒绝。完整复现与最小修复见下文及行内评论。
动机
本 PR 对应 source-owned GoalRef 生命周期的 M3 attached-host Chat 切片：同名 Goal A 被 B 替换后，不应让 A 的旧 Session 获得 B 的新执行权；已对 A 持久化 admission 的迟到结果可以只返回 A。这是新鲜 source profile 的有限 owner qualification，不是整个 M3、既有项目 activation 或 managed provider 的完成。当前 RFC 的 missing-binding matrix、source 不可用时 fail-closed、禁止猜测/自动升级历史记录，是本轮判断依据。
改动思路
decideChatSessionLifecycle 在既有 TS effect-runtime 中拥有 select/admit/claim/replay/complete 的精确身份规则；Python 收集 source/session/turn facts，并在既有 source-session lifetime guard 内执行 File effects。Session、queued Turn 和 claim admission 记录可选 identity 字段，非 source writer 仍省略。正确方向是让 durable binding 决定受允许的关系，同时禁止 alias、历史记录或缺失 locator 生成新的 authority。
具体改动
17 个文件、+1,841/-55：attached_session 增加 source-aware bind/selection/shared lifetime 和受 guard 保护的 enqueue/resume/claim/complete；chat_runtime 接入这些路径并继续拒绝未资格化的 strict managed startup。chat_store 抽出完整/可恢复 candidate 枚举，保存可选 Session/Turn/admission identity；API 与 worker-bridge CLI 使用 source-aware registry owner。TS handler 与 tsconfig 注册新 decision，inventory/census 更新 owner/read locations，两份 RFC 记入 bounded M3 candidate，Python/TS/architecture 测试覆盖 ABA、历史结果、锁、CLI 与 compatibility。
registry 存在时，前次指出的 unstamped Session 绕过已修好：五个入口都拒绝且不改 Session/Turn。正向 ABA 对照也通过：A 已领取结果只写回 A，A 未领取队列拒绝继续，新建精确 B Session 可以 submit→claim→complete，重启读回与重复 completion 只有一条各自响应，B 不接受 A 的结果。
缺口在 registry 不存在的分支：exact_session=false 不是“profile 已关闭”的证据。它也不应与 selection 的 known-exact-history refusal 分裂成两套上下文解释。
对主干的风险
独立使用真实临时 source registry 与 File Session store，先保留同一上下文的 exact-bound 历史，再选择 unstamped Session，仅删除测试 registry。12 个隔离 arm 中，五个 mutation 入口全部错误接受并改变 business files；五个 exact Session 和两个 selection 对照全部正确拒绝、文件 SHA-256 不变。这不是只看错误码、mock admission 或篡改活跃 Goal。
同时跑了 21 个 legacy base/head 对照，覆盖 preview/bind/重复提交、queue、claim/replay、complete/replay、resume、公开读回及无 registry/无 exact history 的正常 legacy 流程。仅归一化随机 ID、时间和临时路径后，完整返回值及持久化字节一致；no-write 断言使用归一化前的文件 SHA-256，保留 ID 关系而不合并不同 Turn。应修 strict 降级，同时保留这些相反方向的 compatibility 控制。
在新 base/head 上完整重跑：head 108 项、base 94 项通过；各有同一个 census 失败，明确不作为该 PR 的阻塞项。失败 detail 是 loopx/contract.py::<module>.check_contract::codec_read:load_registry#1 metadata changed：main #5239 把该调用移到第 1027 行，两侧 manifest 都仍是 1014，且本 PR 未改动这一 row。18 TS 项、control-plane typecheck、changed-file Ruff 与配置中的 19 文件 mypy 通过。current source worktree 的 standard premerge 19 selected + 5 direct 命令全部通过；聚合 gate 为 quality_invalid_receipt，质量回执如实记录上述 P1 及原始 census 基线失败。review 已把 census 标为 pre_existing_unrelated，仅 P1 需要此 PR 修改；不拿普通绿测试覆盖这一缺口，也不因不相关 CI 要求改这个 PR。没有查询/等待 GitHub CI，未安装 candidate、调用 live provider、改变 active Goal 或迁移 store。UI 没有新增 identity 输入/视觉 schema，既有 public payload 隐藏 identity；这不代表 packaged App 或整个 M3 已完成。
我的整体评价
总体边界与 TS owner 选择合理，之前的修复和历史结果保护是实质进展；但 attached owner inventory 现在写 m3_qualified/exact_goal_ref_enforced，不能在这个入口仍 fail-open 时交付。最小修复是统一 source-context 判断：已知 strict 上下文遇到 source unavailable 应拒绝所有 mutation；历史只可提供否决证据，不能猜当前 GoalRef 或自动升级旧 Session。保留纯 legacy/no-exact-history 路径并补五入口 negative + opposite legacy control 后再资格化。
future-facing pass 已通过 shared lifetime/candidate seam 做了一部分；这次应在同一 owning boundary 修掉 selection 与 mutation 的缺失分支分歧，不扩成新的框架或平行 Python state machine。仅在限定 attached-host Chat owner 完整通过后恢复其 qualification；managed provider、first-party host 与整体 activation 的既有 holds 继续独立存在。
English verdict: REQUEST_CHANGES - 61d2b08; missing source registry lets five unstamped-session mutation entrypoints fall back to legacy despite exact-bound context history. Current strict/ABA and 21 legacy parity controls pass, but the reproduced fail-closed gap must be fixed before attached-owner qualification.
| if not registry_path.exists(): | ||
| if exact_session: | ||
| raise FileNotFoundError(registry_path) | ||
| yield session, None, None | ||
| return |
There was a problem hiding this comment.
[P1] Do not downgrade a known strict source context when its registry is unavailable
This branch treats an unstamped selected Session as proof of legacy mode. That does not hold when an exact-bound Session already exists for the same Goal/Agent/channel. After only the disposable source registry is removed, all five real mutation paths (submit, enqueue, resume, claim, complete) accept the unstamped Session and change business files, while five exact-Session controls and both selection controls correctly refuse without writes. Selection already checks exact-bound context history at lines 521–540; mutations bypass that check here and never reach typed admission. Share the source-context availability rule: known strict context must fail closed when source is unavailable; history may supply refusal evidence but must not invent a current GoalRef or automatically stamp legacy records. Preserve genuine legacy/no-exact-history behavior, and cover all five operations with opposite legacy controls before marking the attached owner qualified.
Frame alignment — exact head
|
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
This branch has not been deployed
2 participants
Goal And Delivered Outcome
attached_host_chat_sessionowner for the source-session Goal lifetime model. Attached Chat sessions previously persisted onlygoal_id, so a recreated Goal could reuse or mutate work from the retired lifetime.main; this change is independent of feat(collaboration): bind inbox continuity to GoalRef #5106.Scope And Continuation
attached_host_chat_sessioninventory row. It keeps managed provider startup blocked forsource_session_v1, leaves public broker payloads unchanged, and preserves legacy serialized bytes when the profile is not active.first_party_host_runtimeand the remaining M3 owner rows retain the overall activation hold. No Desktop or UI code changes are included.Validation
f7b48f7e4a9417a41f7f1cae58b4c0c10ea33d00unitpassednpm run test:control-plane: 3154 tests, 3124 passed, 30 environment-gated skips, 0 failures.integrationpassedstaticpassedruff check,npm run typecheck:control-plane,git diff --check origin/main, and the project-registry I/O manifest validator passed.real_entrypointpassedloopx canary premerge --from-git-diff: 19/19 checks passed across control-plane, docs, public-boundary, and Python surfaces; no manual holds.regression_paritypassedSee validation disclosure guidance.
Frontend / Visual Evidence
Type of Change
LoopX Area
Technical Direction
attached_host_chat_sessionqualification ingoal-instance-identity-and-orphan-recovery-v0.Shared-authority RFC fixture impact
source_session_v1.Boundary Checklist
.loopx/,.codex/goals/, and liveACTIVE_GOAL_STATE.md).none.Signed-off-bytrailer (git commit -s).