Repository navigation
0.3.2: fix comment-guard false positives (DEV-28/41/43/49, DEV-57) - #1
Merged
Merged
Conversation
…guard - Regex literals with `/` no longer misdetected as `//` comments (DEV-41). - Bare-token HTML comments (`<!--app-head-->`) are template placeholders, not prose, and are exempted; prose HTML comments still count (DEV-41). - Bash commands that are entirely git checkout/switch/mv/restore/merge/ rebase/cherry-pick/pull/reset/stash apply|pop skip the snapshot/audit entirely, since they change working-tree content without the agent authoring anything (DEV-43). - A Write to a brand-new file is no longer denied outright for any comment: if every added comment line's text also exists elsewhere in the repo's HEAD tree, it's treated as moved, not authored (DEV-28). - Edit/Write/MultiEdit no longer deny restoring a comment that still exists in the file's HEAD-committed blob (DEV-49). 150 tests passing (21 new). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EAvmyrEwiqgQhbaA3w4TsM
…ests PR #1's Windows/macOS CI runs intermittently failed the new "restoring a comment still present in HEAD is allowed" test with the hook denying instead of allowing. Root cause: node --test runs test files (bash.test.mjs, hook.test.mjs) concurrently by default, and both spin up many temporary git repos; under CI/AV load the git subprocess timeout in gitOut() (4s) was occasionally exceeded, making headBlobFor() silently fail (fails-open to the deny path). Raised the timeout to 10s (matching the existing snapshot-audit timeout) and pinned test-concurrency=1 to remove the concurrent-git-process contention this exposed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EAvmyrEwiqgQhbaA3w4TsM
git show HEAD:<path> returns CRLF-converted text on Windows/macOS runners when core.autocrlf is set, which never matched the LF-only edit text compared against it in relaxForMovedOrRestoredComments. This deterministically failed the "restoring a comment still present in HEAD is allowed" test on windows-latest/macos-latest CI while passing on ubuntu-latest. Reuse the existing normalise() helper (already used for the same class of problem elsewhere in this file). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EAvmyrEwiqgQhbaA3w4TsM
GitHub Actions Windows runners expose the temp directory through an 8.3 short-name path segment (RUNNER~1) in some contexts. If git's rev-parse --show-toplevel and Node's resolve(path) disagree on long vs short form for the same directory, relative() computes garbage, git show fails, headBlobFor returns null, and the restore-from-HEAD relaxation never engages. Route both sides through realpathSync (falling back to the original value if it throws) so they agree. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EAvmyrEwiqgQhbaA3w4TsM
Diagnostic only, to be reverted once the root cause of the Windows-only 'restoring a comment still present in HEAD' failure is confirmed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EAvmyrEwiqgQhbaA3w4TsM
…h math Root cause confirmed via a temporary CI debug step (now reverted): on GitHub's windows-latest runner, mkdtempSync()/resolve() return the temp path through its 8.3 short name (C:\Users\RUNNER~1\...), while git rev-parse --show-toplevel returns the canonicalized long form (C:/Users/runneradmin/...). Node's realpathSync does not expand Windows 8.3 short names, so relative(root, resolve(path)) produced a bogus path that fell outside the repo, git show failed, and the restore-from-HEAD relaxation silently never engaged. Fix: never mix a Node-resolved path with a git-resolved one. Ask git itself for the file's repo-relative prefix (git rev-parse --show-prefix, run from the file's own directory) and append the basename, so the whole computation stays inside git's own path resolution. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EAvmyrEwiqgQhbaA3w4TsM
5 tasks done
The post-Bash audit diffed the pre-command git-tree snapshot against whatever the cwd's working tree looked like afterward. If a command cd'd into a different worktree (linked worktrees share the object database, so the diff resolves without error), or ran git checkout/fetch mixed with a non-plumbing verb like git status (falling outside the existing plumbing-only fast path), the audit compared unrelated trees and blamed the command for pre-existing, human-authored comments it never touched. Track the git root and HEAD at snapshot time and skip the audit (logging skip-tree-moved, not blocking) if either has moved by post-command time. A same-repo, same-HEAD genuine comment addition still blocks as before. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EAvmyrEwiqgQhbaA3w4TsM
TAJD
changed the title
2 tasks done
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
/no longer misdetected as//comments; bare-token HTML comments (<!--app-head-->) treated as placeholders, not prose.Writeto a brand-new file is no longer denied outright for any comment — if every added comment line's text exists elsewhere in the repo's HEAD tree, it's treated as moved.Edit/Write/MultiEditno longer deny restoring a comment still present in the file's HEAD-committed blob.skip-tree-movedlog line) if either has moved by the time the command finishes. This catches two false-positive classes DEV-43's plumbing-only fast path missed: (1) a command thatcds into a different worktree (linked worktrees share the object database, so the cross-worktree diff resolves without erroring and blames that worktree's pre-existing comments on the command), and (2) a compound command mixinggit checkout/fetchwith a non-plumbing verb likegit status, which isn't "plumbing-only" so previously fell through to a full audit against the new HEAD. A same-repo, same-HEAD genuine comment addition still blocks as before — verified with a dedicated regression test.Test plan
npm test— 153/153 passing (3 new DEV-57 regression tests: cross-worktree cd suppressed, mixed-command HEAD-move suppressed, genuine same-HEAD addition still blocks)HUMAN CHECK
npm testfrom a checkout of this branch — expect# pass 153,# fail 0.REGEX_LITERAL_RE) is a best-effort lookbehind, not a real JS parser — it can misclassify an unusual division expression as a regex literal in rare cases (this only risks a false negative, not a new false-positive block).git revert 218f4f4— it's a single self-contained commit on top of 0.3.1's fixes. No hook config or settings.json changes are involved, so reverting only requires updating this plugin's installed version.🤖 Generated with Claude Code
https://claude.ai/code/session_01EAvmyrEwiqgQhbaA3w4TsM