feat: Wave 2 — project knowledge system (decisions + pitfalls)

[dean0x]

Summary

Closes #99. Implements the Project Knowledge system — structured extraction of architectural decisions (ADR-NNN) and known pitfalls (PF-NNN) from command flows, with lightweight injection at session start.

• 2 knowledge files in .memory/knowledge/: decisions.md (append-only ADRs) and pitfalls.md (area-specific gotchas)
• Session-start injection: TL;DR comment headers only (~30-50 tokens via head -1), graceful degradation if files missing
• Agent consumption: Coder reads decisions before implementation, Reviewer checks pitfalls against diff, Skimmer includes decision count in orientation
• Knowledge extraction: /implement → Phase 11.5 records ADRs; /code-review → Phase 4.5, /debug → Phase 4.5/7.5, /resolve → Phase 5.5 record pitfalls
• Both variants: Base and Teams command files updated (teammate prompts include knowledge reads)
• CLI: createMemoryDir() now creates knowledge/ subdirectory
• Tests: 8 new tests covering TL;DR parsing, sequential numbering, deduplication, graceful degradation, and TL;DR update after append (246 total, all passing)

Key design choices

• Token conservative: only TL;DR headers at session start, agents read full files on demand
• No new agents or CLI commands — extraction is inline in existing command phases
• 50-entry cap per file with mkdir-based lock for concurrent write safety
• Seed entries from existing codebase knowledge (Result types decision, synthesizer glob pitfall)

Test plan

• npm run build passes (31 skills, 17 plugins)
• npm test — 246/246 tests pass
• Session-start hook verified: TL;DR injected into additionalContext
• Manual: run /implement with architectural decisions, verify ADR appended
• Manual: run /code-review on branch with issues, verify PF appended
• Manual: delete knowledge files, confirm graceful degradation (no errors)

[dean0x]

Duplicate step numbering: Step 5 appears twice consecutively. The "Report completion: SendMessage" step should be numbered 6, not 5, since step 2 was inserted above it.

