mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-19 10:33:29 +00:00
fix(tooling): stop worktree commits from skipping prettier
The pre-commit frontend formatter resolved prettier at REPO_ROOT/frontend-modern/node_modules/.bin/prettier, where REPO_ROOT is derived from the script's own path. In a linked worktree that is the worktree root, which never runs npm install, so prettier_bin() returned None and the formatter silently returned 0. Every frontend commit made from a Claude or Codex agent worktree skipped formatting entirely, and the drift accumulated in already-committed files until someone ran `make format` and picked up 14 files of unrelated churn. The skip path's comment claimed "CI's prettier check still catches drift that slips through here." No such check existed -- nothing under .github/workflows referenced prettier -- so there was no backstop at all. - Fall back to the primary worktree's node_modules, resolved via `git rev-parse --git-common-dir`, and say so in the hook output so a version mismatch between the two checkouts stays visible. - Add the whole-tree "Check frontend formatting" step to the frontend CI job that the comment already promised. Staged-only formatting cannot see drift in untouched files; this can. - Pin prettier exactly. "^3.3.0" let the tracked package-lock.json land on 3.9.5 while the gitignored pnpm-lock.yaml resolved 3.9.6, so CI and the dev machine were free to run different formatters. - Resolve the test suite's prettier the same way, so its two real coverage tests stop silently skipping in worktrees. Contract-Neutral: devDependency prettier version pin plus pre-commit/CI formatter tooling; no runtime, API, or deployment-surface delta
This commit is contained in:
parent
dd894bd1f7
commit
ff58c33cfb
5 changed files with 117 additions and 17 deletions
7
.github/workflows/build-and-test.yml
vendored
7
.github/workflows/build-and-test.yml
vendored
|
|
@ -122,6 +122,13 @@ jobs:
|
|||
working-directory: frontend-modern
|
||||
run: npm ci
|
||||
|
||||
# Whole-tree, not staged-only: the pre-commit formatter only ever sees
|
||||
# staged files, so drift in untouched files is invisible to it. This is
|
||||
# the backstop that keeps `make format` a no-op on a clean tree.
|
||||
- name: Check frontend formatting
|
||||
working-directory: frontend-modern
|
||||
run: npm run format:check
|
||||
|
||||
- name: Lint frontend
|
||||
working-directory: frontend-modern
|
||||
run: npm run lint
|
||||
|
|
|
|||
8
frontend-modern/package-lock.json
generated
8
frontend-modern/package-lock.json
generated
|
|
@ -35,7 +35,7 @@
|
|||
"jscpd": "^4.0.8",
|
||||
"jsdom": "^24.1.0",
|
||||
"postcss": "^8.5.13",
|
||||
"prettier": "^3.3.0",
|
||||
"prettier": "3.9.6",
|
||||
"tailwindcss": "^3.4.18",
|
||||
"typescript": "^5.3.0",
|
||||
"typescript-eslint": "^8.54.0",
|
||||
|
|
@ -5739,9 +5739,9 @@
|
|||
}
|
||||
},
|
||||
"node_modules/prettier": {
|
||||
"version": "3.9.5",
|
||||
"resolved": "https://registry.npmjs.org/prettier/-/prettier-3.9.5.tgz",
|
||||
"integrity": "sha512-/FVl766LpUfB5vXgCYOYa0MeV/441Ia99AeICQIQFTY/Nw0roZwULcXpku5i1/m5kt/baz+s4Zogspd839HSMg==",
|
||||
"version": "3.9.6",
|
||||
"resolved": "https://registry.npmjs.org/prettier/-/prettier-3.9.6.tgz",
|
||||
"integrity": "sha512-OpN0zzVdiaiAhxpuuj5efpIS4sY9j7bY6uR5mnj5yPzGkdkjNKSJeUThPb60Jw29QuAZgA4o+/iB49kFiaBX6g==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"bin": {
|
||||
|
|
|
|||
|
|
@ -81,7 +81,7 @@
|
|||
"jscpd": "^4.0.8",
|
||||
"jsdom": "^24.1.0",
|
||||
"postcss": "^8.5.13",
|
||||
"prettier": "^3.3.0",
|
||||
"prettier": "3.9.6",
|
||||
"tailwindcss": "^3.4.18",
|
||||
"typescript": "^5.3.0",
|
||||
"typescript-eslint": "^8.54.0",
|
||||
|
|
|
|||
|
|
@ -54,13 +54,48 @@ def git(*args: str, text: bool, input_data: str | bytes | None = None) -> subpro
|
|||
)
|
||||
|
||||
|
||||
def primary_worktree_root() -> Path | None:
|
||||
"""Resolve the primary worktree's root, or None when that fails.
|
||||
|
||||
Linked worktrees (the Claude and Codex agent checkouts under
|
||||
.claude/worktrees and ~/.codex/worktrees) never run `npm install`, so
|
||||
frontend-modern/node_modules only ever exists in the primary checkout.
|
||||
--git-common-dir points at the shared .git directory from anywhere in the
|
||||
repository, and its parent is the primary worktree root.
|
||||
"""
|
||||
try:
|
||||
common = git("rev-parse", "--git-common-dir", text=True).stdout.strip()
|
||||
except (subprocess.CalledProcessError, OSError):
|
||||
return None
|
||||
if not common:
|
||||
return None
|
||||
common_dir = Path(common)
|
||||
if not common_dir.is_absolute():
|
||||
common_dir = (REPO_ROOT / common_dir).resolve()
|
||||
# A bare or otherwise unusual layout has no worktree to fall back to.
|
||||
if common_dir.name != ".git":
|
||||
return None
|
||||
return common_dir.parent
|
||||
|
||||
|
||||
def prettier_search_roots() -> list[Path]:
|
||||
roots = [REPO_ROOT]
|
||||
primary = primary_worktree_root()
|
||||
if primary is not None and primary != REPO_ROOT:
|
||||
roots.append(primary)
|
||||
return roots
|
||||
|
||||
|
||||
def prettier_bin() -> Path | None:
|
||||
override = os.environ.get("PULSE_PRETTIER_BIN")
|
||||
if override:
|
||||
candidate = Path(override)
|
||||
return candidate if candidate.exists() else None
|
||||
candidate = REPO_ROOT / FRONTEND_DIR / "node_modules" / ".bin" / "prettier"
|
||||
return candidate if candidate.exists() else None
|
||||
for root in prettier_search_roots():
|
||||
candidate = root / FRONTEND_DIR / "node_modules" / ".bin" / "prettier"
|
||||
if candidate.exists():
|
||||
return candidate
|
||||
return None
|
||||
|
||||
|
||||
def staged_frontend_files() -> list[str]:
|
||||
|
|
@ -142,12 +177,19 @@ def format_staged_frontend_files() -> int:
|
|||
|
||||
prettier = prettier_bin()
|
||||
if prettier is None:
|
||||
# Fresh clones and linked worktrees may not have node_modules; skip
|
||||
# gracefully like the golangci-lint availability check does. CI's
|
||||
# prettier check still catches drift that slips through here.
|
||||
# A fresh clone that has never run `npm install` has no prettier
|
||||
# anywhere, so skip gracefully like the golangci-lint availability
|
||||
# check does. The "Check frontend formatting" step in the frontend CI
|
||||
# job is the backstop that catches anything skipped here.
|
||||
print("Skipping frontend formatter (prettier not installed under frontend-modern/node_modules).")
|
||||
return 0
|
||||
|
||||
if not prettier.is_relative_to(REPO_ROOT):
|
||||
# Linked worktrees borrow the primary checkout's prettier. Say so, so a
|
||||
# version mismatch between the two is visible in the hook output rather
|
||||
# than silently reformatting against the wrong prettier.
|
||||
print(f"Using prettier from the primary worktree: {prettier}")
|
||||
|
||||
print("Running prettier on staged frontend files...")
|
||||
formatted_count = 0
|
||||
synced_count = 0
|
||||
|
|
|
|||
|
|
@ -10,13 +10,24 @@ from format_staged_frontend import format_staged_frontend_files
|
|||
from repo_file_io import strip_local_git_env
|
||||
|
||||
|
||||
REAL_PRETTIER = (
|
||||
Path(format_staged_frontend.DEFAULT_REPO_ROOT)
|
||||
/ "frontend-modern"
|
||||
/ "node_modules"
|
||||
/ ".bin"
|
||||
/ "prettier"
|
||||
)
|
||||
def _resolve_real_prettier() -> Path:
|
||||
# Linked worktrees have no node_modules of their own. Without this the
|
||||
# tests below silently skip in exactly the checkouts where the formatter's
|
||||
# worktree fallback matters.
|
||||
for root in format_staged_frontend.prettier_search_roots():
|
||||
candidate = root / "frontend-modern" / "node_modules" / ".bin" / "prettier"
|
||||
if candidate.exists():
|
||||
return candidate
|
||||
return (
|
||||
Path(format_staged_frontend.DEFAULT_REPO_ROOT)
|
||||
/ "frontend-modern"
|
||||
/ "node_modules"
|
||||
/ ".bin"
|
||||
/ "prettier"
|
||||
)
|
||||
|
||||
|
||||
REAL_PRETTIER = _resolve_real_prettier()
|
||||
|
||||
|
||||
class FormatStagedFrontendTest(unittest.TestCase):
|
||||
|
|
@ -99,6 +110,46 @@ class FormatStagedFrontendTest(unittest.TestCase):
|
|||
staged = self.git(repo_root, "show", ":frontend-modern/src/sample.ts").stdout
|
||||
self.assertEqual(staged, "const x = { a: 1 };\n")
|
||||
|
||||
def test_linked_worktree_falls_back_to_primary_checkout_prettier(self) -> None:
|
||||
# Regression: prettier resolved only under REPO_ROOT, so every frontend
|
||||
# commit made from a Claude or Codex worktree silently skipped
|
||||
# formatting and let drift accumulate in already-committed files.
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
primary = Path(tmpdir) / "primary"
|
||||
primary.mkdir()
|
||||
self.git(primary, "init")
|
||||
(primary / "seed.txt").write_text("seed\n", encoding="utf-8")
|
||||
self.git(primary, "add", "seed.txt")
|
||||
self.git(
|
||||
primary,
|
||||
"-c",
|
||||
"user.email=test@example.com",
|
||||
"-c",
|
||||
"user.name=test",
|
||||
"commit",
|
||||
"-m",
|
||||
"seed",
|
||||
)
|
||||
|
||||
installed = primary / "frontend-modern" / "node_modules" / ".bin" / "prettier"
|
||||
installed.parent.mkdir(parents=True)
|
||||
installed.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8")
|
||||
installed.chmod(0o755)
|
||||
|
||||
linked = Path(tmpdir) / "linked"
|
||||
self.git(primary, "worktree", "add", str(linked))
|
||||
self.assertFalse((linked / "frontend-modern" / "node_modules").exists())
|
||||
|
||||
with patch.dict("os.environ", {}, clear=False):
|
||||
os.environ.pop("PULSE_PRETTIER_BIN", None)
|
||||
with patch("format_staged_frontend.REPO_ROOT", linked):
|
||||
resolved = format_staged_frontend.prettier_bin()
|
||||
|
||||
# --git-common-dir comes back resolved, so compare resolved paths:
|
||||
# on macOS the temp dir is /var -> /private/var.
|
||||
self.assertIsNotNone(resolved)
|
||||
self.assertEqual(resolved.resolve(), installed.resolve())
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue