fix(codex): keep member hook groups at their keys and trust with the newest build (#1244, #1250) by SaulMoro · Pull Request #1263 · Tencent/teamai-cli

SaulMoro · 2026-10-21T09:54:16Z

Why

Codex keys a hook's approval by its group position (<file>:<event>:<group>:<handler>) and hashes handlers with additionalContextLimit differently across versions (0.142.5 ignores the field, 0.163.0 includes it). Rebuilding a Codex hooks.json moved foreign hook groups off their trusted keys, and trusting with a stale codex on PATH overwrote the desktop app's trust every session.

What changed

Route every Codex hook writer through editCodexHookFile in src/hooks.ts so teamai updates its own groups in place, appends new ones, and carries over existing approvals via keepCodexHookTrust in src/codex-trust.ts when a removal shifts a foreign group.
Resolve the newest Codex binary across PATH and the desktop app bundles (src/codex-desktop.ts) in src/codex-trust.ts for trust writes, doctor, and the stat-only trust cache fingerprint.
Compare listed hook hashes across installed versions and report Codex builds disagree: once per pull and in doctor when an older build hashes a teamai hook differently.
Keep the Codex team-hook dispatcher command on bare bash on Windows too, like the Codex built-ins, with the Windows form in commandWindows. Since main runs codex-team-hooks.test.ts on the Windows runner, the dispatcher rendered a Git Bash path there and failed it. The Windows test list now accepts a file that pins command beside commandWindows.

Scope

Covers all Codex hooks.json writers (reconcileHooks, removeHooks, reconcileCodexDispatchers, applyAgentHook, removeAgentHook, uninstall) and Codex build resolution on macOS and Windows. Leaves additionalContextLimit: 0 unchanged.

Tradeoffs

When an older build fails to list hooks, trustCodexForScope retries the comparison on the next pull and then caches if the same build fails again, avoiding repeated app-server spawns on every pull when a binary predates hooks/list.

Blast Radius

Touches Codex hook file reconciliation and codex app-server trust calls. Non-Codex tools and configurations with codexTrustEnabled: false do not spawn Codex.

Verification

On 8586c041 (current main merged in): npx tsc --noEmit, npm run lint, and npx vitest run (492 test files, 12,201 tests passed, 21 skipped). Earlier, on 14821532: npm run build and the real-CLI runs below.
The Windows dispatcher change is not run locally (macOS); the Windows CI job runs codex-team-hooks.test.ts and asserts both forms.
Real-CLI reproduction for [bug] Codex hook reconciliation reorders unchanged third-party hooks and invalidates their trust #1250 (inject, pull, remove with a trusted member group ahead of and between teamai groups): all 3 cases exit 0 with the member group's trusted status preserved.
Real-CLI e2e for Codex hook trust: two installed versions compute different hashes for additionalContextLimit, so the review prompt returns every session #1244 (codex-cli 0.142.5 on PATH + 0.163.0 desktop bundle): 0.163.0 lists SessionStart as trusted, and pull and doctor surface the Codex builds disagree: diagnostic.

Fixes #1244
Fixes #1250

Every teamai writer of a Codex hooks file must leave a member's group at its key unless a teamai removal shifts it, and then carry its approval. A partial edit keeps the approvals in events it does not touch, a group that lands on a key without carrying an approval does not show one, and a caller that states no trust policy starts no Codex. Fourteen cases fail on main. The two trust-off guards and the internal-variant guard pass there because main never starts Codex from a hooks write, and the agent hook removal in a relocated Codex home passes because main already asked that file's home.

Codex keys an approval by group position. teamai now rewrites its own groups in their slots and appends new ones, so a member group moves only when a teamai group before it is removed. Every such move goes through editCodexHookFile, which carries the approval to the new key and clears the stale approvals left behind. Each production caller states the active config's Codex trust policy, and an agent hook's write asks the Codex home its file sits in; a caller that states none writes no trust and starts no Codex.

) PATH codex-cli 0.142.5 plus a desktop 0.163.0 must trust and diagnose with the newest, report a skew only when the builds hash a teamai hook differently, warn on every pull while it lasts without starting Codex, compare again on the next pull after an older build failed to list, start no older build when there is nothing to compare, and invalidate the trust cache when a desktop build appears or changes. The fake Codex answers --version, can hash like 0.142.5, can fail a method in one install only, and records which install answered.