Suggested fix:

    5. Document findings with file:path references.
    6. Report completion: SendMessage(type: "message", recipient: "team-lead",

This issue affects all four explorer prompts (architecture, integration, reusable-code, edge-case). Use consistent sequential numbering (2-6) after inserting the new knowledge-reading step.

[dean0x]

Non-standard step numbering: Using "1.5" breaks sequential integer numbering used throughout the codebase. Agents may interpret this as a sub-step rather than a full responsibility.

Suggested fix: Renumber to use sequential integers (1, 2, 3, ..., 11) since you're inserting a new responsibility between steps 1 and 2. All subsequent steps would be incremented accordingly.

Alternative: If fractional numbering is intentional for non-breaking additions, document this convention in CLAUDE.md.

[dean0x]

Non-standard step numbering: Step "2.5" is inserted between sequential steps, breaking the standard integer pattern. Agents may not recognize this as a full sequential step in the review process.

Suggested fix: Renumber all steps sequentially (2 → 3, old 3 → 4, etc.) so the pitfall-reading step is step 3 and subsequent steps follow consecutively. Apply this consistently across all four reviewer teammate prompts in this file.

[dean0x]

Code Review Summary

Inline Comments Created (High Confidence ≥80%)

I've created targeted inline comments for critical findings:

• ✅ Duplicate step 5 in implement-teams.md explorer prompts (95% confidence)
• ✅ Fractional numbering "1.5" in reviewer.md responsibilities (82% confidence)
• ✅ Fractional numbering "2.5" in code-review-teams.md reviewer prompts (82% confidence)

---

Additional High-Confidence Findings (Consolidated)

Architectural Issues (88-85% confidence):

• Duplicated knowledge extraction logic across 8 command files (DRY violation). Recommend extracting 7-step pitfall-recording and decision-recording procedures to docs-framework/references/knowledge-procedures.md.
• Knowledge injection hook has minor inconsistent newline handling when WORKING-MEMORY.md is missing.

Lock Specification Inconsistency (82-85% confidence):

• Debug/resolve commands omit timeout/recovery parameters from lock specification, while code-review/implement include (30s timeout, 60s stale recovery). Standardize across all 8 command files to prevent orphaned locks.

Test Coverage Gaps (80% confidence):

• Knowledge format tests are self-referential — they test inline logic, not exported TypeScript functions or actual shell hook behavior.
• No integration tests for session-start-memory hook's new knowledge TL;DR injection.
• No test coverage for shell hook failure paths.

Missing Integration (80% confidence):

• /specify and /self-review commands not updated to read knowledge files (may be intentional scoping).

Performance (Low impact, 80% confidence):

• Session-start hook: head -1 | sed could combine into single sed to reduce subprocess spawns.

Documentation (80% confidence):

• Asymmetry between Skimmer (reads TL;DR only) and Coder (reads full file) not documented.

---

Verification Checklist

• BLOCKING: Fix duplicate step 5 numbering in all 4 explorer prompts (implement-teams.md)
• Standardize lock specification across debug/resolve/code-review/implement commands
• Extract duplicated pitfall/decision procedures to shared reference
• Document fractional numbering convention or standardize to sequential
• Consider integration tests for shell hooks in follow-up PR

---

Assessment

Blocking Issues: 3 (step numbering issues and lock inconsistencies)
Should Fix: 6 (DRY violations, test coverage, missing integration)
Pre-existing: Several (noted but not blocking)

Recommendation: CHANGES_REQUESTED
The step numbering bugs should be fixed. Lock standardization is important for reliability. Consider the DRY violation extraction as a follow-up if the scope is too large for this PR.

---

Created by Claude Code — Analysis based on 9 review reports (Architecture, Performance, Tests, Regression, Documentation, TypeScript, Security, Complexity, Consistency)

[dean0x]

Code Review Resolution Status

Commit 40b167b resolves findings from the 9-reviewer code review.

Fixed (14 issues)

#	Issue	Resolution
1	Duplicate step "5" in explorer prompts	Renumbered to 5→6 in all 4 prompts
2	DRY: extraction procedure duplicated 8×	Created knowledge-persistence skill — single source of truth
4	Inconsistent lock specs	Solved by #2 — unified in skill
5	Fractional "2.5" in reviewer teammate prompts	Renumbered 2.5→3, shifted 3→10 in all 4 prompts
6	Fractional "1.5" in reviewer.md	Renumbered 1.5→2, shifted 2→11
7	Inconsistent template headers	Solved by #2 — templates defined once in skill
8	No hook integration test	Added 3 integration tests for session-start-memory hook
11	Undocumented Skimmer asymmetry	Added TL;DR-only rationale comment to step 6
12	Leading newlines when Section 1 skipped	Guarded section joins with emptiness check
13	Stale counts in file-organization.md	Updated 24→31 skills, 8→17 plugins
14	Missing ambient hook in file-organization.md	Added ambient-prompt.sh to hooks listing
15	Skimmer instruction/output mismatch	Aligned step 6 to mention <!-- TL;DR: ... --> format
16	Silent catch in createMemoryDir	Logs error in verbose mode
20	Redundant subprocesses	Collapsed head+sed+grep into single sed -n

Deferred (3 issues)

#	Issue	Rationale
9	TL;DR input sanitization	Trust model: .memory/ is gitignored, local-only. Requires filesystem access to exploit.
10	Knowledge integrity verification	Same trust model. Append-only is instruction-enforced, matching all agent behavior.
17	Missing createMemoryDir failure test	mkdir({ recursive: true }) basically never fails. Low value.

Dismissed (2 issues)

#	Issue	Rationale
18	Phase ordering code-review vs teams	Both record pitfalls post-synthesis. Display difference is cosmetic.
19	Missing /specify and /self-review integration	Intentional scope: /specify has no implementation decisions, /self-review is ephemeral.

Verification

• npm run build — 32 skills, 17 plugins ✅
• npm test — 249/249 tests pass ✅
• Hook integration — TL;DR injection verified, no leading newlines ✅
• All 8 command files reference knowledge-persistence skill instead of inline procedures ✅

[dean0x]

Follow-up: Additional Resolutions (b52b8b1)

After discussing the deferred/dismissed issues:

#	Issue	Updated Status
17	Missing createMemoryDir failure test	Fixed — added test that blocks mkdir with a file
19	Missing /specify and /self-review integration	Fixed — added knowledge awareness (read-only) to both commands

/specify: New Phase 2.5 reads decisions.md + pitfalls.md before exploration. Constraints and Failure Modes explorers now receive prior decisions and known pitfalls as context.

/self-review: Phase 0 now reads knowledge files. Both Simplifier and Scrutinizer receive KNOWLEDGE_CONTEXT to check for reintroduced pitfall patterns and verify architectural consistency.

Confirmed deferred (after discussion):

• 🔒 SECURITY (Long-term): Implement cryptographic script signing #9 (TL;DR sanitization) — trust model correct, .memory/ is local-only
• 🔒 SECURITY (Future): Explore sandboxed script execution #10 (Knowledge integrity) — instruction-enforced, consistent with all agent behavior

Confirmed non-issue:

• fix: install skills in flat structure for auto-discovery #18 (Phase ordering) — structural difference between teams (has cleanup phase) and non-teams (no cleanup needed), not a bug

Tests: 250/250 pass ✅