Conversation
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
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
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).