Skip to content

fix: unify SCM audience and echo routing - #2124

Merged
baixiaohang merged 1 commit into
mainfrom
fix/shared-scm-audience-actor-echo
Aug 3, 2026
Merged

fix: unify SCM audience and echo routing#2124
baixiaohang merged 1 commit into
mainfrom
fix/shared-scm-audience-actor-echo

Conversation

@baixiaohang

Copy link
Copy Markdown
Collaborator

Summary

  • centralize SCM audience composition around exact (humanAgentId, wakeAgentId) personnel authority while preserving distinct existing and legacy routes
  • carry discriminated target-chat intent through GitHub persistence so strict personnel lines cannot fall back to another delegate's human-scoped mapping
  • keep actor-echo cards in chat while suppressing only matching-line wakes, and move GitHub App Task data behind an opaque provider-task context
  • update GitHub/GitLab parity coverage and the reusable cross-surface QA case

Validation

  • targeted SCM, provider-adapter parity, target-chat, echo, and App Task suites (including an 8-file / 210-test pass and final adapter-specific reruns)
  • pnpm exec vitest run --maxWorkers=2 --reporter=dot in packages/server (257 files, 2923 tests)
  • pnpm typecheck (11/11 tasks)
  • pnpm check (passes with 16 existing warnings and 1 info)
  • git diff --check

Formal cross-surface QA is warranted for production webhook delivery, but was not started as part of this implementation task.

Scope

  • no generic message.ts behavior changes
  • no schema or migration changes
  • no lifecycle, delivery-claim, cleanup, or orphan-chat reliability changes
  • no merge requested; Context Tree reconciliation is required before merge because the GitHub routing node is decision-locked

@baixiaohang
baixiaohang requested a review from yuezengwu as a code owner July 31, 2026 10:52

@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.

Implementation goal and core changes: this PR centralizes SCM audience composition around the exact (humanAgentId, wakeAgentId) pair, preserves distinct existing/legacy routes, carries strict personnel target-chat intent through GitHub persistence, keeps provider-task capability data discriminated, and changes actor echo handling so cards remain in chat while only the matching line's wake is suppressed. The core implementation is coherent, and I did not find a code-level defect in the reviewed diff.

Blocking governance issue: the current decision-locked Context Tree contract has not been reconciled with these behavior changes. system/cloud/automation/github-webhook-routing.md still says actor-owned entries are pruned before delivery and a self-only chat receives no card (S7/S11), while this PR deliberately writes a silent card. Its S2 target wording also still describes human-scoped existing-line reuse, while this PR moves authority to the exact human/wake pair and creates a strict current-delegate line when only a same-human/different-wake carrier line exists. decisionLocksCode: true means the existing Tree contract wins until the owners approve a replacement.

Please land the companion Context Tree reconciliation (and ensure both the GitHub-specific and provider-neutral SCM nodes state the same settled contract), then re-request review. There are no schema or migration changes, but the core in-memory routing data structures and delivery semantics changed, so this needs explicit human-owner inspection before merge.

I did not run tests per review scope. The current Test Server CI failure is a Docker pull failure, not a product-test failure.

@baixiaohang

Copy link
Copy Markdown
Collaborator Author

The governance blocker from the previous review is now resolved:

  • The companion Context Tree reconciliation, agent-team-foundation/first-tree-context#872, has merged as 7c56b43d1769782289b55b7a3806e6fc01a148ed.
  • The provider-neutral SCM node and the decision-locked GitHub routing node now both define exact (humanAgentId, wakeAgentId) personnel authority, strict target-chat intent, and card routing separated from wake eligibility.
  • The source head remains f86954ba8f45d1d234a6fd1a7c6afb511cc9dcfa; no code changes were needed after reconciliation.

The failed Test Server job never reached checkout or product tests: GitHub Actions timed out three times while pulling postgres:17 from Docker Hub. I am rerunning the failed jobs. Re-requesting review against the reconciled contract.

@baixiaohang
baixiaohang requested a review from yuezengwu August 3, 2026 01:50

@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.

Re-reviewed the unchanged source head f86954ba8f45d1d234a6fd1a7c6afb511cc9dcfa against the reconciled Context Tree contract.

Implementation goal and core changes: centralize GitHub/GitLab audience composition around exact (humanAgentId, wakeAgentId) authority; preserve distinct existing and legacy routes; carry strict personnel target-chat intent through provider persistence; separate card routing from actor-echo wake eligibility; and keep GitHub provider-task capability data discriminated and opaque to shared SCM.

The previous governance blocker is resolved. Context Tree PR #872 is merged (PR head 7c56b43d1769782289b55b7a3806e6fc01a148ed, merge commit 07a99e83321877b08fdaf1ff58657045a557c689), and the current provider-neutral SCM, GitHub, and GitLab nodes now match the implementation. No code changes occurred since the prior diff review, which found no code-level defect. No schema or migration changes are present.

Approved. I did not rerun tests and did not wait for the infrastructure-failed Server job rerun, per review scope.

@baixiaohang
baixiaohang merged commit 11a2901 into main Aug 3, 2026
28 of 30 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 3, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants