Skip to content

feat: persistent memory system surviving auto-compaction#36

Open
morten1231 wants to merge 1 commit into
dsifry:mainfrom
morten1231:feat/memory-system
Open

feat: persistent memory system surviving auto-compaction#36
morten1231 wants to merge 1 commit into
dsifry:mainfrom
morten1231:feat/memory-system

Conversation

@morten1231

Copy link
Copy Markdown

Summary

  • Adds a lightweight memory system that persists across auto-compaction and session restarts
  • PreCompact hook reads .metaswarm/memory/ files + session-state.json and injects them as additionalContext
  • Zero dependencies, ~1500 tokens (~0.7% of 200K context window), 89ms average execution

Problem

Claude Code auto-compacts when context approaches the limit. After compaction, agents lose all conversation history — task state, decisions, gotchas, and user preferences. This forces expensive rediscovery of context that was already known.

metaswarm's existing beads integration helps with plan persistence, but there's no general-purpose memory for decisions, gotchas, user feedback, or session state that agents can write to and hooks can read from.

Solution

4 markdown files + 1 JSON checkpoint in .metaswarm/memory/:

File Purpose Update frequency
active-state.md Current task, phase, progress, key files Every phase transition
decisions.md Architecture decisions with reasoning When significant choices are made
gotchas.md Project-specific pitfalls When bugs/gotchas are discovered
feedback.md User corrections and preferences When the user corrects behavior
session-state.json Machine-readable execution checkpoint Every phase transition

The pre-compact.sh hook reads all memory files and outputs them as additionalContext. It's wired to both PreCompact (survives in compressed summary) and SessionStart (re-injected into fresh context).

Files changed

