fix(adopt): idempotency stamp (#48) + recipe-collision detection (#49) - #53
Merged
Conversation
…mpotency stamp
adopt.sh's only "already scaffolded" guard was a filesystem heuristic
([[ -f justfile && -d scripts/ci ]]), inferring "was rust-forge adopted
here" from paths anyone can create for unrelated reasons. False positive
(hit live): a repo with an old, hand-rolled justfile + scripts/ci trips
the guard even though it has nothing to do with this template - the only
workaround was renaming both paths out of the way, which throws away the
one signal the script had. False negative: delete/rename justfile after a
real adopt and a second run happily starts over.
The template already has the right primitive for the init direction:
docs/forge/FORGE-STAMP.toml + TEMPLATE_VERSION, a real birth-certificate
pair adopt.sh never copied. Now it does:
- FORGE-STAMP.toml/TEMPLATE_VERSION added to MACHINERY, so they flow
through the existing copy + acmex->slug rename machinery unchanged
(project = "acmex" -> project = "$SLUG", same as every other file).
- New `origin` field ("init" for `just init`, "adopted" for adopt.sh)
distinguishes provenance; adopt.sh flips it after the copy.
- The precondition now checks for docs/forge/FORGE-STAMP.toml instead
of justfile/scripts-ci - a repo with a stamp has genuinely been
forged or adopted; a repo with a justfile merely has *a* justfile.
Fixes #48.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Found by testing #48's fix: the stamp lives on adopt/rust-forge-scaffolding until that branch merges (adopt.sh never touches the base branch), so re-running adopt.sh from base while a trial is still unmerged doesn't see it - the precondition passes, and the script switches onto the existing adopt branch and re-runs the whole copy/wire/commit sequence on top of already-adopted content, producing a redundant, confusing second commit (verified live: 89 files that should have copied clean instead became .forge-suggested collisions against themselves). Adds the belt-and-suspenders check from #48's proposal: if adopt/rust-forge-scaffolding already exists, stop and point at `just adopt-status` / `just adopt-undo` instead of silently doubling up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
adopt.sh's never-clobber guarantee operates at file-path granularity: if the destination FILE already exists, the template's version lands beside it as .forge-suggested instead of overwriting. That's the right rule for file content, but just recipes from every imported .just file share one flat namespace - two files defining a recipe with the same name breaks `just` entirely (not just that recipe), regardless of which files they live in. A new template file (just/analysis.just) sailing past the file-existence check can still collide with a recipe name already defined in an existing file with a different name (your own just/dev.just) - reported live: this repo's own just/analysis.just defines `audit:`, which collided with an adoptee's hand-rolled just/dev.just also defining `audit:`, and `just` refused to run at all until one was renamed. After the copy step, diffs the set of top-level recipe names across every just/*.just file (existing + newly copied) and warns - never blocks - on any name that now appears in more than one file, naming both files. Same "surface, don't block" posture as the .forge-suggested list already printed in this step. Tested against the exact reported scenario (a just/dev.just already defining audit:, template's just/analysis.just also defining audit:) - correctly names both files and suggests the fix. Verified no false positives against a clean adoption (no pre-existing just/ directory) and a repo with an unrelated pre-existing justfile/scripts/ci (the scenario that motivated #48's stamp fix). Fixes #49. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes both issues filed from the same live adoption run, both tested end-to-end against fixture repos:
#48: replace the justfile+scripts/ci heuristic with a real idempotency stamp
adopt.sh's only "already scaffolded" guard was[[ -f justfile && -d scripts/ci ]]- a heuristic that false-positives on any repo with an unrelated hand-rolled justfile, and false-negatives once those paths are deleted/renamed.docs/forge/FORGE-STAMP.toml+TEMPLATE_VERSION(the existing birth-certificate pair forjust init) are now inadopt.sh'sMACHINERYlist, flow through the existing copy + acmex->slug rename machinery unchanged, and gain a neworiginfield ("init"vs"adopted") that adopt.sh flips after copying.adopt/rust-forge-scaffoldinguntil that branch merges, so re-running from the base branch while a trial is still unmerged wouldn't see it. Added a second precondition checking for the branch itself, pointing atjust adopt-status/just adopt-undo.#49: warn on just recipe-name collisions across files
justrecipes share one flat namespace across every imported.justfile. A new template file (just/analysis.just) colliding with an existing file's recipe of the same name (just/dev.just'saudit:) breaksjustentirely - the exact scenario reported live.just/*.justfiles and warns (never blocks) on any name appearing in more than one file, naming both.Test plan
justfile, nojust/) - stamp written withorigin = "adopted", no spurious collision warning.adopt/rust-forge-scaffoldingexists unmerged - blocked with the new branch-existence message (confirmed this reproduces cleanly before the fix: a redundant second commit with 89 files turned into self-collisions).justfile+scripts/ci(the original adopt.sh: replace the justfile+scripts/ci heuristic with a real idempotency stamp #48 false-positive) - no longer blocked, handled as a normal.forge-suggestedcase.just/dev.justdefinesaudit:(the exact adopt.sh: just recipe-name collisions across files aren't detected #49 scenario) - correctly warns, naming bothjust/analysis.justandjust/dev.just.bash -n adopt.sh- syntax OK.lint-fast+lint-pre-pushgates passed locally on every commit.Closes #48, closes #49.