Every app-server call, doctor's read, and the trust-cache fingerprint now resolve one build from codex on PATH and the desktop app's bundled ones (src/codex-desktop.ts), picking the newest by --version. While that build answers, each build of another version lists the same hooks; when one hashes a teamai hook differently, pull and doctor name both builds, and a pull warns once. Nothing else starts when the newest build lists none of teamai's hooks. The skew is stored with the trust cache, so every pull warns while it lasts. A pass in which another build could not list is cached only when the same builds failed in the pass before, so the next pull compares once more. The fingerprint stays stat-only, so a cached pull runs no binary.

github-actions · 2026-10-21T09:54:49Z

Code Erosion Report

Informational — never blocks the merge. scb-check==0.2.0 SlopCodeBench metrics; method & caveats in docs/dev/ci-code-erosion.md.

Metric	Value	Human band	Agent band	Reading
Verbosity*	0.060	0.15	0.33	human
Erosion	0.713	0.31	0.68	agent
Cognitive erosion	0.879	–	–	–

Scanned 402 files / 92012 SLOC · high-CC functions 722/7877

* On TypeScript, verbosity is partial and the bands are Python-calibrated.

Rule hits (TS verbosity layer)

Standalone ast-grep, separate from the number above.

Rule	Hits
`unnecessary-else-after-return`	58
`empty-catch-block`	5

63 distinct lines flagged across src/.

github-actions · 2026-10-21T10:03:47Z

No actionable findings in the specified PR diff.

The PR description documents sufficient testing: build, type check, lint, unit tests, and representative real-CLI verification for both fixes.
The earlier code-erosion report is informational, not an actionable finding to re-raise or mark resolved.
Review was read-only; I did not run, build, or install anything from the PR.

SaulMoro · 2026-10-21T10:33:08Z

@natinli

…1244-1250 # Conflicts: # src/init.ts

main now runs codex-team-hooks.test.ts on the Windows runner, and that file pins the POSIX dispatcher command. The team-hook dispatcher still put a Git Bash path into command on Windows, while the Codex built-ins keep bare bash on every platform and carry the Windows form in commandWindows. The dispatcher now does the same, so a committed hooks.json does not change on a Windows pull, and the Windows test list accepts a file that pins command beside commandWindows.

github-actions · 2026-10-22T06:56:17Z

No actionable findings in the specified PR diff.

The PR description documents sufficient testing, including build and representative real-CLI verification for both fixes. The older verification commit is a note, not blocking.
Windows-specific verification is explicitly deferred to CI; this does not block merge.
The earlier pass raised no actionable findings to resolve or re-raise.
Review was read-only and limited to the specified diff. I did not run, build, or install anything from the PR.

# Conflicts: # src/types.ts

github-actions · 2026-10-23T09:42:23Z

No actionable findings in the specified PR diff.

The PR description documents sufficient testing, including a build and representative real-CLI verification for both fixes. The older verification commit is a note, not blocking.
Windows-specific verification is explicitly deferred to CI; this does not block merge.
Earlier passes raised no actionable findings to resolve or re-raise.
Review was read-only and limited to the specified diff. I did not run, build, or install anything from the PR.

# Conflicts: # src/hooks.ts # src/pull.ts

github-actions · 2026-10-23T14:32:05Z

No actionable findings in the specified PR diff.

The PR description documents sufficient testing, including a build and representative real-CLI verification for both fixes. The older verification commit is a note, not blocking.
Windows-specific verification is explicitly deferred to CI; this does not block merge.
Earlier passes raised no actionable findings to resolve or re-raise.
Review was read-only and limited to the specified diff. I did not run, build, or install anything from the PR.

jeff-r2026

Approving 24008b0; please rebase onto current main and I'll merge. Merged locally with main: tsc, lint, build, full unit suite (only the 2 submodule-checkout cases that also fail on plain main), codex-trust/codex-team-hooks/hooks/hooks-cmd/doctor 239/239, Codex hook e2e 17/17.