File Change
hooks/pre-compact.sh New — core hook that reads memory + knowledge base
hooks/hooks.json Wires pre-compact.sh to PreCompact + SessionStart
skills/memory/SKILL.md New — documents the checkpoint protocol for agents
templates/memory/* New — 4 markdown + 1 JSON template for new projects
commands/memory.md New/memory command shim
lib/setup-mandatory-files.sh Creates .metaswarm/memory/ on project setup
templates/CLAUDE.md Documents memory system + checkpoint protocol

Design decisions

  • Files, not database — zero dependencies, works everywhere, easy to read/debug
  • 350-byte threshold — empty templates (244-299 bytes) are automatically skipped, zero noise on fresh projects
  • Knowledge base filtering — example entries (id contains "example") are excluded from highlights
  • Extra files auto-discovered — users can create additional .md files in memory/ and they'll be loaded automatically
  • Graceful degradation — corrupt JSON, missing files, missing directory all produce valid empty output

Test plan

Battle-tested locally with:

  • 18/18 content recovery checks (session state, decisions, gotchas, feedback, knowledge)
  • Empty project produces zero context (no template noise)
  • Corrupt JSON graceful fallback
  • Missing session-state.json graceful fallback
  • Missing memory directory produces empty output
  • Extra user-created memory files auto-discovered
  • Example knowledge entries filtered out
  • Performance: 89ms average over 10 runs
  • Output size: ~1500 tokens (0.7% of 200K window)
  • Valid JSON output in all scenarios

🤖 Generated with Claude Code

Adds a lightweight memory system (4 markdown files + 1 JSON checkpoint)
that persists across auto-compaction and session restarts. The PreCompact
hook reads memory files and injects them as additionalContext, so critical
project state survives context compression.

Components:
- hooks/pre-compact.sh — reads session state, memory files, and knowledge
  base highlights; outputs as additionalContext for both PreCompact and
  SessionStart events (89ms avg, ~1500 tokens / 0.7% of 200K context)
- skills/memory/SKILL.md — documents the checkpoint protocol for agents
- templates/memory/ — 4 markdown templates + session-state.json
- commands/memory.md — /memory command shim

What survives compaction:
- Current task, phase, completed/next steps (session-state.json)
- Project state and key files (active-state.md)
- Architecture decisions with reasoning (decisions.md)
- Project-specific pitfalls (gotchas.md)
- User corrections and preferences (feedback.md)
- Knowledge base highlights (filtered, no examples)

Integration:
- hooks.json wires pre-compact.sh to both PreCompact and SessionStart
- setup-mandatory-files.sh creates .metaswarm/memory/ on project setup
- CLAUDE.md template documents the memory + checkpoint protocol
- Empty templates (<350 bytes) are automatically skipped
- Extra .md files in memory/ are auto-discovered

Battle-tested with 18/18 recovery checks, edge cases (corrupt JSON,
missing files, missing directory), and 10-run performance benchmarks.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Apr 17, 2026

Copy link
Copy Markdown

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced a persistent memory system that automatically tracks work progress, decisions, gotchas, and user feedback across sessions.
    • Memory is auto-loaded at session start to provide context recovery.
  • Documentation

    • Updated documentation to describe the new memory system and session state tracking features.

Walkthrough

Introduces a persistent file-based memory system for metaswarm with auto-activated skill, pre-compact hook that collects session state and knowledge highlights, setup initialization logic, command documentation, and updated context recovery documentation.

Changes

Cohort / File(s) Summary
Memory System Core
skills/memory/SKILL.md, hooks/hooks.json, hooks/pre-compact.sh
Adds persistent memory skill and pre-compact hook that collects memory files, session state, and knowledge highlights into contextual output for SessionStart/PreCompact events. Hook configuration updated to invoke pre-compact.sh in addition to session-start.sh.
Memory Initialization & Setup
lib/setup-mandatory-files.sh
Replaces command-shim creation with initialization of .metaswarm/memory/ directory and session-state.json. Copies memory markdown templates from plugin templates and conditionally initializes session state.
Memory Templates
templates/memory/active-state.md, templates/memory/decisions.md, templates/memory/feedback.md, templates/memory/gotchas.md, templates/memory/session-state.json
Adds five template files defining the structure and format for tracking active work state, architectural decisions, user feedback, project pitfalls, and machine-readable session checkpoint data.
Documentation & Commands
commands/memory.md, templates/CLAUDE.md
Adds memory command documentation and updates context recovery model documentation to describe memory file location, structure, and checkpoint semantics alongside beads persistence as fallback.

Sequence Diagram

sequenceDiagram
    participant Hook as Hook System
    participant Script as pre-compact.sh
    participant FS as File System
    participant Node as Node.js<br/>(JSON Processing)
    participant Output as Hook Output

    Hook->>Script: SessionStart/PreCompact event
    Script->>FS: Read .metaswarm/session-state.json
    FS-->>Script: Session state data
    Script->>Node: Parse & summarize session state
    Node-->>Script: Formatted metadata
    
    Script->>FS: Read .metaswarm/memory/*.md
    FS-->>Script: Memory files (decisions, gotchas, etc.)
    Script->>FS: Filter files by size (>200 bytes)
    
    Script->>FS: Read knowledge/gotchas.jsonl, decisions.jsonl, anti-patterns.jsonl
    FS-->>Script: Knowledge entries
    Script->>Node: Extract facts, format Key Knowledge
    Node-->>Script: Knowledge summary
    
    Script->>Script: Concatenate session state + memory + knowledge
    Script->>Node: Escape content to safe JSON string
    Node-->>Script: Escaped JSON content
    
    Script->>Output: Emit JSON with hookEventName & additionalContext
    Output-->>Hook: Context loaded into session
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly Related PRs

Poem

🐰 A memory warren, deep and true,
Captures decisions old and new,
States and gotchas, feedback too—
The hive remembers what to do!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: persistent memory system surviving auto-compaction' directly and clearly describes the main change: introducing a persistent memory system that survives auto-compaction, which is the core feature of this PR.
Description check ✅ Passed The description is highly relevant and comprehensive, detailing the problem, solution, file changes, design decisions, and test plan for the memory system implementation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Apr 17, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a lightweight persistent memory system for metaswarm — five files in .metaswarm/memory/ plus a pre-compact.sh hook that injects saved state as additionalContext on both PreCompact and SessionStart events.

  • P1 — wrong hookEventName on SessionStart: pre-compact.sh always outputs \"hookEventName\": \"PreCompact\", even when triggered from SessionStart. Compare with session-start.sh, which correctly outputs \"hookEventName\": \"SessionStart\". If Claude Code validates this field against the triggering event, the additionalContext payload will be silently dropped on every session start — defeating the primary use case of the feature.
  • P1 — path injection in embedded Node.js: $state_file and $kb_dir are interpolated as bare shell variables inside single-quoted Node.js string literals. A project directory containing a single quote will break the JS syntax; a path containing backslash sequences will corrupt the resolved path.

Confidence Score: 3/5

Not safe to merge until the hookEventName bug is fixed — memory injection on SessionStart (the main recovery path) will silently fail.

Two P1 defects in the core hook: the hardcoded "PreCompact" event name breaks context injection on session start, and direct variable interpolation into Node.js string literals will corrupt file paths containing single quotes or backslashes. The rest of the PR (templates, documentation, setup script) is solid and the design is sound, but the hook needs fixes before this reliably works in production.

hooks/pre-compact.sh requires the most attention — both P1 issues are in this file.

Important Files Changed

Filename Overview
hooks/pre-compact.sh Core hook with two P1 bugs: hookEventName always outputs "PreCompact" even when invoked from SessionStart (breaking memory re-injection on session start), and path variables are interpolated directly into Node.js source strings (breaks on paths with single quotes or backslashes). Also has unused dead variables and no memory file size cap.
hooks/hooks.json Correctly wires pre-compact.sh to both SessionStart (alongside existing session-start.sh) and PreCompact; replaces the previously incorrect session-start.sh entry in the PreCompact event.
lib/setup-mandatory-files.sh Adds memory directory setup, but session-state.json creation is gated inside the memory directory check — won't be created/repaired if the directory already exists but the file is missing.
skills/memory/SKILL.md Good documentation of the memory protocol, but documents a 200-byte skip threshold that contradicts the 350-byte threshold in the actual hook code.
templates/CLAUDE.md Cleanly extends the Context Recovery section to document both the new memory system and existing beads persistence; demotes beads to optional.
commands/memory.md Standard command shim routing to the memory skill; consistent with other shims in the project.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: hooks/pre-compact.sh
Line: 139-142

Comment:
**`hookEventName` hardcoded as `PreCompact` when run from `SessionStart`**

The script outputs `"hookEventName": "PreCompact"` regardless of which event triggered it. `session-start.sh` demonstrates the correct pattern — it outputs `"hookEventName": "SessionStart"` when invoked from `SessionStart`. If Claude Code validates the event name in the response (matching against the event that triggered the hook), this hook's `additionalContext` will be silently ignored on every session start. That breaks the "memory re-injected into fresh sessions" use case that is the primary reason this is wired to `SessionStart`.

The fix is to detect the calling event and emit the matching name. One approach:

```bash
# Detect calling event: if SESSION_HOOK_EVENT is set by the runtime, use it;
# otherwise default to PreCompact (which is its primary purpose)
HOOK_EVENT_NAME="${SESSION_HOOK_EVENT:-PreCompact}"
# ... then in the output:
#   "hookEventName": "${HOOK_EVENT_NAME}"
```

Or simply maintain two separate scripts — `session-start.sh` already does this correctly.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: hooks/pre-compact.sh
Line: 23

Comment:
**Path injection into Node.js source string**

`$state_file` is shell-interpolated directly inside a Node.js `-e` string delimited by single quotes (`'$state_file'`). If `PROJECT_DIR` contains a single quote (e.g., a project named `don't-break`), the JS syntax breaks and the whole block silently fails. If it contains backslash sequences (`\n`, `\t`, etc.), they're interpreted as JS escape sequences and the path becomes wrong.

The same issue applies to `$kb_dir` on line 81.

Pass the path as a command-line argument instead:

```bash
state_summary=$(node -e "
  try {
    const s = JSON.parse(require('fs').readFileSync(process.argv[1], 'utf8'));
    ...
  } catch {}
" "$state_file" 2>/dev/null || true)
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: skills/memory/SKILL.md
Line: 169

Comment:
**Documented threshold (200 bytes) doesn't match the code (350 bytes)**

The SKILL.md states "Files smaller than 200 bytes are skipped", but `hooks/pre-compact.sh` uses `[ "${byte_count:-0}" -gt 350 ]`. The templates themselves are 244–299 bytes per the PR description, so the 200-byte threshold would cause them to be loaded — defeating the "zero noise on fresh projects" design goal. One of these needs to match the other.

```suggestion
Files smaller than 350 bytes are skipped (assumed to be empty templates). This means memory only loads when there's real content — zero overhead for fresh projects.
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: hooks/pre-compact.sh
Line: 53-56

Comment:
**No upper size limit on memory files — context window can grow unbounded**

The hook loads entire memory files without any size cap. After months of active use, `decisions.md` and `gotchas.md` in particular can grow to thousands of tokens. The 1500-token estimate in the PR description only holds for fresh projects. There's no protection against a single large file consuming a disproportionate share of the context window.

The knowledge-base section already does `slice(-5)` + `slice(0, 10)` to cap output. Apply the same discipline to the raw markdown files — e.g., read only the last N kilobytes or truncate content beyond a configurable token budget.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: hooks/pre-compact.sh
Line: 10-11

Comment:
**`SCRIPT_DIR` and `PLUGIN_ROOT` are computed but never used**

Both variables are set at the top of the script but referenced nowhere below. Unlike `session-start.sh` (which uses `PLUGIN_ROOT` to call `setup-mandatory-files.sh`), `pre-compact.sh` only ever reads from `PROJECT_DIR`. Remove these dead assignments to keep the script clean.

```suggestion
PROJECT_DIR="$(pwd)"
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: lib/setup-mandatory-files.sh
Line: 134-149

Comment:
**`session-state.json` is only created if memory directory doesn't already exist**

The `session-state.json` copy is nested inside `if [ ! -d "$memory_dir" ]`. If a user or migration has the `memory/` directory but is missing the state file (or if someone deletes it), setup will skip creating it — the `skipped` message only mentions the directory, not the missing file. Move the state-file creation outside the directory guard:

```bash
if [ ! -d "$memory_dir" ] && [ -d "$memory_template_dir" ]; then
  mkdir -p "$memory_dir"
  for tmpl in "$memory_template_dir"/*.md; do
    [ -f "$tmpl" ] || continue
    cp "$tmpl" "$memory_dir/"
    created+=(".metaswarm/memory/$(basename "$tmpl")")
  done
else
  if [ -d "$memory_dir" ]; then
    skipped+=(".metaswarm/memory/ (already exists)")
  fi
fi
# Always ensure session-state.json exists, regardless of memory_dir state
if [ -f "$memory_template_dir/session-state.json" ] && [ ! -f "$state_file" ]; then
  cp "$memory_template_dir/session-state.json" "$state_file"
  created+=(".metaswarm/session-state.json")
fi
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "feat: add persistent memory system that ..." | Re-trigger Greptile

Comment thread hooks/pre-compact.sh
Comment on lines +139 to +142
"hookEventName": "PreCompact",
"additionalContext": "${escaped}"
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 hookEventName hardcoded as PreCompact when run from SessionStart

The script outputs "hookEventName": "PreCompact" regardless of which event triggered it. session-start.sh demonstrates the correct pattern — it outputs "hookEventName": "SessionStart" when invoked from SessionStart. If Claude Code validates the event name in the response (matching against the event that triggered the hook), this hook's additionalContext will be silently ignored on every session start. That breaks the "memory re-injected into fresh sessions" use case that is the primary reason this is wired to SessionStart.

The fix is to detect the calling event and emit the matching name. One approach:

# Detect calling event: if SESSION_HOOK_EVENT is set by the runtime, use it;
# otherwise default to PreCompact (which is its primary purpose)
HOOK_EVENT_NAME="${SESSION_HOOK_EVENT:-PreCompact}"
# ... then in the output:
#   "hookEventName": "${HOOK_EVENT_NAME}"

Or simply maintain two separate scripts — session-start.sh already does this correctly.

Prompt To Fix With AI
This is a comment left during a code review.
Path: hooks/pre-compact.sh
Line: 139-142

Comment:
**`hookEventName` hardcoded as `PreCompact` when run from `SessionStart`**

The script outputs `"hookEventName": "PreCompact"` regardless of which event triggered it. `session-start.sh` demonstrates the correct pattern — it outputs `"hookEventName": "SessionStart"` when invoked from `SessionStart`. If Claude Code validates the event name in the response (matching against the event that triggered the hook), this hook's `additionalContext` will be silently ignored on every session start. That breaks the "memory re-injected into fresh sessions" use case that is the primary reason this is wired to `SessionStart`.

The fix is to detect the calling event and emit the matching name. One approach:

```bash
# Detect calling event: if SESSION_HOOK_EVENT is set by the runtime, use it;
# otherwise default to PreCompact (which is its primary purpose)
HOOK_EVENT_NAME="${SESSION_HOOK_EVENT:-PreCompact}"
# ... then in the output:
#   "hookEventName": "${HOOK_EVENT_NAME}"
```

Or simply maintain two separate scripts — `session-start.sh` already does this correctly.

How can I resolve this? If you propose a fix, please make it concise.

Comment thread hooks/pre-compact.sh
if command -v node >/dev/null 2>&1; then
state_summary=$(node -e "
try {
const s = JSON.parse(require('fs').readFileSync('$state_file', 'utf8'));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Path injection into Node.js source string

$state_file is shell-interpolated directly inside a Node.js -e string delimited by single quotes ('$state_file'). If PROJECT_DIR contains a single quote (e.g., a project named don't-break), the JS syntax breaks and the whole block silently fails. If it contains backslash sequences (\n, \t, etc.), they're interpreted as JS escape sequences and the path becomes wrong.

The same issue applies to $kb_dir on line 81.

Pass the path as a command-line argument instead:

state_summary=$(node -e "
  try {
    const s = JSON.parse(require('fs').readFileSync(process.argv[1], 'utf8'));
    ...
  } catch {}
" "$state_file" 2>/dev/null || true)
Prompt To Fix With AI
This is a comment left during a code review.
Path: hooks/pre-compact.sh
Line: 23

Comment:
**Path injection into Node.js source string**

`$state_file` is shell-interpolated directly inside a Node.js `-e` string delimited by single quotes (`'$state_file'`). If `PROJECT_DIR` contains a single quote (e.g., a project named `don't-break`), the JS syntax breaks and the whole block silently fails. If it contains backslash sequences (`\n`, `\t`, etc.), they're interpreted as JS escape sequences and the path becomes wrong.

The same issue applies to `$kb_dir` on line 81.

Pass the path as a command-line argument instead:

```bash
state_summary=$(node -e "
  try {
    const s = JSON.parse(require('fs').readFileSync(process.argv[1], 'utf8'));
    ...
  } catch {}
" "$state_file" 2>/dev/null || true)
```

How can I resolve this? If you propose a fix, please make it concise.

Comment thread skills/memory/SKILL.md
The `pre-compact.sh` hook reads all memory files and session-state.json. It runs on:
- **SessionStart** — memory is injected into the fresh session context
- **PreCompact** — memory is injected into the compaction summary (survives compression)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Documented threshold (200 bytes) doesn't match the code (350 bytes)

The SKILL.md states "Files smaller than 200 bytes are skipped", but hooks/pre-compact.sh uses [ "${byte_count:-0}" -gt 350 ]. The templates themselves are 244–299 bytes per the PR description, so the 200-byte threshold would cause them to be loaded — defeating the "zero noise on fresh projects" design goal. One of these needs to match the other.

Suggested change
Files smaller than 350 bytes are skipped (assumed to be empty templates). This means memory only loads when there's real content — zero overhead for fresh projects.
Prompt To Fix With AI
This is a comment left during a code review.
Path: skills/memory/SKILL.md
Line: 169

Comment:
**Documented threshold (200 bytes) doesn't match the code (350 bytes)**

The SKILL.md states "Files smaller than 200 bytes are skipped", but `hooks/pre-compact.sh` uses `[ "${byte_count:-0}" -gt 350 ]`. The templates themselves are 244–299 bytes per the PR description, so the 200-byte threshold would cause them to be loaded — defeating the "zero noise on fresh projects" design goal. One of these needs to match the other.

```suggestion
Files smaller than 350 bytes are skipped (assumed to be empty templates). This means memory only loads when there's real content — zero overhead for fresh projects.
```

How can I resolve this? If you propose a fix, please make it concise.

Comment thread hooks/pre-compact.sh
Comment on lines +53 to +56
byte_count=$(wc -c < "$filepath" 2>/dev/null | tr -d ' ')
if [ "${byte_count:-0}" -gt 350 ]; then
context_parts+=("$(cat "$filepath")")
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 No upper size limit on memory files — context window can grow unbounded

The hook loads entire memory files without any size cap. After months of active use, decisions.md and gotchas.md in particular can grow to thousands of tokens. The 1500-token estimate in the PR description only holds for fresh projects. There's no protection against a single large file consuming a disproportionate share of the context window.

The knowledge-base section already does slice(-5) + slice(0, 10) to cap output. Apply the same discipline to the raw markdown files — e.g., read only the last N kilobytes or truncate content beyond a configurable token budget.

Prompt To Fix With AI
This is a comment left during a code review.
Path: hooks/pre-compact.sh
Line: 53-56

Comment:
**No upper size limit on memory files — context window can grow unbounded**

The hook loads entire memory files without any size cap. After months of active use, `decisions.md` and `gotchas.md` in particular can grow to thousands of tokens. The 1500-token estimate in the PR description only holds for fresh projects. There's no protection against a single large file consuming a disproportionate share of the context window.

The knowledge-base section already does `slice(-5)` + `slice(0, 10)` to cap output. Apply the same discipline to the raw markdown files — e.g., read only the last N kilobytes or truncate content beyond a configurable token budget.

How can I resolve this? If you propose a fix, please make it concise.

Comment thread hooks/pre-compact.sh
Comment on lines +10 to +11
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
PLUGIN_ROOT="${CLAUDE_PLUGIN_ROOT:-${extensionPath:-$(cd "$SCRIPT_DIR/.." && pwd)}}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 SCRIPT_DIR and PLUGIN_ROOT are computed but never used

Both variables are set at the top of the script but referenced nowhere below. Unlike session-start.sh (which uses PLUGIN_ROOT to call setup-mandatory-files.sh), pre-compact.sh only ever reads from PROJECT_DIR. Remove these dead assignments to keep the script clean.

Suggested change
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
PLUGIN_ROOT="${CLAUDE_PLUGIN_ROOT:-${extensionPath:-$(cd "$SCRIPT_DIR/.." && pwd)}}"
PROJECT_DIR="$(pwd)"
Prompt To Fix With AI
This is a comment left during a code review.
Path: hooks/pre-compact.sh
Line: 10-11

Comment:
**`SCRIPT_DIR` and `PLUGIN_ROOT` are computed but never used**

Both variables are set at the top of the script but referenced nowhere below. Unlike `session-start.sh` (which uses `PLUGIN_ROOT` to call `setup-mandatory-files.sh`), `pre-compact.sh` only ever reads from `PROJECT_DIR`. Remove these dead assignments to keep the script clean.

```suggestion
PROJECT_DIR="$(pwd)"
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +134 to +149
if [ ! -d "$memory_dir" ] && [ -d "$memory_template_dir" ]; then
mkdir -p "$memory_dir"
for tmpl in "$memory_template_dir"/*.md; do
[ -f "$tmpl" ] || continue
cp "$tmpl" "$memory_dir/"
created+=(".metaswarm/memory/$(basename "$tmpl")")
done
if [ -f "$memory_template_dir/session-state.json" ] && [ ! -f "$state_file" ]; then
cp "$memory_template_dir/session-state.json" "$state_file"
created+=(".metaswarm/session-state.json")
fi
else
if [ -d "$memory_dir" ]; then
skipped+=(".metaswarm/memory/ (already exists)")
fi
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 session-state.json is only created if memory directory doesn't already exist

The session-state.json copy is nested inside if [ ! -d "$memory_dir" ]. If a user or migration has the memory/ directory but is missing the state file (or if someone deletes it), setup will skip creating it — the skipped message only mentions the directory, not the missing file. Move the state-file creation outside the directory guard:

if [ ! -d "$memory_dir" ] && [ -d "$memory_template_dir" ]; then
  mkdir -p "$memory_dir"
  for tmpl in "$memory_template_dir"/*.md; do
    [ -f "$tmpl" ] || continue
    cp "$tmpl" "$memory_dir/"
    created+=(".metaswarm/memory/$(basename "$tmpl")")
  done
else
  if [ -d "$memory_dir" ]; then
    skipped+=(".metaswarm/memory/ (already exists)")
  fi
fi
# Always ensure session-state.json exists, regardless of memory_dir state
if [ -f "$memory_template_dir/session-state.json" ] && [ ! -f "$state_file" ]; then
  cp "$memory_template_dir/session-state.json" "$state_file"
  created+=(".metaswarm/session-state.json")
fi
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/setup-mandatory-files.sh
Line: 134-149

Comment:
**`session-state.json` is only created if memory directory doesn't already exist**

The `session-state.json` copy is nested inside `if [ ! -d "$memory_dir" ]`. If a user or migration has the `memory/` directory but is missing the state file (or if someone deletes it), setup will skip creating it — the `skipped` message only mentions the directory, not the missing file. Move the state-file creation outside the directory guard:

```bash
if [ ! -d "$memory_dir" ] && [ -d "$memory_template_dir" ]; then
  mkdir -p "$memory_dir"
  for tmpl in "$memory_template_dir"/*.md; do
    [ -f "$tmpl" ] || continue
    cp "$tmpl" "$memory_dir/"
    created+=(".metaswarm/memory/$(basename "$tmpl")")
  done
else
  if [ -d "$memory_dir" ]; then
    skipped+=(".metaswarm/memory/ (already exists)")
  fi
fi
# Always ensure session-state.json exists, regardless of memory_dir state
if [ -f "$memory_template_dir/session-state.json" ] && [ ! -f "$state_file" ]; then
  cp "$memory_template_dir/session-state.json" "$state_file"
  created+=(".metaswarm/session-state.json")
fi
```

How can I resolve this? If you propose a fix, please make it concise.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@commands/memory.md`:
- Around line 1-3: Add a top-level H1 heading to the markdown so MD041 passes
and the file matches other commands/*.md; edit the file that currently contains
the comment "<!-- Created by metaswarm setup. Routes to the metaswarm plugin.
Safe to delete if you uninstall metaswarm. -->" and the line "Invoke the
`/metaswarm:memory` skill..." by inserting a short H1 (e.g. "# Metaswarm Memory"
or "# /metaswarm:memory") at the very top of the document so the heading
precedes the existing content.

In `@hooks/hooks.json`:
- Around line 11-16: pre-compact.sh currently always emits "hookEventName":
"PreCompact", causing the framework to drop additionalContext when the hook is
actually invoked as SessionStart; update pre-compact.sh to inspect the
CLAUDE_HOOK_EVENT (or equivalent env var passed by the runner) and set the
emitted hookEventName to the actual event (e.g., "SessionStart" when
CLAUDE_HOOK_EVENT == "SessionStart", otherwise "PreCompact"), or move the
SessionStart-specific output logic into a shared script or merge with
session-start.sh so the script emits the matching hookEventName and preserves
additionalContext when invoked from SessionStart.

In `@hooks/pre-compact.sh`:
- Around line 21-39: The injected paths are unsafe because the script
interpolates shell variables directly into the Node -e string (see
readFileSync('$state_file', 'utf8') and const kbDir = '$kb_dir'), enabling
shell→JS injection; change these Node invocations to accept the paths via argv
or env instead of embedding them in single-quoted JS literals (e.g., update the
inline script to use process.argv[n] or process.env.KB_DIR and invoke node -e
with the path passed after -- or export the env var), remove the single-quoted
interpolation, and ensure the shell call quotes the argument (e.g., pass
"$state_file" as an argv) so paths with quotes/newlines are handled safely and
not executed.
- Around line 129-134: The sed fallback does not reliably escape all control
characters (0x00–0x1F) and can emit invalid JSON; replace the fragile sed branch
in the node availability check so that when command -v node fails you fail
closed: set escaped to an empty JSON string (e.g., ""), avoid attempting manual
escaping of $joined with sed/tr, and (optionally) emit a brief warning; update
the branch that currently assigns to escaped (and references $joined / the sed +
tr sequence) to return the safe empty value instead so downstream JSON emission
remains valid.
- Line 11: The PLUGIN_ROOT variable (assigned using
PLUGIN_ROOT="${CLAUDE_PLUGIN_ROOT:-${extensionPath:-$(cd "$SCRIPT_DIR/.." &&
pwd)}}") is unused and triggers SC2034; either remove this assignment entirely
from hooks/pre-compact.sh, or if it must be available to child processes
referenced by the node -e invocations, change the line to export PLUGIN_ROOT so
child processes can read it (ensure node -e code actually reads
process.env.PLUGIN_ROOT). Make the change near the PLUGIN_ROOT assignment and
confirm there are no other references to PLUGIN_ROOT in the script.

In `@lib/setup-mandatory-files.sh`:
- Around line 134-149: The session-state.json copy check is incorrectly nested
inside the conditional that runs only when memory_dir does not exist, so when
.metaswarm/memory/ exists but .metaswarm/session-state.json is missing we never
create it; modify the script so the block that checks [ -f
"$memory_template_dir/session-state.json" ] && [ ! -f "$state_file" ] (and the
subsequent cp to "$state_file" and created+=(".metaswarm/session-state.json"))
is executed independently of the memory_dir branch (i.e., move that check out of
the outer if/else), while keeping the existing created and skipped updates for
memory_dir (memory_dir, memory_template_dir, state_file, created, skipped).

In `@skills/memory/SKILL.md`:
- Line 170: The doc in SKILL.md incorrectly states the skip threshold as 200
bytes while the hook checks in hooks/pre-compact.sh use 350; either update the
SKILL.md text to say "350 bytes" to match the hook, or refactor to centralize
the threshold (introduce a named constant like MEMORY_SIZE_THRESHOLD in
hooks/pre-compact.sh and use that value in the script, then update the
documentation to reference that constant) so both the documentation and the -gt
checks (currently in hooks/pre-compact.sh) remain consistent.

In `@templates/CLAUDE.md`:
- Around line 173-185: The fenced JSON example for the session state lacks blank
lines before and after the triple-backtick fence (MD031); add one blank line
immediately above the opening ```json and one blank line immediately after the
closing ``` so the fenced block for the "session-state.json" example renders
consistently across Markdown parsers and satisfies markdownlint.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: e84774ff-5ffa-4ac4-84b6-39e793617d41

📥 Commits

Reviewing files that changed from the base of the PR and between c86fd6c and 5f62366.

📒 Files selected for processing (11)
  • commands/memory.md
  • hooks/hooks.json
  • hooks/pre-compact.sh
  • lib/setup-mandatory-files.sh
  • skills/memory/SKILL.md
  • templates/CLAUDE.md
  • templates/memory/active-state.md
  • templates/memory/decisions.md
  • templates/memory/feedback.md
  • templates/memory/gotchas.md
  • templates/memory/session-state.json

Comment thread commands/memory.md
Comment on lines +1 to +3
<!-- Created by metaswarm setup. Routes to the metaswarm plugin. Safe to delete if you uninstall metaswarm. -->

Invoke the `/metaswarm:memory` skill to handle this request. Pass along any arguments the user provided.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add a top-level heading to satisfy MD041 and improve rendering.

markdownlint flags the missing H1. A short heading also makes this file consistent with other commands/*.md entries.

📝 Proposed fix
 <!-- Created by metaswarm setup. Routes to the metaswarm plugin. Safe to delete if you uninstall metaswarm. -->

+# /memory
+
 Invoke the `/metaswarm:memory` skill to handle this request. Pass along any arguments the user provided.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<!-- Created by metaswarm setup. Routes to the metaswarm plugin. Safe to delete if you uninstall metaswarm. -->
Invoke the `/metaswarm:memory` skill to handle this request. Pass along any arguments the user provided.
<!-- Created by metaswarm setup. Routes to the metaswarm plugin. Safe to delete if you uninstall metaswarm. -->
# /memory
Invoke the `/metaswarm:memory` skill to handle this request. Pass along any arguments the user provided.
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)

[warning] 3-3: First line in a file should be a top-level heading

(MD041, first-line-heading, first-line-h1)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@commands/memory.md` around lines 1 - 3, Add a top-level H1 heading to the
markdown so MD041 passes and the file matches other commands/*.md; edit the file
that currently contains the comment "<!-- Created by metaswarm setup. Routes to
the metaswarm plugin. Safe to delete if you uninstall metaswarm. -->" and the
line "Invoke the `/metaswarm:memory` skill..." by inserting a short H1 (e.g. "#
Metaswarm Memory" or "# /metaswarm:memory") at the very top of the document so
the heading precedes the existing content.

Comment thread hooks/hooks.json
Comment on lines +11 to 16
},
{
"type": "command",
"command": "${CLAUDE_PLUGIN_ROOT}/hooks/pre-compact.sh",
"async": false
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

Claude Code hooks JSON schema: does hookSpecificOutput.hookEventName need to match the dispatched event (SessionStart vs PreCompact), and can multiple command hooks under one matcher each emit their own hookSpecificOutput JSON?

💡 Result:

Yes, hookSpecificOutput.hookEventName must match the dispatched hook event name (e.g., "SessionStart" for a SessionStart hook, "PreCompact" for a PreCompact hook). Official docs and examples consistently show the hookEventName set to the current event name, such as "PreToolUse" in PreToolUse examples and "SessionStart" explicitly supported in the TypeScript SDK union type. The input JSON includes "hook_event_name" matching the event, and outputs echo this for validation/processing. Yes, multiple command hooks under one matcher can each emit their own hookSpecificOutput JSON. All matching hooks run in parallel; each processes independently and outputs its own JSON to stdout (on exit 0), which Claude Code parses individually. For fields like additionalContext, values from multiple hooks are concatenated where applicable (e.g., SessionStart). Decisions from multiple PreToolUse hooks follow precedence rules (deny > defer > ask > allow).

Citations:


Fix pre-compact.sh to emit the correct hookEventName when invoked from SessionStart.

hooks/pre-compact.sh hard-codes "hookEventName": "PreCompact" in its output. When invoked from the SessionStart hook, it must emit "hookEventName": "SessionStart" instead—the framework validates that the output's event name matches the dispatched event and discards additionalContext if they don't match.

Branch on $CLAUDE_HOOK_EVENT (or equivalent) inside pre-compact.sh to emit the correct hookEventName, or extract the SessionStart-specific logic into a dedicated script merged with session-start.sh's output.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hooks/hooks.json` around lines 11 - 16, pre-compact.sh currently always emits
"hookEventName": "PreCompact", causing the framework to drop additionalContext
when the hook is actually invoked as SessionStart; update pre-compact.sh to
inspect the CLAUDE_HOOK_EVENT (or equivalent env var passed by the runner) and
set the emitted hookEventName to the actual event (e.g., "SessionStart" when
CLAUDE_HOOK_EVENT == "SessionStart", otherwise "PreCompact"), or move the
SessionStart-specific output logic into a shared script or merge with
session-start.sh so the script emits the matching hookEventName and preserves
additionalContext when invoked from SessionStart.

Comment thread hooks/pre-compact.sh
set -euo pipefail

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
PLUGIN_ROOT="${CLAUDE_PLUGIN_ROOT:-${extensionPath:-$(cd "$SCRIPT_DIR/.." && pwd)}}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

PLUGIN_ROOT is computed but never referenced (SC2034).

Drop the assignment (or prefix with export if intentionally exported to child processes — the node -e invocations below don't currently read it).

🧹 Proposed fix
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
-PLUGIN_ROOT="${CLAUDE_PLUGIN_ROOT:-${extensionPath:-$(cd "$SCRIPT_DIR/.." && pwd)}}"
 PROJECT_DIR="$(pwd)"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
PLUGIN_ROOT="${CLAUDE_PLUGIN_ROOT:-${extensionPath:-$(cd "$SCRIPT_DIR/.." && pwd)}}"
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
PROJECT_DIR="$(pwd)"
🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 11-11: PLUGIN_ROOT appears unused. Verify use (or export if used externally).

(SC2034)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hooks/pre-compact.sh` at line 11, The PLUGIN_ROOT variable (assigned using
PLUGIN_ROOT="${CLAUDE_PLUGIN_ROOT:-${extensionPath:-$(cd "$SCRIPT_DIR/.." &&
pwd)}}") is unused and triggers SC2034; either remove this assignment entirely
from hooks/pre-compact.sh, or if it must be available to child processes
referenced by the node -e invocations, change the line to export PLUGIN_ROOT so
child processes can read it (ensure node -e code actually reads
process.env.PLUGIN_ROOT). Make the change near the PLUGIN_ROOT assignment and
confirm there are no other references to PLUGIN_ROOT in the script.

Comment thread hooks/pre-compact.sh
Comment on lines +21 to +39
state_summary=$(node -e "
try {
const s = JSON.parse(require('fs').readFileSync('$state_file', 'utf8'));
if (s.task) {
let out = '## Session Recovery State';
out += '\n- Task: ' + s.task;
if (s.phase) out += '\n- Phase: ' + s.phase;
if (s.completedSteps && s.completedSteps.length > 0)
out += '\n- Completed: ' + s.completedSteps.join(', ');
if (s.nextSteps && s.nextSteps.length > 0)
out += '\n- Next: ' + s.nextSteps.join(', ');
if (s.fileScope && s.fileScope.length > 0)
out += '\n- Files: ' + s.fileScope.join(', ');
if (s.blockedBy) out += '\n- BLOCKED: ' + s.blockedBy;
if (s.lastUpdated) out += '\n- State saved: ' + s.lastUpdated;
console.log(out);
}
} catch {}
" 2>/dev/null || true)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Shell→JS injection via unquoted path interpolation into node -e scripts.

Both Node invocations interpolate bash variables directly into single-quoted JS string literals:

  • Line 23: readFileSync('$state_file', 'utf8')
  • Line 81: const kbDir = '$kb_dir';

state_file and kb_dir are derived from PROJECT_DIR="$(pwd)". If the path contains a single quote, backslash, or newline (e.g., /Users/o'brien/proj, a symlink name with a newline, or any intentionally crafted directory), the generated JS either fails to parse or executes attacker-controlled code. The surrounding 2>/dev/null || true will silently swallow the failure, so memory simply won't load and the root cause is invisible.

Pass paths via process.argv (or environment variables) instead of string interpolation, and drop the single-quoted literals:

🛡️ Proposed fix — pass paths as argv
-    state_summary=$(node -e "
+    state_summary=$(STATE_FILE="$state_file" node -e "
       try {
-        const s = JSON.parse(require('fs').readFileSync('$state_file', 'utf8'));
+        const s = JSON.parse(require('fs').readFileSync(process.env.STATE_FILE, 'utf8'));
         if (s.task) {
-  kb_summary=$(node -e "
+  kb_summary=$(KB_DIR="$kb_dir" node -e "
     const fs = require('fs');
     const path = require('path');
-    const kbDir = '$kb_dir';
+    const kbDir = process.env.KB_DIR;
     const highlights = [];

Also applies to: 78-104

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hooks/pre-compact.sh` around lines 21 - 39, The injected paths are unsafe
because the script interpolates shell variables directly into the Node -e string
(see readFileSync('$state_file', 'utf8') and const kbDir = '$kb_dir'), enabling
shell→JS injection; change these Node invocations to accept the paths via argv
or env instead of embedding them in single-quoted JS literals (e.g., update the
inline script to use process.argv[n] or process.env.KB_DIR and invoke node -e
with the path passed after -- or export the env var), remove the single-quoted
interpolation, and ensure the shell call quotes the argument (e.g., pass
"$state_file" as an argv) so paths with quotes/newlines are handled safely and
not executed.

Comment thread hooks/pre-compact.sh
Comment on lines +129 to +134
if command -v node >/dev/null 2>&1; then
escaped=$(printf '%s' "$joined" | node -e "let d='';process.stdin.on('data',c=>d+=c.toString());process.stdin.on('end',()=>process.stdout.write(JSON.stringify(d)))")
escaped="${escaped:1:${#escaped}-2}"
else
escaped=$(printf '%s' "$joined" | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g' -e 's/ /\\t/g' | tr '\n' '\036' | sed 's/\x1e/\\n/g')
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

sed fallback doesn't escape control characters and will produce invalid JSON for some inputs.

The Node path is fine, but the sed fallback (line 133) only escapes \, ", \t, and \n. Any other ASCII control byte (0x00–0x1F) in a memory file—e.g., a stray CR, form-feed, or NUL—will be emitted raw into the JSON string value and rejected by strict JSON parsers. Also the tr '\n' '\036' / sed 's/\x1e/\\n/g' hop is fragile: \x1e in sed BRE isn't portable across BSD/GNU sed.

Since Node is already a hard dependency of the rest of this hook (session-state parsing and kb-summary both require it), consider simply failing closed (emit additionalContext: "") when Node is unavailable rather than maintaining a second, weaker escaper.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hooks/pre-compact.sh` around lines 129 - 134, The sed fallback does not
reliably escape all control characters (0x00–0x1F) and can emit invalid JSON;
replace the fragile sed branch in the node availability check so that when
command -v node fails you fail closed: set escaped to an empty JSON string
(e.g., ""), avoid attempting manual escaping of $joined with sed/tr, and
(optionally) emit a brief warning; update the branch that currently assigns to
escaped (and references $joined / the sed + tr sequence) to return the safe
empty value instead so downstream JSON emission remains valid.

Comment on lines +134 to +149
if [ ! -d "$memory_dir" ] && [ -d "$memory_template_dir" ]; then
mkdir -p "$memory_dir"
for tmpl in "$memory_template_dir"/*.md; do
[ -f "$tmpl" ] || continue
cp "$tmpl" "$memory_dir/"
created+=(".metaswarm/memory/$(basename "$tmpl")")
done
if [ -f "$memory_template_dir/session-state.json" ] && [ ! -f "$state_file" ]; then
cp "$memory_template_dir/session-state.json" "$state_file"
created+=(".metaswarm/session-state.json")
fi
else
if [ -d "$memory_dir" ]; then
skipped+=(".metaswarm/memory/ (already exists)")
fi
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

session-state.json will not be created when memory_dir already exists.

The block that copies session-state.json (lines 141‑144) is nested inside the outer if [ ! -d "$memory_dir" ] branch. That means on any upgrade or partial install where .metaswarm/memory/ already exists but .metaswarm/session-state.json is missing, setup silently leaves the state file uninitialized. These two paths are independent and should be checked independently.

🔧 Proposed fix
 # --- File 3: Memory directory ---
 memory_dir="$PROJECT_DIR/.metaswarm/memory"
 state_file="$PROJECT_DIR/.metaswarm/session-state.json"
 memory_template_dir="$PLUGIN_ROOT/templates/memory"
-if [ ! -d "$memory_dir" ] && [ -d "$memory_template_dir" ]; then
-  mkdir -p "$memory_dir"
-  for tmpl in "$memory_template_dir"/*.md; do
-    [ -f "$tmpl" ] || continue
-    cp "$tmpl" "$memory_dir/"
-    created+=(".metaswarm/memory/$(basename "$tmpl")")
-  done
-  if [ -f "$memory_template_dir/session-state.json" ] && [ ! -f "$state_file" ]; then
-    cp "$memory_template_dir/session-state.json" "$state_file"
-    created+=(".metaswarm/session-state.json")
-  fi
-else
-  if [ -d "$memory_dir" ]; then
-    skipped+=(".metaswarm/memory/ (already exists)")
-  fi
+if [ -d "$memory_template_dir" ]; then
+  if [ ! -d "$memory_dir" ]; then
+    mkdir -p "$memory_dir"
+    for tmpl in "$memory_template_dir"/*.md; do
+      [ -f "$tmpl" ] || continue
+      cp "$tmpl" "$memory_dir/"
+      created+=(".metaswarm/memory/$(basename "$tmpl")")
+    done
+  else
+    skipped+=(".metaswarm/memory/ (already exists)")
+  fi
+  if [ -f "$memory_template_dir/session-state.json" ] && [ ! -f "$state_file" ]; then
+    mkdir -p "$(dirname "$state_file")"
+    cp "$memory_template_dir/session-state.json" "$state_file"
+    created+=(".metaswarm/session-state.json")
+  fi
 fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/setup-mandatory-files.sh` around lines 134 - 149, The session-state.json
copy check is incorrectly nested inside the conditional that runs only when
memory_dir does not exist, so when .metaswarm/memory/ exists but
.metaswarm/session-state.json is missing we never create it; modify the script
so the block that checks [ -f "$memory_template_dir/session-state.json" ] && [ !
-f "$state_file" ] (and the subsequent cp to "$state_file" and
created+=(".metaswarm/session-state.json")) is executed independently of the
memory_dir branch (i.e., move that check out of the outer if/else), while
keeping the existing created and skipped updates for memory_dir (memory_dir,
memory_template_dir, state_file, created, skipped).

Comment thread skills/memory/SKILL.md
- **SessionStart** — memory is injected into the fresh session context
- **PreCompact** — memory is injected into the compaction summary (survives compression)

Files smaller than 200 bytes are skipped (assumed to be empty templates). This means memory only loads when there's real content — zero overhead for fresh projects.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Byte‑threshold doc is out of sync with the hook implementation.

The SKILL claims files smaller than 200 bytes are skipped, but hooks/pre-compact.sh uses 350 bytes (see the -gt 350 checks at lines 54 and 69). Please align the doc with the actual threshold (or make the threshold a named constant referenced from both places) to avoid user confusion when small-but-real memory entries silently disappear.

📝 Proposed fix
-Files smaller than 200 bytes are skipped (assumed to be empty templates). This means memory only loads when there's real content — zero overhead for fresh projects.
+Files smaller than 350 bytes are skipped (assumed to be empty templates). This means memory only loads when there's real content — zero overhead for fresh projects.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Files smaller than 200 bytes are skipped (assumed to be empty templates). This means memory only loads when there's real content — zero overhead for fresh projects.
Files smaller than 350 bytes are skipped (assumed to be empty templates). This means memory only loads when there's real content — zero overhead for fresh projects.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/memory/SKILL.md` at line 170, The doc in SKILL.md incorrectly states
the skip threshold as 200 bytes while the hook checks in hooks/pre-compact.sh
use 350; either update the SKILL.md text to say "350 bytes" to match the hook,
or refactor to centralize the threshold (introduce a named constant like
MEMORY_SIZE_THRESHOLD in hooks/pre-compact.sh and use that value in the script,
then update the documentation to reference that constant) so both the
documentation and the -gt checks (currently in hooks/pre-compact.sh) remain
consistent.

Comment thread templates/CLAUDE.md
Comment on lines +173 to +185
**2. Session state** (`.metaswarm/session-state.json`) — machine-readable execution checkpoint:
```json
{
"task": "current task description",
"phase": "IMPLEMENT",
"completedSteps": ["step1", "step2"],
"nextSteps": ["step3"],
"fileScope": ["src/auth/*.ts"],
"blockedBy": null,
"lastUpdated": "ISO-8601"
}
```
Update after each phase transition so recovery knows exactly where to resume.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add blank lines around the JSON fence (MD031).

markdownlint flags lines 174 and 184; the fence needs surrounding blank lines for consistent rendering across parsers.

📝 Proposed fix
 **2. Session state** (`.metaswarm/session-state.json`) — machine-readable execution checkpoint:
+
 ```json
 {
   "task": "current task description",
   "phase": "IMPLEMENT",
   "completedSteps": ["step1", "step2"],
   "nextSteps": ["step3"],
   "fileScope": ["src/auth/*.ts"],
   "blockedBy": null,
   "lastUpdated": "ISO-8601"
 }

Update after each phase transition so recovery knows exactly where to resume.

</details>

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 markdownlint-cli2 (0.22.0)</summary>

[warning] 174-174: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

---

[warning] 184-184: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

</details>

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @templates/CLAUDE.md around lines 173 - 185, The fenced JSON example for the
session state lacks blank lines before and after the triple-backtick fence
(MD031); add one blank line immediately above the opening json and one blank line immediately after the closing so the fenced block for the
"session-state.json" example renders consistently across Markdown parsers and
satisfies markdownlint.


</details>

<!-- fingerprinting:phantom:poseidon:nectarine:695e9f3e-7938-4b63-8259-6e0209b97e31 -->

<!-- This is an auto-generated comment by CodeRabbit -->

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants