Skip to content

Fix hook repo quoting and restore docs repo_root#192

Open
GrimoireScribe wants to merge 1 commit intotirth8205:mainfrom
GrimoireScribe:fix/hook-quoting-docs-repo-root
Open

Fix hook repo quoting and restore docs repo_root#192
GrimoireScribe wants to merge 1 commit intotirth8205:mainfrom
GrimoireScribe:fix/hook-quoting-docs-repo-root

Conversation

@GrimoireScribe
Copy link
Copy Markdown
Contributor

Summary

Following feedback on #154, this separates the remaining non-JSX fixes into their own PR.

This PR does two things:

  • quotes generated Claude hook --repo paths so repos under paths with spaces do not break
  • restores the repo_root parameter on get_docs_section_tool so the MCP schema remains backward-compatible

Why

The JSX parser work now lives on its own branch/PR.
These two changes are still useful independently:

  • generated hook commands should be shell-safe for repo paths with spaces
  • existing callers that pass repo_root to get_docs_section_tool should not break

Verification

Ran targeted validation locally for the affected areas.

@tirth8205
Copy link
Copy Markdown
Owner

The fixes here are correct and important — hook format now matches Claude Code's actual schema (nested hooks array with type:command), PreCommit removed (invalid hook event), --repo path is properly shell-quoted via json.dumps(), and get_docs_section_tool repo_root param is restored for backwards compat.

However the tests have a platform-specific bug that will fail on Linux/macOS CI:

The three assertions like this:
assert hooks[0]['hooks'][0]['command'].endswith('--repo "E:/repo"')

Path('/repo').resolve().as_posix() returns '/repo' on Linux/macOS, not 'E:/repo' — the 'E:' prefix is your Windows drive. Please replace these three assertions with OS-agnostic checks, for example:

import json
from pathlib import Path
expected_arg = json.dumps(Path('/repo').resolve().as_posix())
assert f'--repo {expected_arg}' in command

Or simply check that the command contains '--repo' and that the path is quoted (contains a double-quote character) without hardcoding the drive letter.

Please fix the three affected test assertions and rebase on main, then this is good to merge.

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