Repository navigation
Conversation
Bound Git discovery only in the native hook fixture subprocess. Cover plain and broken-ancestor directories and assert bound session input, request sequence, and silent Pi behavior. Focused regression: six leaf cases passed after controlled RED. internal/project suite passed. Full plugin suite still fails TestCodexUnixUserPromptSubmitDetachesPromptPersistenceStdin on Windows; its cause remains unresolved. No full-suite or CI green claim. Refs: Gentleman-Programming#2089
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Fixture isolation and hook assertions plugin/claude_code_pi_guard_test.go |
The test runs plain and broken-Git-ancestor fixtures with bounded Git discovery while retaining inherited Git ceilings. It checks empty stderr, silent output and no requests with the Pi marker, and response fields and ordered session requests without the marker. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Low
Possibly related issues
- Gentleman-Programming/gentle-ai issue 5674: The test checks silent Claude hook behavior when
PI_CODING_AGENTis set, which matches the issue’s suppression objective for other hook entry points.
Possibly related PRs
- Gentleman-Programming/engram#1979: The test exercises native Claude first registration with a fixture
cwdand Git discovery ceiling. - Gentleman-Programming/engram#1954: The test checks silent Claude hook behavior when
PI_CODING_AGENTis set. - Gentleman-Programming/engram#2084: Both changes isolate test fixtures from ambient Git project discovery.
Suggested reviewers: edwinsaavedran, gentleman-programming
Merge Risk: 
ce31e
-
No change-related issue is established; merge after normal validation.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: isolating Claude Pi guard test fixtures from ancestor Git metadata. Linked Issues check Issue #2089requires isolation of the Claude Pi-guard fixtures from unrelated ancestor Git metadata. The diff adds plain and broken-ancestor fixtures, sets a fixture-local GIT_CEILING_DIRECTORIES valu…Out of Scope Changes check The diff changes only plugin/claude_code_pi_guard_test.go. The added fixture setup and response assertions directly support issue #2089. No production code, timeout, dependency, public contract, or un…Full details: Linked Issues check
-
Explanation
Issue
#2089requires isolation of the Claude Pi-guard fixtures from unrelated ancestor Git metadata. The diff adds plain and broken-ancestor fixtures, sets a fixture-local GIT_CEILING_DIRECTORIES value while retaining inherited ceilings, and preserves explicit repositories below the ceiling. It verifies unset and empty PI_CODING_AGENT registration, bound updatedInput, request order, exit status, and empty stderr. It verifies that a nonempty marker produces no output and no requests. The change is limited to the test fixture, so production identity validation is unchanged.
Full details: Out of Scope Changes check
-
Explanation
The diff changes only plugin/claude_code_pi_guard_test.go. The added fixture setup and response assertions directly support issue
#2089. No production code, timeout, dependency, public contract, or unrelated test surface changed.
-
Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
aec0f4f
🔗 Linked Issue
Closes #2089
Lifecycle Evidence
16e06bb0176e060b98ceab08f27dd391ce365b82, which matched remote main when work began.b5c036532c1924888e4c4bdd4305325e16f7c59ebefore publication; this candidate has not been locally integrated with or tested against that newer revision.🏷️ PR Type
type:bug— Bug fixtype:feature— New featuretype:question— Question requiring tracked worktype:docs— Documentation onlytype:refactor— Code refactoringtype:chore— Maintenance, dependencies, toolingtype:breaking-change— Breaking changeThe maintainer authorized applying exactly one GitHub label:
type:bug. Its application was verified on this PR.📝 Summary
📂 Changes
plugin/claude_code_pi_guard_test.goCommit:
ce31e16c1fe2a547c02dc9e4f236ebf17949bc3b(one file, 71 additions and 30 deletions). No production scripts, timeouts, dependencies, or public contracts changed. Local.tmp/diagnostic artifacts are excluded.🧪 Test Plan
Existing evidence, not checks rerun for publication. Local execution used Go 1.26.4 on Windows/amd64.
go test ./plugin -run '^TestClaudeCodeHooksAreNoOpUnderPi/(unset|empty|true)/claude-pre-tool-use$' -count=1 -v— all six final leaf cases passed, package 7.053s.go test ./plugin ./internal/project -count=1—internal/projectpassed, 16.244s; the plugin package failed on missing explicit Windows tool selectors and a separate native-jq CRLF mismatch.go test ./plugin -count=1— FAILED, 388.562s, with one reported failure:TestCodexUnixUserPromptSubmitDetachesPromptPersistenceStdin,codex_unix_user_prompt_test.go:337: prompt persistence did not finish the stdin probe.git diff --cached --checkpassed before committing. The committed fixture blob matches the previously checked candidate.🤖 Automated Checks
All hosted results remain pending until observed on this PR. No hosted green or merge-readiness claim is made. The initial pre-label check failed before
type:bugwas applied. That historical result is retained; the label-event check must establish the current policy result.✅ Contributor Checklist
type:*label —type:bug, explicitly authorized and verified.Co-Authored-Bytrailers.💬 Notes for Reviewers
The Codex stdin-probe failure is on a different test surface, but its root cause and baseline behavior remain unproven. Do not treat it as an established independent Engram bug or as a proven regression from this change.
An isolated path/EOF probe succeeded for native Windows and converted marker paths; it did not reproduce the proposed path mismatch or establish whole-hook dispatch behavior. Two later attempts to prepare fixture-only diagnostic instrumentation failed in our generators before Go ran. Those are diagnostic-tooling failures, not additional Engram runtime failures. Neither attempt produced runtime evidence. No assertions or deadlines were relaxed.
Native assessment returned the RDD-off writer-self-verification plan. Later assessments were unassessable because local untracked diagnostics require declaration; there is no native approval receipt. This draft preserves the verification gap rather than claiming the plugin suite is green.
Risk: none — source changes are limited to this fixture; production repository binding, security guards, state, concurrency, delivery, and dependencies are untouched.
Summary by CodeRabbit