diff options
| author | Craig Jennings <c@cjennings.net> | 2026-07-16 13:42:53 -0500 |
|---|---|---|
| committer | Craig Jennings <c@cjennings.net> | 2026-07-16 13:42:53 -0500 |
| commit | c98fda556e9211573dab3e32eaa5c42c2dedbfcb (patch) | |
| tree | 3e9e9cb7c6060d2edaa813ebaf852637a2244c7a | |
| parent | 125c1e9777d66645cf23b30b3d2de1c91fb492aa (diff) | |
| download | rulesets-c98fda556e9211573dab3e32eaa5c42c2dedbfcb.tar.gz rulesets-c98fda556e9211573dab3e32eaa5c42c2dedbfcb.zip | |
fix(install-lang): refuse a colliding second bundle instead of clobbering
Installing a second language bundle into a project silently replaced the first one's config. settings.json and githooks are copied with cp -rT (always overwrite), so installing bash over elisp rewired the validate hook to validate-bash.sh and dropped check-parens from pre-commit, leaving validate-el.sh orphaned on disk. The output said [ok] for both. A project could lose its paren check or secret scan and read the install as success.
The guard detects which bundles a project already has, by the same rule fingerprint sync-language-bundle uses, and refuses when the incoming bundle would overwrite a file another one ships. It names each file at risk. FORCE=1 still overrides, and the message says that also re-seeds CLAUDE.md, which is destructive on a customized project.
Only three of the five shared filenames actually collide. gitignore-add.txt is appended and deduped, and CLAUDE.md is seed-only, so both compose across bundles already.
This doesn't decide whether polyglot projects are supported, and the guard shouldn't be read as "no". A non-overlapping pair still installs: bash ships settings.json and githooks with no coverage fragment, python ships only a coverage fragment, so the two compose today. The real line is overlap, not polyglot, and nothing chose it. The open question, along with the identical coverage target names both fragments define, is filed.
home reported this after scaffolding clock-panel with python and typescript, which hit the coverage fragment. The settings.json and githooks cases are worse and hadn't been noticed. No project was damaged: the three bundles that collide aren't doubled up anywhere.
| -rw-r--r-- | docs/design/2026-07-16-polyglot-bundle-collision.txt | 30 | ||||
| -rwxr-xr-x | scripts/install-lang.sh | 69 | ||||
| -rw-r--r-- | scripts/tests/install-lang-collision.bats | 112 | ||||
| -rw-r--r-- | todo.org | 41 |
4 files changed, 252 insertions, 0 deletions
diff --git a/docs/design/2026-07-16-polyglot-bundle-collision.txt b/docs/design/2026-07-16-polyglot-bundle-collision.txt new file mode 100644 index 0000000..b7e1257 --- /dev/null +++ b/docs/design/2026-07-16-polyglot-bundle-collision.txt @@ -0,0 +1,30 @@ +Proposal: language bundles collide on coverage-makefile.txt in a polyglot project, and the second install loses silently. + +Context: scaffolded a new project (clock-panel) on 2026-07-16 with both the python and typescript bundles installed into the same project. + +What happened. The second install printed: + + [skip] coverage-makefile.txt already exists (use FORCE=1 to overwrite) + +Both bundles ship a file at that same path. Python installed first and won, so the project ended up with Python's coverage fragment only. The TypeScript one (c8, Istanbul json-summary) never landed. The line reads like routine idempotence, not a dropped deliverable, so it's easy to skim past. I only caught it by checking which targets were actually in the file. + +The workaround is bad. FORCE=1 gets the TypeScript fragment but overwrites the Python one, so you can't have both without renaming by hand between installs. FORCE=1 also re-seeds CLAUDE.md, which is fine on a fresh project and destructive on a customized one. What I did in clock-panel: install python, mv coverage-makefile.txt coverage-makefile-python.txt, install typescript FORCE=1, mv coverage-makefile.txt coverage-makefile-typescript.txt. Both fragments now coexist under language-suffixed names. + +The deeper problem, which the filename collision only hints at: both fragments define targets with the SAME names. + + coverage: + coverage-summary: + +So even with both files present, a polyglot project can't copy both into one Makefile. It gets duplicate targets. The fragments assume they're the only language in the project. Renaming the files doesn't fix that, it just makes both sets of instructions visible while leaving the conflict for whoever pastes them. + +Options, in the order I'd weigh them: + +Suffix the shipped filename per bundle (coverage-makefile-python.txt, coverage-makefile-typescript.txt) so installs never collide. Cheap, and it makes the multi-bundle case work by default. It leaves the target-name conflict. + +Namespace the targets too (coverage-python / coverage-typescript, with a coverage target that runs both). Fixes the real problem for polyglot projects. Bigger change, and it makes single-language projects type a longer target name unless there's an alias. + +At minimum, make the skip line loud when the skipped file came from a DIFFERENT bundle than the one that wrote it. A skip that means "already yours" and a skip that means "another language's file is here and yours is being dropped" currently look identical. + +Worth deciding whether polyglot projects are a supported case at all. If they are, the bundles need a collision story. If they aren't, install-lang could say so when it detects a second bundle going into a project that already has one, rather than half-installing. + +Also flagged separately in a note from home today: the same install-lang run is where I'd look for other per-bundle files that could collide on a shared filename. I only checked coverage-makefile.txt. diff --git a/scripts/install-lang.sh b/scripts/install-lang.sh index 2f38fcd..6e8d806 100755 --- a/scripts/install-lang.sh +++ b/scripts/install-lang.sh @@ -33,6 +33,75 @@ fi # Resolve to absolute path PROJECT="$(cd "$PROJECT" && pwd)" +# 0. Cross-bundle collision guard. +# +# Several bundles ship a file at the same path. gitignore-add.txt is appended +# and deduped, and CLAUDE.md is seed-only, so both compose across bundles. Three +# do not: claude/settings.json and githooks/* are installed with `cp -rT` +# (always overwrite), and coverage-makefile.txt is seeded under a fixed name. So +# installing a second bundle replaced the first's hook wiring and pre-commit +# while printing [ok], so a project could lose its paren check or secret scan +# and read the output as success. Refuse instead, naming what would go. +# +# Detection is by rule fingerprint, matching sync-language-bundle.sh: a project +# has bundle X iff one of X's own rule files is in .claude/rules/. No marker +# file, so this works on installs that predate the guard. +project_has_bundle() { + local b="$1" rf + for rf in "$REPO_ROOT/languages/$b/claude/rules"/*.md; do + [ -f "$rf" ] || continue + [ -f "$PROJECT/.claude/rules/$(basename "$rf")" ] && return 0 + done + return 1 +} + +# Files $1's bundle and $LANG both ship, and that install would overwrite. +shared_overwritten_files() { + local other="$1" rel + for rel in claude/settings.json coverage-makefile.txt; do + [ -f "$SRC/$rel" ] && [ -f "$REPO_ROOT/languages/$other/$rel" ] \ + && echo " $(printf '%s' "$rel" | sed 's|^claude/|.claude/|')" + done + if [ -d "$SRC/githooks" ] && [ -d "$REPO_ROOT/languages/$other/githooks" ]; then + for rel in "$SRC/githooks"/*; do + [ -f "$rel" ] || continue + [ -f "$REPO_ROOT/languages/$other/githooks/$(basename "$rel")" ] \ + && echo " githooks/$(basename "$rel")" + done + fi +} + +if [ "$FORCE" != "1" ]; then + collisions="" + for other_dir in "$REPO_ROOT/languages"/*/; do + other="$(basename "$other_dir")" + [ "$other" = "$LANG" ] && continue + [ -d "$other_dir/claude/rules" ] || continue + project_has_bundle "$other" || continue + files="$(shared_overwritten_files "$other")" + [ -n "$files" ] && collisions="${collisions}The '$other' bundle is already installed here. Installing '$LANG' would replace: +${files} +" + done + + if [ -n "$collisions" ]; then + { + echo "ERROR: bundle collision. Refusing to install '$LANG' into $PROJECT" + echo + printf '%s' "$collisions" + echo "Both bundles ship these files, and installing overwrites rather than merges," + echo "so the bundle already here would silently lose them." + echo + echo "Whether a project can carry two bundles at once is an open question." + echo "Until it's settled, install one bundle per project." + echo + echo "To override: re-run with FORCE=1. That also re-seeds CLAUDE.md from the" + echo "bundle template, which overwrites any project-specific edits to it." + } >&2 + exit 1 + fi +fi + echo "Installing '$LANG' ruleset into $PROJECT" # 1. Generic rules from claude-rules/ (shared across all languages) diff --git a/scripts/tests/install-lang-collision.bats b/scripts/tests/install-lang-collision.bats new file mode 100644 index 0000000..36abb5b --- /dev/null +++ b/scripts/tests/install-lang-collision.bats @@ -0,0 +1,112 @@ +#!/usr/bin/env bats +# Tests for install-lang's cross-bundle collision guard. +# +# Several bundles ship files at the same path. gitignore-add.txt merges +# (appended, deduped) and CLAUDE.md is seed-only, so both compose across +# bundles. Three do not: +# +# claude/settings.json elisp, bash, go — cp -rT, silently overwritten +# githooks/pre-commit elisp, bash, go — cp -rT, silently overwritten +# coverage-makefile.txt 4 bundles — [skip]ped, fragment dropped +# +# Installing a second bundle used to replace the first's settings.json and +# pre-commit while printing [ok], so a project could lose its paren check or +# secret scan and read the output as success. The guard refuses instead, naming +# what would be replaced. FORCE=1 still overrides. + +INSTALL="${BATS_TEST_DIRNAME}/../install-lang.sh" + +setup() { + PROJ="$(mktemp -d)" + git init -q "$PROJ" +} + +teardown() { + [ -n "${PROJ:-}" ] && rm -rf "$PROJ" +} + +# ---- Normal: single-bundle installs are unaffected ---- + +@test "install-lang: a fresh single-bundle install succeeds" { + run bash "$INSTALL" elisp "$PROJ" + [ "$status" -eq 0 ] + [ -f "$PROJ/.claude/settings.json" ] + grep -q 'validate-el.sh' "$PROJ/.claude/settings.json" +} + +@test "install-lang: reinstalling the SAME bundle is idempotent, not a collision" { + bash "$INSTALL" elisp "$PROJ" + run bash "$INSTALL" elisp "$PROJ" + [ "$status" -eq 0 ] + [[ "$output" != *"collision"* ]] + grep -q 'validate-el.sh' "$PROJ/.claude/settings.json" +} + +# ---- The guard: a second, different bundle must not silently replace ---- + +@test "install-lang: a second bundle sharing settings.json and githooks is refused" { + bash "$INSTALL" elisp "$PROJ" + run bash "$INSTALL" bash "$PROJ" + [ "$status" -ne 0 ] || { echo "second bundle installed without refusal"; return 1; } + [[ "$output" == *"elisp"* ]] || { echo "refusal does not name the existing bundle"; return 1; } +} + +@test "install-lang: the refusal names each file that would be replaced" { + bash "$INSTALL" elisp "$PROJ" + run bash "$INSTALL" bash "$PROJ" + [[ "$output" == *"settings.json"* ]] || { echo "refusal omits settings.json"; return 1; } + [[ "$output" == *"pre-commit"* ]] || { echo "refusal omits githooks/pre-commit"; return 1; } +} + +@test "install-lang: a refused install leaves the first bundle intact" { + bash "$INSTALL" elisp "$PROJ" + bash "$INSTALL" bash "$PROJ" || true + grep -q 'validate-el.sh' "$PROJ/.claude/settings.json" \ + || { echo "elisp settings.json was replaced despite refusal"; return 1; } + grep -q 'check-parens' "$PROJ/githooks/pre-commit" \ + || { echo "elisp pre-commit was replaced despite refusal"; return 1; } +} + +@test "install-lang: bundles colliding only on coverage-makefile.txt are refused too" { + # python and typescript ship no settings.json or githooks, but both ship a + # coverage fragment. The second one's used to be silently dropped. + bash "$INSTALL" python "$PROJ" + run bash "$INSTALL" typescript "$PROJ" + [ "$status" -ne 0 ] || { echo "typescript installed over python's coverage fragment"; return 1; } + [[ "$output" == *"coverage-makefile.txt"* ]] +} + +@test "install-lang: two bundles that share no overwritten file install together" { + # bash ships settings.json + githooks and no coverage fragment; python ships + # only the coverage fragment. Nothing overlaps, so the guard must stay out of + # the way. This is the path where a false refusal would be easiest to + # introduce: the bundle IS detected, and only the empty file-list stops it. + bash "$INSTALL" bash "$PROJ" + run bash "$INSTALL" python "$PROJ" + [ "$status" -eq 0 ] || { echo "guard falsely refused a non-colliding pair: $output"; return 1; } + [ -f "$PROJ/coverage-makefile.txt" ] || { echo "python's coverage fragment did not land"; return 1; } + # bash's config survives untouched. + grep -q 'validate-bash.sh' "$PROJ/.claude/settings.json" + [ -f "$PROJ/.claude/rules/bash.md" ] && [ -f "$PROJ/.claude/rules/python-testing.md" ] +} + +# ---- The escape hatch ---- + +@test "install-lang: FORCE=1 overrides the collision guard" { + bash "$INSTALL" elisp "$PROJ" + run bash "$INSTALL" bash "$PROJ" 1 + [ "$status" -eq 0 ] || { echo "FORCE=1 did not override: $output"; return 1; } + grep -q 'validate-bash.sh' "$PROJ/.claude/settings.json" +} + +@test "install-lang: the refusal points at FORCE=1 and warns it re-seeds CLAUDE.md" { + bash "$INSTALL" elisp "$PROJ" + run bash "$INSTALL" bash "$PROJ" + # Assert the refusal fired first: the pre-existing "[skip] CLAUDE.md already + # exists (use FORCE=1 to overwrite)" line mentions both strings on its own, so + # without this the test passes against an unguarded install. + [ "$status" -ne 0 ] || { echo "no refusal fired"; return 1; } + refusal="$(printf '%s\n' "$output" | grep -v '^ \[skip\]')" + [[ "$refusal" == *"FORCE=1"* ]] || { echo "refusal does not name the override"; return 1; } + [[ "$refusal" == *"CLAUDE.md"* ]] || { echo "refusal does not warn about the CLAUDE.md re-seed"; return 1; } +} @@ -48,6 +48,47 @@ triage-intake.org), live trial night on rulesets. Origin: work project's proposal, [[file:docs/design/2026-07-14-sentry-workflow-proposal.org][docs/design/2026-07-14-sentry-workflow-proposal.org]]. All nine design decisions resolved with Craig 2026-07-14 (recorded in the spec). +** TODO [#C] Polyglot projects — supported, or refused? :spec: +SCHEDULED: <2026-07-20 Mon> +Do we support more than one language bundle per project? The honest answer today +is "partly, by accident." The collision guard added 2026-07-16 refuses a +*colliding* second bundle rather than silently replacing the first's config, but +a non-overlapping pair still installs fine: bash ships =settings.json= + +githooks and no coverage fragment, python ships only a coverage fragment, so +=bash= + =python= composes cleanly today and yields a real polyglot project with +both rule sets. So the line isn't polyglot-vs-not, it's overlap-vs-not — and +nobody chose that line, it fell out of which bundle happens to ship what. Origin: +home's report after scaffolding clock-panel with python + typescript, +[[file:docs/design/2026-07-16-polyglot-bundle-collision.txt][docs/design/2026-07-16-polyglot-bundle-collision.txt]]. + +Pair this with the subproject scouting below — it's the same question in a +different costume ("which projects would actually be polyglot, and why"), so +they should be one conversation. + +The three options, in the order they'd be weighed: + +1. *Unsupported, explicitly.* Keep the guard as the answer. Cheapest, and + matches how little polyglot exists (one project, clock-panel). +2. *Supported.* Needs per-bundle filenames, a merged =settings.json= (the hooks + arrays compose rather than clobber), composed githooks, and namespaced + Makefile targets with a =coverage= aggregate. This is the real work. +3. *Case-by-case.* Support the pairs that come up, refuse the rest. + +What the decision needs to know: + +- *The target-name collision is the deeper half* (home's point, and it's right). + Every bundle's fragment defines =coverage:= and =coverage-summary:=, so even + with both files present a polyglot project can't paste both into one Makefile. + Renaming files doesn't fix it. +- *Only three of five shared filenames actually collide.* =gitignore-add.txt= + (5 bundles) appends deduped and composes. =CLAUDE.md= (3) is seed-only, and + its fallback comment shows multi-bundle was already considered there. + =claude/settings.json= (3), =githooks/*= (3), and =coverage-makefile.txt= (4) + are the real ones. +- *=FORCE=1= is a poor escape hatch* (home's catch): it also re-seeds + =CLAUDE.md=, which is destructive on a customized project. If polyglot + becomes supported, the override wants to be its own flag. + ** TODO [#C] Subproject pattern — promote to claude-rules? :spec: SCHEDULED: <2026-07-20 Mon> home proposes promoting its subproject pattern (a former standalone project |