Conflicts are mechanical: src/hooks.ts (keep your codexTrust line and #1330's ZCode legacy cleanup block; the ZCode reconcileHooks call needs no codexTrust) and src/pull.ts reinjectLegacyHooks (use main's hookToolPathsForConfig(teamConfig, localConfig) and append codexTrustPolicy(localConfig)).

[P2 non-blocking] On macOS desktopCodexBinaries() looks for /Applications/ChatGPT.app/Contents/Resources/codex, but current ChatGPT.app ships the CLI at Contents/Resources/codex-cli/bin/codex, so on mac the #1244 fix never sees the newer desktop build. Worth adding that candidate here or in a follow-up.

# Conflicts: # src/uninstall.ts

github-actions · 2026-10-23T15:40:43Z

No actionable findings in the specified PR diff.

The PR description documents sufficient testing, including a build and representative real-CLI verification for both fixes. The older verification commit is a note, not blocking.
Windows-specific verification is explicitly deferred to CI; this does not block merge.
Earlier passes raised no actionable findings to resolve or re-raise.
Review was read-only and limited to the specified diff. I did not run, build, or install anything from the PR.

SaulMoro · 2026-10-23T17:55:52Z

Thanks. Brought up to date with main at 2c1995d by a merge commit, not a rebase, so your approval stays on the same history. The resolutions are the ones you described, plus src/uninstall.ts after #1265 (main's mcpLeft plus codexTrustPolicy(localConfig)). CI is 18/18 on that head. The macOS ChatGPT.app path (Contents/Resources/codex-cli/bin/codex) is tracked as a follow-up in #1351.

SaulMoro added 5 commits October 21, 2026 11:46

docs: tell the agent what to do when Codex builds disagree (#1244)

1482153

SaulMoro assigned SaulMoro and jeff-r2026 and unassigned SaulMoro Oct 21, 2026

This was referenced Oct 21, 2026

fix(session): only end a Codex tool round on items that start a message #1260

Merged

fix(hooks): make Codex hook commands portable shell-free argv (#1215) #1231

Merged

SaulMoro added 2 commits October 22, 2026 08:44

Merge remote-tracking branch 'origin/main' into fix/codex-hook-trust-…

15d49c7

…1244-1250 # Conflicts: # src/init.ts

Merge remote-tracking branch 'origin/main' into HEAD

24008b0

# Conflicts: # src/types.ts

Merge remote-tracking branch 'origin/main' into HEAD

cadd8b7

# Conflicts: # src/hooks.ts # src/pull.ts

jeff-r2026 approved these changes Oct 23, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into HEAD

2c1995d

# Conflicts: # src/uninstall.ts

SaulMoro mentioned this pull request Oct 23, 2026

fix(codex): find the desktop Codex CLI at ChatGPT.app's codex-cli/bin/codex on macOS #1351

Closed

jeff-r2026 merged commit 7bb2e27 into main Oct 24, 2026
18 checks passed

This was referenced Oct 24, 2026

fix(init): retire side worktrees before replacing repo #1327

Merged

feat(roles): assign roles in manifest/members.yaml instead of per member (#1186) #1191

Open

Navigation Menu

Repository navigation

fix(codex): keep member hook groups at their keys and trust with the newest build (#1244, #1250) - #1263

fix(codex): keep member hook groups at their keys and trust with the newest build (#1244, #1250)#1263
jeff-r2026 merged 10 commits into
mainfrom
fix/codex-hook-trust-1244-1250

SaulMoro commented Oct 21, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 21, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 21, 2026

Uh oh!

SaulMoro commented Oct 21, 2026

Uh oh!

github-actions Bot commented Oct 22, 2026

Uh oh!

github-actions Bot commented Oct 23, 2026

Uh oh!

github-actions Bot commented Oct 23, 2026

Uh oh!

jeff-r2026 left a comment

Uh oh!

github-actions Bot commented Oct 23, 2026

Uh oh!

SaulMoro commented Oct 23, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

SaulMoro commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What changed

Scope

Tradeoffs

Blast Radius

Verification

Uh oh!

github-actions Bot commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Erosion Report

Rule hits (TS verbosity layer)

Uh oh!

github-actions Bot commented Oct 21, 2026

Uh oh!

SaulMoro commented Oct 21, 2026

Uh oh!

github-actions Bot commented Oct 22, 2026

Uh oh!

github-actions Bot commented Oct 23, 2026

Uh oh!

github-actions Bot commented Oct 23, 2026

Uh oh!

jeff-r2026 left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 23, 2026

Uh oh!

SaulMoro commented Oct 23, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SaulMoro commented Oct 21, 2026 •

edited

Loading

github-actions Bot commented Oct 21, 2026 •

edited

Loading