test(client): guard retired hook steering drift#1571
Conversation
baixiaohang
left a comment
There was a problem hiding this comment.
Review by baixiaohang-code-reviewer - PR #1571
Recommendation: request changes
- Rationale: The source-side guard is reasonable, but it now codifies a durable steering-channel decision that still conflicts with the current Context Tree architecture nodes and has no paired tree PR linked.
Risk level: A
- Path baseline: includes
packages/client/**. - Semantic lift: none beyond the A baseline; the test is about generated briefing / shipped skill steering, but the path already puts it at A.
PR 摘要
- Author / repo: yuezengwu / agent-team-foundation/first-tree
- Problem: Agents could drift back toward retired hook-based steering instructions even after the supported behavior moved to provider-neutral briefing/runtime-contract surfaces.
- Approach: Add a deterministic client drift guard over the generated briefing plus shipped First Tree skill payloads, and add a manual eval checklist case for assigned PR-delivery authority.
- Impacted modules:
packages/client/src/__tests__,packages/skill-evals/docs.
Review findings
❌ 1. Please open/link the paired Context Tree PR for the hook-steering decision before this lands. The new guard explicitly treats .claude/settings.json, .codex/hooks.json, SessionStart, Stop-hook markers, etc. as retired agent hook steering (packages/client/src/__tests__/retired-tree-commands-drift.test.ts:175 and :289), but agent-team-foundation/first-tree-context@main still records agent runtime hooks as an active steering channel in system/cloud/agents/agent-behavior-steering.md and system/architecture.md. Since generated briefing/tree-read rules make the Context Tree the durable source for future agents, merging the source-only guard leaves future reviewers with contradictory sources of truth. A draft tree PR updating those nodes to the current provider-neutral steering model, cross-linked from this PR, is enough; I do not see a needed source-code change here.
Action taken
- Submitted request changes.
- I did not run checks or tests; reviewed the PR diff, current source evidence, GitHub comments, and relevant Context Tree nodes.
Summary
.claude/settings.json,.codex/hooks.json,SessionStart, Stop hook markers, etc.).Context
This follows the P1 brief/base-prompt gap audit. Current
origin/mainalready contains the accepted.mdContext Tree write carve-out and the PR-task authority runtime contract. The remaining historical Stop-hook item is no longer an active code path after the retired tree-command surface was removed, so this PR locks that state with deterministic drift coverage instead of reviving provider-specific hooks.Verification
pnpm install --frozen-lockfilepnpm exec biome check packages/client/src/__tests__/retired-tree-commands-drift.test.ts packages/skill-evals/docs/agent-briefing-manual-eval.mdpnpm --filter @first-tree/client exec vitest run src/__tests__/retired-tree-commands-drift.test.ts src/__tests__/chat-context.test.ts src/__tests__/agent-briefing.test.ts --maxWorkers=2 --minWorkers=1pnpm --filter @first-tree/skill-evals eval:floorpnpm --filter @first-tree/skill-evals eval:select -- --changed-file packages/client/src/__tests__/retired-tree-commands-drift.test.ts --changed-file packages/skill-evals/docs/agent-briefing-manual-eval.mdgit diff --check