fix(cli): verify BYO Context readiness before setup - #2207
Conversation
yuezengwu
left a comment
There was a problem hiding this comment.
这项改动的目标是把 BYO Context Tree 的可用性检查前移到 context enable 的本地 setup 边界,避免根 SCOPE.md 缺失或无效时,仍先展示 setup 选择或写入 Plugin、grant、session receipt。
核心改动:
- 抽出统一的 exact-root SCOPE reader,固定读取绑定分支的精确提交,只接受根目录 regular blob,并在解析前验证大小和 UTF-8。
context enable在 live Team activation 后读取 member-safe binding,并在 plan/apply 共用的 plan 构造路径里执行 preflight;typed、sanitized 错误会在 setup mutation 前返回。- 既有 task route 改为复用同一 reader,保持 exact commit、SCOPE schema 和 unavailable-candidate 语义一致;CLI 文档与针对性测试覆盖同步更新。
我检查了 mutation ordering、错误分类/清洗、binding/schema 边界、原路由语义,以及第二个 hardening commit。没有发现 blocker;本 PR 不涉及核心数据结构或数据库变更。按本次 review 要求未额外运行测试或 QA。
baixiaohang
left a comment
There was a problem hiding this comment.
Recommendation: request changes
- Rationale: The readiness gate is placed before the known setup mutations, but its Git transport currently violates the documented root-SCOPE-only data boundary.
Risk level: A
- Path baseline: includes
apps/cli/**-> A - Semantic lift: none; the path baseline already covers the CLI setup and routing boundary.
PR summary
- Author / repo: Gandy2025 / agent-team-foundation/first-tree
- Problem: A member can currently reach BYO Context setup choices—and potentially activate a persistent or session scope—when the selected Team's bound Tree cannot actually route tasks because root
SCOPE.mdis absent or invalid. - Approach: Re-check the live member-safe binding and root SCOPE during plan construction, reuse one exact-root reader for setup and routing, and map failures to stable CLI errors before Plugin/grant/session-receipt mutation.
- Impacted modules: CLI Context enable planning, root-SCOPE routing, focused tests, and Context integration documentation.
Review findings
❌ 1. git fetch --depth=1 still downloads every blob reachable from the branch-tip commit into the temporary repository; calling ls-tree/cat-file only for SCOPE.md afterward does not undo that transfer. Because this PR now invokes the reader from context enable --plan, setup can fetch the full Tree before a scope is chosen, contradicting the new guarantees in both docs that only root SCOPE.md is fetched/read and the rest of the Tree is not materialized. Make the canonical reader enforce an object-level root-SCOPE-only fetch (and fail closed when the remote cannot honor that boundary), so both setup and routing share the promised data minimization. [R5 / apps/cli/src/core/context-integration/exact-root-scope.ts:44]
invalid, but rootScopeFailure discards the specific bounded-size reason and tells users only to fix UTF-8/schema-v1 validity. A well-formed file over 16 KiB already satisfies those stated conditions, and the limit is not documented elsewhere, so the advertised actionable error cannot guide repair. Preserve a sanitized oversized subtype or include the byte limit in the stable invalid guidance. [R4 / apps/cli/src/core/context-integration/context-enable-scope-preflight.ts:107]
✅ 3. The shared reader removes duplicate parsing logic, and the command-level ordering places the readiness check before the currently known Plugin, grant, and session-candidate mutation calls.
Action taken
- Submitted request changes.
yuezengwu
left a comment
There was a problem hiding this comment.
更正我先前的 approve:另一份 review 指出的 transport 问题成立,当前 head 仍有 blocker。
git fetch --depth=1 只限制提交历史,并不会限制 branch-tip commit 可达的 blob;当前 reader 会在执行 ls-tree / cat-file 之前把该提交的整棵 Tree 对象下载进临时仓库。因此 setup plan 新增的 Git preflight 实际会 materialize 根 SCOPE.md 之外的 Tree 内容,与本 PR 文档和现行 BYO 边界所承诺的“只获取根 SCOPE、选择前不 materialize 其余 Tree”直接冲突。canonical reader 需要使用可验证的 object-level filtering,并在 remote 不支持或未遵守该边界时 fail closed。
同时,oversized SCOPE 被归类为 invalid 后,稳定错误只要求用户修复 UTF-8/schema v1;一个合法但超过 16 KiB 的文件已经满足这两个条件,现有指引无法指导修复。请在 sanitized subtype 或稳定错误文案中明确 byte limit。
其余结论不变:preflight 的 command-level ordering 位于已知 Plugin、grant、session-receipt mutation 之前,共用 reader 也统一了 route/setup 的解析逻辑;本 PR 不涉及核心数据结构或数据库变更。按 review 要求未额外运行测试或 QA。
|
Recommendation: request changes Reviewed exact head
Positive checks: the shared reader removes duplicate route parsing; plan/apply ordering is before known setup mutation; auth, binding, missing/nonregular, UTF-8, schema, and fetch failures remain sanitized/fail-closed. Focused Vitest passed 44/44, CLI typecheck passed, and current CI is green. QA status is FAIL for this focused scope because the transport defect is reproducible; case disposition: |
|
Recommendation: request changes Reviewed exact head
Positive checks: the shared reader removes duplicate route parsing; plan/apply ordering is before known setup mutation; auth, binding, missing/nonregular, UTF-8, schema, and fetch failures remain sanitized/fail-closed. Focused Vitest passed 44/44, CLI typecheck passed, and current CI is green. QA status is FAIL for this focused scope because the transport defect is reproducible; case disposition: |
|
Recommendation: request changes Reviewed exact head
Positive checks: the shared reader removes duplicate route parsing; plan/apply ordering is before known setup mutation; auth, binding, missing/nonregular, UTF-8, schema, and fetch failures remain sanitized/fail-closed. Focused Vitest passed 44/44, CLI typecheck passed, and current CI is green. QA status is FAIL for this focused scope because the transport defect is reproducible; case disposition: |
|
Recommendation: request changes Reviewed exact head
Positive checks: the shared reader removes duplicate route parsing; plan/apply ordering is before known setup mutation; auth, binding, missing/nonregular, UTF-8, schema, and fetch failures remain sanitized/fail-closed. Focused Vitest passed 44/44, CLI typecheck passed, and current CI is green. QA status is FAIL for this focused scope because the transport defect is reproducible; case disposition: |
|
Recommendation: request changes Reviewed exact head
Positive checks: the shared reader removes duplicate route parsing; plan/apply ordering is before known setup mutation; auth, binding, missing/nonregular, UTF-8, schema, and fetch failures remain sanitized/fail-closed. Focused Vitest passed 44/44, CLI typecheck passed, and current CI is green. QA status is FAIL for this focused scope because the transport defect is reproducible; case disposition: |
Summary
first-tree context enable --planon the selected Team's live member-safe Context Tree bindingSCOPE.mdreader for setup and task routingSCOPE.md, not the full TreeWhy
The Web setup flow intentionally uses coarse binding eligibility. A Team with a bound Tree but a missing or invalid root
SCOPE.mdcould therefore reach CLI setup choices and mutate local activation state before task routing discovered that the Tree was unusable.The CLI plan is the correct local preflight seam. Apply already rebuilds that plan, so the same check also protects the mutation path without adding Server state, endpoints, or Web polling.
Design
exact-root-scope.tsowns the Git fetch, exact commit/root blob lookup, bounded size check, UTF-8 decode, schema parse, cleanup, and typedmissing | invalid | fetchfailures.context-enable-scope-preflight.tsowns the member binding read and maps binding/root failures to the stableCONTEXT_ENABLE_*CLI contract.enable.tsremains thin: it invokes the preflight after live Team activation and renders typed errors.Impact
BYO Context setup now fails early with actionable errors until the selected Team's exact root
SCOPE.mdis readable and valid. Oversized SCOPE content is reported as invalid rather than as a repository/network failure. Web eligibility behavior, provider selection, onboarding completion, write confirmation, and the wider architecture are unchanged.Validation
pnpm --filter ./apps/cli exec vitest run src/__tests__/exact-root-scope.test.ts src/__tests__/context-enable-scope-preflight.test.ts src/__tests__/context-enable-command.test.ts src/__tests__/context-route.test.ts(44 tests passed)pnpm checkpnpm typecheckpnpm testwas attempted; the parallel monorepo run hit the pre-existing 10-second build-hook timeout intests/context-policy-runtime-layout.test.ts. The same suite passed standalone (5/5).