Conversation
added 4 commits
September 18, 2026 07:33
…stead of silently writing cwd (#417) `${CLAUDE_PROJECT_DIR:-}` reads "" both when the variable was never exported (the ordinary, documented `bash scripts/rebuild-tsv.sh` usage, which must keep resolving against $PWD) and when something exported it as an empty string -- a caller that meant to name a tree, computed nothing, and exported the empty result anyway. The #231 cross-tree guard's `[ -n ... ]` precondition could not tell those two apart, so the empty-but-set case took the same silent skip as a genuine unset and reached JIT_BASE's own $PWD fallback (common.sh) with zero refusal and zero note -- observed on a maintainer clone as a live test fixture written into the real .claude/jit-context/ and 12 real index rows lost in the same rebuild. ${CLAUDE_PROJECT_DIR+set} now tells the two apart: an explicitly empty CLAUDE_PROJECT_DIR is refused outright (exit 2, FATAL), and a genuinely unset one still falls through to $PWD exactly as before. Adds sections H (red before the fix, green after) and I (positive control: a genuinely unset CLAUDE_PROJECT_DIR keeps working) to tests/test-cross-tree-write-231.sh, and a changelog.d/417.fixed.md fragment. Co-Authored-By: Max <noreply>
…OJECT_DIR (#417 review) Self-review findings on the #417 fix: - Explore reviewer found that scripts/jit-init.sh's own `--base /.claude/jit-context` case (seeding at the filesystem root) sets PROJECT to the empty string by design (resolve_dir()'s own comment), and then passed it straight through as `CLAUDE_PROJECT_DIR="$PROJECT"` to rebuild-tsv.sh -- exactly the shape the #417 fix now refuses outright. Fixed by deriving `CPD="${PROJECT:-/}"` so the literal root is passed instead of its empty spelling; a fixed-string-pinned regression test was added to tests/test-jit-init.sh (driving the real root-seed path would require writing under the actual filesystem root, which this suite does not do). - Same reviewer flagged that the new refusal's own comment overstated "before anything ever gets a chance to write" -- common.sh's pre-existing #51 scaffolding (.discovery/state, .discovery/logs) can still be created before the check runs, gated on the tree already existing, same as the ordinary legitimate case. Tightened the comment and the changelog fragment to say "before the index is written" rather than "before anything". The oss:auditor spawn returned NO FINDINGS on the original commit. Co-Authored-By: Max <noreply>
…PROJECT (#417 second-pass review) The second-pass review round on b2c089a caught a third site the first fix round missed: scripts/jit-init.sh's success-path receipt ("Then write your own beside it, and rebuild:") still interpolated raw $PROJECT rather than $CPD, because it is an unquoted `echo` interpolation and not an assignment, so the earlier pinning test's assignment-shaped grep could not see it. In the root-seed case (--base /.claude/jit-context, PROJECT=""), a user following this tool's own printed advice would run `CLAUDE_PROJECT_DIR= bash scripts/rebuild-tsv.sh` and hit the very #417 refusal this fix exists to route around. Both Explore and oss:auditor's second-pass spawns found this identically. Fixed by using $CPD at all three sites, and strengthened tests/test-jit-init.sh's pinning assertions to count every CLAUDE_PROJECT_DIR print/export site rather than only the assignment form -- confirmed red against the prior commit (1 CPD use, one raw $PROJECT interpolation) and green now (2 CPD uses, none raw). Co-Authored-By: Max <noreply>
…t-init.sh (#417) tests/test-assertion-helpers.sh's own structural guard ("nothing pipes into an early-exiting reader") correctly flagged a line the #417 second-pass fix added: `grep -F ... | grep -vqF '# '` -- the same #56 ordering trap assert_contains() in tests/test-cross-tree-write-231.sh already avoids (`| grep -q` exits on its first match and the WRITER takes SIGPIPE, so under pipefail a real match can report the opposite of what was found). This failed identically on all three CI platform legs. Fixed by capturing the grep -F output into a variable first and testing it with a here-string, matching the existing convention. Confirmed the assertion still catches the regression it was written for (red against the b2c089a-era jit-init.sh, green against the current one), and confirmed directly against test-assertion-helpers.sh's own scan_file() awk pattern that the rewritten line no longer matches. Co-Authored-By: Max <noreply>
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.
rebuild-tsv.shnow refuses (exit 2) whenCLAUDE_PROJECT_DIRis exported empty, instead of silently falling through to$PWDand rebuilding whatever tree the shell happens to be standing in.The bug
${CLAUDE_PROJECT_DIR:-}reads""both when the variable was never exported (the ordinary, documentedbash scripts/rebuild-tsv.shcase, which must keep resolving against$PWD) and when something exported it as an empty string -- a caller that meant to name a tree, computed nothing, and exported the empty result anyway. The#231cross-tree guard's own[ -n ... ]precondition could not tell those two apart, so the empty-but-set case took the same silent skip as a genuine unset and reachedJIT_BASE's own$PWDfallback (common.sh) with zero refusal and zero note.Observed on a maintainer clone: a tick running three unrelated lanes left a live test fixture (
tools/00-manual/guard.md, aremind-mode rule matchinggit push) written into the real.claude/jit-context/, plus 12 real index rows lost in the same rebuild. Any session rooted at the clone in that window would have had the fixture rule fire ongit push, and the 12 lost rows went inert with no error anywhere.The fix
${CLAUDE_PROJECT_DIR+set}tells the two cases apart -- it readssetwhenever the variable was exported at all, blank value included, andunsetonly when nothing ever touched it. An explicitly-emptyCLAUDE_PROJECT_DIRis now refused outright (FATAL, exit 2) before the index write ever runs; a genuinely unset one still falls through to$PWDexactly as before.jit-init.sh's own--base /.claude/jit-contextcase (seeding at the filesystem root) producesCLAUDE_PROJECT_DIR=""by design --PROJECTis documented as the empty string for that case. It now derivesCPD="${PROJECT:-/}"and passes"/"literally at every site that prints or exportsCLAUDE_PROJECT_DIR, so that legitimate call keeps working under the tightened guard rather than hitting the new refusal (caught in two review rounds, including a third site -- the success-path receipt -- missed by the first round's own pinning test).Testing
tests/test-cross-tree-write-231.shgained two sections: H pins the new refusal (exit 2, stderr names the reason, tree not written), confirmed red before the fix and green after; I is a positive control confirming a genuinely-unsetCLAUDE_PROJECT_DIRstill works exactly as before, passing both before and after (proves the fix is additive).tests/test-jit-init.shgained a regression test pinning everyCLAUDE_PROJECT_DIRprint/export site injit-init.shto the$CPDfallback via fixed-string grep, and asserting no raw$PROJECTinterpolation remains outside a comment.Ran the changelog fragment checker (
python3 .oss/assemble_changelog.py --check) and the sibling suites that callrebuild-tsv.shwith realCLAUDE_PROJECT_DIRvalues -- all green. Did not run the fullbash tests/run-all.sh; CI is the authority for the whole matrix.Out of scope
scripts/jit-doctor.shandcommon.sh'sjit_worktree_mismatch_line()(shared by every hook) have the same unset-vs-set-empty ambiguity in their own diagnostic prose -- reporting "unset" for what may actually be set-but-empty. Left for a follow-up issue: a different file/subsystem (read-only diagnostic, not a writer) and shared logic used by every hook, out of this issue's declared blast radius.Closes #417
[AI-generated]