Skip to content

test(hooks): isolate reconcile scope fixture - #1264

Merged
SaulMoro merged 2 commits into
Tencent:mainfrom
huong8373tt-beep:fix/hooks-reconcile-scope
Oct 21, 2026
Merged

SaulMoro merged 2 commits into
Tencent:mainfrom
huong8373tt-beep:fix/hooks-reconcile-scope

Conversation

@huong8373tt-beep

@huong8373tt-beep huong8373tt-beep commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • initialize each real-shell gate fixture as its own Git repository so hook reconciliation cannot walk up into the contributor checkout
  • assert that the fixture repository receives the expected TeamAI Git hooks while the contributor checkout's local hook.teamai-* configuration remains unchanged

Validation

  • in Git Bash with PATH including C:\Program Files\Git\bin:
    node node_modules/vitest/vitest.mjs run src/__tests__/hooks-reconcile-scope.test.ts --testNamePattern='fires inside the project'
  • npm run typecheck
  • npm run lint
  • npm run build
  • git diff --check upstream/main

The focused real-shell test exercises the gate from fixture directories under node_modules, verifies it runs inside the fixture and is a no-op outside it, and verifies both that the fixture has the TeamAI Git hooks and that no TeamAI Git-hook entries are added to the contributor checkout.

Real CLI verification

Using the built dist/cli.js, I created an isolated temporary Git repository with a fixture-only generic Git remote and ran:

node dist/cli.js init --self --provider git --agent codebuddy --force

The command exited 0. It created .teamai/teamai.yaml and .codebuddy/settings.json, and local Git config contained all six expected hook.teamai-{post-checkout,post-merge,post-rewrite}.{command,event} entries. The fixture was removed afterward. The deliberately unreachable fixture remote produced only the expected non-blocking member-registration warning; no remote push occurred.


Slai Agent Runtime assisted with investigation, implementation, and testing. The account owner reviewed and authorized this contribution.

@github-actions

Copy link
Copy Markdown
  • [P1 blocking] Missing real-CLI verification record — src/git-hook.ts:228 changes runtime hook-path resolution, so this is not a tests-only PR. The description lists a focused Vitest real-shell test, type checking, lint, and build, but no end-to-end run of the real CLI. The first Code Review Rules item explicitly makes that omission blocking. Document one representative real-CLI verification after building; no full provider × agent matrix is required.

No other confident findings in the supplied diff.

@huong8373tt-beep

Copy link
Copy Markdown
Contributor Author

Addressed in the PR description. Slai Agent Runtime performed the isolated real-CLI verification after building:

node dist/cli.js init --self --provider git --agent codebuddy --force

It exited successfully; the expected hook configuration and generated files were verified, and the temporary fixture was removed. The account owner reviewed and authorized this contribution.

@github-actions

Copy link
Copy Markdown

No new findings in the specified diff.

Resolved: The earlier [P1 blocking] Missing real-CLI verification record finding is fixed. The updated description documents a successful run of the built CLI in an isolated Git repository and verifies the generated configuration and all six expected hook entries. This satisfies the representative real-CLI requirement.

Review was read-only; I did not run, build, or install anything from the PR.

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

This was generated by AI during triage.

What was checked. I checked 1003e9b on macOS (Git 2.56.0, Node 24) in a clean clone. On main, the "fires inside the project" test writes all six hook.teamai-* entries into the checkout's own .git/config. On this branch it writes none. The built CLI behaves the same on main and on this branch when run from a symlinked directory. init, git worktree add and uninstall exit 0 with the same hook results. The doctor git-hook check passes on both, with identical output.

[P2 non-blocking] src/git-hook.ts:228 and src/git-hook.ts:269. The test fix does not need this change. The updated test passes against main's src/git-hook.ts. The change also has no test. The description says it is for linked worktrees, but Git returns absolute paths there, so the base directory is ignored. The case it actually fixes is a repoDir that reaches a repository subdirectory through a symlink. Please either drop it from this PR, or describe that case and add a focused test through installGitHook or gitHookStatus.

[P3 nit] src/__tests__/hooks-reconcile-scope.test.ts:1380. The before/after comparison still passes when the checkout already has the six hook.teamai-* entries. With the fixture's git init removed, the test fails on a clean config but passes on a populated one. An assertion on the fixture's own hook with gitHookStatus(root) would show the hook went where intended, whatever state the checkout is in.

Not checked. I did not run on Windows, where realpath can spell drive and UNC paths differently. I did not run a real Git older than 2.54; the script-hook path ran through a version shim. I did not run on Linux.

@github-actions

Copy link
Copy Markdown

No new findings in the specified diff. The changes are tests-only, isolating the fixtures in Git repositories and checking that reconciliation leaves the checkout’s TeamAI hook configuration unchanged.

Resolved: The earlier [P1 blocking] Missing real-CLI verification record finding remains resolved. The description documents a successful built-CLI run and verification of the generated configuration and six hook entries. The current tests-only diff does not require an additional e2e record.

Review was read-only; I did not run, build, or install anything from the PR.

@huong8373tt-beep

Copy link
Copy Markdown
Contributor Author

Addressed the collaborator's non-blocking review notes in 17feef5:

  • removed the untested realpath runtime changes from src/git-hook.ts, leaving this as a fixture-isolation fix;
  • added a gitHookStatus(root) assertion so the test verifies the fixture repository received the expected hooks, in addition to verifying the contributor checkout was unchanged.

Slai Agent Runtime reran the focused real-shell test, typecheck, lint, and build; details are in the PR description.

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

This was generated by AI during triage.

This re-reviews 17feef5 against the approval of 1003e9b. Both notes are resolved. The realpath edit is gone and src/git-hook.ts matches main. The fixture asserts gitHookStatus(root) is { installed: true }. Versus main, the only change is the test file. Runtime behavior is unchanged, and the real-CLI record already in the PR body is enough. npm test -- src/__tests__/hooks-reconcile-scope.test.ts exited 0 in a fresh clone, 86 tests passed.

@SaulMoro
SaulMoro merged commit d60e68f into Tencent:main Oct 21, 2026
17 checks passed
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