Skip to content

fix(hooks): run every handler in the scope the dispatcher resolved (#752) - #769

Merged
jeff-r2026 merged 2 commits into
Tencent:mainfrom
SaulMoro:fix/752-handlers-dispatcher-config
Sep 24, 2026
Merged

jeff-r2026 merged 2 commits into
Tencent:mainfrom
SaulMoro:fix/752-handlers-dispatcher-config

Conversation

@SaulMoro

@SaulMoro SaulMoro commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

Part of #752 (item 5).

Summary

 hookDispatchCli(event, stdin)
   chdir(payload cwd)                      ← can fail: deleted worktree
   localConfig = resolveConfigForDir(payload cwd)
   handlers = filterHandlersForConfig(registry, localConfig)
-  createDispatcher({ handlers })
+  createDispatcher({ handlers, localConfig })
-    handler.execute(stdin, tool)
+    handler.execute(stdin, tool, localConfig)

 dashboard-report  teamCorrectionKeywords
-  autoDetectInit()          ← process cwd
+  loadTeamConfig(localConfig.repo.localPath)
 votes-sync
-  autoDetectInit()          ← process cwd
+  localConfig
 webhook-dispatch
-  loadWebhookConfig()       ← autoDetectInit(), process cwd
+  loadWebhookConfig(localConfig)

With a payload cwd that no longer exists, the dispatcher resolves the user scope. The three handlers above then read the config of whichever directory the host started the hook in:

payload cwd: deleted worktree   → dispatcher: user scope
process cwd: project A          → keywords, votes, webhook: project A

The contribute hint is not affected: it already resolves from the payload cwd (#747). loadWebhookConfig() with no argument keeps its CLI behaviour (teamai webhook list / test).

Second commit: the background pass starts again when the payload cwd is gone. Found while verifying the first. The detached child that runs the background handlers was spawned in the payload cwd. On macOS and Linux a directory that no longer exists fails the spawn, the error is swallowed (child.on('error', () => {})), and no background handler runs: no session-start pull, no webhook, no update check.

 spawnPlainDetached(command, args, stdin, cwd)
-  spawn(…, { cwd })                              ← ENOENT, swallowed
+  spawn(…, { cwd: exists(cwd) ? cwd : tmpdir })  ← as the WMI launch on Windows

It needs the first commit. The child resolves its scope from the payload, not from the directory it starts in, but before this PR its handlers read the process cwd. Started from the host's directory, they would have landed in project A. The temp dir is also safer than inheriting the host's directory for the session-start pull, which still reads the process cwd (item 2 of #752).

Evidence

Red → green, hook-dispatch-scope.test.ts (real hookDispatchCli, sandbox HOME, both passes):

handlers follow the scope the dispatcher resolved, not the directory the hook process runs in (#752)
  user scope (member tester) · project A (member alice, keyword "rehazlo", webhook)
  payload cwd deleted, process.chdir(project A)
  prompt "rehazlo", then Stop with a transcript that declares a recalled doc
                                  before          after
  prompt_submit.correction        true            false
  ~/.teamai/votes/                [alice.yaml]    [tester.yaml]
  webhook requests                1               0

Real CLI (dist/index.js hook-dispatch), same setup, one sandbox per agent. Each scope's team has its own webhook (/a, /user) on a local receiver. Builds: main (dad371c), the first commit (cb1d0ff), and this PR:

build agent keywords votes webhooks received
main claude / codex / codebuddy / opencode correction: true (A's keyword) alice.yaml none
cb1d0ff claude / codex / codebuddy / opencode correction: false tester.yaml none
this PR claude / codex / codebuddy / opencode correction: false tester.yaml 1 × /user

"none" in the first two rows is the background pass never starting. A control run with the payload cwd in project A fires A's webhook once (checked on both commits of this PR).

Session start with a deleted cwd, hook process in project A: with this PR the background pull runs (it installs ~/.teamai/bin/teamai, which cb1d0ff never gets to). Project A's data directory holds the same files before and after, on both builds.

Red → green, hook-dispatch-cli.test.ts:

starts the background pass from the temp dir when the payload cwd no longer exists
  spawn cwd   before: <payload cwd, deleted>   after: os.tmpdir()

Also: hook-dispatch.test.ts checks that the dispatcher hands every handler, foreground and background, the scope it was created with. The keyword tests in hook-handlers.test.ts now read a real teamai.yaml from the passed scope instead of mocking autoDetectInit.

Test plan

  • npx tsc --noEmit, npx vitest run (4341 passed, 1 skipped), npm run test:e2e (228 passed, 26 skipped), npm run build
  • Real CLI, git provider, Claude, Codex, CodeBuddy, OpenCode: the table above. Only the agent axis is covered: these handlers do not depend on the provider, and nothing here pushes (votes go through updateReports, which fails offline in the sandbox either way).

Merge Danger

Door: two-way. No data format or file layout changes.

Blast Radius: hooks

HookHandler.execute takes a third parameter. Every handler in the registry is updated. Handlers other than these three ignore it.

Sessions whose cwd was deleted now run their background handlers on macOS and Linux, as they already did on Windows. That includes a session-start pull of the scope the dispatcher resolved (the user scope).

…encent#752)

hook-dispatch resolves the scope from the payload cwd, then chdirs there.
The correction keywords, votes-sync and webhook-dispatch read their config
again through autoDetectInit() on the process cwd, so when that chdir failed
(a deleted worktree) and the host started the hook inside another project,
they used that project: its keywords, its member for the session's votes,
and its webhooks.

HookHandler.execute now receives the config the dispatcher resolved, and the
three handlers read their scope from it.
…ncent#752)

The detached child was spawned in the payload cwd. On macOS and Linux a cwd
that no longer exists (a deleted worktree) fails the spawn, the error is
swallowed, and no background handler runs: no session-start pull, webhook or
update check. Start it in the temp dir instead, as the Windows WMI launch
already does. The child resolves its scope from the payload, and its handlers
now use that config, so the directory it starts in names no project.
@jeff-r2026 jeff-r2026 self-assigned this Sep 23, 2026
@github-actions

Copy link
Copy Markdown

Findings

  • [P1 blocking] src/hook-dispatch-cli.ts:110 assumes os.tmpdir() is an existing, scope-neutral directory. It honors TMPDIR/TMP/TEMP, which may be missing or located inside another TeamAI project. Because the session-start pull still detects scope from the child’s process cwd, a deleted-worktree hook can still fail to spawn or pull/deploy the unrelated project instead of the dispatcher-resolved user scope.
  • [P1 blocking] The PR description’s Test Plan covers real-CLI verification only with the git provider and explicitly omits gitlab and github. The trusted AGENTS.md requires real-CLI verification across git, gitlab, and github, so the required end-to-end record is incomplete.

@jeff-r2026
jeff-r2026 merged commit cc27721 into Tencent:main Sep 24, 2026
13 checks passed
@SaulMoro
SaulMoro deleted the fix/752-handlers-dispatcher-config branch September 24, 2026 06:05
SaulMoro added a commit to SaulMoro/teamai-cli that referenced this pull request Sep 24, 2026
- The session-start handler returns when the dispatcher resolved no
  config for the hook's cwd, which is what an unreadable project config
  resolves to since Tencent#748. That one guard replaces the unreadable-config
  sinks added to seedProjectAgentRoot and the package-hint context, and
  follows the Tencent#769 contract that handlers read their scope from the
  dispatcher. The handler tests that exercise cwd routing now pass a
  resolved scope; a new one pins that nothing runs without one.
- CHANGELOG and usage guide (en, zh-CN): a session start there runs no
  pull; only `teamai pull --silent` from a pre-dispatch hook writes the
  reason to debug.log.
- skill-data troubleshooting: what `Nothing was synced` means, and that
  moving the config aside and re-running init needs the user's consent.
SaulMoro added a commit to SaulMoro/teamai-cli that referenced this pull request Sep 24, 2026
- The session-start handler returns when the dispatcher resolved no
  config for the hook's cwd, which is what an unreadable project config
  resolves to since Tencent#748. That one guard replaces the unreadable-config
  sinks added to seedProjectAgentRoot and the package-hint context, and
  follows the Tencent#769 contract that handlers read their scope from the
  dispatcher. The handler tests that exercise cwd routing now pass a
  resolved scope; a new one pins that nothing runs without one.
- CHANGELOG and usage guide (en, zh-CN): a session start there runs no
  pull; only `teamai pull --silent` from a pre-dispatch hook writes the
  reason to debug.log.
- skill-data troubleshooting: what `Nothing was synced` means, and that
  moving the config aside and re-running init needs the user's consent.
jeff-r2026 pushed a commit that referenced this pull request Sep 24, 2026
… (#792)

* fix(pull): sync nothing in a project whose config cannot be read (#784)

Detection skips a project config it cannot read and returns what loads
next: a legacy .teamai/ behind a broken partition, which may name another
team, or the user scope. pull() deployed and reported for that team, and
the session-start hook did so on every session (reports-wt/ and
learnings-wt/ appeared in the legacy .teamai/).

pull() now listens for the unreadable config, syncs no scope, prints the
problem with BROKEN_CONFIG_ADVICE and exits 1. A silent pull (the
session-start hook, or a pre-dispatch hook running `teamai pull --silent`)
records it in debug.log only. Agent-root seeding and the package hint
refuse the same way, so a session start there does nothing. Hooks and
usage already follow this rule since #748. The message trimming detectTeam
used moves to config.ts as describeUnreadableConfig so both share it.

* fix(pull): address pre-review findings (#784)

- The session-start handler returns when the dispatcher resolved no
  config for the hook's cwd, which is what an unreadable project config
  resolves to since #748. That one guard replaces the unreadable-config
  sinks added to seedProjectAgentRoot and the package-hint context, and
  follows the #769 contract that handlers read their scope from the
  dispatcher. The handler tests that exercise cwd routing now pass a
  resolved scope; a new one pins that nothing runs without one.
- CHANGELOG and usage guide (en, zh-CN): a session start there runs no
  pull; only `teamai pull --silent` from a pre-dispatch hook writes the
  reason to debug.log.
- skill-data troubleshooting: what `Nothing was synced` means, and that
  moving the config aside and re-running init needs the user's consent.

* fix(pull): address CI review findings (#784)

- The session-start pull is registered with `requiresConfig` instead of
  returning early inside the handler: the dispatcher drops it wherever no
  config resolves, which covers an unreadable project config (#748), and
  spawns no detached pass for it. Where no teamai config exists at all
  it did nothing on main either (no scope to pull, no project root to
  seed, no config for a package hint). The #748 registry test and the
  docs no longer list it as machine-level work.
- `teamai pull --silent` exits 1 on the refusal too. Pre-dispatch hooks
  run it as `… 2>/dev/null || true` (`; exit 0` on Windows), so hosts
  still see success.
- The dispatch-scope test asserts the pull is skipped and resets the
  pull mock it queues.
- skill-serving design doc: `teamai pull` now reports an unreadable
  project config too. skill-data troubleshooting: `teamai doctor` can
  pass there.

* fix(hooks): still pull at session start where teamai is not set up (#784)

A null config from the dispatcher means either "no teamai here" or "the
project config cannot be read". Gating the session-start pull on
`requiresConfig` stopped it in both; only the second must stop it. The
handler now asks `findUnreadableProjectConfig` for the hook's cwd when
no config resolved (a cwd that no longer exists holds none) and runs
nothing when it reports a file. Everywhere else it runs as on main, so
the docs list it as machine-level work again.
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.

2 participants