Skip to content

fix(cli): verify BYO Context readiness before setup - #2207

Open
Gandy2025 wants to merge 2 commits into
mainfrom
feat/byo-context-scope-readiness
Open

fix(cli): verify BYO Context readiness before setup#2207
Gandy2025 wants to merge 2 commits into
mainfrom
feat/byo-context-scope-readiness

Conversation

@Gandy2025

@Gandy2025 Gandy2025 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • gate first-tree context enable --plan on the selected Team's live member-safe Context Tree binding
  • reuse one canonical exact root SCOPE.md reader for setup and task routing
  • validate regular-file mode, bounded blob size, UTF-8, and the current SCOPE schema before setup choices
  • return stable, sanitized CLI errors for authentication, binding, missing/invalid SCOPE, and fetch failures
  • prove plan and apply failures occur before Plugin/grant mutation or session candidate receipt issuance
  • document that the local preflight reads only root SCOPE.md, not the full Tree

Why

The Web setup flow intentionally uses coarse binding eligibility. A Team with a bound Tree but a missing or invalid root SCOPE.md could 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.ts owns the Git fetch, exact commit/root blob lookup, bounded size check, UTF-8 decode, schema parse, cleanup, and typed missing | invalid | fetch failures.
  • context-enable-scope-preflight.ts owns the member binding read and maps binding/root failures to the stable CONTEXT_ENABLE_* CLI contract.
  • enable.ts remains 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.md is 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)
  • packaged-CLI focused-local E2E: valid, missing, nonregular, invalid, non-UTF-8, oversized, binding-invalid/unreadable, auth, fetch, and plan/apply recheck paths passed; failure paths produced no Plugin, grant, BYO repo, or session receipt mutation
  • pnpm check
  • pnpm typecheck
  • pnpm test was attempted; the parallel monorepo run hit the pre-existing 10-second build-hook timeout in tests/context-policy-runtime-layout.test.ts. The same suite passed standalone (5/5).

@Gandy2025
Gandy2025 marked this pull request as ready for review August 6, 2026 07:24

@yuezengwu yuezengwu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这项改动的目标是把 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 baixiaohang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.md is 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]

⚠️ 2. Oversized SCOPE content is classified as 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 yuezengwu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

更正我先前的 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。

@first-tree-hub-staging

Copy link
Copy Markdown

Recommendation: request changes

Reviewed exact head 32e5d2d4decad32273612f7ddeeba0b16f31b618. The readiness check is ordered before the known Plugin, grant, and session-receipt mutations, but two current-head defects remain.

  1. Blocker — the Git transport violates the root-SCOPE-only boundary. exact-root-scope.ts:44 uses git fetch --depth=1 without an object filter. I invoked the actual helper with Git Trace2 and independently replayed that exact fetch against the current Context Tree: the pack contained 324 objects, including 257 blobs, and a non-SCOPE.md root blob was present in the temporary object database. Depth limits history, not branch-tip content. This contradicts the documented guarantee that setup/routing fetch only exact root SCOPE.md and do not materialize the rest of the Tree before selection (CLI reference, integration guide). The shared reader must enforce a verifiable object-level boundary and fail closed when a remote cannot honor it.

  2. The oversized-SCOPE error is not actionable. exact-root-scope.ts:56 correctly detects the 16,384-byte limit, but context-enable-scope-preflight.ts:106 collapses every invalid subtype to UTF-8/schema-v1 guidance. A schema-valid oversized file already satisfies that guidance, and the CLI docs do not state the limit. Preserve a sanitized oversized subtype or name the byte limit in the stable error.

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: move-to-product-test for deterministic assertions covering fetched-object isolation and oversized guidance.

@first-tree-hub-staging

Copy link
Copy Markdown

Recommendation: request changes

Reviewed exact head 32e5d2d4decad32273612f7ddeeba0b16f31b618. The readiness check is ordered before the known Plugin, grant, and session-receipt mutations, but two current-head defects remain.

  1. Blocker — the Git transport violates the root-SCOPE-only boundary. exact-root-scope.ts:44 uses git fetch --depth=1 without an object filter. I invoked the actual helper with Git Trace2 and independently replayed that exact fetch against the current Context Tree: the pack contained 324 objects, including 257 blobs, and a non-SCOPE.md root blob was present in the temporary object database. Depth limits history, not branch-tip content. This contradicts the documented guarantee that setup/routing fetch only exact root SCOPE.md and do not materialize the rest of the Tree before selection (CLI reference, integration guide). The shared reader must enforce a verifiable object-level boundary and fail closed when a remote cannot honor it.

  2. The oversized-SCOPE error is not actionable. exact-root-scope.ts:56 correctly detects the 16,384-byte limit, but context-enable-scope-preflight.ts:106 collapses every invalid subtype to UTF-8/schema-v1 guidance. A schema-valid oversized file already satisfies that guidance, and the CLI docs do not state the limit. Preserve a sanitized oversized subtype or name the byte limit in the stable error.

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: move-to-product-test for deterministic assertions covering fetched-object isolation and oversized guidance.

