mirror of
https://github.com/thedotmack/claude-mem
synced 2026-04-25 17:15:04 +02:00
fix: Claude Code 2.1.1 compatibility + log-level audit + path validation fixes (#614)
* Refactor CLAUDE.md and related files for December 2025 updates - Updated CLAUDE.md in src/services/worker with new entries for December 2025, including changes to Search.ts, GeminiAgent.ts, SDKAgent.ts, and SessionManager.ts. - Revised CLAUDE.md in src/shared to reflect updates and new entries for December 2025, including paths.ts and worker-utils.ts. - Modified hook-constants.ts to clarify exit codes and their behaviors. - Added comprehensive hooks reference documentation for Claude Code, detailing usage, events, and examples. - Created initial CLAUDE.md files in various directories to track recent activity. * fix: Merge user-message-hook output into context-hook hookSpecificOutput - Add footer message to additionalContext in context-hook.ts - Remove user-message-hook from SessionStart hooks array - Fixes issue where stderr+exit(1) approach was silently discarded Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Update logs and documentation for recent plugin and worker service changes - Added detailed logs for worker service activities from Dec 10, 2025 to Jan 7, 2026, including initialization patterns, cleanup confirmations, and diagnostic logging. - Updated plugin documentation with recent activities, including plugin synchronization and configuration changes from Dec 3, 2025 to Jan 7, 2026. - Enhanced the context hook and worker service logs to reflect improvements and fixes in the plugin architecture. - Documented the migration and verification processes for the Claude memory system and its integration with the marketplace. * Refactor hooks architecture and remove deprecated user-message-hook - Updated hook configurations in CLAUDE.md and hooks.json to reflect changes in session start behavior. - Removed user-message-hook functionality as it is no longer utilized in Claude Code 2.1.0; context is now injected silently. - Enhanced context-hook to handle session context injection without user-visible messages. - Cleaned up documentation across multiple files to align with the new hook structure and removed references to obsolete hooks. - Adjusted timing and command execution for hooks to improve performance and reliability. * fix: Address PR #610 review issues - Replace USER_MESSAGE_ONLY test with BLOCKING_ERROR test in hook-constants.test.ts - Standardize Claude Code 2.1.0 note wording across all three documentation files - Exclude deprecated user-message-hook.ts from logger-usage-standards test Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Remove hardcoded fake token counts from context injection Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Address PR #610 review issues by fixing test files, standardizing documentation notes, and verifying code quality improvements. * fix: Add path validation to CLAUDE.md distribution to prevent invalid directory creation - Add isValidPathForClaudeMd() function to reject invalid paths: - Tilde paths (~) that Node.js doesn't expand - URLs (http://, https://) - Paths with spaces (likely command text or PR references) - Paths with # (GitHub issue/PR references) - Relative paths that escape project boundary - Integrate validation in updateFolderClaudeMdFiles loop - Add 6 unit tests for path validation - Update .gitignore to prevent accidental commit of malformed directories - Clean up existing invalid directories (~/, PR #610..., git diff..., https:) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: Implement path validation in CLAUDE.md generation to prevent invalid directory creation - Added `isValidPathForClaudeMd()` function to validate file paths in `src/utils/claude-md-utils.ts`. - Integrated path validation in `updateFolderClaudeMdFiles` to skip invalid paths. - Added 6 new unit tests in `tests/utils/claude-md-utils.test.ts` to cover various rejection cases. - Updated `.gitignore` to prevent tracking of invalid directories. - Cleaned up existing invalid directories in the repository. * feat: Promote critical WARN logs to ERROR level across codebase Comprehensive log-level audit promoting 38+ WARN messages to ERROR for improved debugging and incident response: - Parser: observation type errors, data contamination - SDK/Agents: empty init responses (Gemini, OpenRouter) - Worker/Queue: session recovery, auto-recovery failures - Chroma: sync failures, search failures (now treated as critical) - SQLite: search failures (primary data store) - Session/Generator: failures, missing context - Infrastructure: shutdown, process management failures - File Operations: CLAUDE.md updates, config reads - Branch Management: recovery checkout failures Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: Address PR #614 review issues - Remove incorrectly tracked tilde-prefixed files from git - Fix absolute path validation to check projectRoot boundaries - Add test coverage for absolute path validation edge cases Closes review issues: - Issue 1: ~/ prefixed files removed from tracking - Issue 3: Absolute paths now validated against projectRoot - Issue 4: Added 3 new test cases for absolute path scenarios Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * build assets and context --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -3,169 +3,5 @@
|
||||
|
||||
<!-- This section is auto-generated by claude-mem. Edit content outside the tags. -->
|
||||
|
||||
### Dec 26, 2025
|
||||
|
||||
**adaptive-orbiting-sundae.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #32927 | 10:32 PM | ⚖️ | Three-Phase Fix Plan Created for PR #448 OpenRouter Error Handling Issues | ~543 |
|
||||
|
||||
**floating-petting-snowglobe.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #32915 | 10:08 PM | 🔵 | OpenRouter Provider PR #448 Fix Plan Loaded | ~345 |
|
||||
|
||||
### Dec 27, 2025
|
||||
|
||||
**greedy-puzzling-teapot.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #33132 | 7:53 PM | ⚖️ | Session Continuity Regression Investigation and Fix Plan Created | ~567 |
|
||||
|
||||
**parallel-snuggling-kazoo.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #33008 | 4:54 PM | ⚖️ | Auto-Restart Worker Strategy After Plugin Updates | ~514 |
|
||||
|
||||
**fancy-leaping-orbit.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #32999 | 4:43 PM | ⚖️ | Worker Update Strategy Changed from Auto-Restart to Manual | ~265 |
|
||||
| #32997 | 4:42 PM | ✅ | Implementation Plan Documented for Windows Concurrency Fix | ~597 |
|
||||
|
||||
### Dec 28, 2025
|
||||
|
||||
**scalable-exploring-wadler.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #33597 | 11:14 PM | 🔵 | MCP search tool successfully retrieving mem-search to mcp-search rename history | ~361 |
|
||||
|
||||
**pr-458-fixes.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #33291 | 3:08 PM | ✅ | Completed PR #458 Worker Lifecycle Fix Plan - Phase 6 Build Verification | ~334 |
|
||||
| #33275 | 3:01 PM | 🔴 | Unix orphaned process cleanup error handling fixed | ~330 |
|
||||
| #33274 | " | 🔴 | Fixed waitForProcessesExit crash when child processes exit | ~312 |
|
||||
| #33273 | " | 🔴 | Added PID validation after daemon spawn to prevent invalid PID file writes | ~341 |
|
||||
| #33272 | " | 🔴 | Windows taskkill error handling in orphaned process cleanup | ~266 |
|
||||
|
||||
**mellow-discovering-meerkat.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #33262 | 2:58 PM | ✅ | PR #456 Closed and Fresh Branch Created for Worker Lifecycle Fix | ~313 |
|
||||
|
||||
**functional-shimmying-lake.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #33257 | 2:37 PM | ✅ | Plan Simplified to Minimal Fix Approach | ~320 |
|
||||
|
||||
### Dec 29, 2025
|
||||
|
||||
**worker-executable.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #33895 | 5:13 PM | 🟣 | Implemented install-time worker executable compilation | ~423 |
|
||||
| #33883 | 5:11 PM | ⚖️ | Plan created for worker executable compilation strategy | ~432 |
|
||||
|
||||
**phased-worker-executable-plan.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #33873 | 4:56 PM | ⚖️ | Worker Service Migration to Standalone Executables | ~531 |
|
||||
|
||||
**precious-dancing-rossum.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #33872 | 4:56 PM | ⚖️ | Build-time executable strategy replaced with install-time compilation approach | ~524 |
|
||||
| #33870 | 4:52 PM | ⚖️ | Migrate Worker Service from Runtime .cjs to Compiled Platform Executables | ~567 |
|
||||
|
||||
### Dec 30, 2025
|
||||
|
||||
**lively-twirling-garden.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #34411 | 1:55 PM | ⚖️ | Agent SDK V2 Migration Plan Created | ~519 |
|
||||
|
||||
### Jan 1, 2026
|
||||
|
||||
**cozy-orbiting-hopper.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #35485 | 9:06 PM | ⚖️ | Comprehensive error handling remediation plan completed and submitted for approval | ~555 |
|
||||
| #35484 | 9:05 PM | ✅ | Quick reference checklist added for all 45 critical issues | ~390 |
|
||||
| #35483 | " | ✅ | Plan implementation section replaced with practical execution guidance | ~375 |
|
||||
| #35482 | " | ✅ | Verification progress metrics updated with accurate wave counts | ~313 |
|
||||
| #35481 | " | ✅ | Wave 3 Group 3C scope reduced after code verification | ~341 |
|
||||
| #35480 | " | ✅ | Wave 2 specific fixes detailed with exact code patterns | ~401 |
|
||||
| #35479 | 9:04 PM | ✅ | Wave 2 fix pattern updated to target worst offender | ~363 |
|
||||
| #35478 | " | ✅ | Plan updated with accurate Wave 2 critical path failures | ~424 |
|
||||
| #35471 | 9:02 PM | ⚖️ | Systematic plan created to fix 45 critical error handling anti-patterns | ~540 |
|
||||
|
||||
**ticklish-humming-pixel.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #35296 | 1:25 PM | ⚖️ | Revised implementation plan: Generate deterministic memory session IDs to prevent orphaned sessions | ~619 |
|
||||
| #35295 | " | ⚖️ | Updated Implementation Plan - Deterministic Session ID with Immediate Persistence | ~505 |
|
||||
| #35251 | 1:14 PM | ✅ | Plan updated with correct import paths for provider check functions | ~301 |
|
||||
| #35246 | " | ⚖️ | Implementation plan created: Add provider check to startup-recovery mechanism | ~525 |
|
||||
|
||||
### Jan 2, 2026
|
||||
|
||||
**gleaming-imagining-cray.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #36104 | 10:31 PM | ⚖️ | Database Transaction Strategy for Observation Deduplication | ~474 |
|
||||
|
||||
### Jan 3, 2026
|
||||
|
||||
**nifty-honking-goblet.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #36644 | 10:59 PM | ✅ | Test Fixes Plan Updated with Logger Coverage Implementation Details | ~469 |
|
||||
|
||||
**glimmering-coalescing-pnueli.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #36160 | 6:41 PM | ⚖️ | Final Architecture Approved: Grep-Optimized Progressive Disclosure with Named Re-Exports | ~757 |
|
||||
| #36159 | 6:39 PM | ⚖️ | Phase 1 Refactor Plan Finalized: Grep-Optimized Progressive Disclosure with Named Re-Exports | ~830 |
|
||||
|
||||
### Jan 4, 2026
|
||||
|
||||
**greedy-soaring-sphinx.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #36762 | 12:37 AM | ✅ | Created Implementation Plan for Four GitHub Issues | ~438 |
|
||||
|
||||
### Jan 5, 2026
|
||||
|
||||
**CLAUDE.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #38078 | 9:54 PM | ✅ | CLAUDE.md Documentation Cleanup - 1,233 Lines Removed Across 18 Files | ~590 |
|
||||
|
||||
**fix-claudemd-worktree-bug.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #38005 | 9:03 PM | 🔵 | Comprehensive exploration of PR review items completed | ~438 |
|
||||
| #38004 | " | 🔵 | Prior plan documented worktree bug fix with projectRoot parameter | ~416 |
|
||||
|
||||
**velvety-churning-globe.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #37763 | 6:32 PM | 🔵 | Comprehensive test suite cleanup plan for claude-mem project | ~446 |
|
||||
| #37762 | 6:31 PM | ⚖️ | Test Cleanup Plan Approved: Delete 6 Harmful Tests, Add 4 Integration Tests | ~793 |
|
||||
| #37761 | 6:26 PM | ✅ | Test Cleanup Plan Updated with Detailed File Lists and Impact Summary | ~643 |
|
||||
| #37760 | " | ⚖️ | Test Plan Expanded: Four New Integration Tests Added to Cleanup Scope | ~500 |
|
||||
| #37759 | 6:25 PM | ⚖️ | Test Suite Cleanup Plan Created Based on Comprehensive Audit | ~697 |
|
||||
| #37757 | 6:22 PM | ⚖️ | Test Suite Cleanup Plan Created Based on Audit Findings | ~640 |
|
||||
|
||||
**2026-01-05-fix-81-test-failures.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #37704 | 6:05 PM | ⚖️ | Plan Abandoned: Mock Logger Approach Rejected for Test Quality Audit | ~312 |
|
||||
| #37699 | 5:59 PM | 🔵 | Test Failure Root Cause: Incomplete Logger Mocks Polluting Module Cache | ~455 |
|
||||
|
||||
**2026-01-05-github-issues-fix-plan.md**
|
||||
| ID | Time | T | Title | Read |
|
||||
|----|------|---|-------|------|
|
||||
| #37559 | 4:49 PM | 🔵 | GitHub Issues Fix Plan Loaded | ~428 |
|
||||
*No recent activity*
|
||||
</claude-mem-context>
|
||||
262
.claude/plans/claude-md-path-validation-fix.md
Normal file
262
.claude/plans/claude-md-path-validation-fix.md
Normal file
@@ -0,0 +1,262 @@
|
||||
# CLAUDE.md Path Validation Bug Fix
|
||||
|
||||
## Problem Summary
|
||||
|
||||
Claude-Mem 9.0's distributed CLAUDE.md feature has a **critical path validation bug** that creates invalid directories when Claude SDK agent outputs non-path strings in file tracking XML tags (`<files_read>`, `<files_modified>`).
|
||||
|
||||
### Root Cause
|
||||
|
||||
In `src/utils/claude-md-utils.ts:234-239`:
|
||||
```typescript
|
||||
if (projectRoot && !path.isAbsolute(filePath)) {
|
||||
absoluteFilePath = path.join(projectRoot, filePath);
|
||||
}
|
||||
```
|
||||
|
||||
- `path.isAbsolute('~/.claude-mem/logs')` returns `false` (Node.js doesn't recognize `~`)
|
||||
- Code joins: `path.join(projectRoot, '~/.claude-mem/logs')` → `/project/~/.claude-mem/logs`
|
||||
- `mkdirSync` creates literal directories
|
||||
|
||||
### Invalid Directories Currently in Repo
|
||||
|
||||
```
|
||||
./~/ ← literal tilde directory
|
||||
./PR #610 on thedotmack/ ← GitHub PR reference
|
||||
./git diff for src/ ← git command text
|
||||
./https:/code.claude.com/docs/en/ ← URL
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
### Phase 1: Add Path Validation Function
|
||||
|
||||
**File:** `src/utils/claude-md-utils.ts`
|
||||
|
||||
Add new validation function after the imports (around line 16):
|
||||
|
||||
```typescript
|
||||
/**
|
||||
* Validate that a file path is safe for CLAUDE.md generation.
|
||||
* Rejects tilde paths, URLs, command-like strings, and paths with invalid chars.
|
||||
*
|
||||
* @param filePath - The file path to validate
|
||||
* @param projectRoot - Optional project root for boundary checking
|
||||
* @returns true if path is valid for CLAUDE.md processing
|
||||
*/
|
||||
function isValidPathForClaudeMd(filePath: string, projectRoot?: string): boolean {
|
||||
// Reject empty or whitespace-only
|
||||
if (!filePath || !filePath.trim()) return false;
|
||||
|
||||
// Reject tilde paths (Node.js doesn't expand ~)
|
||||
if (filePath.startsWith('~')) return false;
|
||||
|
||||
// Reject URLs
|
||||
if (filePath.startsWith('http://') || filePath.startsWith('https://')) return false;
|
||||
|
||||
// Reject paths with spaces (likely command text or PR references)
|
||||
if (filePath.includes(' ')) return false;
|
||||
|
||||
// Reject paths with # (GitHub issue/PR references)
|
||||
if (filePath.includes('#')) return false;
|
||||
|
||||
// If projectRoot provided, ensure resolved path stays within project
|
||||
if (projectRoot) {
|
||||
const resolved = path.resolve(projectRoot, filePath);
|
||||
const normalizedRoot = path.resolve(projectRoot);
|
||||
if (!resolved.startsWith(normalizedRoot + path.sep) && resolved !== normalizedRoot) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
```
|
||||
|
||||
### Phase 2: Integrate Validation in updateFolderClaudeMdFiles
|
||||
|
||||
**File:** `src/utils/claude-md-utils.ts`
|
||||
|
||||
Modify the file path loop in `updateFolderClaudeMdFiles` (around line 232):
|
||||
|
||||
```typescript
|
||||
for (const filePath of filePaths) {
|
||||
if (!filePath || filePath === '') continue;
|
||||
|
||||
// VALIDATE PATH BEFORE PROCESSING
|
||||
if (!isValidPathForClaudeMd(filePath, projectRoot)) {
|
||||
logger.debug('FOLDER_INDEX', 'Skipping invalid file path', {
|
||||
filePath,
|
||||
reason: 'Failed path validation'
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
// ... rest of existing logic unchanged
|
||||
}
|
||||
```
|
||||
|
||||
### Phase 3: Add Unit Tests
|
||||
|
||||
**File:** `tests/utils/claude-md-utils.test.ts`
|
||||
|
||||
Add new test block after existing tests:
|
||||
|
||||
```typescript
|
||||
describe('path validation in updateFolderClaudeMdFiles', () => {
|
||||
it('should reject tilde paths', async () => {
|
||||
const fetchMock = mock(() => Promise.resolve({ ok: true } as Response));
|
||||
global.fetch = fetchMock;
|
||||
|
||||
await updateFolderClaudeMdFiles(
|
||||
['~/.claude-mem/logs/worker.log'],
|
||||
'test-project',
|
||||
37777,
|
||||
tempDir
|
||||
);
|
||||
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should reject URLs', async () => {
|
||||
const fetchMock = mock(() => Promise.resolve({ ok: true } as Response));
|
||||
global.fetch = fetchMock;
|
||||
|
||||
await updateFolderClaudeMdFiles(
|
||||
['https://example.com/file.ts'],
|
||||
'test-project',
|
||||
37777,
|
||||
tempDir
|
||||
);
|
||||
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should reject paths with spaces', async () => {
|
||||
const fetchMock = mock(() => Promise.resolve({ ok: true } as Response));
|
||||
global.fetch = fetchMock;
|
||||
|
||||
await updateFolderClaudeMdFiles(
|
||||
['PR #610 on thedotmack/CLAUDE.md'],
|
||||
'test-project',
|
||||
37777,
|
||||
tempDir
|
||||
);
|
||||
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should reject paths with hash symbols', async () => {
|
||||
const fetchMock = mock(() => Promise.resolve({ ok: true } as Response));
|
||||
global.fetch = fetchMock;
|
||||
|
||||
await updateFolderClaudeMdFiles(
|
||||
['issue#123/file.ts'],
|
||||
'test-project',
|
||||
37777,
|
||||
tempDir
|
||||
);
|
||||
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should reject path traversal outside project', async () => {
|
||||
const fetchMock = mock(() => Promise.resolve({ ok: true } as Response));
|
||||
global.fetch = fetchMock;
|
||||
|
||||
await updateFolderClaudeMdFiles(
|
||||
['../../../etc/passwd'],
|
||||
'test-project',
|
||||
37777,
|
||||
tempDir
|
||||
);
|
||||
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should accept valid relative paths', async () => {
|
||||
const apiResponse = {
|
||||
content: [{ text: '| #123 | 4:30 PM | 🔵 | Test | ~100 |' }]
|
||||
};
|
||||
const fetchMock = mock(() => Promise.resolve({
|
||||
ok: true,
|
||||
json: () => Promise.resolve(apiResponse)
|
||||
} as Response));
|
||||
global.fetch = fetchMock;
|
||||
|
||||
await updateFolderClaudeMdFiles(
|
||||
['src/utils/logger.ts'],
|
||||
'test-project',
|
||||
37777,
|
||||
tempDir
|
||||
);
|
||||
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
### Phase 4: Update .gitignore
|
||||
|
||||
**File:** `.gitignore`
|
||||
|
||||
Add at end of file:
|
||||
|
||||
```gitignore
|
||||
# Prevent literal tilde directories (path validation bug artifacts)
|
||||
~*/
|
||||
|
||||
# Prevent other malformed path directories
|
||||
http*/
|
||||
https*/
|
||||
```
|
||||
|
||||
### Phase 5: Clean Up Invalid Directories
|
||||
|
||||
**Command sequence:**
|
||||
```bash
|
||||
rm -rf "~/."
|
||||
rm -rf "PR #610 on thedotmack"
|
||||
rm -rf "git diff for src"
|
||||
rm -rf "https:"
|
||||
```
|
||||
|
||||
### Phase 6: Verify and Commit
|
||||
|
||||
1. Run test suite: `npm test`
|
||||
2. Run build: `npm run build`
|
||||
3. Verify no invalid directories remain
|
||||
4. Commit with message: `fix: Add path validation to CLAUDE.md distribution to prevent invalid directory creation`
|
||||
|
||||
---
|
||||
|
||||
## Files Modified
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `src/utils/claude-md-utils.ts` | Add `isValidPathForClaudeMd()` function + integrate in loop |
|
||||
| `tests/utils/claude-md-utils.test.ts` | Add 6 new path validation tests |
|
||||
| `.gitignore` | Add `~*/`, `http*/`, `https*/` patterns |
|
||||
|
||||
## Files Deleted
|
||||
|
||||
| Path | Reason |
|
||||
|------|--------|
|
||||
| `~/` (directory tree) | Invalid literal tilde directory |
|
||||
| `PR #610 on thedotmack/` | Invalid PR reference directory |
|
||||
| `git diff for src/` | Invalid git command directory |
|
||||
| `https:/` | Invalid URL directory |
|
||||
|
||||
---
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
**Low Risk:**
|
||||
- Validation is additive (only skips invalid paths, doesn't change valid path handling)
|
||||
- Existing tests remain unchanged
|
||||
- Fire-and-forget design means failures are logged but don't break hooks
|
||||
|
||||
**Testing Coverage:**
|
||||
- 6 new unit tests covering all rejection cases
|
||||
- Existing 27 tests verify valid path behavior unchanged
|
||||
144
.claude/plans/pr-610-review-fixes.md
Normal file
144
.claude/plans/pr-610-review-fixes.md
Normal file
@@ -0,0 +1,144 @@
|
||||
# Plan: Address PR #610 Review Issues
|
||||
|
||||
## Overview
|
||||
This plan addresses the issues identified in the PR review for PR #610 "fix: Update hooks for Claude Code 2.1.0/1 - SessionStart no longer shows user messages".
|
||||
|
||||
## Phase 0: Verification and Discovery
|
||||
|
||||
### 0.1 Verify Test Failure
|
||||
- **File**: `tests/hook-constants.test.ts`
|
||||
- **Issue**: Lines 61-63 test for `HOOK_EXIT_CODES.USER_MESSAGE_ONLY` which was removed
|
||||
- **Verification**: Run `bun test tests/hook-constants.test.ts` to confirm failure
|
||||
|
||||
### 0.2 Verify No Code References USER_MESSAGE_ONLY
|
||||
- **Finding**: Grep found references only in:
|
||||
- `tests/hook-constants.test.ts` (test file - needs fix)
|
||||
- `src/services/CLAUDE.md` (memory context - auto-generated, not code)
|
||||
- `plugin/scripts/CLAUDE.md` (memory context - auto-generated, not code)
|
||||
- **Conclusion**: Only the test file needs updating; CLAUDE.md files are memory records
|
||||
|
||||
### 0.3 Verify CLAUDE.md Files Are Legitimate
|
||||
- **Clarification**: The PR reviewer mentioned "user-specific CLAUDE.md files starting with ~/"
|
||||
- **Finding**: All CLAUDE.md files in the commit are within the repository (`docs/`, `src/`, `plugin/`)
|
||||
- **Conclusion**: These are legitimate in-repo context files, not user-specific paths
|
||||
|
||||
---
|
||||
|
||||
## Phase 1: Fix Test File (REQUIRED)
|
||||
|
||||
### Task 1.1: Remove USER_MESSAGE_ONLY Test
|
||||
**File**: `tests/hook-constants.test.ts`
|
||||
**Action**: Delete lines 61-63 that test for the removed constant
|
||||
|
||||
```typescript
|
||||
// DELETE THESE LINES:
|
||||
it('should define USER_MESSAGE_ONLY exit code', () => {
|
||||
expect(HOOK_EXIT_CODES.USER_MESSAGE_ONLY).toBe(3);
|
||||
});
|
||||
```
|
||||
|
||||
### Task 1.2: Add Test for BLOCKING_ERROR
|
||||
**File**: `tests/hook-constants.test.ts`
|
||||
**Action**: Add test for the new `BLOCKING_ERROR` constant (exit code 2) that replaced it
|
||||
|
||||
```typescript
|
||||
// ADD THIS TEST:
|
||||
it('should define BLOCKING_ERROR exit code', () => {
|
||||
expect(HOOK_EXIT_CODES.BLOCKING_ERROR).toBe(2);
|
||||
});
|
||||
```
|
||||
|
||||
### Verification
|
||||
- Run `bun test tests/hook-constants.test.ts`
|
||||
- Expect: All tests pass
|
||||
|
||||
---
|
||||
|
||||
## Phase 2: Documentation Consistency (NICE TO HAVE)
|
||||
|
||||
### Issue
|
||||
Three similar notes about Claude Code 2.1.0 have slightly different wording:
|
||||
|
||||
1. `docs/public/architecture/hooks.mdx:254`:
|
||||
> "SessionStart hooks no longer display any user-visible messages. Context is still injected via `hookSpecificOutput.additionalContext` but users don't see startup output in the UI."
|
||||
|
||||
2. `docs/public/hooks-architecture.mdx:31`:
|
||||
> "SessionStart hooks no longer display any user-visible messages. Context is silently injected via `hookSpecificOutput.additionalContext`."
|
||||
|
||||
3. `docs/public/hooks-architecture.mdx:441`:
|
||||
> "SessionStart hooks output is never displayed to users. Context is injected silently via `hookSpecificOutput.additionalContext`."
|
||||
|
||||
### Task 2.1: Standardize Note Wording
|
||||
**Action**: Use consistent wording across all three locations
|
||||
|
||||
**Standard text**:
|
||||
```
|
||||
As of Claude Code 2.1.0 (ultrathink update), SessionStart hooks no longer display user-visible messages. Context is silently injected via `hookSpecificOutput.additionalContext`.
|
||||
```
|
||||
|
||||
### Files to Update
|
||||
1. `docs/public/architecture/hooks.mdx:253-255` - Update Note block
|
||||
2. `docs/public/hooks-architecture.mdx:30-32` - Update Note block
|
||||
3. `docs/public/hooks-architecture.mdx:440-442` - Update Note block
|
||||
|
||||
### Verification
|
||||
- Grep for the standard text in all three files
|
||||
- Visual review of documentation
|
||||
|
||||
---
|
||||
|
||||
## Phase 3: Code Quality Improvements (OPTIONAL)
|
||||
|
||||
### Issue 3.1: Hardcoded Promotional Message
|
||||
**File**: `src/hooks/context-hook.ts:66-68`
|
||||
**Current code**:
|
||||
```typescript
|
||||
const enhancedContext = `${text}
|
||||
|
||||
Access 300k tokens of past research & decisions for just 19,008t. Use MCP search tools to access memories by ID.`;
|
||||
```
|
||||
|
||||
### Options
|
||||
1. **Leave as-is**: The token count is a rough estimate and doesn't need to be exact
|
||||
2. **Make configurable**: Add to settings (over-engineering for this use case)
|
||||
3. **Remove hardcoded numbers**: Use relative language instead
|
||||
|
||||
### Recommendation
|
||||
Leave as-is for now. The token counts are marketing copy, not critical functionality. Creating a PR just for this adds unnecessary complexity.
|
||||
|
||||
---
|
||||
|
||||
## Phase 4: Final Verification
|
||||
|
||||
### 4.1 Run Full Test Suite
|
||||
```bash
|
||||
bun test
|
||||
```
|
||||
|
||||
### 4.2 Build Verification
|
||||
```bash
|
||||
npm run build
|
||||
```
|
||||
|
||||
### 4.3 Grep Verification
|
||||
```bash
|
||||
grep -r "USER_MESSAGE_ONLY" src/ --include="*.ts" --include="*.js"
|
||||
```
|
||||
Expected: No results (CLAUDE.md files excluded as they're memory records)
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
| Phase | Priority | Effort | Description |
|
||||
|-------|----------|--------|-------------|
|
||||
| 1 | REQUIRED | 5 min | Fix test file - remove USER_MESSAGE_ONLY test, add BLOCKING_ERROR test |
|
||||
| 2 | Nice to have | 10 min | Standardize documentation note wording |
|
||||
| 3 | Skip | - | Hardcoded token counts are fine as-is |
|
||||
| 4 | REQUIRED | 5 min | Run tests and build to verify |
|
||||
|
||||
## Expected Outcome
|
||||
- All tests pass
|
||||
- Build succeeds
|
||||
- No code references to removed USER_MESSAGE_ONLY constant
|
||||
- Documentation uses consistent wording (if Phase 2 is done)
|
||||
Reference in New Issue
Block a user