Repository navigation
test(host-service): pin core.hooksPath in the dirty-hook fixture - #9192
serhii-kucherenko wants to merge 2 commits into
Conversation
The materialize test installs a post-checkout hook in its repo's .git/hooks and expects a plain checkout to fail on it. On a machine with a global core.hooksPath, git never runs that hook, the checkout succeeds, and the test fails. Pinning core.hooksPath in the repo's own config makes the hook run on every machine.
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Configure checkout hook packages/host-service/test/integration/pr-branch-materialize.integration.test.ts |
The helper accepts a SimpleGit instance and repository path, sets core.hooksPath to the repository’s .git/hooks directory, and returns a promise. The test awaits the helper and passes both arguments. |
Estimated code review effort: 2 (Simple) | ~5 minutes
Suggested reviewers: avipeltz
Merge Risk: 
9e1cd
-
No actionable merge-blocking risk remains in this test-only change.
Pre-merge checks: 
-
Check name Status Explanation Title check The title follows conventional commit format (type(scope): subject) and names the primary change: pinning core.hooksPath in the dirty-hook test fixture. The raw summary confirms the change sets core.h… Description check The description follows the template's main sections. The 'What & why' section explains the problem (a global core.hooksPath stops the post-checkout hook from running) and the fix (setting core.hooksP… Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title follows conventional commit format (type(scope): subject) and names the primary change: pinning core.hooksPath in the dirty-hook test fixture. The raw summary confirms the change sets core.hooksPath in the fixture repository before installing the hook, and the title matches that scope.
Full details: Description check
-
Explanation
The description follows the template's main sections. The 'What & why' section explains the problem (a global core.hooksPath stops the post-checkout hook from running) and the fix (setting core.hooksPath in the fixture repo's config with git config). The 'How I tested it' section lists concrete commands, results, and a red-first run. The checklist is present and marked. The template asks for a linked issue, which the description does not include, and it does not include screenshots. Neither is required for this test-only change, so these gaps are non-critical.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files.
-
Finishing Touches
-
Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
 [Low impact] The change affects only a temporary integration-test repository. The PR appears safe to merge, with a non-blocking path-escaping issue in the test fixture. Findings
Fix with agent prompt### Issue 1
packages/host-service/test/integration/pr-branch-materialize.integration.test.ts:79-82
`hooksDir` is written into Git config without escaping. If `TMPDIR` contains `#` or `;`, Git reads the rest of the path as a comment. The hook is then skipped, and the dirty-hook test fails even without a global `core.hooksPath`. Use `git config --local core.hooksPath` to write the value with proper escaping.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryPins
Reviews (1) · Last reviewed commit: "test(host-service): pin core.hooksPath i..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| appendFileSync( | ||
| join(repoPath, ".git", "config"), | ||
| `[core]\n\thooksPath = ${hooksDir}\n`, | ||
| ); |
There was a problem hiding this comment.
hooksDir is written into Git config without escaping. If TMPDIR contains # or ;, Git reads the rest of the path as a comment. The hook is then skipped, and the dirty-hook test fails even without a global core.hooksPath. Use git config --local core.hooksPath to write the value with proper escaping.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/test/integration/pr-branch-materialize.integration.test.ts
Line: 79-82
Comment:
**Special characters skip the hook**
`hooksDir` is written into Git config without escaping. If `TMPDIR` contains `#` or `;`, Git reads the rest of the path as a comment. The hook is then skipped, and the dirty-hook test fails even without a global `core.hooksPath`. Use `git config --local core.hooksPath` to write the value with proper escaping.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.A raw [core] append breaks when the temp path holds # or ;, because git reads those as a comment start. git config quotes the value.
What & why
The materialize integration test installs a post-checkout hook in its fixture repo's
.git/hooksand expects a plain checkout to fail on it. On a machine with a globalcore.hooksPath, git never runs that hook: the checkout succeeds and the test fails. This setscore.hooksPathin the fixture repo's own config withgit config, so the hook runs on every machine.git configquotes the value, so a temp path with#or;still works. Only the test fixture changes.How I tested it
core.hooksPathset,bun test test/integration/pr-branch-materialize.integration.test.tsinpackages/host-service: 3/3 pass.mainfails "materialize-first worktree creation survives hooks that dirty tracked files during checkout" (expect(oldCheckoutError).toBeInstanceOf(Error), receivednull).TMPDIRset to a folder whose name has#: 3/3 pass. The same run with a raw[core] hooksPath = ...line appended to.git/configfails the same test, because git reads#as a comment start.tscpasses inpackages/host-service.Checklist
type(scope): subject)bun run lintandbun run typecheckpass (CI fails on lint warnings too)Summary by CodeRabbit