@first-tree-hub-staging

Copy link
Copy Markdown

Recommendation: request changes

Reviewed exact head 32e5d2d4decad32273612f7ddeeba0b16f31b618. The readiness check is ordered before the known Plugin, grant, and session-receipt mutations, but two current-head defects remain.

  1. Blocker — the Git transport violates the root-SCOPE-only boundary. exact-root-scope.ts:44 uses git fetch --depth=1 without an object filter. I invoked the actual helper with Git Trace2 and independently replayed that exact fetch against the current Context Tree: the pack contained 324 objects, including 257 blobs, and a non-SCOPE.md root blob was present in the temporary object database. Depth limits history, not branch-tip content. This contradicts the documented guarantee that setup/routing fetch only exact root SCOPE.md and do not materialize the rest of the Tree before selection (CLI reference, integration guide). The shared reader must enforce a verifiable object-level boundary and fail closed when a remote cannot honor it.

  2. The oversized-SCOPE error is not actionable. exact-root-scope.ts:56 correctly detects the 16,384-byte limit, but context-enable-scope-preflight.ts:106 collapses every invalid subtype to UTF-8/schema-v1 guidance. A schema-valid oversized file already satisfies that guidance, and the CLI docs do not state the limit. Preserve a sanitized oversized subtype or name the byte limit in the stable error.

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: move-to-product-test for deterministic assertions covering fetched-object isolation and oversized guidance.

@first-tree-hub-staging

Copy link
Copy Markdown

Recommendation: request changes

Reviewed exact head 32e5d2d4decad32273612f7ddeeba0b16f31b618. The readiness check is ordered before the known Plugin, grant, and session-receipt mutations, but two current-head defects remain.

  1. Blocker — the Git transport violates the root-SCOPE-only boundary. exact-root-scope.ts:44 uses git fetch --depth=1 without an object filter. I invoked the actual helper with Git Trace2 and independently replayed that exact fetch against the current Context Tree: the pack contained 324 objects, including 257 blobs, and a non-SCOPE.md root blob was present in the temporary object database. Depth limits history, not branch-tip content. This contradicts the documented guarantee that setup/routing fetch only exact root SCOPE.md and do not materialize the rest of the Tree before selection (CLI reference, integration guide). The shared reader must enforce a verifiable object-level boundary and fail closed when a remote cannot honor it.

  2. The oversized-SCOPE error is not actionable. exact-root-scope.ts:56 correctly detects the 16,384-byte limit, but context-enable-scope-preflight.ts:106 collapses every invalid subtype to UTF-8/schema-v1 guidance. A schema-valid oversized file already satisfies that guidance, and the CLI docs do not state the limit. Preserve a sanitized oversized subtype or name the byte limit in the stable error.

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: move-to-product-test for deterministic assertions covering fetched-object isolation and oversized guidance.

@first-tree-hub-staging

Copy link
Copy Markdown

Recommendation: request changes

Reviewed exact head 32e5d2d4decad32273612f7ddeeba0b16f31b618. The readiness check is ordered before the known Plugin, grant, and session-receipt mutations, but two current-head defects remain.

  1. Blocker — the Git transport violates the root-SCOPE-only boundary. exact-root-scope.ts:44 uses git fetch --depth=1 without an object filter. I invoked the actual helper with Git Trace2 and independently replayed that exact fetch against the current Context Tree: the pack contained 324 objects, including 257 blobs, and a non-SCOPE.md root blob was present in the temporary object database. Depth limits history, not branch-tip content. This contradicts the documented guarantee that setup/routing fetch only exact root SCOPE.md and do not materialize the rest of the Tree before selection (CLI reference, integration guide). The shared reader must enforce a verifiable object-level boundary and fail closed when a remote cannot honor it.

  2. The oversized-SCOPE error is not actionable. exact-root-scope.ts:56 correctly detects the 16,384-byte limit, but context-enable-scope-preflight.ts:106 collapses every invalid subtype to UTF-8/schema-v1 guidance. A schema-valid oversized file already satisfies that guidance, and the CLI docs do not state the limit. Preserve a sanitized oversized subtype or name the byte limit in the stable error.

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: move-to-product-test for deterministic assertions covering fetched-object isolation and oversized guidance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants