diff --git a/CHANGELOG.md b/CHANGELOG.md index fbd3a68..3c37811 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,8 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Self-learning system: detects repeated workflows and creates slash commands/skills automatically - **Learning**: `devflow learn --purge` command to remove invalid entries from learning log - **Learning**: debug logging mode (`devflow learn --configure`) — logs to `~/.devflow/logs/` +- **Knowledge citations in review & resolve outputs**: Resolvers and Reviewers cite matching ADR-NNN/PF-NNN IDs inline with an explicit hallucination guard (verbatim-only, no inference). `/resolve` aggregates cited IDs into a `## Knowledge Citations` section at the top of `resolution-summary.md`. ### Changed +- **Knowledge index + on-demand Read pattern across all knowledge-consuming commands**: `/resolve`, `/plan`, `/self-review`, `/code-review`, and `/debug` (plus their Teams variants and ambient orch equivalents `resolve:orch`, `plan:orch`, `review:orch`, `debug:orch`) now fan a compact index instead of the full ADR/PF corpus. Downstream agents (resolver, designer, simplifier, scrutinizer, reviewer) Read full entry bodies on demand. For `/debug`, knowledge stays orchestrator-local (hypothesis generation) and is not fanned to Explore investigators. Shared algorithm extracted to new `devflow:apply-knowledge` skill. Unified placeholder convention: all 11 invocation sites use `"{worktree}"`. Closes PF-011 and fills pre-existing ambient gaps for plan:orch, review:orch, and debug:orch. Token savings: ~75K/run at 10 resolvers with current corpus; scales as O(1) instead of O(entries × agents) as corpus grows. - **Learning**: Moved from Stop → SessionEnd hook with 3-session batching (adaptive: 5 at 15+ observations) - **Learning**: Raised procedural thresholds from 2 to 3 observations with 24h+ temporal spread for both types - **Learning**: Reduced default `max_daily_runs` from 10 to 5 @@ -26,6 +28,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Learning**: Race condition in batch file handoff (atomic `mv` replaces `cp`+`rm`) - **Learning**: `--enable` now auto-upgrades legacy Stop hook to SessionEnd - **Learning**: `--status` detects legacy hook and shows upgrade instructions +- **Self-learning reconciler self-heal**: `reconcile-manifest` now recovers from `render-ready` crash-window states. When a knowledge file contains an ADR/PF anchor absent from the manifest, and exactly one `status: 'ready'` log observation matches by normalized pattern, the observation is upgraded to `status: 'created'` and the manifest entry is reconstructed. Zero matches are treated as user-curated (left alone); multiple matches are silently skipped as ambiguous. Adds `healed` counter to all reconcile-manifest output shapes. Heal is gated by the `- **Source**: self-learning:` marker on the knowledge-file section, preventing false-positive heals against pre-v2 seeded entries. +- **Legacy knowledge purge v3 migration** (`purge-legacy-knowledge-v3`): sweeps all remaining pre-v2 seeded knowledge entries using the `- **Source**: self-learning:` format discriminator. Any ADR/PF section lacking this marker is removed. Replaces the v2 hardcoded allow-list approach with a format-based approach that catches entries the v2 migration missed. Self-learning-generated entries and user-opted-in entries (entries containing the source marker) survive. --- diff --git a/CLAUDE.md b/CLAUDE.md index 45620fe..ff749c8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -42,13 +42,13 @@ Commands with Teams Variant ship as `{name}.md` (parallel subagents) and `{name} **Ambient Mode**: Three-layer architecture for always-on intent classification. SessionStart hook (`session-start-classification`) reads lean classification rules (`~/.claude/skills/devflow:router/references/classification-rules.md`, ~30 lines) and injects as `additionalContext` — once per session, deterministic, zero model overhead. UserPromptSubmit hook (`preamble`) injects a one-sentence prompt per message triggering classification + router loading via Skill tool. Router SKILL.md is a pure skill lookup table (~50 lines) loaded on-demand only for GUIDED/ORCHESTRATED depth — maps intent×depth to domain and orchestration skills. Toggleable via `devflow ambient --enable/--disable/--status` or `devflow init`. -**Self-Learning**: A SessionEnd hook (`session-end-learning`) accumulates session IDs and triggers a background `claude -p --model sonnet` every 3 sessions (5 at 15+ observations) to detect **4 observation types** — workflow, procedural, decision, and pitfall — from batch transcripts. Transcript content is split into two channels by `scripts/hooks/lib/transcript-filter.cjs`: `USER_SIGNALS` (plain user messages, feeds workflow/procedural detection) and `DIALOG_PAIRS` (prior-assistant + user turns, feeds decision/pitfall detection). Detection uses per-type linguistic markers and quality gates stored in each observation as `quality_ok`. Per-type thresholds govern promotion (workflow: 3 required; procedural: 4 required; decision/pitfall: 2 required), each with independent temporal spread requirements. Observations accumulate in `.memory/learning-log.jsonl`; their lifecycle is `observing → ready → created → deprecated`. When thresholds are met, `json-helper.cjs render-ready` renders deterministically to 4 targets: slash commands (`.claude/commands/self-learning/`), skills (`.claude/skills/{slug}/`), decisions.md ADR entries, and pitfalls.md PF entries. A session-start feedback reconciler (`json-helper.cjs reconcile-manifest`) checks the manifest at `.memory/.learning-manifest.json` against the filesystem to detect deletions (applies 0.3× confidence penalty) and edits (ignored per D13). Loaded artifacts are reinforced locally (no LLM) on each session end. Single toggle mechanism: hook presence in `settings.json` IS the enabled state — no `enabled` field in `learning.json`. Toggleable via `devflow learn --enable/--disable/--status` or `devflow init --learn/--no-learn`. Configurable model/throttle/caps/debug via `devflow learn --configure`. Use `devflow learn --reset` to remove all artifacts + log + transient state. Use `devflow learn --purge` to remove invalid observations. Use `devflow learn --review` to inspect observations needing attention. Debug logs stored at `~/.devflow/logs/{project-slug}/`. The `knowledge-persistence` skill is a format specification only; the actual writer is `scripts/hooks/background-learning` via `json-helper.cjs render-ready`. +**Self-Learning**: A SessionEnd hook (`session-end-learning`) accumulates session IDs and triggers a background `claude -p --model sonnet` every 3 sessions (5 at 15+ observations) to detect **4 observation types** — workflow, procedural, decision, and pitfall — from batch transcripts. Transcript content is split into two channels by `scripts/hooks/lib/transcript-filter.cjs`: `USER_SIGNALS` (plain user messages, feeds workflow/procedural detection) and `DIALOG_PAIRS` (prior-assistant + user turns, feeds decision/pitfall detection). Detection uses per-type linguistic markers and quality gates stored in each observation as `quality_ok`. Per-type thresholds govern promotion (workflow: 3 required; procedural: 4 required; decision/pitfall: 2 required), each with independent temporal spread requirements. Observations accumulate in `.memory/learning-log.jsonl`; their lifecycle is `observing → ready → created → deprecated`. When thresholds are met, `json-helper.cjs render-ready` renders deterministically to 4 targets: slash commands (`.claude/commands/self-learning/`), skills (`.claude/skills/{slug}/`), decisions.md ADR entries, and pitfalls.md PF entries. A session-start feedback reconciler (`json-helper.cjs reconcile-manifest`) checks the manifest at `.memory/.learning-manifest.json` against the filesystem to detect deletions (applies 0.3× confidence penalty) and edits (ignored per D13). The reconciler also **self-heals** from render-ready crash-window states: when a knowledge file contains an ADR/PF anchor that is absent from the manifest *and* the section carries the `- **Source**: self-learning:` marker, the heal scans the log for `status: 'ready'` observations matching by normalized pattern (exactly one match = upgrade to `status: 'created'` and reconstruct manifest entry; zero or multiple matches = silently skipped). The marker check excludes pre-v2 seeded entries from the heal path so they cannot be falsely paired with a current ready obs. Loaded artifacts are reinforced locally (no LLM) on each session end. Single toggle mechanism: hook presence in `settings.json` IS the enabled state — no `enabled` field in `learning.json`. Toggleable via `devflow learn --enable/--disable/--status` or `devflow init --learn/--no-learn`. Configurable model/throttle/caps/debug via `devflow learn --configure`. Use `devflow learn --reset` to remove all artifacts + log + transient state. Use `devflow learn --purge` to remove invalid observations. Use `devflow learn --review` to inspect observations needing attention. Debug logs stored at `~/.devflow/logs/{project-slug}/`. The `knowledge-persistence` skill is a format specification only; the actual writer is `scripts/hooks/background-learning` via `json-helper.cjs render-ready`. **Claude Code Flags**: Typed registry (`src/cli/utils/flags.ts`) for managing Claude Code feature flags (env vars and top-level settings). Pure functions `applyFlags`/`stripFlags`/`getDefaultFlags` follow the `applyTeamsConfig`/`stripTeamsConfig` pattern. Initial flags: `tool-search`, `lsp`, `clear-context-on-plan` (default ON), `brief`, `disable-1m-context` (default OFF). Manageable via `devflow flags --enable/--disable/--status/--list`. Stored in manifest `features.flags: string[]`. **Two-Mode Init**: `devflow init` offers Recommended (sensible defaults, quick setup) or Advanced (full interactive flow) after plugin selection. `--recommended` / `--advanced` CLI flags for non-interactive use. Recommended applies: ambient ON, memory ON, learn ON, HUD ON, teams OFF, default-ON flags, .claudeignore ON, auto-install safe-delete if trash CLI detected, user-mode security deny list. -**Migrations**: Run-once migrations execute automatically on `devflow init`, tracked at `~/.devflow/migrations.json` (scope-independent; single file regardless of user-scope vs local-scope installs). Registry: append an entry to `MIGRATIONS` in `src/cli/utils/migrations.ts`. Scopes: `global` (runs once per machine, no project context) vs `per-project` (sweeps all discovered Claude-enabled projects in parallel). Failures are non-fatal — migrations retry on next init. **D37 edge case**: a project cloned *after* migrations have run won't be swept (the marker is global, not per-project). Recovery: `rm ~/.devflow/migrations.json` forces a re-sweep on next `devflow init`. +**Migrations**: Run-once migrations execute automatically on `devflow init`, tracked at `~/.devflow/migrations.json` (scope-independent; single file regardless of user-scope vs local-scope installs). Registry: append an entry to `MIGRATIONS` in `src/cli/utils/migrations.ts`. Scopes: `global` (runs once per machine, no project context) vs `per-project` (sweeps all discovered Claude-enabled projects in parallel). Failures are non-fatal — migrations retry on next init. Currently registered per-project migrations include `purge-legacy-knowledge-v2` (removes 4 hardcoded pre-v2 ADR/PF IDs and orphan `PROJECT-PATTERNS.md`) and `purge-legacy-knowledge-v3` (v3: sweeps all remaining pre-v2 seeded entries using the `- **Source**: self-learning:` format discriminator — any ADR/PF section lacking this marker is removed; entries the user edited to include the marker survive). **D37 edge case**: a project cloned *after* migrations have run won't be swept (the marker is global, not per-project). Recovery: `rm ~/.devflow/migrations.json` forces a re-sweep on next `devflow init`. ## Project Structure @@ -142,12 +142,12 @@ Working memory files live in a dedicated `.memory/` directory: ## Agent & Command Roster **Orchestration commands** (spawn agents, never do agent work in main session): -- `/plan` — Skimmer + Explore + Designer + Synthesizer + Plan + Designer → design artifact +- `/plan` — Skimmer + Explore + Designer + Synthesizer + Plan + Designer → design artifact; consumes knowledge via index + on-demand Read via `devflow:apply-knowledge` - `/implement` — Git + Coder + Validator + Simplifier + Scrutinizer + Evaluator + Tester → PR (accepts plan documents, issues, or task descriptions) -- `/code-review` — 7-11 Reviewer agents + Git + Synthesizer -- `/resolve` — N Resolver agents + Git +- `/code-review` — 7-11 Reviewer agents + Git + Synthesizer; consumes knowledge via index + on-demand Read via `devflow:apply-knowledge` +- `/resolve` — N Resolver agents + Git; loads compact knowledge index (`knowledge-context.cjs index`) per worktree and passes it as `KNOWLEDGE_CONTEXT` to each Resolver; Resolvers use `devflow:apply-knowledge` to Read full bodies on demand; aggregates cited ADR-NNN/PF-NNN IDs into a `## Knowledge Citations` section at the top of `resolution-summary.md` - `/debug` — Agent Teams competing hypotheses -- `/self-review` — Simplifier then Scrutinizer (sequential) +- `/self-review` — Simplifier then Scrutinizer (sequential); consumes knowledge via index + on-demand Read via `devflow:apply-knowledge` - `/audit-claude` — CLAUDE.md audit (optional plugin) **Shared agents** (12): git, synthesizer, skimmer, simplifier, coder, reviewer, resolver, evaluator, tester, scrutinizer, validator, designer diff --git a/docs/self-learning.md b/docs/self-learning.md index 8be60b5..2afb9fb 100644 --- a/docs/self-learning.md +++ b/docs/self-learning.md @@ -84,6 +84,67 @@ On session start, `json-helper.cjs reconcile-manifest ` compares manifest e This creates a feedback loop: deleting a generated artifact reduces its observation's confidence, eventually causing it to stop promoting. +#### Self-Heal: Crash-Window Recovery + +`render-ready` writes to the knowledge file first, then updates the log and manifest. If the process crashes in the window between file write and log update, the knowledge file contains the new ADR/PF entry but the log still shows `status: 'ready'` — a duplicate would be written on the next render-ready call. + +The reconciler detects and heals these orphans automatically: + +1. Scans `decisions.md` and `pitfalls.md` for ADR/PF anchors not tracked in the manifest. Only sections containing the `- **Source**: self-learning:` marker qualify — pre-v2 seeded entries (which lack the marker) are excluded so they cannot be falsely paired with a current ready obs. +2. For each unmanaged anchor, searches the log for `status: 'ready'` observations whose normalized pattern matches the anchor's heading text. +3. **Exactly one match** → upgrades the observation to `status: 'created'`, reconstructs the manifest entry, and registers usage. The `healed` counter in the reconcile output increments. +4. **Zero matches** → the entry is user-curated (written manually). Left untouched. +5. **Multiple matches** → ambiguous; silently skipped. The `healed` counter does not increment. + +The `healed` field is present in all three reconcile-manifest output shapes (main path and both early-return paths) and is backward-compatible — callers that discard the output are unaffected. + +## Knowledge Index + On-Demand Read Pattern + +Knowledge consumers (slash commands and orch skills) do not fan the full ADR/PF corpus to spawned agents. Instead they use a two-step pattern: + +### Step 1: Load compact index at orchestrator + +```bash +KNOWLEDGE_CONTEXT=$(node scripts/hooks/lib/knowledge-context.cjs index "{worktree}") +``` + +This produces a compact index listing each active entry's ID, truncated title, status, and area: + +``` +Decisions (2): + ADR-001 Use Result types instead of thrown errors [Active] + ... + +Pitfalls (3): + PF-004 Background hook scripts become god scripts [Active] — scripts/hooks/ + ... + +ADR-NNN entries live in /path/to/project/.memory/knowledge/decisions.md +PF-NNN entries live in /path/to/project/.memory/knowledge/pitfalls.md +Read the relevant file and locate the matching `## ADR-NNN:` or `## PF-NNN:` heading for the full body. +``` + +> **Note**: Pre-v2 seeded entries may show `[unknown]` instead of `[Active]` if they predate the standard `- **Status**: Active` line format. New entries created by the learning system always include the status line. + +### Step 2: Agent reads full body on demand + +Agents that receive `KNOWLEDGE_CONTEXT` follow the `devflow:apply-knowledge` skill algorithm: + +1. Scan the index and identify plausibly-relevant entries for the current task +2. Use `Read` on the knowledge file and locate the matching `## ADR-NNN:` or `## PF-NNN:` heading +3. Read the full entry body +4. Cite `applies ADR-NNN` / `avoids PF-NNN` inline — verbatim IDs only, no fabrication + +### Commands using this pattern + +| Command / Orch | Agents that consume | +|----------------|---------------------| +| `/resolve`, `resolve:orch` | Resolver | +| `/plan`, `plan:orch` | Designer, Explore | +| `/self-review` | Simplifier, Scrutinizer | +| `/code-review`, `review:orch` | Reviewer | +| `debug:orch` | Orchestrator-local (not fanned to Explore) | + ## CLI Commands ```bash @@ -97,7 +158,11 @@ npx devflow-kit learn --purge # Remove invalid/corrupted entri npx devflow-kit learn --review # Inspect observations needing attention (stale, capped, low-quality) ``` -Removal of pre-v2 low-signal knowledge entries (ADR-002, PF-001, PF-003, PF-005) and orphan `PROJECT-PATTERNS.md` now runs automatically as a one-time migration on `devflow init` — no CLI flag needed. Migration state is tracked at `~/.devflow/migrations.json`. +Two one-time migrations run automatically on `devflow init` to remove pre-v2 seeded knowledge entries — no CLI flag needed. Migration state is tracked at `~/.devflow/migrations.json`. + +**v2 migration (`purge-legacy-knowledge-v2`)**: Removes 4 hardcoded low-signal IDs (ADR-002, PF-001, PF-003, PF-005) and the orphan `PROJECT-PATTERNS.md` file seeded by earlier devflow versions. + +**v3 migration (`purge-legacy-knowledge-v3`)**: Sweeps all remaining pre-v2 seeded entries using a format discriminator. Any ADR/PF section in `decisions.md` or `pitfalls.md` that lacks the line `- **Source**: self-learning:` is treated as pre-v2 seeded content and removed. Self-learning-generated entries all carry this marker, so they are preserved. User-edited entries survive too — add the `- **Source**: self-learning:manual_xxx` line to any entry you want to keep through future migrations. ## HUD Row diff --git a/plugins/devflow-code-review/commands/code-review-teams.md b/plugins/devflow-code-review/commands/code-review-teams.md index afc7a27..afb19c7 100644 --- a/plugins/devflow-code-review/commands/code-review-teams.md +++ b/plugins/devflow-code-review/commands/code-review-teams.md @@ -82,6 +82,16 @@ Per worktree, detect file types in diff using `DIFF_RANGE` to determine conditio **Skill availability check**: Language/ecosystem reviews (typescript, react, accessibility, ui-design, go, java, python, rust) require their optional skill plugin to be installed. Before adding a conditional perspective, use Read to check if `~/.claude/skills/devflow:{focus}/SKILL.md` exists. If Read returns an error (file not found), **skip that perspective** — the language plugin isn't installed. Non-language reviews (database, dependencies, documentation) use skills bundled with this plugin and are always available. +### Phase 1b: Load Knowledge Index + +Load the knowledge index for the current worktree before spawning the review team: + +```bash +KNOWLEDGE_CONTEXT=$(node scripts/hooks/lib/knowledge-context.cjs index "{worktree}") +``` + +This produces a compact index of active ADR/PF entries. Pass `KNOWLEDGE_CONTEXT` to each reviewer teammate prompt. Reviewers use `devflow:apply-knowledge` to Read full entry bodies on demand. + ### Phase 2: Spawn Review Team **Per worktree**, create an agent team for adversarial review. Always include 4 core perspectives; conditionally add more based on Phase 1 analysis. @@ -110,77 +120,51 @@ Per worktree, detect file types in diff using `DIFF_RANGE` to determine conditio ``` Create a team named "review-{branch_slug}" to review PR #{pr_number}. -Spawn review teammates with self-contained prompts: +Spawn review teammates. For each teammate, compose a self-contained prompt using the template below, substituting the per-reviewer values from the table. -- Name: "security-reviewer" - Prompt: | - You are reviewing PR #{pr_number} on branch {branch} (base: {base_branch}). - WORKTREE_PATH: {worktree_path} (omit if cwd) - 1. Read your skill: `Read ~/.claude/skills/devflow:security/SKILL.md` - 2. Read review methodology: `Read ~/.claude/skills/devflow:review-methodology/SKILL.md` - 3. Read `.memory/knowledge/pitfalls.md` if it exists. Check for known pitfall patterns in the diff. - 4. Get the diff: `git -C {WORKTREE_PATH} diff {DIFF_RANGE}` - 5. Apply the 6-step review process from devflow:review-methodology - 6. Focus: injection, auth bypass, crypto misuse, OWASP vulnerabilities - 7. Classify each finding: 🔴 BLOCKING / ⚠️ SHOULD-FIX / ℹ️ PRE-EXISTING - 8. Include file:line references for every finding - 9. Write your report: `Write to {worktree_path}/.docs/reviews/{branch_slug}/{timestamp}/security.md` - 10. Report completion: SendMessage(type: "message", recipient: "team-lead", summary: "Security review done") +**Reviewer prompt template:** -- Name: "architecture-reviewer" - Prompt: | You are reviewing PR #{pr_number} on branch {branch} (base: {base_branch}). WORKTREE_PATH: {worktree_path} (omit if cwd) - 1. Read your skill: `Read ~/.claude/skills/devflow:architecture/SKILL.md` + KNOWLEDGE_CONTEXT: {knowledge_context} + 1. Read your skill(s): `Read {SKILL_PATHS}` 2. Read review methodology: `Read ~/.claude/skills/devflow:review-methodology/SKILL.md` - 3. Read `.memory/knowledge/pitfalls.md` if it exists. Check for known pitfall patterns in the diff. + 3. Follow devflow:apply-knowledge to scan KNOWLEDGE_CONTEXT index and Read full ADR/PF bodies on demand. Skip if (none). 4. Get the diff: `git -C {WORKTREE_PATH} diff {DIFF_RANGE}` 5. Apply the 6-step review process from devflow:review-methodology - 6. Focus: SOLID violations, coupling, layering issues, modularity problems + 6. Focus: {FOCUS} 7. Classify each finding: 🔴 BLOCKING / ⚠️ SHOULD-FIX / ℹ️ PRE-EXISTING 8. Include file:line references for every finding - 9. Write your report: `Write to {worktree_path}/.docs/reviews/{branch_slug}/{timestamp}/architecture.md` - 10. Report completion: SendMessage(type: "message", recipient: "team-lead", summary: "Architecture review done") - -- Name: "performance-reviewer" - Prompt: | - You are reviewing PR #{pr_number} on branch {branch} (base: {base_branch}). - WORKTREE_PATH: {worktree_path} (omit if cwd) - 1. Read your skill: `Read ~/.claude/skills/devflow:performance/SKILL.md` - 2. Read review methodology: `Read ~/.claude/skills/devflow:review-methodology/SKILL.md` - 3. Read `.memory/knowledge/pitfalls.md` if it exists. Check for known pitfall patterns in the diff. - 4. Get the diff: `git -C {WORKTREE_PATH} diff {DIFF_RANGE}` - 5. Apply the 6-step review process from devflow:review-methodology - 6. Focus: N+1 queries, memory leaks, algorithm issues, I/O bottlenecks - 7. Classify each finding: 🔴 BLOCKING / ⚠️ SHOULD-FIX / ℹ️ PRE-EXISTING - 8. Include file:line references for every finding - 9. Write your report: `Write to {worktree_path}/.docs/reviews/{branch_slug}/{timestamp}/performance.md` - 10. Report completion: SendMessage(type: "message", recipient: "team-lead", summary: "Performance review done") - -- Name: "quality-reviewer" - Prompt: | - You are reviewing PR #{pr_number} on branch {branch} (base: {base_branch}). - WORKTREE_PATH: {worktree_path} (omit if cwd) - 1. Read your skills: - - `Read ~/.claude/skills/devflow:complexity/SKILL.md` - - `Read ~/.claude/skills/devflow:consistency/SKILL.md` - - `Read ~/.claude/skills/devflow:testing/SKILL.md` - - `Read ~/.claude/skills/devflow:regression/SKILL.md` - 2. Read review methodology: `Read ~/.claude/skills/devflow:review-methodology/SKILL.md` - 3. Read `.memory/knowledge/pitfalls.md` if it exists. Check for known pitfall patterns in the diff. - 4. Get the diff: `git -C {WORKTREE_PATH} diff {DIFF_RANGE}` - 5. Apply the 6-step review process from devflow:review-methodology - 6. Focus: complexity, test gaps, pattern violations, regressions, naming - 7. Classify each finding: 🔴 BLOCKING / ⚠️ SHOULD-FIX / ℹ️ PRE-EXISTING - 8. Include file:line references for every finding - 9. Write your report: `Write to {worktree_path}/.docs/reviews/{branch_slug}/{timestamp}/quality.md` - 10. Report completion: SendMessage(type: "message", recipient: "team-lead", summary: "Quality review done") - -[Add conditional perspectives based on Phase 1 — follow same pattern: - explicit skill path, diff command with DIFF_RANGE, output path in timestamped dir, SendMessage for completion] + 9. Write your report: `Write to {worktree_path}/.docs/reviews/{branch_slug}/{timestamp}/{REPORT_NAME}.md` + 10. Report completion: SendMessage(type: "message", recipient: "team-lead", summary: "{SUMMARY}") + +**Core reviewers (always spawn):** + +| Name | SKILL_PATHS | FOCUS | REPORT_NAME | SUMMARY | +|------|-------------|-------|-------------|---------| +| security-reviewer | `~/.claude/skills/devflow:security/SKILL.md` | injection, auth bypass, crypto misuse, OWASP vulnerabilities | security | Security review done | +| architecture-reviewer | `~/.claude/skills/devflow:architecture/SKILL.md` | SOLID violations, coupling, layering issues, modularity problems | architecture | Architecture review done | +| performance-reviewer | `~/.claude/skills/devflow:performance/SKILL.md` | N+1 queries, memory leaks, algorithm issues, I/O bottlenecks | performance | Performance review done | +| quality-reviewer | `~/.claude/skills/devflow:complexity/SKILL.md`, `~/.claude/skills/devflow:consistency/SKILL.md`, `~/.claude/skills/devflow:testing/SKILL.md`, `~/.claude/skills/devflow:regression/SKILL.md` | complexity, test gaps, pattern violations, regressions, naming | quality | Quality review done | + +**Conditional reviewers** — add based on Phase 1 changed-file analysis, using the same template: + +| Name | Condition | SKILL_PATHS | FOCUS | REPORT_NAME | +|------|-----------|-------------|-------|-------------| +| typescript-reviewer | .ts/.tsx changed | `devflow:typescript` | type safety, generics, utility types | typescript | +| react-reviewer | .tsx/.jsx changed | `devflow:react` | hooks, state, rendering, composition | react | +| accessibility-reviewer | .tsx/.jsx changed | `devflow:accessibility` | ARIA, keyboard nav, focus management | accessibility | +| frontend-design-reviewer | .tsx/.jsx/.css changed | `devflow:ui-design` | visual consistency, spacing, typography | frontend-design | +| go-reviewer | .go changed | `devflow:go` | error handling, interfaces, concurrency | go | +| java-reviewer | .java changed | `devflow:java` | records, sealed classes, composition | java | +| python-reviewer | .py changed | `devflow:python` | type hints, protocols, data modeling | python | +| rust-reviewer | .rs changed | `devflow:rust` | ownership, error handling, type system | rust | +| database-reviewer | DB files changed | `devflow:database` | schema, queries, migrations, indexes | database | +| dependencies-reviewer | package files changed | `devflow:dependencies` | CVEs, versions, licenses, supply chain | dependencies | +| documentation-reviewer | docs or significant code changed | `devflow:documentation` | doc drift, missing docs, stale comments | documentation | ``` -### Phase 3: Debate Round +### Phase 2b: Debate Round After all reviewers complete initial analysis, lead initiates adversarial debate: @@ -210,7 +194,7 @@ Reviewers message each other directly using SendMessage: - `SendMessage(type: "message", recipient: "team-lead", summary: "Escalation: {topic}")` for unresolvable disagreements - Update or withdraw findings based on peer evidence -### Phase 4: Synthesis and PR Comments +### Phase 3: Synthesis and PR Comments **WAIT** for debate to complete, then lead produces outputs. @@ -254,7 +238,7 @@ Check for existing inline comments at same file:line before creating new ones." {Key exchanges that changed findings} ``` -### Phase 5: Write Review Head Marker +### Phase 4: Write Review Head Marker Per worktree, after successful completion: 1. Write current HEAD SHA to `{worktree_path}/.docs/reviews/{branch-slug}/.last-review-head` @@ -262,7 +246,7 @@ Per worktree, after successful completion: -### Phase 6: Cleanup and Report +### Phase 5: Cleanup and Report Shut down all review teammates explicitly: @@ -304,16 +288,16 @@ In multi-worktree mode, report results per worktree with aggregate summary. │ ├─ Quality Reviewer (teammate) │ └─ [Conditional: TypeScript, React, A11y, Design, Go, Java, Python, Rust, DB, Deps, Docs] │ -├─ Phase 3: Debate round +├─ Phase 2b: Debate round │ └─ Reviewers challenge each other (max 2 rounds) │ -├─ Phase 4: Synthesis +├─ Phase 3: Synthesis │ ├─ Git agent (comment-pr with consensus findings + dedup) │ └─ Lead writes review-summary with confidence levels │ -├─ Phase 5: Write .last-review-head per worktree +├─ Phase 4: Write .last-review-head per worktree │ -└─ Phase 6: Cleanup and display results +└─ Phase 5: Cleanup and display results ``` ## Edge Cases diff --git a/plugins/devflow-code-review/commands/code-review.md b/plugins/devflow-code-review/commands/code-review.md index 8ba5751..84bb6d5 100644 --- a/plugins/devflow-code-review/commands/code-review.md +++ b/plugins/devflow-code-review/commands/code-review.md @@ -89,6 +89,16 @@ Per worktree, detect file types in diff using `DIFF_RANGE` to determine conditio **Skill availability check**: Language/ecosystem reviews (typescript, react, accessibility, ui-design, go, java, python, rust) require their optional skill plugin to be installed. Before spawning a conditional Reviewer for these focuses, use Read to check if `~/.claude/skills/devflow:{focus}/SKILL.md` exists. If Read returns an error (file not found), **skip that review** — the language plugin isn't installed. Non-language reviews (database, dependencies, documentation) use skills bundled with this plugin and are always available. +### Phase 1b: Load Knowledge Index + +While file analysis runs (or just before spawning reviewers), load the knowledge index for the current worktree: + +```bash +KNOWLEDGE_CONTEXT=$(node scripts/hooks/lib/knowledge-context.cjs index "{worktree}") +``` + +This produces a compact index of active ADR/PF entries. Pass `KNOWLEDGE_CONTEXT` to all Reviewer agents. Reviewers use `devflow:apply-knowledge` to Read full entry bodies on demand. + ### Phase 2: Run Reviews (Parallel) Spawn Reviewer agents **in a single message**. Always run 7 core reviews; conditionally add more based on changed file types: @@ -122,6 +132,8 @@ Follow 6-step process from devflow:review-methodology. PR: #{pr_number}, Base: {base_branch} WORKTREE_PATH: {worktree_path} (omit if cwd) DIFF_COMMAND: git -C {WORKTREE_PATH} diff {DIFF_RANGE} (omit -C flag if no WORKTREE_PATH) +KNOWLEDGE_CONTEXT: {knowledge_context} +Follow devflow:apply-knowledge to scan the index and Read full ADR/PF bodies on demand. IMPORTANT: Write report to {worktree_path}/.docs/reviews/{branch-slug}/{timestamp}/{focus}.md using Write tool" ``` diff --git a/plugins/devflow-core-skills/.claude-plugin/plugin.json b/plugins/devflow-core-skills/.claude-plugin/plugin.json index b3470dc..1e1a8cf 100644 --- a/plugins/devflow-core-skills/.claude-plugin/plugin.json +++ b/plugins/devflow-core-skills/.claude-plugin/plugin.json @@ -18,6 +18,7 @@ ], "agents": [], "skills": [ + "apply-knowledge", "software-design", "docs-framework", "git", diff --git a/plugins/devflow-debug/commands/debug-teams.md b/plugins/devflow-debug/commands/debug-teams.md index 9a38f3d..38e32ed 100644 --- a/plugins/devflow-debug/commands/debug-teams.md +++ b/plugins/devflow-debug/commands/debug-teams.md @@ -23,9 +23,15 @@ Investigate bugs by spawning a team of agents, each pursuing a different hypothe ## Phases -### Phase 1: Load Project Knowledge +### Phase 1: Load Knowledge Index (Orchestrator-Local) -Read `.memory/knowledge/decisions.md` and `.memory/knowledge/pitfalls.md`. Known pitfalls from prior debugging sessions and code reviews can directly inform hypothesis generation — pass their content as context to investigators in Phase 2. +Before hypothesizing, load the knowledge index: + +```bash +KNOWLEDGE_CONTEXT=$(node scripts/hooks/lib/knowledge-context.cjs index "{worktree}") +``` + +The orchestrator uses `KNOWLEDGE_CONTEXT` locally when generating hypotheses (Phase 2) — prior pitfalls and decisions can suggest specific root causes to investigate. Follow `devflow:apply-knowledge` to Read full entry bodies on demand. **Do NOT pass `KNOWLEDGE_CONTEXT` to investigator teammates** — knowledge context stays in the orchestrator; teammates examine code directly. ### Phase 2: Context Gathering @@ -201,7 +207,7 @@ Lead produces final report: ``` /debug (orchestrator) │ -├─ Phase 1: Load Project Knowledge +├─ Phase 1: Load Knowledge Index (Orchestrator-Local) │ ├─ Phase 2: Context gathering │ └─ Git agent (fetch issue, if #N provided) diff --git a/plugins/devflow-debug/commands/debug.md b/plugins/devflow-debug/commands/debug.md index d4768f2..18b6236 100644 --- a/plugins/devflow-debug/commands/debug.md +++ b/plugins/devflow-debug/commands/debug.md @@ -31,9 +31,15 @@ Investigate bugs by spawning parallel agents, each pursuing a different hypothes ## Phases -### Phase 1: Load Project Knowledge +### Phase 1: Load Knowledge Index (Orchestrator-Local) -Read `.memory/knowledge/decisions.md` and `.memory/knowledge/pitfalls.md`. Known pitfalls from prior debugging sessions and code reviews can directly inform hypothesis generation — pass their content as context to investigators in Phase 2. +Before hypothesizing, load the knowledge index: + +```bash +KNOWLEDGE_CONTEXT=$(node scripts/hooks/lib/knowledge-context.cjs index "{worktree}") +``` + +The orchestrator uses `KNOWLEDGE_CONTEXT` locally when generating hypotheses (Phase 2) — prior pitfalls and decisions can suggest specific root causes to investigate. Follow `devflow:apply-knowledge` to Read full entry bodies on demand. **Do NOT pass `KNOWLEDGE_CONTEXT` to Explore investigators** — knowledge context stays in the orchestrator; investigators examine code directly. ### Phase 2: Context Gathering @@ -146,7 +152,7 @@ Produce the final report: ``` /debug (orchestrator) │ -├─ Phase 1: Load Project Knowledge +├─ Phase 1: Load Knowledge Index (Orchestrator-Local) │ ├─ Phase 2: Context gathering │ └─ Git agent (fetch issue, if #N provided) diff --git a/plugins/devflow-plan/agents/designer.md b/plugins/devflow-plan/agents/designer.md index 0e56cb9..7a2be47 100644 --- a/plugins/devflow-plan/agents/designer.md +++ b/plugins/devflow-plan/agents/designer.md @@ -2,7 +2,7 @@ name: Designer description: Design analysis agent with mode-driven skill loading. Modes: gap-analysis (completeness, architecture, security, performance, consistency, dependencies), design-review (anti-pattern detection). model: opus -skills: devflow:worktree-support +skills: devflow:worktree-support, devflow:apply-knowledge --- # Designer Agent @@ -18,6 +18,8 @@ The orchestrator provides: **Worktree Support**: If `WORKTREE_PATH` is provided, follow the `devflow:worktree-support` skill for path resolution. If omitted, use cwd. +- **KNOWLEDGE_CONTEXT** (optional): Compact index of active ADR/PF entries for this worktree (generated by `knowledge-context.cjs index`). `(none)` when absent. Use `devflow:apply-knowledge` to Read full bodies on demand. + ## Modes | Mode | Focus (optional) | Skill File (Read this first) | @@ -29,9 +31,10 @@ The orchestrator provides: 1. **Load mode skill** — Read the skill file from the table above for your assigned mode. This gives you detection patterns and checklists specific to your analysis type. 2. **Apply focus-specific analysis** — Use detection patterns from the loaded skill to scan the provided artifacts. For `gap-analysis`, apply only the patterns for your assigned focus. For `design-review`, apply all 6 anti-pattern rules. -3. **Assess confidence (0-100%)** — For each finding, assess certainty. Report at 80%+, suggest at 60-79%, drop below 60%. -4. **Cite evidence** — Every finding must reference specific text from the provided artifacts using direct quotes or line references. -5. **Write findings to output** — Format findings clearly with severity, confidence, evidence, and resolution. +3. **Apply Knowledge** — Follow `devflow:apply-knowledge` to scan the index, Read relevant bodies on demand, and cite `applies ADR-NNN` / `avoids PF-NNN` in findings. Skip when `KNOWLEDGE_CONTEXT` is empty or `(none)`. +4. **Assess confidence (0-100%)** — For each finding, assess certainty. Report at 80%+, suggest at 60-79%, drop below 60%. +5. **Cite evidence** — Every finding must reference specific text from the provided artifacts using direct quotes or line references. +6. **Write findings to output** — Format findings clearly with severity, confidence, evidence, and resolution. ## Output diff --git a/plugins/devflow-plan/commands/plan-teams.md b/plugins/devflow-plan/commands/plan-teams.md index f7e2eb8..38eb85f 100644 --- a/plugins/devflow-plan/commands/plan-teams.md +++ b/plugins/devflow-plan/commands/plan-teams.md @@ -72,7 +72,13 @@ Run rskim on source directories (NOT repo root) to identify: Return codebase context for requirements analysis." ``` -While Skimmer runs, read `.memory/knowledge/decisions.md` and `.memory/knowledge/pitfalls.md`. Pass Skimmer context and project knowledge to all subsequent agents and teammates. +While Skimmer runs, run: + +```bash +KNOWLEDGE_CONTEXT=$(node scripts/hooks/lib/knowledge-context.cjs index "{worktree}") +``` + +This produces a compact index of active ADR/PF entries. Pass Skimmer context and `KNOWLEDGE_CONTEXT` to all subsequent agents and teammates — prior decisions constrain design, known pitfalls inform gap analysis. Agents use `devflow:apply-knowledge` to Read full entry bodies on demand. #### Phase 3: Exploration Team @@ -87,7 +93,8 @@ Spawn exploration teammates with self-contained prompts: Prompt: | You are exploring requirements for: {feature/issues} 1. Skimmer context: {Phase 2 output} - 2. Project knowledge: {Phase 2 decisions + pitfalls} + 2. KNOWLEDGE_CONTEXT: {knowledge_context} + Follow devflow:apply-knowledge to scan the index and Read full ADR/PF bodies on demand. Skip if (none). 3. Your deliverable: Target users, their goals, pain points, user journeys, and success scenarios. What does the user need this to do? 4. Report completion: SendMessage(type: "message", recipient: "team-lead", @@ -97,7 +104,8 @@ Spawn exploration teammates with self-contained prompts: Prompt: | You are exploring requirements for: {feature/issues} 1. Skimmer context: {Phase 2 output} - 2. Project knowledge: {Phase 2 decisions + pitfalls} + 2. KNOWLEDGE_CONTEXT: {knowledge_context} + Follow devflow:apply-knowledge to scan the index and Read full ADR/PF bodies on demand. Skip if (none). 3. Your deliverable: Comparable features in the codebase or domain, scope patterns, edge cases discovered from similar implementations. 4. Report completion: SendMessage(type: "message", recipient: "team-lead", @@ -107,7 +115,8 @@ Spawn exploration teammates with self-contained prompts: Prompt: | You are exploring requirements for: {feature/issues} 1. Skimmer context: {Phase 2 output} - 2. Project knowledge: {Phase 2 decisions + pitfalls} + 2. KNOWLEDGE_CONTEXT: {knowledge_context} + Follow devflow:apply-knowledge to scan the index and Read full ADR/PF bodies on demand. Skip if (none). 3. Your deliverable: Dependencies, business rules, security constraints, performance constraints, and prior architectural decisions that constrain scope. 4. Report completion: SendMessage(type: "message", recipient: "team-lead", @@ -117,7 +126,8 @@ Spawn exploration teammates with self-contained prompts: Prompt: | You are exploring requirements for: {feature/issues} 1. Skimmer context: {Phase 2 output} - 2. Project knowledge: {Phase 2 decisions + pitfalls} + 2. KNOWLEDGE_CONTEXT: {knowledge_context} + Follow devflow:apply-knowledge to scan the index and Read full ADR/PF bodies on demand. Skip if (none). 3. Your deliverable: Error states, edge cases, validation needs, known pitfalls, and failure scenarios that must be handled. 4. Report completion: SendMessage(type: "message", recipient: "team-lead", @@ -192,7 +202,7 @@ Each designer receives: - Focus: (their assigned focus) - Exploration synthesis from Phase 4 - Skimmer context from Phase 2 -- Project knowledge from Phase 2 +- KNOWLEDGE_CONTEXT: knowledge index from Phase 2 (or `(none)`) — designers follow `devflow:apply-knowledge` to Read full ADR/PF bodies on demand - Multi-issue: all issue bodies #### Phase 6: Synthesize Gap Analysis @@ -405,7 +415,7 @@ Display: artifact path, issue URL, gap analysis summary, design review summary, │ ├─ Phase 1: GATE 0 - Confirm Understanding ⛔ MANDATORY │ ├─ Phase 2: Orient + Load Knowledge │ │ ├─ Skimmer agent (codebase context) -│ │ └─ Read decisions.md + pitfalls.md +│ │ └─ Load knowledge index (knowledge-context.cjs index) │ ├─ Phase 3: Exploration Team (4 teammates + debate) │ │ ├─ user-perspective-explorer │ │ ├─ similar-features-explorer diff --git a/plugins/devflow-plan/commands/plan.md b/plugins/devflow-plan/commands/plan.md index 59892b7..51fab42 100644 --- a/plugins/devflow-plan/commands/plan.md +++ b/plugins/devflow-plan/commands/plan.md @@ -72,11 +72,17 @@ Run rskim on source directories (NOT repo root) to identify: Return codebase context for requirements analysis." ``` -While Skimmer runs, read `.memory/knowledge/decisions.md` and `.memory/knowledge/pitfalls.md`. Pass Skimmer context and project knowledge to all subsequent agents — prior decisions constrain design, known pitfalls inform gap analysis. +While Skimmer runs, run: + +```bash +KNOWLEDGE_CONTEXT=$(node scripts/hooks/lib/knowledge-context.cjs index "{worktree}") +``` + +This produces a compact index of active ADR/PF entries. Pass Skimmer context and `KNOWLEDGE_CONTEXT` to all subsequent agents — prior decisions constrain design, known pitfalls inform gap analysis. Agents use `devflow:apply-knowledge` to Read full entry bodies on demand. #### Phase 3: Explore Requirements (Parallel) -Spawn 4 Explore agents **in a single message**, each with Skimmer context and project knowledge: +Spawn 4 Explore agents **in a single message**, each with Skimmer context and `KNOWLEDGE_CONTEXT` (from Phase 2). Include instruction: "follow `devflow:apply-knowledge` for KNOWLEDGE_CONTEXT". | Focus | Thoroughness | Find | |-------|-------------|------| @@ -124,19 +130,20 @@ Each designer receives: - Focus: (their assigned focus from table) - Exploration synthesis from Phase 4 - Skimmer context from Phase 2 -- Project knowledge from Phase 2 +- `KNOWLEDGE_CONTEXT` (index from Phase 2) - Multi-issue: all issue bodies ``` Agent(subagent_type="Designer"): "Mode: gap-analysis Focus: {completeness|architecture|security|performance|consistency|dependencies} +KNOWLEDGE_CONTEXT: {knowledge_context} Artifacts: Feature/Issues: {feature description or issue bodies} Exploration synthesis: {Phase 4 output} Codebase context: {Phase 2 output} - Project knowledge: {decisions + pitfalls} -Analyze only your assigned focus area. Cite evidence from provided artifacts." +Analyze only your assigned focus area. Follow devflow:apply-knowledge for KNOWLEDGE_CONTEXT. +Cite evidence from provided artifacts." ``` #### Phase 6: Synthesize Gap Analysis @@ -347,7 +354,7 @@ Display completion summary: │ │ └─ AskUserQuestion: Validate interpretation │ ├─ Phase 2: Orient + Load Knowledge │ │ ├─ Skimmer agent (codebase context) -│ │ └─ Read decisions.md + pitfalls.md +│ │ └─ Load knowledge index (knowledge-context.cjs index) │ ├─ Phase 3: Explore Requirements (PARALLEL) │ │ ├─ Explore: User perspective │ │ ├─ Explore: Similar features diff --git a/plugins/devflow-resolve/commands/resolve-teams.md b/plugins/devflow-resolve/commands/resolve-teams.md index 4f58724..e9149d8 100644 --- a/plugins/devflow-resolve/commands/resolve-teams.md +++ b/plugins/devflow-resolve/commands/resolve-teams.md @@ -60,6 +60,16 @@ For each worktree: Set `TARGET_DIR` to the selected review directory path. +#### Step 0d: Load Project Knowledge + +For each worktree, run: + +```bash +KNOWLEDGE_CONTEXT=$(node scripts/hooks/lib/knowledge-context.cjs index "{worktree}") +``` + +This produces a compact index of active ADR/PF entries from `decisions.md` and `pitfalls.md`, with Deprecated/Superseded entries already stripped. Falls back to `(none)` when both files are absent or all entries are filtered. Pass `KNOWLEDGE_CONTEXT` to every Resolver agent in Phase 4. Resolver agents use `devflow:apply-knowledge` to Read full entry bodies on demand — no fan-out of the full corpus. + ### Phase 1: Parse Issues Read review reports from `{TARGET_DIR}/*.md` and extract: @@ -116,7 +126,9 @@ Each resolver teammate receives the following instructions (only the issue list You are resolving review issues on branch {branch} (PR #{pr_number}). WORKTREE_PATH: {worktree_path} (omit if cwd) + KNOWLEDGE_CONTEXT: {knowledge_context} 1. Read your skill: `Read ~/.claude/skills/devflow:patterns/SKILL.md` + Follow devflow:apply-knowledge to scan KNOWLEDGE_CONTEXT and Read full ADR/PF bodies on demand. Skip if (none). 2. Your issues to resolve: {BATCH_ISSUES} 3. For each issue: @@ -181,6 +193,8 @@ Aggregate from all Resolvers: - **Deferred**: High-risk issues marked for tech debt - **Blocked**: Issues that couldn't be fixed +Extract all knowledge citations from Resolver Reasoning columns. Collect unique `applies ADR-NNN` and `avoids PF-NNN` references across all batches. These will populate the `## Knowledge Citations` section in Phase 8. + @@ -250,7 +264,8 @@ In multi-worktree mode, report results per worktree with aggregate summary. ├─ Phase 0: Worktree Discovery & Pre-flight │ ├─ Step 0a: git worktree list → filter resolvable │ ├─ Step 0b: Git agent (validate-branch) per worktree [parallel] -│ └─ Step 0c: Target latest review directory per worktree +│ ├─ Step 0c: Target latest review directory per worktree +│ └─ Step 0d: Load project knowledge → KNOWLEDGE_CONTEXT │ ├─ Phase 1: Parse issues from TARGET_DIR │ └─ Extract ALL issues (including Suggestions, exclude summaries) @@ -317,6 +332,13 @@ Written by orchestrator in Phase 8 to `{TARGET_DIR}/resolution-summary.md`: **Review**: {TARGET_DIR} **Command**: /resolve +## Knowledge Citations + +- applies ADR-{NNN} — {batch-id}, {issue-id} +- avoids PF-{NNN} — {batch-id}, {issue-id} + +(Omit section if no citations were made) + ## Statistics | Metric | Value | |--------|-------| diff --git a/plugins/devflow-resolve/commands/resolve.md b/plugins/devflow-resolve/commands/resolve.md index ad1cb8a..2dbf7cb 100644 --- a/plugins/devflow-resolve/commands/resolve.md +++ b/plugins/devflow-resolve/commands/resolve.md @@ -67,6 +67,16 @@ For each worktree: Set `TARGET_DIR` to the selected review directory path. +#### Step 0d: Load Project Knowledge + +For each worktree, run: + +```bash +KNOWLEDGE_CONTEXT=$(node scripts/hooks/lib/knowledge-context.cjs index "{worktree}") +``` + +This produces a compact index of active ADR/PF entries from `decisions.md` and `pitfalls.md`, with Deprecated/Superseded entries already stripped. Falls back to `(none)` when both files are absent or all entries are filtered. Pass `KNOWLEDGE_CONTEXT` to every Resolver agent in Phase 4. Resolver agents use `devflow:apply-knowledge` to Read full entry bodies on demand — no fan-out of the full corpus. + ### Phase 1: Parse Issues Read review reports from `{TARGET_DIR}/*.md` and extract: @@ -116,7 +126,8 @@ Agent(subagent_type="Resolver"): BRANCH: {branch-slug} BATCH_ID: batch-{n} WORKTREE_PATH: {worktree_path} (omit if cwd) -Validate, decide FIX vs TECH_DEBT, implement fixes" +KNOWLEDGE_CONTEXT: {knowledge_context} +Validate, decide FIX vs TECH_DEBT, implement fixes. Follow devflow:apply-knowledge to Read full ADR/PF bodies on demand." ``` > Resolvers follow a 3-tier risk approach: @@ -134,6 +145,8 @@ Aggregate from all Resolvers: - **Deferred**: High-risk issues marked for tech debt - **Blocked**: Issues that couldn't be fixed +Extract all knowledge citations from Resolver Reasoning columns. Collect unique `applies ADR-NNN` and `avoids PF-NNN` references across all batches. These will populate the `## Knowledge Citations` section in Phase 8. + ### Phase 6: Simplify If any fixes were made, spawn Simplifier agent to refine the changed code: @@ -200,7 +213,8 @@ In multi-worktree mode, report results per worktree with aggregate summary. ├─ Phase 0: Worktree Discovery & Pre-flight │ ├─ Step 0a: git worktree list → filter resolvable │ ├─ Step 0b: Git agent (validate-branch) per worktree [parallel] -│ └─ Step 0c: Target latest review directory per worktree +│ ├─ Step 0c: Target latest review directory per worktree +│ └─ Step 0d: Load project knowledge → KNOWLEDGE_CONTEXT │ ├─ Phase 1: Parse issues from TARGET_DIR │ └─ Extract ALL issues (including Suggestions, exclude summaries) @@ -266,6 +280,13 @@ Written by orchestrator in Phase 8 to `{TARGET_DIR}/resolution-summary.md`: **Review**: {TARGET_DIR} **Command**: /resolve +## Knowledge Citations + +- applies ADR-{NNN} — {batch-id}, {issue-id} +- avoids PF-{NNN} — {batch-id}, {issue-id} + +(Omit section if no citations were made) + ## Statistics | Metric | Value | |--------|-------| diff --git a/plugins/devflow-self-review/commands/self-review.md b/plugins/devflow-self-review/commands/self-review.md index 29a306c..5338939 100644 --- a/plugins/devflow-self-review/commands/self-review.md +++ b/plugins/devflow-self-review/commands/self-review.md @@ -21,7 +21,11 @@ Detect changed files and build context: 2. Else run `git diff --name-only HEAD` + `git diff --name-only --cached` to get staged + unstaged 3. If no changes found, report "No changes to review" and exit 4. Build TASK_DESCRIPTION from recent commit messages or branch name -5. Read `.memory/knowledge/pitfalls.md` and `.memory/knowledge/decisions.md`. Pass as KNOWLEDGE_CONTEXT to Simplifier and Scrutinizer — known pitfalls help identify reintroduced issues, prior decisions help validate architectural consistency. +5. Load knowledge index: + ```bash + KNOWLEDGE_CONTEXT=$(node scripts/hooks/lib/knowledge-context.cjs index "{worktree}") + ``` + Pass `KNOWLEDGE_CONTEXT` to Scrutinizer — the compact index lists active ADR/PF entries; Scrutinizer uses `devflow:apply-knowledge` to Read full entry bodies on demand. Known pitfalls help identify reintroduced issues, prior decisions help validate architectural consistency. (Simplifier does not consume knowledge — it operates at code-shape level and Scrutinizer runs after to catch any architectural drift.) **Extract:** FILES_CHANGED (list), TASK_DESCRIPTION (string), KNOWLEDGE_CONTEXT (string, optional) @@ -32,9 +36,7 @@ Spawn Simplifier agent to refine code for clarity and consistency: Agent(subagent_type="Simplifier", run_in_background=false): "TASK_DESCRIPTION: {task_description} FILES_CHANGED: {files_changed} -KNOWLEDGE_CONTEXT: {knowledge_context or 'None'} -Simplify and refine the code for clarity and consistency while preserving functionality. -If knowledge context is provided, verify no known pitfall patterns are being reintroduced." +Simplify and refine the code for clarity and consistency while preserving functionality." **Wait for completion.** Simplifier commits changes directly. @@ -45,9 +47,9 @@ Spawn Scrutinizer agent for quality evaluation and fixing: Agent(subagent_type="Scrutinizer", run_in_background=false): "TASK_DESCRIPTION: {task_description} FILES_CHANGED: {files_changed} -KNOWLEDGE_CONTEXT: {knowledge_context or 'None'} +KNOWLEDGE_CONTEXT: {knowledge_context} Evaluate against 9-pillar framework. Fix P0/P1 issues. Return structured report. -If knowledge context is provided, check whether any known pitfall patterns are being reintroduced and verify architectural consistency with prior decisions." +Follow devflow:apply-knowledge to scan KNOWLEDGE_CONTEXT and Read full ADR/PF bodies on demand. Skip if (none)." **Wait for completion.** Extract: STATUS (PASS|FIXED|BLOCKED), changes_made (bool) @@ -99,7 +101,7 @@ Display summary: │ ├─ Phase 0: Context gathering │ ├─ Git diff for changed files -│ └─ Read project knowledge (decisions.md + pitfalls.md) +│ └─ Load knowledge index (knowledge-context.cjs index) │ ├─ Phase 1: Simplifier │ └─ Code refinement (commits directly) diff --git a/scripts/hooks/json-helper.cjs b/scripts/hooks/json-helper.cjs index 357c987..448a2d7 100755 --- a/scripts/hooks/json-helper.cjs +++ b/scripts/hooks/json-helper.cjs @@ -188,6 +188,37 @@ function nextKnowledgeId(matches, prefix) { return { nextN, anchorId: `${prefix}-${nextN}` }; } +/** + * Extract the content of a single ADR-NNN / PF-NNN section from a knowledge file. + * Returns the text from the matching `## ` heading through the next `## ADR-` + * or `## PF-` heading (exclusive), or to end-of-file. Returns null when the anchor is + * not present. The anchorId is sanitised before use to eliminate ReDoS surface. + * + * Shared by reconcileExisting (anchored hash path) and the heal block — eliminates the + * duplicated inline regex that appeared at three reconcile-manifest call sites (A2). + * + * @param {string} content - Full file content + * @param {string} anchorId - e.g. 'ADR-001' or 'PF-007' + * @returns {string|null} + */ +function sliceKnowledgeSection(content, anchorId) { + const safe = anchorId.replace(/[^A-Z0-9-]/gi, ''); + const sectionRe = new RegExp(`(##\\s+${safe}[\\s\\S]*?)(?=\\n##\\s+(?:ADR|PF)-|\\s*$)`); + const m = content.match(sectionRe); + return m ? m[1] : null; +} + +/** + * Return a zeroed reconcile-manifest result object. + * Centralises the five-place inline shape `{ deletions: 0, edits: 0, unchanged: 0, healed: 0 }` + * so that adding a counter in the future is a one-line change (C3). + * + * @returns {{ deletions: number, edits: number, unchanged: number, healed: number }} + */ +function emptyReconcileResult() { + return { deletions: 0, edits: 0, unchanged: 0, healed: 0 }; +} + /** * D18: Count only non-deprecated headings in a knowledge file. * Scans ## ADR-NNN: or ## PF-NNN: headings, then checks the next Status @@ -218,6 +249,59 @@ function countActiveHeadings(content, entryType) { return count; } +/** + * Scan decisions.md and pitfalls.md for anchors (ADR-NNN / PF-NNN) that are present in + * the files but not tracked in the manifest. Returns an array of unmanaged anchor descriptors. + * Used by reconcile-manifest to self-heal render-ready crash-window duplicates. + * + * Only sections that contain the `- **Source**: self-learning:` marker qualify — pre-v2 + * seeded entries (which lack the marker) are excluded so they cannot be falsely paired + * with a current `ready` log obs by normalised heading match. Pre-v2 entries are removed + * separately by the v3 migration; until that runs they must remain inert here. + * + * `fileContent` is threaded through the returned descriptor so the heal block can reuse + * the already-read bytes instead of re-reading the file a second time (A3). + * + * Skips scanning when `logMap` contains no `ready` observations — there is nothing to + * pair with, so the I/O would be wasted (P2). + * + * @param {string} memoryDir - Path to .memory dir + * @param {Set} managedAnchors - Anchor IDs already tracked in the manifest + * @param {Map} logMap - Current observation log keyed by obs ID + * @returns {Array<{anchorId: string, type: string, path: string, headingText: string, fileContent: string}>} + */ +function findUnmanagedAnchors(memoryDir, managedAnchors, logMap) { + // P2: short-circuit when no ready observations exist — nothing can be healed. + if (!Array.from(logMap.values()).some(o => o.status === 'ready')) return []; + + // Use only literal (non-dynamic) regexes to avoid ReDoS surface on tainted data. + // prefix values are hardcoded: 'ADR' for decisions, 'PF' for pitfalls. + const result = []; + const files = [ + { file: path.join(memoryDir, 'knowledge', 'decisions.md'), type: 'decision', re: /^## (ADR-\d+):\s*([^\n]+)/gm }, + { file: path.join(memoryDir, 'knowledge', 'pitfalls.md'), type: 'pitfall', re: /^## (PF-\d+):\s*([^\n]+)/gm }, + ]; + for (const { file, type, re } of files) { + if (!fs.existsSync(file)) continue; + const content = fs.readFileSync(file, 'utf8'); + let m; + while ((m = re.exec(content)) !== null) { + if (managedAnchors.has(m[1])) continue; + // Slice out this section's body (heading → next ## heading or eof) and require the + // self-learning source marker — this excludes pre-v2 seeded content from the heal path. + const sectionStart = m.index; + const nextHeadingIdx = content.indexOf('\n## ', sectionStart + 1); + const section = nextHeadingIdx !== -1 + ? content.slice(sectionStart, nextHeadingIdx) + : content.slice(sectionStart); + if (!section.includes('\n- **Source**: self-learning:')) continue; + // A3: thread fileContent so the heal block does not re-read this file. + result.push({ anchorId: m[1], type, path: file, headingText: m[2].trim(), fileContent: content }); + } + } + return result; +} + /** * Read .knowledge-usage.json from .memory dir. Returns {version, entries} or empty default. * @param {string} memoryDir @@ -1356,31 +1440,42 @@ try { const logFile = path.join(cwd, '.memory', 'learning-log.jsonl'); const lockDir = path.join(cwd, '.memory', '.learning.lock'); - if (!fs.existsSync(manifestPath) || !fs.existsSync(logFile)) { - console.log(JSON.stringify({ deletions: 0, edits: 0, unchanged: 0 })); + // A1: require only the log file (not the manifest) before proceeding. + // The heal path must be able to run even when the manifest has never been written + // (e.g. render-ready crashed before its manifest write). A missing manifest is + // treated as an empty one; the heal block then reconstructs it from the log + files. + if (!fs.existsSync(logFile)) { + console.log(JSON.stringify(emptyReconcileResult())); break; } if (!acquireMkdirLock(lockDir, 15000, 60000)) { learningLog('reconcile-manifest: timeout acquiring lock, skipping'); - console.log(JSON.stringify({ deletions: 0, edits: 0, unchanged: 0 })); + console.log(JSON.stringify(emptyReconcileResult())); break; } try { + // C1: loadReconcileState — read manifest (or construct empty) + build logMap. let manifest; - try { - manifest = JSON.parse(fs.readFileSync(manifestPath, 'utf8')); - if (!manifest.entries) manifest.entries = []; - } catch { - console.log(JSON.stringify({ deletions: 0, edits: 0, unchanged: 0 })); - break; + if (fs.existsSync(manifestPath)) { + try { + manifest = JSON.parse(fs.readFileSync(manifestPath, 'utf8')); + if (!manifest.entries) manifest.entries = []; + } catch { + // Corrupt manifest — treat as empty so the heal path can still recover. + manifest = { schemaVersion: 1, entries: [] }; + } + } else { + // A1: no manifest yet; construct empty in-memory so heal can populate it. + manifest = { schemaVersion: 1, entries: [] }; } const logEntries = parseJsonl(logFile); const logMap = new Map(logEntries.map(e => [e.id, e])); - let deletions = 0, edits = 0, unchanged = 0; + // C1: reconcileExisting — walk manifest entries, detect deletions / edits. + const counters = emptyReconcileResult(); const keptEntries = []; for (const entry of manifest.entries) { @@ -1399,7 +1494,7 @@ try { obs.status = 'deprecated'; obs.deprecated_at = new Date().toISOString(); learningLog(`reconcile: deletion detected for ${entry.observationId}, confidence -> ${obs.confidence}`); - deletions++; + counters.deletions++; // Remove manifest entry (don't keep it) continue; } @@ -1414,20 +1509,18 @@ try { obs.status = 'deprecated'; obs.deprecated_at = new Date().toISOString(); learningLog(`reconcile: anchor ${entry.anchorId} missing for ${entry.observationId}`); - deletions++; + counters.deletions++; continue; } - // For anchored entries, hash just the section bytes - const sectionRe = new RegExp(`(##\\s+${entry.anchorId}[\\s\\S]*?)(?=\\n##\\s+(?:ADR|PF)-|\\s*$)`); - const sectionMatch = content.match(sectionRe); - const sectionContent = sectionMatch ? sectionMatch[1] : content; + // A2: use shared sliceKnowledgeSection — eliminates duplicated inline regex. + const sectionContent = sliceKnowledgeSection(content, entry.anchorId) ?? content; const currentHash = contentHash(sectionContent); if (currentHash !== entry.contentHash) { // D13: silently update hash only, no confidence penalty entry.contentHash = currentHash; - edits++; + counters.edits++; } else { - unchanged++; + counters.unchanged++; } } else { const content = fs.readFileSync(filePath, 'utf8'); @@ -1435,21 +1528,55 @@ try { if (currentHash !== entry.contentHash) { // D13: silently update hash only entry.contentHash = currentHash; - edits++; + counters.edits++; } else { - unchanged++; + counters.unchanged++; } } keptEntries.push(entry); } + // C1: healUnmanagedAnchors — recover from render-ready crash-window duplicates. + // If render-ready wrote the knowledge file but crashed before updating the log + // and manifest, the anchor exists in the file but the log still shows status=ready + // and the manifest has no entry. We detect this by scanning knowledge files for + // anchors not tracked in the manifest, then matching them against ready log + // observations with a matching normalised pattern. + // DESIGN: D-D — skip silently when zero or multiple log entries match (ambiguity guard). + const memoryDir = path.join(cwd, '.memory'); + const managedAnchors = new Set(keptEntries.filter(e => e.anchorId).map(e => e.anchorId)); + // P2 early-exit is inside findUnmanagedAnchors; pass logMap so it can check. + const unmanaged = findUnmanagedAnchors(memoryDir, managedAnchors, logMap); + for (const u of unmanaged) { + const headingNorm = normalizeForDedup(u.headingText); + const candidates = Array.from(logMap.values()).filter(o => + o.type === u.type && o.status === 'ready' && + normalizeForDedup(o.pattern) === headingNorm, + ); + if (candidates.length !== 1) continue; // 0 = user-curated, >1 = ambiguous (D-D: silent) + const obs = candidates[0]; + obs.status = 'created'; + obs.artifact_path = `${u.path}#${u.anchorId}`; + // A2: use shared sliceKnowledgeSection; A3: use fileContent already read by findUnmanagedAnchors. + const section = sliceKnowledgeSection(u.fileContent, u.anchorId); + keptEntries.push({ + observationId: obs.id, type: u.type, path: u.path, + contentHash: contentHash(section ?? u.headingText), + renderedAt: new Date().toISOString(), anchorId: u.anchorId, + }); + registerUsageEntry(memoryDir, u.anchorId); + counters.healed++; + learningLog(`reconcile: healed ${obs.id} → ${u.anchorId}`); + } + // Atomic writes writeJsonlAtomic(logFile, Array.from(logMap.values())); manifest.entries = keptEntries; + fs.mkdirSync(path.dirname(manifestPath), { recursive: true }); writeFileAtomic(manifestPath, JSON.stringify(manifest, null, 2)); - console.log(JSON.stringify({ deletions, edits, unchanged })); + console.log(JSON.stringify(counters)); } finally { releaseLock(lockDir); } diff --git a/scripts/hooks/lib/knowledge-context.cjs b/scripts/hooks/lib/knowledge-context.cjs new file mode 100644 index 0000000..0e4e252 --- /dev/null +++ b/scripts/hooks/lib/knowledge-context.cjs @@ -0,0 +1,247 @@ +// scripts/hooks/lib/knowledge-context.cjs +// Deterministic project knowledge loader for orchestration surfaces. +// +// DESIGN: Orchestration surfaces (resolve.md, plan.md, code-review.md, etc.) +// instruct the orchestrator to strip Deprecated and Superseded knowledge entries +// before passing KNOWLEDGE_CONTEXT to consumer agents. +// Having this logic as a pure CJS module gives us: +// 1. Deterministic filtering — not LLM-interpreted, always consistent. +// 2. Real test coverage — tests import this module directly. +// 3. CLI interface — orchestrators invoke as: +// node scripts/hooks/lib/knowledge-context.cjs index {worktree} +// and capture the output as KNOWLEDGE_CONTEXT (compact index format). +// +// This module is the single source of truth for the D-A filter algorithm +// (strip ## ADR-NNN / ## PF-NNN sections marked Deprecated or Superseded). + +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +/** @typedef {{ id: string, title: string, status: string, area: string|null }} IndexEntry */ + +/** Statuses recognised by the index formatter — everything else renders as [unknown]. */ +const KNOWN_STATUSES = ['Active', 'Deprecated', 'Superseded']; + +/** + * Return true when a markdown section is marked Deprecated or Superseded. + * This is the single predicate backing the D-A filter algorithm described in + * the DESIGN comment above — every call-site that needs to strip inactive + * knowledge entries should use this function. + * + * @param {string} section - raw text of one ## ADR-NNN / ## PF-NNN section + * @returns {boolean} + */ +function isDeprecatedOrSuperseded(section) { + return ( + /- \*\*Status\*\*: Deprecated/.test(section) || + /- \*\*Status\*\*: Superseded/.test(section) + ); +} + +/** + * Filter raw decisions.md / pitfalls.md content, removing any ## ADR-NNN: or + * ## PF-NNN: section whose body contains `- **Status**: Deprecated` or + * `- **Status**: Superseded`. + * + * Section boundary = next ## ADR/PF heading or end of string. + * Non-knowledge content before the first section header (e.g., a file-level + * title) is preserved in sections[0] and always kept. + * + * @param {string} raw - raw content from decisions.md or pitfalls.md + * @returns {string} filtered content (trimmed), or '' if nothing remains + */ +function filterKnowledgeContext(raw) { + if (!raw.trim()) return ''; + // Split on ADR-NNN / PF-NNN section boundaries using a lookahead so each + // section includes its own heading. + const sections = raw.split(/(?=^## (?:ADR|PF)-\d+:)/m); + const kept = sections.filter(section => { + const isKnowledgeSection = /^## (?:ADR|PF)-\d+:/m.test(section); + if (!isKnowledgeSection) return true; // keep preamble / non-knowledge content + return !isDeprecatedOrSuperseded(section); + }); + return kept.join('').trim(); +} + +/** + * Extract index entries from raw decisions.md / pitfalls.md content. + * Applies the same D-A filter as filterKnowledgeContext before extracting. + * + * @param {string} raw - raw content from decisions.md or pitfalls.md + * @returns {IndexEntry[]} array of index entries (empty if none survive filter) + */ +function extractIndexEntries(raw) { + if (!raw.trim()) return []; + const sections = raw.split(/(?=^## (?:ADR|PF)-\d+:)/m); + /** @type {IndexEntry[]} */ + const entries = []; + + for (const section of sections) { + const headingMatch = section.match(/^## ((?:ADR|PF)-\d+): (.+)/m); + if (!headingMatch) continue; // preamble or non-knowledge content + + if (isDeprecatedOrSuperseded(section)) continue; + + const id = headingMatch[1]; + const rawTitle = headingMatch[2].trim(); + + // Extract status line + const statusMatch = section.match(/- \*\*Status\*\*: (.+)/); + const status = statusMatch ? statusMatch[1].trim() : null; + + // Extract area line (pitfalls only, optional) + const areaMatch = section.match(/- \*\*Area\*\*: (.+)/); + const area = areaMatch ? areaMatch[1].trim() : null; + + entries.push({ id, title: rawTitle, status, area }); + } + + return entries; +} + +/** + * Truncate a string to maxLen characters, appending '…' if truncated. + * + * @param {string} str + * @param {number} maxLen + * @returns {string} + */ +function truncate(str, maxLen) { + if (str.length <= maxLen) return str; + return str.slice(0, maxLen) + '…'; +} + +/** + * Format a single index line for an ADR or PF entry. + * ADR entries have `area: null`, so the area suffix is naturally omitted. + * + * @param {IndexEntry} entry + * @returns {string} + */ +function formatEntryLine(entry) { + const title = truncate(entry.title, 60); + const tag = entry.status && KNOWN_STATUSES.includes(entry.status) ? `[${entry.status}]` : '[unknown]'; + const areaSuffix = entry.area ? ` — ${truncate(entry.area, 80)}` : ''; + return ` ${entry.id} ${title} ${tag}${areaSuffix}`; +} + +/** + * Load a compact index of project knowledge entries for a given worktree. + * + * Returns a compact index listing each ADR/PF entry with ID, truncated + * title, status, and (for pitfalls) area. Includes a footer describing how to + * Read full bodies on demand. Returns '(none)' when both files are absent or + * their filtered content is empty. + * + * @param {string} worktreePath - absolute path to the worktree root + * @param {{ decisionsFile?: string, pitfallsFile?: string }} [opts] - override + * file paths for testing (relative paths resolved against worktreePath) + * @returns {string} compact index string, or '(none)' + */ +function loadKnowledgeIndex(worktreePath, opts = {}) { + const decisionsFile = opts.decisionsFile + ? path.resolve(worktreePath, opts.decisionsFile) + : path.join(worktreePath, '.memory', 'knowledge', 'decisions.md'); + + const pitfallsFile = opts.pitfallsFile + ? path.resolve(worktreePath, opts.pitfallsFile) + : path.join(worktreePath, '.memory', 'knowledge', 'pitfalls.md'); + + /** @type {IndexEntry[]} */ + let adrEntries = []; + /** @type {IndexEntry[]} */ + let pfEntries = []; + let hasDecisionsFile = false; + let hasPitfallsFile = false; + + try { + const raw = fs.readFileSync(decisionsFile, 'utf8'); + hasDecisionsFile = true; + adrEntries = extractIndexEntries(raw); + } catch { + // Skip silently if absent + } + + try { + const raw = fs.readFileSync(pitfallsFile, 'utf8'); + hasPitfallsFile = true; + pfEntries = extractIndexEntries(raw); + } catch { + // Skip silently if absent + } + + if (adrEntries.length === 0 && pfEntries.length === 0) return '(none)'; + + const blocks = []; + + if (adrEntries.length > 0) { + const lines = [`Decisions (${adrEntries.length}):`]; + for (const entry of adrEntries) { + lines.push(formatEntryLine(entry)); + } + blocks.push(lines.join('\n')); + } + + if (pfEntries.length > 0) { + const lines = [`Pitfalls (${pfEntries.length}):`]; + for (const entry of pfEntries) { + lines.push(formatEntryLine(entry)); + } + blocks.push(lines.join('\n')); + } + + // Footer: explain how to read full bodies + const footerLines = []; + if (adrEntries.length > 0) { + footerLines.push( + `ADR-NNN entries live in ${decisionsFile}` + ); + } + if (pfEntries.length > 0) { + footerLines.push( + `PF-NNN entries live in ${pitfallsFile}` + ); + } + footerLines.push( + 'Read the relevant file and locate the matching `## ADR-NNN:` or `## PF-NNN:` heading for the full body.' + ); + + blocks.push(footerLines.join('\n')); + + return blocks.join('\n\n'); +} + +// --------------------------------------------------------------------------- +// CLI interface +// +// Usage: +// node knowledge-context.cjs index → compact index +// --------------------------------------------------------------------------- + +if (require.main === module) { + const argv = process.argv.slice(2); + + if (argv[0] !== 'index' || !argv[1]) { + process.stderr.write( + 'Usage: node knowledge-context.cjs index \n' + ); + process.exit(1); + } + + const worktreePath = path.resolve(argv[1]); + const result = loadKnowledgeIndex(worktreePath); + if (result !== '(none)') { + const adrCount = (result.match(/^\s+ADR-\d+/gm) || []).length; + const pfCount = (result.match(/^\s+PF-\d+/gm) || []).length; + const entries = adrCount + pfCount; + process.stderr.write( + `[knowledge-context] mode=index worktree=${worktreePath} entries=${entries}\n` + ); + } + process.stdout.write(result + '\n'); + process.exit(0); +} + +module.exports = { filterKnowledgeContext, loadKnowledgeIndex, extractIndexEntries }; diff --git a/shared/agents/designer.md b/shared/agents/designer.md index 0e56cb9..2d1ba58 100644 --- a/shared/agents/designer.md +++ b/shared/agents/designer.md @@ -2,7 +2,7 @@ name: Designer description: Design analysis agent with mode-driven skill loading. Modes: gap-analysis (completeness, architecture, security, performance, consistency, dependencies), design-review (anti-pattern detection). model: opus -skills: devflow:worktree-support +skills: devflow:worktree-support, devflow:apply-knowledge --- # Designer Agent @@ -18,6 +18,12 @@ The orchestrator provides: **Worktree Support**: If `WORKTREE_PATH` is provided, follow the `devflow:worktree-support` skill for path resolution. If omitted, use cwd. +- **KNOWLEDGE_CONTEXT** (optional): Compact index of active ADR/PF entries for this worktree (generated by `knowledge-context.cjs index`). `(none)` when absent. Use `devflow:apply-knowledge` to Read full bodies on demand. + +## Apply Knowledge + +Follow the `devflow:apply-knowledge` skill to scan the `KNOWLEDGE_CONTEXT` index, Read full ADR/PF bodies on demand, and cite `applies ADR-NNN` / `avoids PF-NNN` in findings. Skip when `KNOWLEDGE_CONTEXT` is empty or `(none)`. + ## Modes | Mode | Focus (optional) | Skill File (Read this first) | @@ -29,9 +35,10 @@ The orchestrator provides: 1. **Load mode skill** — Read the skill file from the table above for your assigned mode. This gives you detection patterns and checklists specific to your analysis type. 2. **Apply focus-specific analysis** — Use detection patterns from the loaded skill to scan the provided artifacts. For `gap-analysis`, apply only the patterns for your assigned focus. For `design-review`, apply all 6 anti-pattern rules. -3. **Assess confidence (0-100%)** — For each finding, assess certainty. Report at 80%+, suggest at 60-79%, drop below 60%. -4. **Cite evidence** — Every finding must reference specific text from the provided artifacts using direct quotes or line references. -5. **Write findings to output** — Format findings clearly with severity, confidence, evidence, and resolution. +3. **Apply Knowledge** — See [Apply Knowledge](#apply-knowledge) section above. Skip when `KNOWLEDGE_CONTEXT` is empty or `(none)`. +4. **Assess confidence (0-100%)** — For each finding, assess certainty. Report at 80%+, suggest at 60-79%, drop below 60%. +5. **Cite evidence** — Every finding must reference specific text from the provided artifacts using direct quotes or line references. +6. **Write findings to output** — Format findings clearly with severity, confidence, evidence, and resolution. ## Output diff --git a/shared/agents/resolver.md b/shared/agents/resolver.md index e7b4c10..47b69ff 100644 --- a/shared/agents/resolver.md +++ b/shared/agents/resolver.md @@ -2,7 +2,7 @@ name: Resolver description: Validates review issues, implements fixes with risk-proportional care. Tech debt only for architectural overhauls. model: sonnet -skills: devflow:software-design, devflow:git, devflow:patterns, devflow:test-driven-development, devflow:worktree-support +skills: devflow:software-design, devflow:git, devflow:patterns, devflow:test-driven-development, devflow:worktree-support, devflow:apply-knowledge --- # Resolver Agent @@ -15,6 +15,7 @@ You receive from orchestrator: - **ISSUES**: Array of issues to resolve, each with `id`, `file`, `line`, `severity`, `type`, `description`, `suggested_fix` - **BRANCH**: Current branch slug - **BATCH_ID**: Identifier for this batch of issues +- **KNOWLEDGE_CONTEXT** (optional): Compact index of active ADR/PF entries for this worktree (generated by `knowledge-context.cjs index`). `(none)` when absent. Use `devflow:apply-knowledge` to Read full bodies on demand. **Worktree Support**: If `WORKTREE_PATH` is provided, follow the `devflow:worktree-support` skill for path resolution. If omitted, use cwd. @@ -30,13 +31,15 @@ You receive from orchestrator: - Reviewer misunderstood context - Code is intentional (e.g., magic number with comment, deliberate complexity for performance, placeholder for planned feature) -3. **Assess risk for valid issues**: Apply risk criteria to decide FIX vs TECH_DEBT. +3. **Apply Knowledge**: See [Apply Knowledge](#apply-knowledge) section below. -4. **Implement fixes**: Make changes following existing patterns. One logical change per commit. +4. **Assess risk for valid issues**: Apply risk criteria to decide FIX vs TECH_DEBT. -5. **Document all decisions**: Record reasoning for every classification and risk assessment. +5. **Implement fixes**: Make changes following existing patterns. One logical change per commit. -6. **Commit batch**: Create atomic commit with all fixes in this batch. +6. **Document all decisions**: Record reasoning for every classification and risk assessment. + +7. **Commit batch**: Create atomic commit with all fixes in this batch. ## Risk Assessment @@ -73,6 +76,10 @@ For careful fixes, follow the systematic refactoring protocol: This is the ONLY case where deferral is appropriate. "Touches many files" or "changes public API" are NOT reasons to defer — they're reasons to be careful. +## Apply Knowledge + +Follow the `devflow:apply-knowledge` skill to scan the index, Read full bodies on demand, and cite `applies ADR-NNN` / `avoids PF-NNN` inline in the Reasoning column. Skip when `KNOWLEDGE_CONTEXT` is empty or `(none)`. Use only verbatim IDs from the index — do not fabricate entry IDs. + ## Decision Flow ``` diff --git a/shared/agents/reviewer.md b/shared/agents/reviewer.md index 6322a25..48618b8 100644 --- a/shared/agents/reviewer.md +++ b/shared/agents/reviewer.md @@ -2,7 +2,7 @@ name: Reviewer description: Universal code review agent with parameterized focus. Dynamically loads pattern skill for assigned focus area. model: opus -skills: devflow:review-methodology, devflow:worktree-support +skills: devflow:review-methodology, devflow:worktree-support, devflow:apply-knowledge --- # Reviewer Agent @@ -16,6 +16,7 @@ The orchestrator provides: - **Branch context**: What changes to review - **Output path**: Where to save findings (e.g., `.docs/reviews/{branch}/{timestamp}/{focus}.md`) - **DIFF_COMMAND** (optional): Specific diff command to use (e.g., `git diff {sha}...HEAD` for incremental reviews). If not provided, default to `git diff {base_branch}...HEAD`. +- **KNOWLEDGE_CONTEXT** (optional): Compact index of active ADR/PF entries for this worktree (generated by `knowledge-context.cjs index`). `(none)` when absent. Use `devflow:apply-knowledge` to Read full bodies on demand. **Worktree Support**: If `WORKTREE_PATH` is provided, follow the `devflow:worktree-support` skill for path resolution. If omitted, use cwd. @@ -42,14 +43,18 @@ The orchestrator provides: | `python` | `~/.claude/skills/devflow:python/SKILL.md` | | `rust` | `~/.claude/skills/devflow:rust/SKILL.md` | -## Responsibilities +## Apply Knowledge + +Follow the `devflow:apply-knowledge` skill to scan the `KNOWLEDGE_CONTEXT` index, Read full ADR/PF bodies on demand, and cite `applies ADR-NNN` / `avoids PF-NNN` inline in findings. Skip when `KNOWLEDGE_CONTEXT` is empty or `(none)`. -1. **Load focus skill** - Read the pattern skill file for your focus area from the table above. This gives you detection rules and patterns specific to your review type. -2. **Check known pitfalls** - If `.memory/knowledge/pitfalls.md` exists, read it. Check if any pitfall Areas overlap with files in the current diff. Verify the Resolution was applied. Flag if a known pitfall pattern is being reintroduced. - -When you apply a decision from `.memory/knowledge/decisions.md` or avoid a pitfall from `.memory/knowledge/pitfalls.md`, cite the entry ID in your final summary (e.g., 'applying ADR-003' or 'per PF-002') so usage can be tracked for capacity reviews. +When you apply a decision or avoid a pitfall identified via the KNOWLEDGE_CONTEXT index (after reading its full body per the `devflow:apply-knowledge` skill), cite the entry ID inline: `applies ADR-NNN` or `avoids PF-NNN`. + +## Responsibilities + +1. **Load focus skill** - Read the pattern skill file for your focus area from the table above. This gives you detection rules and patterns specific to your review type. +2. **Apply Knowledge** - Follow `devflow:apply-knowledge` (see section above) to scan the index and cite relevant entries in findings. 3. **Identify changed lines** - Get diff against base branch (main/master/develop/integration/trunk) 4. **Apply 3-category classification** - Sort issues by where they occur 5. **Apply focus-specific analysis** - Use pattern skill detection rules from the loaded skill file diff --git a/shared/agents/scrutinizer.md b/shared/agents/scrutinizer.md index b71e9fd..dcdebef 100644 --- a/shared/agents/scrutinizer.md +++ b/shared/agents/scrutinizer.md @@ -2,7 +2,7 @@ name: Scrutinizer description: Self-review agent that evaluates and fixes implementation issues using 9-pillar framework. Runs in fresh context after Coder completes. model: opus -skills: devflow:quality-gates, devflow:software-design, devflow:worktree-support +skills: devflow:quality-gates, devflow:software-design, devflow:worktree-support, devflow:apply-knowledge --- # Scrutinizer Agent @@ -14,9 +14,14 @@ You are a meticulous self-review specialist. You evaluate implementations agains You receive from orchestrator: - **TASK_DESCRIPTION**: What was implemented - **FILES_CHANGED**: List of modified files from Coder output +- **KNOWLEDGE_CONTEXT** (optional): Compact index of active ADR/PF entries for this worktree (generated by `knowledge-context.cjs index`). `(none)` when absent. Use `devflow:apply-knowledge` to Read full bodies on demand. **Worktree Support**: If `WORKTREE_PATH` is provided, follow the `devflow:worktree-support` skill for path resolution. If omitted, use cwd. +## Apply Knowledge + +Follow the `devflow:apply-knowledge` skill to scan the index, Read full bodies on demand, and verify the implementation is consistent with prior architectural decisions and avoids known pitfalls. Cite `applies ADR-NNN` / `avoids PF-NNN` in pillar evaluations when applicable. Skip when `KNOWLEDGE_CONTEXT` is empty or `(none)`. + ## Responsibilities 1. **Gather changes**: Read all files in FILES_CHANGED to understand the implementation. diff --git a/shared/skills/apply-knowledge/SKILL.md b/shared/skills/apply-knowledge/SKILL.md new file mode 100644 index 0000000..9d8bb91 --- /dev/null +++ b/shared/skills/apply-knowledge/SKILL.md @@ -0,0 +1,99 @@ +--- +name: apply-knowledge +description: Canonical algorithm for consuming KNOWLEDGE_CONTEXT index — scan index, identify relevant entries, Read full bodies on demand, cite verbatim IDs inline. +allowed-tools: Read +--- + +# Apply Knowledge + +Canonical consumer algorithm for the `KNOWLEDGE_CONTEXT` index passed by orchestrators. The index lists each ADR/PF entry with ID, truncated title, status, and area — not the full body. Use this skill to surface the right decisions and pitfalls for your task without loading the entire corpus. + +## Iron Law + +> **VERBATIM IDs ONLY — NEVER FABRICATE** +> +> Cite only IDs that appear verbatim in KNOWLEDGE_CONTEXT. If an ADR/PF ID is not +> in the index, do not cite it. If an entry looks relevant but you haven't Read its +> full body, do not cite it. Fabricated citations are worse than no citations. + +--- + +## 5-Step Algorithm + +### Step 1: Scan the index + +Read through all entries in `KNOWLEDGE_CONTEXT`. The index format is: + +``` +Decisions (N): + ADR-001 Title truncated to 60 chars [Active] + ADR-002 Another decision [Active] + +Pitfalls (M): + PF-004 Background hook god scripts [Active] — scripts/hooks/foo.cjs + PF-011 KNOWLEDGE_CONTEXT fan-out [Active] — plugins/devflow-resolve/... + +ADR-NNN entries live in {worktree}/.memory/knowledge/decisions.md +PF-NNN entries live in {worktree}/.memory/knowledge/pitfalls.md +Read the relevant file and locate the matching `## ADR-NNN:` or `## PF-NNN:` heading for the full body. +``` + +### Step 2: Identify plausibly-relevant entries + +From the index, identify entries whose title or area plausibly overlaps with: +- The files you are modifying or reviewing +- The category of issue you are addressing (e.g., error handling, hook scripts, JSON parsing) +- The architectural area of your change + +Titles are truncated to 60 characters — if a truncated title looks relevant, proceed to Step 3. + +### Step 3: Read the full body + +For each plausibly-relevant entry, use the Read tool to open the knowledge file listed in the `KNOWLEDGE_CONTEXT` footer and locate the matching `## ADR-NNN:` or `## PF-NNN:` heading. Read the full section to confirm relevance and understand the decision or pitfall completely. + +**The footer is the single source of truth for file paths.** Never substitute hardcoded paths — the footer resolves to the correct worktree, which may differ from your cwd in multi-worktree flows. + +``` +Use the exact paths from the KNOWLEDGE_CONTEXT footer, e.g.: + {worktree-from-footer}/.memory/knowledge/decisions.md → find ## ADR-NNN: heading + {worktree-from-footer}/.memory/knowledge/pitfalls.md → find ## PF-NNN: heading +``` + +Only cite an entry after you have read its full body and confirmed it applies. + +### Step 4: Cite inline + +When applying a prior decision, cite as `applies ADR-NNN` in your reasoning or output. When avoiding a known pitfall, cite as `avoids PF-NNN`. Place citations in the Reasoning column of decision tables, in inline comments, or in your structured output — wherever your agent's output format captures rationale. + +### Step 5: Use verbatim IDs only + +Cite only IDs that appear verbatim in `KNOWLEDGE_CONTEXT`. Do not guess at IDs that might exist. Do not construct IDs from memory. If no entry is clearly relevant, skip citation entirely — silence is correct when nothing applies. + +--- + +## Worked Example + +**Scenario**: Reviewing `scripts/hooks/background-learning` for issues. + +1. **Scan** — Index shows `PF-004 Background hook god scripts [Active] — scripts/hooks/foo.cjs` +2. **Identify** — Area field includes `scripts/hooks/` which overlaps with the file under review +3. **Read** — Open the pitfalls file at the path given in the KNOWLEDGE_CONTEXT footer (e.g., `/.memory/knowledge/pitfalls.md`), find `## PF-004:` section, read full body +4. **Cite** — If the file shows signs of the god-script pattern, note `avoids PF-004` in reasoning +5. **Verbatim** — ID `PF-004` appeared in the index; citation is valid + +--- + +## Skip Guard + +When `KNOWLEDGE_CONTEXT` is empty, `(none)`, or not provided: skip this skill entirely. Do not attempt to load knowledge files independently. Do not speculate about what decisions or pitfalls might exist. + +--- + +## Citation Format Reference + +| Situation | Citation | +|-----------|----------| +| Applying a prior architectural decision | `applies ADR-NNN` | +| Avoiding a known pitfall | `avoids PF-NNN` | +| Entry not in index | (no citation — silence is correct) | +| Entry in index but not read yet | (no citation — read first) | diff --git a/shared/skills/debug:orch/SKILL.md b/shared/skills/debug:orch/SKILL.md index e0b8055..e5d0c70 100644 --- a/shared/skills/debug:orch/SKILL.md +++ b/shared/skills/debug:orch/SKILL.md @@ -24,6 +24,16 @@ This is a lightweight variant of `/debug` for ambient ORCHESTRATED mode. Exclude If the orchestrator receives a `WORKTREE_PATH` context (e.g., from multi-worktree workflows), pass it through to all spawned agents. Each agent's "Worktree Support" section handles path resolution. +## Phase 0: Load Knowledge Index (Orchestrator-Local) + +Before hypothesizing, load the knowledge index: + +```bash +KNOWLEDGE_CONTEXT=$(node scripts/hooks/lib/knowledge-context.cjs index "{worktree}") +``` + +The orchestrator uses `KNOWLEDGE_CONTEXT` locally when generating hypotheses (Phase 1) — prior pitfalls and decisions can suggest specific root causes to investigate. Follow `devflow:apply-knowledge` to Read full entry bodies on demand. **Do NOT pass `KNOWLEDGE_CONTEXT` to Explore sub-agents** — knowledge context stays in the orchestrator, not in the investigation workers. + ## Phase 1: Hypothesize Analyze the bug description, error messages, and conversation context. Generate 3-5 hypotheses that are: diff --git a/shared/skills/plan:orch/SKILL.md b/shared/skills/plan:orch/SKILL.md index 27f2082..6e30239 100644 --- a/shared/skills/plan:orch/SKILL.md +++ b/shared/skills/plan:orch/SKILL.md @@ -34,6 +34,16 @@ If the orchestrator receives a `WORKTREE_PATH` context (e.g., from multi-worktre --- +## Phase 0: Load Knowledge Index + +Before spawning any agents, load the knowledge index for the current worktree: + +```bash +KNOWLEDGE_CONTEXT=$(node scripts/hooks/lib/knowledge-context.cjs index "{worktree}") +``` + +This produces a compact index of active ADR/PF entries. Pass `KNOWLEDGE_CONTEXT` to Explorer and Designer agents — prior decisions constrain design, known pitfalls inform gap analysis. Agents use `devflow:apply-knowledge` to Read full entry bodies on demand. + ## Phase 1: Orient Spawn `Agent(subagent_type="Skimmer")` to get codebase overview relevant to the planning question: @@ -51,6 +61,8 @@ Based on Skimmer findings, spawn 2-3 `Agent(subagent_type="Explore")` agents **i - **Pattern explorer**: Find existing implementations of similar features to follow as templates - **Constraint explorer**: Identify constraints — test infrastructure, build system, CI requirements, deployment concerns +Each Explore agent receives `KNOWLEDGE_CONTEXT` (from Phase 0) and the instruction: "follow `devflow:apply-knowledge` for KNOWLEDGE_CONTEXT". + Adjust explorer focus based on the specific planning question. ## Phase 3: Gap Analysis Lite @@ -61,20 +73,24 @@ Spawn 2 `Agent(subagent_type="Designer")` agents **in a single message** (parall Agent(subagent_type="Designer"): "Mode: gap-analysis Focus: completeness +KNOWLEDGE_CONTEXT: {knowledge_context} Artifacts: Planning question: {user's intent} Exploration findings: {Phase 2 outputs} Codebase context: {Phase 1 output} -Identify missing requirements, undefined error states, vague acceptance criteria." +Identify missing requirements, undefined error states, vague acceptance criteria. +Follow devflow:apply-knowledge for KNOWLEDGE_CONTEXT." Agent(subagent_type="Designer"): "Mode: gap-analysis Focus: architecture +KNOWLEDGE_CONTEXT: {knowledge_context} Artifacts: Planning question: {user's intent} Exploration findings: {Phase 2 outputs} Codebase context: {Phase 1 output} -Identify pattern violations, missing integration points, layering issues." +Identify pattern violations, missing integration points, layering issues. +Follow devflow:apply-knowledge for KNOWLEDGE_CONTEXT." ``` ## Phase 4: Synthesize diff --git a/shared/skills/resolve:orch/SKILL.md b/shared/skills/resolve:orch/SKILL.md index 4c42a5e..5989b6e 100644 --- a/shared/skills/resolve:orch/SKILL.md +++ b/shared/skills/resolve:orch/SKILL.md @@ -30,6 +30,13 @@ If no unresolved review found: halt with "No unresolved review found. Run a revi Extract branch slug from the directory path. + +## Phase 1.5: Load Project Knowledge + +Run `node scripts/hooks/lib/knowledge-context.cjs index "{worktree}"` to produce a compact index of active ADR/PF entries from `decisions.md` and `pitfalls.md`, with Deprecated/Superseded entries already stripped. Falls back to `(none)` when both files are absent or all entries are filtered. Pass `KNOWLEDGE_CONTEXT` to every Resolver agent in Phase 4. Resolver agents use `devflow:apply-knowledge` to Read full entry bodies on demand — no fan-out of the full corpus. + ## Phase 2: Parse Issues Read all `{focus}.md` files in the timestamped directory (exclude `review-summary.md` and `resolution-summary.md`). @@ -57,6 +64,7 @@ Each receives: - **ISSUES**: Array of issues in the batch - **BRANCH**: Branch slug - **BATCH_ID**: Identifier for this batch +- **KNOWLEDGE_CONTEXT**: Knowledge index from Phase 1.5 (or `(none)`). Resolvers follow `devflow:apply-knowledge` to Read full ADR/PF bodies on demand. Resolvers follow a 3-tier risk approach: - **Standard fixes**: Applied directly @@ -74,11 +82,14 @@ Spawn `Agent(subagent_type="Simplifier")` on all files modified by Resolvers. Write `resolution-summary.md` to the same timestamped review directory. +The report includes a `## Knowledge Citations` section at the top (before Statistics) listing all unique `applies ADR-NNN` and `avoids PF-NNN` references extracted from Resolver Reasoning columns. Omit the section entirely if no citations were made. + Report to user: - Issues resolved vs deferred vs false positives - Files modified - Commits created - Remaining issues (if any deferred) +- Knowledge citations applied (if any) ## Error Handling diff --git a/shared/skills/review:orch/SKILL.md b/shared/skills/review:orch/SKILL.md index 8292175..6e33ae5 100644 --- a/shared/skills/review:orch/SKILL.md +++ b/shared/skills/review:orch/SKILL.md @@ -37,6 +37,16 @@ Check `.docs/reviews/{branch_slug}/.last-review-head`: Generate timestamp: `YYYY-MM-DD_HHMM` Create directory: `mkdir -p .docs/reviews/{branch_slug}/{timestamp}` +## Phase 2b: Load Knowledge Index + +After incremental detection, load the knowledge index: + +```bash +KNOWLEDGE_CONTEXT=$(node scripts/hooks/lib/knowledge-context.cjs index "{worktree}") +``` + +This produces a compact index of active ADR/PF entries. Pass `KNOWLEDGE_CONTEXT` to all Reviewer agents. Reviewers use `devflow:apply-knowledge` to Read full entry bodies on demand. + ## Phase 3: File Analysis Run `git diff --name-only {DIFF_RANGE}` to get changed files. @@ -72,6 +82,7 @@ Each reviewer receives: - **Branch context**: branch → base_branch - **Output path**: `.docs/reviews/{branch_slug}/{timestamp}/{focus}.md` - **DIFF_COMMAND**: `git diff {DIFF_RANGE}` (incremental or full) +- **KNOWLEDGE_CONTEXT**: compact index from Phase 2b (or `(none)` when absent) — follow `devflow:apply-knowledge` to Read full ADR/PF bodies on demand ## Phase 5: Synthesis (Parallel) diff --git a/src/cli/plugins.ts b/src/cli/plugins.ts index e23a3fe..7be2872 100644 --- a/src/cli/plugins.ts +++ b/src/cli/plugins.ts @@ -47,7 +47,7 @@ export const DEVFLOW_PLUGINS: PluginDefinition[] = [ description: 'Auto-activating quality enforcement skills - foundation layer for all Devflow plugins', commands: [], agents: [], - skills: ['software-design', 'docs-framework', 'git', 'boundary-validation', 'research', 'test-driven-development', 'testing'], + skills: ['apply-knowledge', 'software-design', 'docs-framework', 'git', 'boundary-validation', 'research', 'test-driven-development', 'testing'], }, { name: 'devflow-plan', @@ -391,6 +391,8 @@ export const LEGACY_SKILL_NAMES: string[] = [ // v2.x plan plugin: new skills bare names for pre-namespace installs 'gap-analysis', 'design-review', + // v2.x knowledge index pattern: new shared skill bare name for pre-namespace installs + 'apply-knowledge', ]; /** diff --git a/src/cli/utils/legacy-knowledge-purge.ts b/src/cli/utils/legacy-knowledge-purge.ts index 92a2be3..a5c1923 100644 --- a/src/cli/utils/legacy-knowledge-purge.ts +++ b/src/cli/utils/legacy-knowledge-purge.ts @@ -39,6 +39,9 @@ export interface PurgeLegacyKnowledgeResult { files: string[]; } +/** Typed pair of (file path, section-prefix). Prefix is 'ADR' for decisions.md, 'PF' for pitfalls.md. */ +type KnowledgeFilePair = readonly [string, 'ADR' | 'PF']; + function escapeRegExp(str: string): string { return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); } @@ -73,25 +76,44 @@ async function acquireMkdirLock( } /** - * Remove pre-v2 low-signal knowledge entries from decisions.md and pitfalls.md. + * Regex matching any `## ADR-NNN:` or `## PF-NNN:` section heading and all + * lines that belong to that section (up to the next `## ` heading or end of + * file). The leading `\n` is included so removal does not leave a blank line + * between sections when the preceding section ends with a newline. + */ +const SECTION_REGEX = /\n## (?:ADR|PF)-\d+:[^\n]*(?:\n(?!## )[^\n]*)*/g; + +/** + * Source marker that identifies a v2-era self-learning entry. Any section + * containing this literal string is authored by the background-learning + * extractor and must be preserved. Sections without it are pre-v2 seeded + * content injected at install time. + */ +const SELF_LEARNING_SOURCE_MARKER = '\n- **Source**: self-learning:'; + +/** + * Shared lock-and-loop helper used by both purge functions. * - * The entries targeted are: - * - ADR-002 (decisions.md) - * - PF-001, PF-003, PF-005 (pitfalls.md) + * Acquires the knowledge lock, then for each file in `filePrefixPairs`: + * 1. Reads the file (skips if absent). + * 2. Calls `rewriteContent(content, prefix)` to get an updated string and a + * removed-section count. + * 3. If the content changed, rewrites the TL;DR comment and writes atomically. * - * Returns immediately if `.memory/knowledge/` does not exist. + * The `rewriteContent` callback owns all domain-specific removal logic + * (allowlist-based for v2, source-marker-based for v3), keeping this helper + * free of policy. * - * @param options.memoryDir - absolute path to the `.memory/` directory - * @returns number of sections removed and list of files that were modified - * @throws if lock acquisition times out + * @param memoryDir Absolute path to `.memory/` + * @param filePrefixPairs Files to process with their heading prefix + * @param rewriteContent Per-file transform: returns updated content + sections removed */ -export async function purgeLegacyKnowledgeEntries(options: { - memoryDir: string; -}): Promise { - const { memoryDir } = options; +async function withKnowledgeFiles( + memoryDir: string, + filePrefixPairs: readonly KnowledgeFilePair[], + rewriteContent: (content: string, prefix: 'ADR' | 'PF') => { updated: string; removedCount: number }, +): Promise { const knowledgeDir = path.join(memoryDir, 'knowledge'); - const decisionsPath = path.join(knowledgeDir, 'decisions.md'); - const pitfallsPath = path.join(knowledgeDir, 'pitfalls.md'); // Bail early: nothing to do if knowledge directory doesn't exist try { @@ -110,11 +132,6 @@ export async function purgeLegacyKnowledgeEntries(options: { const modifiedFiles: string[] = []; try { - const filePrefixPairs: [string, string][] = [ - [decisionsPath, 'ADR'], - [pitfallsPath, 'PF'], - ]; - for (const [filePath, prefix] of filePrefixPairs) { let content: string; try { @@ -123,44 +140,135 @@ export async function purgeLegacyKnowledgeEntries(options: { continue; // File doesn't exist — skip } - const legacyInFile = LEGACY_IDS.filter(id => id.startsWith(prefix)); + const { updated, removedCount } = rewriteContent(content, prefix); - let updatedContent = content; - for (const legacyId of legacyInFile) { - // Remove the section from `## LEGACYID:` to the next `## ` or end-of-file - const sectionRegex = new RegExp( - `\\n## ${escapeRegExp(legacyId)}:[^\\n]*(?:\\n(?!## )[^\\n]*)*`, - 'g', - ); - const before = updatedContent; - updatedContent = updatedContent.replace(sectionRegex, ''); - if (updatedContent !== before) removed++; - } + if (updated !== content) { + removed += removedCount; - if (updatedContent !== content) { // Update TL;DR count - const headingMatches = updatedContent.match(/^## (ADR|PF)-/gm) ?? []; + const headingMatches = updated.match(/^## (?:ADR|PF)-/gm) ?? []; const count = headingMatches.length; const label = prefix === 'ADR' ? 'decisions' : 'pitfalls'; - updatedContent = updatedContent.replace( - //, + const withTldr = updated.replace( + //, ``, ); - await writeFileAtomicExclusive(filePath, updatedContent); + + await writeFileAtomicExclusive(filePath, withTldr); modifiedFiles.push(filePath); } } - - // Remove orphan PROJECT-PATTERNS.md — stale artifact, nothing generates/reads it - const projectPatternsPath = path.join(memoryDir, 'PROJECT-PATTERNS.md'); - try { - await fs.unlink(projectPatternsPath); - removed++; - modifiedFiles.push(projectPatternsPath); - } catch { /* File doesn't exist — fine */ } } finally { try { await fs.rmdir(knowledgeLockDir); } catch { /* already cleaned */ } } return { removed, files: modifiedFiles }; } + +/** + * Remove pre-v2 low-signal knowledge entries from decisions.md and pitfalls.md. + * + * The entries targeted are: + * - ADR-002 (decisions.md) + * - PF-001, PF-003, PF-005 (pitfalls.md) + * + * Returns immediately if `.memory/knowledge/` does not exist. + * + * @param options.memoryDir - absolute path to the `.memory/` directory + * @returns number of sections removed and list of files that were modified + * @throws if lock acquisition times out + */ +export async function purgeLegacyKnowledgeEntries(options: { + memoryDir: string; +}): Promise { + const { memoryDir } = options; + const knowledgeDir = path.join(memoryDir, 'knowledge'); + const decisionsPath = path.join(knowledgeDir, 'decisions.md'); + const pitfallsPath = path.join(knowledgeDir, 'pitfalls.md'); + + const filePrefixPairs: readonly KnowledgeFilePair[] = [ + [decisionsPath, 'ADR'], + [pitfallsPath, 'PF'], + ]; + + const result = await withKnowledgeFiles(memoryDir, filePrefixPairs, (content, prefix) => { + const legacyInFile = LEGACY_IDS.filter(id => id.startsWith(prefix)); + let updated = content; + let removedCount = 0; + for (const legacyId of legacyInFile) { + // Remove the section from `## LEGACYID:` to the next `## ` or end-of-file + const sectionRegex = new RegExp( + `\\n## ${escapeRegExp(legacyId)}:[^\\n]*(?:\\n(?!## )[^\\n]*)*`, + 'g', + ); + const before = updated; + updated = updated.replace(sectionRegex, ''); + if (updated !== before) removedCount++; + } + return { updated, removedCount }; + }); + + // Remove orphan PROJECT-PATTERNS.md — stale artifact, nothing generates/reads it + // D39-consistent: lstat guard ensures we only unlink regular files (defense-in-depth) + const projectPatternsPath = path.join(memoryDir, 'PROJECT-PATTERNS.md'); + try { + const stat = await fs.lstat(projectPatternsPath); + if (stat.isFile()) { + await fs.unlink(projectPatternsPath); + result.removed++; + result.files.push(projectPatternsPath); + } + } catch { /* File doesn't exist — fine */ } + + return result; +} + +/** + * Remove ALL pre-v2 seeded knowledge entries from decisions.md and pitfalls.md. + * + * Unlike `purgeLegacyKnowledgeEntries` (which targets a fixed allow-list of 4 + * IDs), this function uses a format discriminator: any `## ADR-NNN:` or + * `## PF-NNN:` section that does NOT contain the literal + * `- **Source**: self-learning:` marker is considered pre-v2 seeded content + * and is removed. Self-learning entries always carry that marker; seeded + * entries never do. + * + * D-Fix3: This widens the v2 migration's coverage from 4 hardcoded IDs to + * ALL seeded entries, fixing the gap where 7 of the original 10 seed entries + * survived the v2 purge on upgraded projects. + * + * Returns immediately if `.memory/knowledge/` does not exist. + * + * Does NOT remove PROJECT-PATTERNS.md — that file is v2's responsibility and + * has already been handled by `purgeLegacyKnowledgeEntries`. + * + * @param options.memoryDir - absolute path to the `.memory/` directory + * @returns number of sections removed and list of files that were modified + * @throws if lock acquisition times out + */ +export async function purgeAllPreV2KnowledgeEntries(options: { + memoryDir: string; +}): Promise { + const { memoryDir } = options; + const knowledgeDir = path.join(memoryDir, 'knowledge'); + const decisionsPath = path.join(knowledgeDir, 'decisions.md'); + const pitfallsPath = path.join(knowledgeDir, 'pitfalls.md'); + + const filePrefixPairs: readonly KnowledgeFilePair[] = [ + [decisionsPath, 'ADR'], + [pitfallsPath, 'PF'], + ]; + + return withKnowledgeFiles(memoryDir, filePrefixPairs, (content) => { + // Remove sections lacking the self-learning marker — those are pre-v2 seeded content. + let removedCount = 0; + const updated = content.replace(SECTION_REGEX, (section) => { + if (!section.includes(SELF_LEARNING_SOURCE_MARKER)) { + removedCount++; + return ''; + } + return section; + }); + return { updated, removedCount }; + }); +} diff --git a/src/cli/utils/migrations.ts b/src/cli/utils/migrations.ts index 24c5b6a..10ee652 100644 --- a/src/cli/utils/migrations.ts +++ b/src/cli/utils/migrations.ts @@ -107,9 +107,51 @@ const MIGRATION_PURGE_LEGACY_KNOWLEDGE: Migration<'per-project'> = { }, }; +/** + * D-Fix3: Widens the v2 purge from 4 hardcoded IDs to ALL pre-v2 seeded + * entries. The discriminator is the `- **Source**: self-learning:` marker: + * any ADR/PF section lacking that marker is pre-v2 seeded content and is + * removed. This fixes the gap where 7 of 10 seed entries survived the v2 + * migration on upgraded projects. + * + * v2 and v3 run independently — both must complete for the migration to be + * considered done. On fresh installs, both are no-ops (no knowledge files + * exist). On projects where only v2 ran, v3 cleans up the remaining 7 entries. + */ +const MIGRATION_PURGE_LEGACY_KNOWLEDGE_V3: Migration<'per-project'> = { + id: 'purge-legacy-knowledge-v3', + description: 'Remove all pre-v2 seeded knowledge entries (entries lacking self-learning: source marker)', + scope: 'per-project', + run: async (ctx: PerProjectMigrationContext): Promise => { + const { purgeAllPreV2KnowledgeEntries } = await import('./legacy-knowledge-purge.js'); + const result = await purgeAllPreV2KnowledgeEntries({ memoryDir: ctx.memoryDir }); + const infos = result.removed > 0 + ? [`Purged ${result.removed} pre-v2 knowledge entry(ies) in ${result.files.length} file(s)`] + : []; + return { infos, warnings: [] }; + }, +}; + +/** + * Migration ID suffix conventions: + * + * - `-vN` A revision of a migration. `-v2`, `-v3`, etc. indicate + * successive sweeps targeting the same data set (e.g. widening + * the purge scope). Each revision runs independently so partially- + * migrated machines get the incremental cleanup on next init. + * + * - `-vN-{tag}` A named variant within a revision. The tag distinguishes + * migrations that operate on the same version epoch but target + * different data (e.g. `shadow-overrides-v2-names` vs a + * hypothetical `shadow-overrides-v2-config`). + * + * All IDs are append-only — never rename an existing ID or already-applied + * machines will re-run the migration. + */ export const MIGRATIONS: readonly Migration[] = [ MIGRATION_SHADOW_OVERRIDES, MIGRATION_PURGE_LEGACY_KNOWLEDGE, + MIGRATION_PURGE_LEGACY_KNOWLEDGE_V3, ]; const MIGRATIONS_FILE = 'migrations.json'; diff --git a/tests/knowledge/apply-knowledge-skill.test.ts b/tests/knowledge/apply-knowledge-skill.test.ts new file mode 100644 index 0000000..369e536 --- /dev/null +++ b/tests/knowledge/apply-knowledge-skill.test.ts @@ -0,0 +1,136 @@ +import { describe, it, expect } from 'vitest' +import { readFileSync, existsSync } from 'fs' +import * as path from 'path' + +const ROOT = path.resolve(import.meta.dirname, '../..') +const SKILL_PATH = path.join(ROOT, 'shared/skills/apply-knowledge/SKILL.md') + +function loadSkill(): string { + return readFileSync(SKILL_PATH, 'utf8') +} + +// ------------------------------------------------------------------------- +// File existence +// ------------------------------------------------------------------------- + +describe('apply-knowledge skill — file existence', () => { + it('shared/skills/apply-knowledge/SKILL.md exists', () => { + expect(existsSync(SKILL_PATH)).toBe(true) + }) +}) + +// ------------------------------------------------------------------------- +// Frontmatter +// ------------------------------------------------------------------------- + +describe('apply-knowledge skill — frontmatter', () => { + it('has name: apply-knowledge in frontmatter', () => { + const content = loadSkill() + expect(content).toMatch(/^name:\s*apply-knowledge/m) + }) + + it('has a description field in frontmatter', () => { + const content = loadSkill() + expect(content).toMatch(/^description:/m) + }) + + it('has allowed-tools: Read in frontmatter', () => { + const content = loadSkill() + expect(content).toMatch(/^allowed-tools:.*Read/m) + }) +}) + +// ------------------------------------------------------------------------- +// 5-step algorithm markers +// ------------------------------------------------------------------------- + +describe('apply-knowledge skill — 5-step algorithm', () => { + it('contains "Scan the index" step', () => { + const content = loadSkill() + expect(content).toMatch(/Scan the index/i) + }) + + it('contains "Identify plausibly-relevant" step', () => { + const content = loadSkill() + expect(content).toMatch(/Identify plausibly.?relevant/i) + }) + + it('contains "Read the full body" step', () => { + const content = loadSkill() + expect(content).toMatch(/Read the full body/i) + }) + + it('contains "Cite inline" step', () => { + const content = loadSkill() + expect(content).toMatch(/Cite inline/i) + }) + + it('contains "verbatim IDs" instruction (hallucination guard)', () => { + const content = loadSkill() + expect(content).toMatch(/verbatim IDs?/i) + }) +}) + +// ------------------------------------------------------------------------- +// Worked example +// ------------------------------------------------------------------------- + +describe('apply-knowledge skill — worked example', () => { + it('contains PF-004 in the worked example', () => { + const content = loadSkill() + expect(content).toContain('PF-004') + }) +}) + +// ------------------------------------------------------------------------- +// Citation format +// ------------------------------------------------------------------------- + +describe('apply-knowledge skill — citation format', () => { + it('specifies "applies ADR-NNN" citation format', () => { + const content = loadSkill() + expect(content).toContain('applies ADR-NNN') + }) + + it('specifies "avoids PF-NNN" citation format', () => { + const content = loadSkill() + expect(content).toContain('avoids PF-NNN') + }) +}) + +// ------------------------------------------------------------------------- +// Skip guard +// ------------------------------------------------------------------------- + +describe('apply-knowledge skill — skip guard', () => { + it('instructs to skip when KNOWLEDGE_CONTEXT is empty or "(none)"', () => { + const content = loadSkill() + expect(content).toMatch(/skip|omit/i) + expect(content).toContain('(none)') + }) +}) + +// ------------------------------------------------------------------------- +// Footer-as-source-of-truth — no hardcoded paths in Step 3 or Worked Example +// ------------------------------------------------------------------------- + +describe('apply-knowledge skill — defers to footer for file paths', () => { + it('Step 3 instructs to use the footer, not hardcoded paths', () => { + const content = loadSkill() + expect(content).toMatch(/footer is the single source of truth/i) + }) + + it('Step 3 example uses {worktree-from-footer} placeholder, not hardcoded .memory/knowledge/', () => { + const content = loadSkill() + // Step 3 section up to Step 4 + const step3 = content.slice( + content.indexOf('### Step 3'), + content.indexOf('### Step 4') + ) + // Must use the footer placeholder in the example + expect(step3).toContain('{worktree-from-footer}') + // Must not show a bare hardcoded .memory/knowledge/decisions.md arrow example + expect(step3).not.toMatch(/^\s*\.memory\/knowledge\/decisions\.md\s+→/m) + expect(step3).not.toMatch(/^\s*\.memory\/knowledge\/pitfalls\.md\s+→/m) + }) +}) diff --git a/tests/knowledge/command-adoption.test.ts b/tests/knowledge/command-adoption.test.ts new file mode 100644 index 0000000..48296a4 --- /dev/null +++ b/tests/knowledge/command-adoption.test.ts @@ -0,0 +1,257 @@ +import { describe, it, expect } from 'vitest' +import { loadFile, extractSection } from './helpers' + +// ------------------------------------------------------------------------- +// Command surfaces — must reference knowledge-context.cjs index +// ------------------------------------------------------------------------- + +describe('Command surfaces — knowledge-context.cjs index invocation', () => { + const surfaces: Array<[string, string]> = [ + ['plan.md', 'plugins/devflow-plan/commands/plan.md'], + ['plan-teams.md', 'plugins/devflow-plan/commands/plan-teams.md'], + ['resolve.md', 'plugins/devflow-resolve/commands/resolve.md'], + ['resolve-teams.md', 'plugins/devflow-resolve/commands/resolve-teams.md'], + ['self-review.md', 'plugins/devflow-self-review/commands/self-review.md'], + ['code-review.md', 'plugins/devflow-code-review/commands/code-review.md'], + ['code-review-teams.md', 'plugins/devflow-code-review/commands/code-review-teams.md'], + ['debug.md', 'plugins/devflow-debug/commands/debug.md'], + ['debug-teams.md', 'plugins/devflow-debug/commands/debug-teams.md'], + ] + + for (const [label, relPath] of surfaces) { + it(`${label} invokes knowledge-context.cjs index with {worktree} placeholder`, () => { + const content = loadFile(relPath) + expect(content).toContain('knowledge-context.cjs index "{worktree}"') + }) + } +}) + +// ------------------------------------------------------------------------- +// Orch skill surfaces — must reference knowledge-context.cjs index +// ------------------------------------------------------------------------- + +describe('Orch skill surfaces — knowledge-context.cjs index invocation', () => { + const orchSkills: Array<[string, string]> = [ + ['plan:orch', 'shared/skills/plan:orch/SKILL.md'], + ['resolve:orch', 'shared/skills/resolve:orch/SKILL.md'], + ['review:orch', 'shared/skills/review:orch/SKILL.md'], + ['debug:orch', 'shared/skills/debug:orch/SKILL.md'], + ] + + for (const [label, relPath] of orchSkills) { + it(`${label} SKILL.md invokes knowledge-context.cjs index with {worktree} placeholder`, () => { + const content = loadFile(relPath) + expect(content).toContain('knowledge-context.cjs index "{worktree}"') + }) + } +}) + +// ------------------------------------------------------------------------- +// debug:orch — knowledge loads orchestrator-locally, NOT fanned to Explore +// ------------------------------------------------------------------------- + +describe('debug:orch — knowledge is orchestrator-local, not fanned to Explore spawns', () => { + it('debug:orch SKILL.md contains KNOWLEDGE_CONTEXT (orchestrator uses it)', () => { + const content = loadFile('shared/skills/debug:orch/SKILL.md') + expect(content).toContain('KNOWLEDGE_CONTEXT') + }) + + it('debug:orch Explore spawn blocks do NOT pass KNOWLEDGE_CONTEXT to sub-agents', () => { + const content = loadFile('shared/skills/debug:orch/SKILL.md') + // Find the Phase 2 Investigate section (Explore spawns) + const phase2Section = extractSection(content, 'Phase 2: Investigate', '## Phase 3') + // KNOWLEDGE_CONTEXT should NOT appear in Explore spawn block parameters + expect(phase2Section).not.toContain('KNOWLEDGE_CONTEXT') + }) +}) + +// ------------------------------------------------------------------------- +// debug.md & debug-teams.md — knowledge orchestrator-local, not fanned +// ------------------------------------------------------------------------- + +describe('debug.md — knowledge is orchestrator-local, not fanned to Explore investigators', () => { + it('debug.md contains KNOWLEDGE_CONTEXT (orchestrator uses it)', () => { + const content = loadFile('plugins/devflow-debug/commands/debug.md') + expect(content).toContain('KNOWLEDGE_CONTEXT') + }) + + it('debug.md Investigate phase does NOT pass KNOWLEDGE_CONTEXT to Explore investigators', () => { + const content = loadFile('plugins/devflow-debug/commands/debug.md') + const phase3 = extractSection(content, 'Phase 3: Investigate', '### Phase 4') + expect(phase3).not.toContain('KNOWLEDGE_CONTEXT') + }) +}) + +describe('debug-teams.md — knowledge is orchestrator-local, not fanned to teammates', () => { + it('debug-teams.md contains KNOWLEDGE_CONTEXT (orchestrator uses it)', () => { + const content = loadFile('plugins/devflow-debug/commands/debug-teams.md') + expect(content).toContain('KNOWLEDGE_CONTEXT') + }) + + it('debug-teams.md teammate spawn block does NOT pass KNOWLEDGE_CONTEXT to investigators', () => { + const content = loadFile('plugins/devflow-debug/commands/debug-teams.md') + const phase3 = extractSection(content, 'Phase 3: Spawn Investigation Team', '### Phase 4') + expect(phase3).not.toContain('KNOWLEDGE_CONTEXT') + }) +}) + +// ------------------------------------------------------------------------- +// KNOWLEDGE_CONTEXT substitution template — single canonical form +// ------------------------------------------------------------------------- + +describe('KNOWLEDGE_CONTEXT template — uses canonical {knowledge_context} form without fallback', () => { + const templateSurfaces: Array<[string, string]> = [ + ['plan.md', 'plugins/devflow-plan/commands/plan.md'], + ['plan-teams.md', 'plugins/devflow-plan/commands/plan-teams.md'], + ['resolve.md', 'plugins/devflow-resolve/commands/resolve.md'], + ['resolve-teams.md', 'plugins/devflow-resolve/commands/resolve-teams.md'], + ['self-review.md', 'plugins/devflow-self-review/commands/self-review.md'], + ['code-review.md', 'plugins/devflow-code-review/commands/code-review.md'], + ['code-review-teams.md', 'plugins/devflow-code-review/commands/code-review-teams.md'], + ['plan:orch', 'shared/skills/plan:orch/SKILL.md'], + ] + + for (const [label, relPath] of templateSurfaces) { + it(`${label} does not use the legacy quoted or prose-fallback forms`, () => { + const content = loadFile(relPath) + // Quoted fallback (Form A) + expect(content).not.toContain(`{knowledge_context or '(none)'}`) + // Prose-descriptive fallback (Form B) + expect(content).not.toMatch(/\{knowledge index from[^}]+, or \(none\)\}/) + expect(content).not.toMatch(/\{Phase \d+ knowledge index, or \(none\)\}/) + }) + } +}) + +// ------------------------------------------------------------------------- +// Consumer agents — must reference devflow:apply-knowledge in skills frontmatter +// ------------------------------------------------------------------------- + +describe('Consumer agents — devflow:apply-knowledge in skills frontmatter', () => { + const agents: Array<[string, string]> = [ + ['resolver.md', 'shared/agents/resolver.md'], + ['designer.md', 'shared/agents/designer.md'], + ['scrutinizer.md', 'shared/agents/scrutinizer.md'], + ['reviewer.md', 'shared/agents/reviewer.md'], + ] + + for (const [label, relPath] of agents) { + it(`${label} references devflow:apply-knowledge in skills frontmatter`, () => { + const content = loadFile(relPath) + // Extract frontmatter (between first --- and second ---) + const frontmatterMatch = content.match(/^---\n([\s\S]*?)\n---/m) + expect(frontmatterMatch).toBeTruthy() + const frontmatter = frontmatterMatch![1] + expect(frontmatter).toContain('devflow:apply-knowledge') + }) + } + + it('simplifier.md does NOT reference devflow:apply-knowledge (code-shape role, not quality gate)', () => { + const content = loadFile('shared/agents/simplifier.md') + const frontmatterMatch = content.match(/^---\n([\s\S]*?)\n---/m) + expect(frontmatterMatch).toBeTruthy() + const frontmatter = frontmatterMatch![1] + expect(frontmatter).not.toContain('devflow:apply-knowledge') + }) +}) + +// ------------------------------------------------------------------------- +// KNOWLEDGE_CONTEXT input description — canonical form across consumer agents +// ------------------------------------------------------------------------- + +describe('KNOWLEDGE_CONTEXT input declaration — canonical form', () => { + const CANONICAL_DESCRIPTION = + '**KNOWLEDGE_CONTEXT** (optional): Compact index of active ADR/PF entries for this worktree (generated by `knowledge-context.cjs index`). `(none)` when absent. Use `devflow:apply-knowledge` to Read full bodies on demand.' + + const consumerAgents: Array<[string, string]> = [ + ['resolver.md', 'shared/agents/resolver.md'], + ['designer.md', 'shared/agents/designer.md'], + ['scrutinizer.md', 'shared/agents/scrutinizer.md'], + ['reviewer.md', 'shared/agents/reviewer.md'], + ] + + for (const [label, relPath] of consumerAgents) { + it(`${label} declares KNOWLEDGE_CONTEXT with canonical description`, () => { + const content = loadFile(relPath) + expect(content).toContain(CANONICAL_DESCRIPTION) + }) + } +}) + +// ------------------------------------------------------------------------- +// KNOWLEDGE_CONTEXT variable present in all consumer surfaces +// ------------------------------------------------------------------------- + +describe('KNOWLEDGE_CONTEXT variable — present in all four command surfaces', () => { + it('plan.md contains KNOWLEDGE_CONTEXT', () => { + expect(loadFile('plugins/devflow-plan/commands/plan.md')).toContain('KNOWLEDGE_CONTEXT') + }) + + it('self-review.md contains KNOWLEDGE_CONTEXT', () => { + expect(loadFile('plugins/devflow-self-review/commands/self-review.md')).toContain('KNOWLEDGE_CONTEXT') + }) + + it('code-review.md contains KNOWLEDGE_CONTEXT', () => { + expect(loadFile('plugins/devflow-code-review/commands/code-review.md')).toContain('KNOWLEDGE_CONTEXT') + }) + + it('plan:orch SKILL.md contains KNOWLEDGE_CONTEXT', () => { + expect(loadFile('shared/skills/plan:orch/SKILL.md')).toContain('KNOWLEDGE_CONTEXT') + }) + + it('review:orch SKILL.md contains KNOWLEDGE_CONTEXT', () => { + expect(loadFile('shared/skills/review:orch/SKILL.md')).toContain('KNOWLEDGE_CONTEXT') + }) + + it('debug:orch SKILL.md contains KNOWLEDGE_CONTEXT', () => { + expect(loadFile('shared/skills/debug:orch/SKILL.md')).toContain('KNOWLEDGE_CONTEXT') + }) +}) + +// ------------------------------------------------------------------------- +// Reviewer agent — apply-knowledge section references skill +// ------------------------------------------------------------------------- + +describe('reviewer.md — Apply Knowledge section', () => { + it('contains Apply Knowledge section referencing devflow:apply-knowledge', () => { + const content = loadFile('shared/agents/reviewer.md') + expect(content).toMatch(/## Apply Knowledge|### Apply Knowledge/) + expect(content).toContain('devflow:apply-knowledge') + }) +}) + +// ------------------------------------------------------------------------- +// plan:orch — knowledge loading phase present +// ------------------------------------------------------------------------- + +describe('plan:orch — knowledge loading phase', () => { + it('contains a knowledge-loading step (load knowledge index)', () => { + const content = loadFile('shared/skills/plan:orch/SKILL.md') + // Should have a phase that loads knowledge + expect(content).toMatch(/[Ll]oad.*[Kk]nowledge|[Kk]nowledge.*[Ll]oad/i) + }) + + it('Explore spawn blocks receive KNOWLEDGE_CONTEXT', () => { + const content = loadFile('shared/skills/plan:orch/SKILL.md') + // The Explore phase section should mention KNOWLEDGE_CONTEXT + const phase2 = extractSection(content, 'Phase 2: Explore', '## Phase 3') + expect(phase2).toContain('KNOWLEDGE_CONTEXT') + }) +}) + +// ------------------------------------------------------------------------- +// review:orch — knowledge loading phase present +// ------------------------------------------------------------------------- + +describe('review:orch — knowledge loading phase', () => { + it('contains a knowledge-loading step', () => { + const content = loadFile('shared/skills/review:orch/SKILL.md') + expect(content).toMatch(/[Ll]oad.*[Kk]nowledge|[Kk]nowledge.*[Ll]oad/i) + }) + + it('Phase 4 Reviews section receives KNOWLEDGE_CONTEXT', () => { + const content = loadFile('shared/skills/review:orch/SKILL.md') + const phase4 = extractSection(content, 'Phase 4: Reviews', '## Phase 5') + expect(phase4).toContain('KNOWLEDGE_CONTEXT') + }) +}) diff --git a/tests/knowledge/fixtures.ts b/tests/knowledge/fixtures.ts new file mode 100644 index 0000000..c7b0b8e --- /dev/null +++ b/tests/knowledge/fixtures.ts @@ -0,0 +1,90 @@ +// Shared test fixtures for knowledge-context module tests. +// Both index-generator.test.ts and knowledge-citation.test.ts import from here +// to avoid drift between fixture definitions. + +import { mkdtempSync, writeFileSync, mkdirSync, rmSync } from 'fs' +import * as path from 'path' +import * as os from 'os' + +// --------------------------------------------------------------------------- +// ADR fixtures +// --------------------------------------------------------------------------- + +export const ACTIVE_ADR = `## ADR-001: Use Result types everywhere across the codebase for errors + +- **Status**: Active +- **Decision**: Always return Result +- **Context**: TypeScript project enforcing functional error handling +` + +export const DEPRECATED_ADR = `## ADR-002: Old approach no longer relevant + +- **Status**: Deprecated +- **Decision**: Do the old thing +` + +export const SUPERSEDED_ADR = `## ADR-003: Superseded approach no longer relevant + +- **Status**: Superseded +- **Decision**: Outdated pattern +` + +// --------------------------------------------------------------------------- +// PF fixtures +// --------------------------------------------------------------------------- + +export const ACTIVE_PF = `## PF-004: Background hook scripts grow into god scripts over time + +- **Status**: Active +- **Area**: scripts/hooks/foo.cjs, scripts/hooks/background-learning +- **Description**: Watch out for growing scripts +` + +export const DEPRECATED_PF = `## PF-001: Old pitfall no longer relevant + +- **Status**: Deprecated +- **Description**: No longer relevant +` + +export const SUPERSEDED_PF = `## PF-005: Superseded pitfall + +- **Status**: Superseded +- **Area**: some/file.ts +- **Description**: No longer relevant +` + +// --------------------------------------------------------------------------- +// Filesystem helpers +// --------------------------------------------------------------------------- + +const createdTmpDirs: string[] = [] + +/** + * Create a temporary worktree directory with optional knowledge files. + * Returns the absolute path to the tmpdir root. + * Directories are tracked — call `cleanupTmpWorktrees()` in afterAll. + */ +export function makeTmpWorktree(decisions?: string, pitfalls?: string): string { + const tmpDir = mkdtempSync(path.join(os.tmpdir(), 'knowledge-index-test-')) + createdTmpDirs.push(tmpDir) + const knowledgeDir = path.join(tmpDir, '.memory', 'knowledge') + mkdirSync(knowledgeDir, { recursive: true }) + if (decisions !== undefined) { + writeFileSync(path.join(knowledgeDir, 'decisions.md'), decisions, 'utf8') + } + if (pitfalls !== undefined) { + writeFileSync(path.join(knowledgeDir, 'pitfalls.md'), pitfalls, 'utf8') + } + return tmpDir +} + +/** + * Remove all temporary worktree directories created by `makeTmpWorktree`. + * Call in `afterAll(() => cleanupTmpWorktrees())`. + */ +export function cleanupTmpWorktrees(): void { + for (const dir of createdTmpDirs) { + try { rmSync(dir, { recursive: true, force: true }) } catch { /* best-effort */ } + } + createdTmpDirs.length = 0 +} diff --git a/tests/knowledge/helpers.ts b/tests/knowledge/helpers.ts new file mode 100644 index 0000000..2ab808c --- /dev/null +++ b/tests/knowledge/helpers.ts @@ -0,0 +1,22 @@ +import { readFileSync } from 'fs' +import * as path from 'path' + +export const ROOT = path.resolve(import.meta.dirname, '../..') + +export function loadFile(relPath: string): string { + return readFileSync(path.join(ROOT, relPath), 'utf8') +} + +/** + * Extract a named section from markdown content. + * Returns the content from startAnchor to endAnchor (or end of string). + * Throws loudly if either anchor is absent. + */ +export function extractSection(content: string, startAnchor: string, endAnchor: string | null): string { + const start = content.indexOf(startAnchor) + if (start === -1) throw new Error(`Anchor not found: "${startAnchor}"`) + if (endAnchor === null) return content.slice(start) + const end = content.indexOf(endAnchor, start + startAnchor.length) + if (end === -1) throw new Error(`End anchor not found after "${startAnchor}": "${endAnchor}"`) + return content.slice(start, end) +} diff --git a/tests/knowledge/index-generator.test.ts b/tests/knowledge/index-generator.test.ts new file mode 100644 index 0000000..8fbfcd3 --- /dev/null +++ b/tests/knowledge/index-generator.test.ts @@ -0,0 +1,234 @@ +import { describe, it, expect, afterAll } from 'vitest' +import * as path from 'path' +import { execSync } from 'child_process' +import { createRequire } from 'module' +import { + ACTIVE_ADR, ACTIVE_PF, DEPRECATED_ADR, SUPERSEDED_PF, + makeTmpWorktree, cleanupTmpWorktrees, +} from './fixtures' + +afterAll(() => cleanupTmpWorktrees()) + +const ROOT = path.resolve(import.meta.dirname, '../..') +const require = createRequire(import.meta.url) + +const { filterKnowledgeContext, loadKnowledgeIndex } = require( + path.join(ROOT, 'scripts/hooks/lib/knowledge-context.cjs') +) as { + filterKnowledgeContext: (raw: string) => string + loadKnowledgeIndex: (worktree: string, opts?: { decisionsFile?: string; pitfallsFile?: string }) => string +} + +const CJS_PATH = path.join(ROOT, 'scripts/hooks/lib/knowledge-context.cjs') + +// ------------------------------------------------------------------------- +// loadKnowledgeIndex — formatting +// ------------------------------------------------------------------------- + +describe('loadKnowledgeIndex — formatting', () => { + it('returns "(none)" when both knowledge files are absent', () => { + const tmpDir = makeTmpWorktree() + expect(loadKnowledgeIndex(tmpDir)).toBe('(none)') + }) + + it('returns "(none)" when all entries are Deprecated/Superseded after filter', () => { + const tmpDir = makeTmpWorktree(DEPRECATED_ADR, SUPERSEDED_PF) + expect(loadKnowledgeIndex(tmpDir)).toBe('(none)') + }) + + it('includes Decisions block when decisions.md has active entries', () => { + const tmpDir = makeTmpWorktree(ACTIVE_ADR) + const result = loadKnowledgeIndex(tmpDir) + expect(result).toContain('Decisions (1):') + expect(result).toContain('ADR-001') + }) + + it('includes Pitfalls block when pitfalls.md has active entries', () => { + const tmpDir = makeTmpWorktree(undefined, ACTIVE_PF) + const result = loadKnowledgeIndex(tmpDir) + expect(result).toContain('Pitfalls (1):') + expect(result).toContain('PF-004') + }) + + it('shows both Decisions and Pitfalls blocks when both files have active entries', () => { + const tmpDir = makeTmpWorktree(ACTIVE_ADR, ACTIVE_PF) + const result = loadKnowledgeIndex(tmpDir) + expect(result).toContain('Decisions (1):') + expect(result).toContain('Pitfalls (1):') + }) + + it('strips Deprecated entries from Decisions block', () => { + const mixed = ACTIVE_ADR + '\n' + DEPRECATED_ADR + const tmpDir = makeTmpWorktree(mixed) + const result = loadKnowledgeIndex(tmpDir) + expect(result).toContain('Decisions (1):') + expect(result).toContain('ADR-001') + expect(result).not.toContain('ADR-002') + }) + + it('strips Superseded entries from Pitfalls block', () => { + const mixed = ACTIVE_PF + '\n' + SUPERSEDED_PF + const tmpDir = makeTmpWorktree(undefined, mixed) + const result = loadKnowledgeIndex(tmpDir) + expect(result).toContain('Pitfalls (1):') + expect(result).toContain('PF-004') + expect(result).not.toContain('PF-005') + }) + + it('truncates title to 60 characters with ellipsis', () => { + const longTitle = 'A'.repeat(70) + const adr = `## ADR-003: ${longTitle}\n\n- **Status**: Active\n- **Decision**: Long title\n` + const tmpDir = makeTmpWorktree(adr) + const result = loadKnowledgeIndex(tmpDir) + expect(result).toContain('ADR-003') + // Title should be truncated to 60 chars with '…' + const lines = result.split('\n') + const adrLine = lines.find(l => l.includes('ADR-003')) + expect(adrLine).toBeDefined() + // The title should contain the ellipsis character, confirming truncation occurred + expect(adrLine).toContain('…') + // The full 70-char title should not appear verbatim + expect(adrLine).not.toContain(longTitle) + }) + + it('truncates area to 80 characters with ellipsis', () => { + const longArea = 'scripts/hooks/' + 'a'.repeat(80) + const pf = `## PF-006: Some pitfall\n\n- **Status**: Active\n- **Area**: ${longArea}\n- **Description**: desc\n` + const tmpDir = makeTmpWorktree(undefined, pf) + const result = loadKnowledgeIndex(tmpDir) + const lines = result.split('\n') + const pfLine = lines.find(l => l.includes('PF-006')) + expect(pfLine).toBeDefined() + // Area suffix should be truncated + if (pfLine!.includes('—')) { + const areaPart = pfLine!.split('—')[1]?.trim() ?? '' + expect(areaPart.length).toBeLessThanOrEqual(81) // 80 + '…' + } + }) + + it('shows [unknown] for entries with no recognized status', () => { + const adr = `## ADR-007: Unknown status entry\n\n- **Status**: Draft\n- **Decision**: Something\n` + const tmpDir = makeTmpWorktree(adr) + const result = loadKnowledgeIndex(tmpDir) + expect(result).toContain('ADR-007') + expect(result).toContain('[unknown]') + }) + + it('omits — Area suffix when Area field is missing', () => { + const pf = `## PF-008: No area field\n\n- **Status**: Active\n- **Description**: desc\n` + const tmpDir = makeTmpWorktree(undefined, pf) + const result = loadKnowledgeIndex(tmpDir) + const lines = result.split('\n') + const pfLine = lines.find(l => l.includes('PF-008')) + expect(pfLine).toBeDefined() + expect(pfLine).not.toContain('—') + }) + + it('includes file path footer for decisions', () => { + const tmpDir = makeTmpWorktree(ACTIVE_ADR) + const result = loadKnowledgeIndex(tmpDir) + expect(result).toContain('decisions.md') + expect(result).toContain('ADR-NNN') + }) + + it('includes file path footer for pitfalls', () => { + const tmpDir = makeTmpWorktree(undefined, ACTIVE_PF) + const result = loadKnowledgeIndex(tmpDir) + expect(result).toContain('pitfalls.md') + expect(result).toContain('PF-NNN') + }) + + it('omits Decisions block when only pitfalls file is present', () => { + const tmpDir = makeTmpWorktree(undefined, ACTIVE_PF) + const result = loadKnowledgeIndex(tmpDir) + expect(result).not.toContain('Decisions (') + expect(result).toContain('Pitfalls (') + }) + + it('omits Pitfalls block when only decisions file is present', () => { + const tmpDir = makeTmpWorktree(ACTIVE_ADR) + const result = loadKnowledgeIndex(tmpDir) + expect(result).toContain('Decisions (') + expect(result).not.toContain('Pitfalls (') + }) +}) + +// ------------------------------------------------------------------------- +// CLI dispatch — subcommand mode +// ------------------------------------------------------------------------- + +describe('CLI dispatch — subcommand mode', () => { + it('index subcommand produces index format output', () => { + const tmpDir = makeTmpWorktree(ACTIVE_ADR, ACTIVE_PF) + const output = execSync(`node "${CJS_PATH}" index "${tmpDir}"`).toString() + expect(output).toContain('Decisions (1):') + expect(output).toContain('Pitfalls (1):') + expect(output).toContain('ADR-001') + expect(output).toContain('PF-004') + }) + + it('bare invocation (no subcommand) exits with code 1', () => { + const tmpDir = makeTmpWorktree(ACTIVE_ADR) + let threw = false + try { + execSync(`node "${CJS_PATH}" "${tmpDir}"`, { + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'], + }) + } catch { + threw = true + } + expect(threw).toBe(true) + }) + + it('unknown subcommand exits with code 1 and prints usage', () => { + const tmpDir = makeTmpWorktree(ACTIVE_ADR) + let threw = false + let stderr = '' + try { + execSync(`node "${CJS_PATH}" foo "${tmpDir}"`, { + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'], + }) + } catch (e: any) { + threw = true + stderr = e.stderr ?? '' + } + expect(threw).toBe(true) + expect(stderr).toMatch(/[Uu]sage/) + }) + + it('index subcommand on empty knowledge returns "(none)"', () => { + const tmpDir = makeTmpWorktree() + const output = execSync(`node "${CJS_PATH}" index "${tmpDir}"`).toString() + expect(output.trim()).toBe('(none)') + }) +}) + +// ------------------------------------------------------------------------- +// Observability — stderr log +// ------------------------------------------------------------------------- + +describe('Observability — stderr log on successful invocation', () => { + it('index subcommand logs mode, worktree, and entries count to stderr', () => { + const tmpDir = makeTmpWorktree(ACTIVE_ADR, ACTIVE_PF) + // Run and capture stderr + const stderr = execSync( + `node "${CJS_PATH}" index "${tmpDir}" 2>&1 1>/dev/null`, + { encoding: 'utf8', shell: true } + ) + expect(stderr).toContain('[knowledge-context]') + expect(stderr).toContain('mode=index') + expect(stderr).toContain('entries=2') + }) + + it('does not log observability when result is "(none)"', () => { + const tmpDir = makeTmpWorktree() + const stderr = execSync( + `node "${CJS_PATH}" index "${tmpDir}" 2>&1 1>/dev/null`, + { encoding: 'utf8', shell: true } + ) + // No log when (none) + expect(stderr).not.toContain('[knowledge-context]') + }) +}) diff --git a/tests/learning/helpers.ts b/tests/learning/helpers.ts index a27ae46..43b405f 100644 --- a/tests/learning/helpers.ts +++ b/tests/learning/helpers.ts @@ -48,6 +48,19 @@ export interface LogEntry { staleReason?: string; } +/** + * djb2 hash — matches the contentHash() implementation in json-helper.cjs. + * Exported so all test files can share a single copy (T3: eliminates duplicate + * inline definitions in reconcile.test.ts and any future test files). + */ +export function djb2(s: string): string { + let h = 5381; + for (let i = 0; i < s.length; i++) { + h = ((h * 33) ^ s.charCodeAt(i)) >>> 0; + } + return h.toString(16); +} + /** * Return a base log entry for the given id and type. * Suitable as a starting point for any test fixture. diff --git a/tests/learning/reconcile.test.ts b/tests/learning/reconcile.test.ts index ab29d6e..c568b08 100644 --- a/tests/learning/reconcile.test.ts +++ b/tests/learning/reconcile.test.ts @@ -6,7 +6,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; -import { runHelper, type LogEntry } from './helpers.js'; +import { runHelper, djb2, type LogEntry } from './helpers.js'; interface ManifestEntry { observationId: string; @@ -169,15 +169,6 @@ describe('reconcile-manifest — edit detection (D13)', () => { const content = '# Stable\n\nThis content does not change\n'; fs.writeFileSync(filePath, content); - // We need to get the real hash first by running render-ready on a file - // Instead, let's manually compute it using same djb2 algorithm - function djb2(s: string): string { - let h = 5381; - for (let i = 0; i < s.length; i++) { - h = ((h * 33) ^ s.charCodeAt(i)) >>> 0; - } - return h.toString(16); - } const hash = djb2(content); writeManifest(manifestPath, [{ @@ -298,3 +289,375 @@ describe('reconcile-manifest — stale manifest entries', () => { expect(result.unchanged).toBe(0); }); }); + +// --------------------------------------------------------------------------- +// Self-healing reconciler tests (Fix 2) +// Validates that reconcile-manifest heals render-ready crash-window duplicates: +// anchors present in knowledge files but missing from manifest + log shows status=ready +// --------------------------------------------------------------------------- + +describe('reconcile-manifest — self-heal (Fix 2)', () => { + let tmpDir: string; + + // Build a decisions.md with the given ADR sections + function buildDecisionsFile(sections: Array<{ anchorId: string; heading: string; body: string }>): string { + const parts = sections.map(s => + `## ${s.anchorId}: ${s.heading}\n\n${s.body}\n` + ); + return `\n# Decisions\n\n${parts.join('\n')}`; + } + + // Build a pitfalls.md with the given PF sections + function buildPitfallsFile(sections: Array<{ anchorId: string; heading: string; body: string }>): string { + const parts = sections.map(s => + `## ${s.anchorId}: ${s.heading}\n\n${s.body}\n` + ); + return `\n# Pitfalls\n\n${parts.join('\n')}`; + } + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'reconcile-heal-test-')); + fs.mkdirSync(path.join(tmpDir, '.memory', 'knowledge'), { recursive: true }); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('heal: anchor in file + ready log entry + missing manifest → status=created, manifest reconstructed', () => { + const { manifestPath, logPath } = setup(tmpDir); + const decisionFile = path.join(tmpDir, '.memory', 'knowledge', 'decisions.md'); + + // decisions.md has ADR-001 written (crash window: file written, log not updated yet) + const adrContent = buildDecisionsFile([{ + anchorId: 'ADR-001', + heading: 'use result types everywhere', + body: '- **Status**: Accepted\n- **Source**: self-learning:obs_heal_001', + }]); + fs.writeFileSync(decisionFile, adrContent); + + // Manifest is empty (crash happened before manifest write) + writeManifest(manifestPath, []); + + // Log still shows status=ready (crash happened before log write) + const obs: LogEntry = { + ...baseEntry('obs_heal_001', 'decision', 'ready'), + pattern: 'use result types everywhere', + confidence: 0.90, + }; + writeLog(logPath, [obs]); + + const result = JSON.parse(runHelper(`reconcile-manifest "${tmpDir}"`)); + + // healed counter must be present and non-zero + expect(result.healed).toBe(1); + + // Log entry upgraded to created + const entries = readLog(logPath); + const healed = entries.find(e => e.id === 'obs_heal_001'); + expect(healed).toBeDefined(); + expect(healed!.status).toBe('created'); + expect(healed!.artifact_path).toContain('ADR-001'); + + // Manifest now has an entry for this obs + const manifest = readManifest(manifestPath); + const manifestEntry = manifest.entries.find(e => e.observationId === 'obs_heal_001'); + expect(manifestEntry).toBeDefined(); + expect(manifestEntry!.anchorId).toBe('ADR-001'); + expect(manifestEntry!.path).toBe(decisionFile); + expect(manifestEntry!.contentHash).toBeTruthy(); + }); + + it('heal: anchor in file, no matching log entry → no-op (user-curated entry)', () => { + const { manifestPath, logPath } = setup(tmpDir); + const decisionFile = path.join(tmpDir, '.memory', 'knowledge', 'decisions.md'); + + // decisions.md has ADR-001 but NO matching log entry (user manually added it) + const adrContent = buildDecisionsFile([{ + anchorId: 'ADR-001', + heading: 'manual decision', + body: '- **Status**: Accepted', + }]); + fs.writeFileSync(decisionFile, adrContent); + + writeManifest(manifestPath, []); + + // Log has a different obs that doesn't match the heading + const obs: LogEntry = { + ...baseEntry('obs_other_001', 'decision', 'ready'), + pattern: 'completely different pattern', + }; + writeLog(logPath, [obs]); + + const result = JSON.parse(runHelper(`reconcile-manifest "${tmpDir}"`)); + + expect(result.healed).toBe(0); + + // The manifest should remain empty + const manifest = readManifest(manifestPath); + expect(manifest.entries.length).toBe(0); + }); + + it('heal: anchor heading does not match any ready log pattern → no-op', () => { + const { manifestPath, logPath } = setup(tmpDir); + const decisionFile = path.join(tmpDir, '.memory', 'knowledge', 'decisions.md'); + + const adrContent = buildDecisionsFile([{ + anchorId: 'ADR-001', + heading: 'use dependency injection', + body: '- **Status**: Accepted\n- **Source**: self-learning:obs_heal_002', + }]); + fs.writeFileSync(decisionFile, adrContent); + + writeManifest(manifestPath, []); + + // Pattern in log uses a different heading text → no match after normalizeForDedup + const obs: LogEntry = { + ...baseEntry('obs_heal_002', 'decision', 'ready'), + pattern: 'prefer factory methods over constructors', + }; + writeLog(logPath, [obs]); + + const result = JSON.parse(runHelper(`reconcile-manifest "${tmpDir}"`)); + + expect(result.healed).toBe(0); + const manifest = readManifest(manifestPath); + expect(manifest.entries.length).toBe(0); + }); + + it('heal: multiple log entries match the same anchor heading → no-op (D-D ambiguity guard)', () => { + const { manifestPath, logPath } = setup(tmpDir); + const decisionFile = path.join(tmpDir, '.memory', 'knowledge', 'decisions.md'); + + const heading = 'use result types everywhere'; + const adrContent = buildDecisionsFile([{ + anchorId: 'ADR-001', + heading, + body: '- **Status**: Accepted', + }]); + fs.writeFileSync(decisionFile, adrContent); + + writeManifest(manifestPath, []); + + // Two log entries with the same normalised pattern — ambiguous, must skip + const obs1: LogEntry = { ...baseEntry('obs_ambig_001', 'decision', 'ready'), pattern: heading }; + const obs2: LogEntry = { ...baseEntry('obs_ambig_002', 'decision', 'ready'), pattern: heading }; + writeLog(logPath, [obs1, obs2]); + + const result = JSON.parse(runHelper(`reconcile-manifest "${tmpDir}"`)); + + expect(result.healed).toBe(0); + // Both log entries remain 'ready' + const entries = readLog(logPath); + expect(entries.every(e => e.status === 'ready')).toBe(true); + }); + + it('heal: pitfalls.md scanned with PF- prefix', () => { + const { manifestPath, logPath } = setup(tmpDir); + const pitfallFile = path.join(tmpDir, '.memory', 'knowledge', 'pitfalls.md'); + + const pfContent = buildPitfallsFile([{ + anchorId: 'PF-001', + heading: 'avoid mutation in reducers', + body: '- **Area**: State management\n- **Issue**: Mutation causes silent bugs\n- **Source**: self-learning:obs_pf_001', + }]); + fs.writeFileSync(pitfallFile, pfContent); + + writeManifest(manifestPath, []); + + const obs: LogEntry = { + ...baseEntry('obs_pf_001', 'pitfall', 'ready'), + pattern: 'avoid mutation in reducers', + }; + writeLog(logPath, [obs]); + + const result = JSON.parse(runHelper(`reconcile-manifest "${tmpDir}"`)); + + expect(result.healed).toBe(1); + + const entries = readLog(logPath); + const healed = entries.find(e => e.id === 'obs_pf_001'); + expect(healed!.status).toBe('created'); + expect(healed!.artifact_path).toContain('PF-001'); + + const manifest = readManifest(manifestPath); + const mEntry = manifest.entries.find(e => e.observationId === 'obs_pf_001'); + expect(mEntry!.anchorId).toBe('PF-001'); + expect(mEntry!.path).toBe(pitfallFile); + }); + + it('heal: multiple anchors healed in a single reconcile pass', () => { + const { manifestPath, logPath } = setup(tmpDir); + const decisionFile = path.join(tmpDir, '.memory', 'knowledge', 'decisions.md'); + const pitfallFile = path.join(tmpDir, '.memory', 'knowledge', 'pitfalls.md'); + + // decisions.md has ADR-001 and ADR-002; pitfalls.md has PF-001 + const adrContent = buildDecisionsFile([ + { anchorId: 'ADR-001', heading: 'use immutable data structures', body: '- **Status**: Accepted\n- **Source**: self-learning:obs_multi_001' }, + { anchorId: 'ADR-002', heading: 'inject dependencies explicitly', body: '- **Status**: Accepted\n- **Source**: self-learning:obs_multi_002' }, + ]); + fs.writeFileSync(decisionFile, adrContent); + + const pfContent = buildPitfallsFile([ + { anchorId: 'PF-001', heading: 'avoid global state mutations', body: '- **Area**: State\n- **Issue**: Silent bugs\n- **Source**: self-learning:obs_multi_003' }, + ]); + fs.writeFileSync(pitfallFile, pfContent); + + writeManifest(manifestPath, []); + + const obs1: LogEntry = { ...baseEntry('obs_multi_001', 'decision', 'ready'), pattern: 'use immutable data structures' }; + const obs2: LogEntry = { ...baseEntry('obs_multi_002', 'decision', 'ready'), pattern: 'inject dependencies explicitly' }; + const obs3: LogEntry = { ...baseEntry('obs_multi_003', 'pitfall', 'ready'), pattern: 'avoid global state mutations' }; + writeLog(logPath, [obs1, obs2, obs3]); + + const result = JSON.parse(runHelper(`reconcile-manifest "${tmpDir}"`)); + + expect(result.healed).toBe(3); + + const manifest = readManifest(manifestPath); + expect(manifest.entries.length).toBe(3); + + const anchorIds = manifest.entries.map(e => e.anchorId); + expect(anchorIds).toContain('ADR-001'); + expect(anchorIds).toContain('ADR-002'); + expect(anchorIds).toContain('PF-001'); + }); + + it('heal: registerUsageEntry called — usage file has entry for healed anchorId', () => { + const { manifestPath, logPath } = setup(tmpDir); + const decisionFile = path.join(tmpDir, '.memory', 'knowledge', 'decisions.md'); + + const adrContent = buildDecisionsFile([{ + anchorId: 'ADR-001', + heading: 'use result types everywhere', + body: '- **Status**: Accepted\n- **Source**: self-learning:obs_usage_001', + }]); + fs.writeFileSync(decisionFile, adrContent); + + writeManifest(manifestPath, []); + + const obs: LogEntry = { + ...baseEntry('obs_usage_001', 'decision', 'ready'), + pattern: 'use result types everywhere', + }; + writeLog(logPath, [obs]); + + runHelper(`reconcile-manifest "${tmpDir}"`); + + // Verify usage file was written with ADR-001 entry + const usagePath = path.join(tmpDir, '.memory', '.knowledge-usage.json'); + expect(fs.existsSync(usagePath)).toBe(true); + const usageData = JSON.parse(fs.readFileSync(usagePath, 'utf8')); + expect(usageData.entries['ADR-001']).toBeDefined(); + expect(usageData.entries['ADR-001'].cites).toBe(0); + }); + + it('result JSON always includes healed counter — zero case (no anchors to heal)', () => { + const { manifestPath, logPath } = setup(tmpDir); + const filePath = path.join(tmpDir, 'my-workflow.md'); + fs.writeFileSync(filePath, '# Workflow\n'); + + // Normal workflow entry — already tracked in manifest and log + writeManifest(manifestPath, [{ + observationId: 'obs_zero_heal', + type: 'workflow', + path: filePath, + contentHash: djb2('# Workflow\n'), + renderedAt: NOW, + }]); + writeLog(logPath, [{ ...baseEntry('obs_zero_heal', 'workflow', 'created') }]); + + const result = JSON.parse(runHelper(`reconcile-manifest "${tmpDir}"`)); + + expect(result).toHaveProperty('healed'); + expect(result.healed).toBe(0); + // Other fields still present + expect(result).toHaveProperty('deletions'); + expect(result).toHaveProperty('edits'); + expect(result).toHaveProperty('unchanged'); + }); + + it('heal: pre-v2 anchor lacking self-learning source marker → no-op even when log obs would match', () => { + // Regression guard: a pre-v2 seeded section whose heading happens to match a + // current ready obs by normalizeForDedup must NOT be paired. Pre-v2 entries + // are removed by the v3 migration; the heal path must not steal their anchor IDs. + const { manifestPath, logPath } = setup(tmpDir); + const decisionFile = path.join(tmpDir, '.memory', 'knowledge', 'decisions.md'); + + // Pre-v2 seeded ADR — has no `- **Source**: self-learning:` marker + const preV2Content = buildDecisionsFile([{ + anchorId: 'ADR-001', + heading: 'use result types everywhere', + body: '- **Status**: Accepted\n- **Source**: /code-review (seed v1)', + }]); + fs.writeFileSync(decisionFile, preV2Content); + + writeManifest(manifestPath, []); + + // Current ready obs whose pattern would normalise-match the pre-v2 heading + const obs: LogEntry = { + ...baseEntry('obs_collision_001', 'decision', 'ready'), + pattern: 'use result types everywhere', + }; + writeLog(logPath, [obs]); + + const result = JSON.parse(runHelper(`reconcile-manifest "${tmpDir}"`)); + + // Heal must NOT trigger — pre-v2 entries lack the source marker + expect(result.healed).toBe(0); + const manifest = readManifest(manifestPath); + expect(manifest.entries.length).toBe(0); + + // Obs stays in `ready` state — it will be rendered as a NEW ADR (e.g., ADR-002) + // by the next render-ready pass, not paired to the pre-v2 ADR-001. + const entries = readLog(logPath); + expect(entries[0].status).toBe('ready'); + }); + + it('heal: manifest absent + knowledge file with anchor + matching ready obs → heal triggers, manifest created (A1)', () => { + // Regression guard for the first-ever render-ready crash: + // render-ready wrote decisions.md but crashed before writing the manifest. + // reconcile-manifest must NOT short-circuit on the missing manifest — it must + // construct an empty in-memory manifest, run the heal path, and persist the + // newly reconstructed manifest. + const logPath = path.join(tmpDir, '.memory', 'learning-log.jsonl'); + const manifestPath = path.join(tmpDir, '.memory', '.learning-manifest.json'); + const decisionFile = path.join(tmpDir, '.memory', 'knowledge', 'decisions.md'); + + // Write the knowledge file (crash-window: file written, manifest never written) + const adrContent = `\n# Decisions\n\n## ADR-001: use pipes for composition\n\n- **Status**: Accepted\n- **Source**: self-learning:obs_a1_001\n`; + fs.writeFileSync(decisionFile, adrContent); + + // Do NOT write a manifest — it doesn't exist yet + expect(fs.existsSync(manifestPath)).toBe(false); + + // Log still shows status=ready (crash happened before log write too) + const obs: LogEntry = { + ...baseEntry('obs_a1_001', 'decision', 'ready'), + pattern: 'use pipes for composition', + confidence: 0.88, + }; + fs.writeFileSync(logPath, JSON.stringify(obs) + '\n'); + + const result = JSON.parse(runHelper(`reconcile-manifest "${tmpDir}"`)); + + // Heal must trigger even though manifest was absent + expect(result.healed).toBe(1); + expect(result.deletions).toBe(0); + + // Manifest must now exist and contain the healed entry + expect(fs.existsSync(manifestPath)).toBe(true); + const manifest = readManifest(manifestPath); + const entry = manifest.entries.find(e => e.observationId === 'obs_a1_001'); + expect(entry).toBeDefined(); + expect(entry!.anchorId).toBe('ADR-001'); + expect(entry!.path).toBe(decisionFile); + expect(entry!.contentHash).toBeTruthy(); + + // Log entry upgraded to created + const logEntries = fs.readFileSync(logPath, 'utf8').trim().split('\n').filter(Boolean).map(l => JSON.parse(l)); + const healed = logEntries.find((e: LogEntry) => e.id === 'obs_a1_001'); + expect(healed!.status).toBe('created'); + expect(healed!.artifact_path).toContain('ADR-001'); + }); +}); diff --git a/tests/legacy-knowledge-purge.test.ts b/tests/legacy-knowledge-purge.test.ts index 9b55b33..a02414a 100644 --- a/tests/legacy-knowledge-purge.test.ts +++ b/tests/legacy-knowledge-purge.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { promises as fs } from 'fs'; import * as path from 'path'; import * as os from 'os'; -import { purgeLegacyKnowledgeEntries } from '../src/cli/utils/legacy-knowledge-purge.js'; +import { purgeLegacyKnowledgeEntries, purgeAllPreV2KnowledgeEntries } from '../src/cli/utils/legacy-knowledge-purge.js'; describe('purgeLegacyKnowledgeEntries', () => { let tmpDir: string; @@ -170,6 +170,31 @@ describe('purgeLegacyKnowledgeEntries', () => { await expect(fs.access(lockDir)).rejects.toThrow(); }); + it('second concurrent caller serializes behind first (mutual exclusion)', async () => { + const decisionsPath = path.join(knowledgeDir, 'decisions.md'); + await fs.writeFile(decisionsPath, ` + +## ADR-002: Legacy + +- **Status**: accepted +`, 'utf-8'); + + // Start two concurrent callers — only one can hold the lock at a time. + // Both should eventually succeed; the second will poll behind the first. + const [r1, r2] = await Promise.all([ + purgeLegacyKnowledgeEntries({ memoryDir }), + purgeLegacyKnowledgeEntries({ memoryDir }), + ]); + + // Between the two runs, exactly one removal happens (second is a no-op on already-clean file) + const totalRemoved = r1.removed + r2.removed; + expect(totalRemoved).toBe(1); + + // Lock must be released after both complete + const lockDir = path.join(memoryDir, '.knowledge.lock'); + await expect(fs.access(lockDir)).rejects.toThrow(); + }); + it('does not modify files when no legacy entries are present', async () => { const decisionsPath = path.join(knowledgeDir, 'decisions.md'); const originalContent = ` @@ -243,3 +268,274 @@ describe('purgeLegacyKnowledgeEntries', () => { expect(updated).not.toContain('ADR-002'); }); }); + +describe('purgeAllPreV2KnowledgeEntries', () => { + let tmpDir: string; + let memoryDir: string; + let knowledgeDir: string; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'devflow-purge-v3-test-')); + memoryDir = path.join(tmpDir, '.memory'); + knowledgeDir = path.join(memoryDir, 'knowledge'); + await fs.mkdir(knowledgeDir, { recursive: true }); + }); + + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + it('returns no-op result when .memory/knowledge/ does not exist', async () => { + const emptyMemory = path.join(tmpDir, 'no-memory'); + const result = await purgeAllPreV2KnowledgeEntries({ memoryDir: emptyMemory }); + expect(result.removed).toBe(0); + expect(result.files).toEqual([]); + }); + + it('removes a section with a /code-review (seed) source marker', async () => { + const decisionsPath = path.join(knowledgeDir, 'decisions.md'); + const content = ` + +## ADR-001: Code review seeded decision + +- **Status**: accepted +- **Source**: /code-review (seed from v1 audit) +- Some content +`; + await fs.writeFile(decisionsPath, content, 'utf-8'); + + const result = await purgeAllPreV2KnowledgeEntries({ memoryDir }); + + expect(result.removed).toBe(1); + expect(result.files).toContain(decisionsPath); + const updated = await fs.readFile(decisionsPath, 'utf-8'); + expect(updated).not.toContain('ADR-001'); + }); + + it('removes a section with a /implement (seed) source marker', async () => { + const pitfallsPath = path.join(knowledgeDir, 'pitfalls.md'); + const content = ` + +## PF-002: Implement seeded pitfall + +- **Status**: active +- **Source**: /implement (seed) +- Some content +`; + await fs.writeFile(pitfallsPath, content, 'utf-8'); + + const result = await purgeAllPreV2KnowledgeEntries({ memoryDir }); + + expect(result.removed).toBe(1); + expect(result.files).toContain(pitfallsPath); + const updated = await fs.readFile(pitfallsPath, 'utf-8'); + expect(updated).not.toContain('PF-002'); + }); + + it('preserves a section with a self-learning: source marker', async () => { + const decisionsPath = path.join(knowledgeDir, 'decisions.md'); + const content = ` + +## ADR-005: Self-learning decision + +- **Status**: accepted +- **Source**: self-learning:obs_abc123 +- Some content +`; + await fs.writeFile(decisionsPath, content, 'utf-8'); + + const result = await purgeAllPreV2KnowledgeEntries({ memoryDir }); + + expect(result.removed).toBe(0); + expect(result.files).not.toContain(decisionsPath); + const unchanged = await fs.readFile(decisionsPath, 'utf-8'); + expect(unchanged).toContain('ADR-005'); + expect(unchanged).toContain('self-learning:obs_abc123'); + }); + + it('removes seeded entries and keeps self-learning entries in a mixed file', async () => { + const decisionsPath = path.join(knowledgeDir, 'decisions.md'); + const content = ` + +## ADR-001: Seeded entry to remove + +- **Status**: accepted +- **Source**: /code-review (seed) +- Content + +## ADR-003: Self-learning entry to keep + +- **Status**: accepted +- **Source**: self-learning:obs_xyz789 +- Content + +## ADR-007: Another seeded entry to remove + +- **Status**: accepted +- **Source**: /implement (seed v1) +- Content +`; + await fs.writeFile(decisionsPath, content, 'utf-8'); + + const result = await purgeAllPreV2KnowledgeEntries({ memoryDir }); + + expect(result.removed).toBe(2); + expect(result.files).toContain(decisionsPath); + + const updated = await fs.readFile(decisionsPath, 'utf-8'); + expect(updated).not.toContain('ADR-001'); + expect(updated).toContain('ADR-003'); + expect(updated).toContain('self-learning:obs_xyz789'); + expect(updated).not.toContain('ADR-007'); + }); + + it('recounts TL;DR after multi-section purge', async () => { + const pitfallsPath = path.join(knowledgeDir, 'pitfalls.md'); + const content = ` + +## PF-001: Seeded pitfall 1 + +- **Status**: active +- **Source**: /code-review (seed) + +## PF-002: Self-learning pitfall + +- **Status**: active +- **Source**: self-learning:obs_111 + +## PF-003: Seeded pitfall 3 + +- **Status**: active +- **Source**: /implement (seed) + +## PF-004: Self-learning pitfall 4 + +- **Status**: active +- **Source**: self-learning:obs_222 +`; + await fs.writeFile(pitfallsPath, content, 'utf-8'); + + await purgeAllPreV2KnowledgeEntries({ memoryDir }); + + const updated = await fs.readFile(pitfallsPath, 'utf-8'); + // 2 seeded removed, 2 self-learning remain + expect(updated).toContain(''); + expect(updated).toContain('PF-002'); + expect(updated).toContain('PF-004'); + expect(updated).not.toContain('PF-001'); + expect(updated).not.toContain('PF-003'); + }); + + it('acquires and releases .knowledge.lock during operation', async () => { + const decisionsPath = path.join(knowledgeDir, 'decisions.md'); + await fs.writeFile(decisionsPath, ` + +## ADR-010: Seeded + +- **Status**: accepted +- **Source**: /code-review (seed) +`, 'utf-8'); + + await purgeAllPreV2KnowledgeEntries({ memoryDir }); + + // Lock directory must be released after the call + const lockDir = path.join(memoryDir, '.knowledge.lock'); + await expect(fs.access(lockDir)).rejects.toThrow(); + }); + + it('second concurrent caller serializes behind first (mutual exclusion)', async () => { + const decisionsPath = path.join(knowledgeDir, 'decisions.md'); + await fs.writeFile(decisionsPath, ` + +## ADR-010: Seeded + +- **Status**: accepted +- **Source**: /code-review (seed) +`, 'utf-8'); + + // Start two concurrent callers — only one can hold the lock at a time. + // Both should eventually succeed; the second will poll behind the first. + const [r1, r2] = await Promise.all([ + purgeAllPreV2KnowledgeEntries({ memoryDir }), + purgeAllPreV2KnowledgeEntries({ memoryDir }), + ]); + + // Between the two runs, exactly one removal happens (second is a no-op on already-clean file) + const totalRemoved = r1.removed + r2.removed; + expect(totalRemoved).toBe(1); + + // Lock must be released after both complete + const lockDir = path.join(memoryDir, '.knowledge.lock'); + await expect(fs.access(lockDir)).rejects.toThrow(); + }); + + it('does not follow a symlink placed at the .tmp path (TOCTOU hardening)', async () => { + // Arrange: create a decisions.md with a seeded entry to trigger atomic write + const decisionsPath = path.join(knowledgeDir, 'decisions.md'); + await fs.writeFile(decisionsPath, ` + +## ADR-020: Seeded TOCTOU test + +- **Status**: accepted +- **Source**: /code-review (seed) +`, 'utf-8'); + + // Place a symlink at the .tmp location pointing to a sentinel file + const tmpPath = `${decisionsPath}.tmp`; + const sentinelPath = path.join(tmpDir, 'v3-attacker-controlled.txt'); + await fs.writeFile(sentinelPath, 'original-sentinel', 'utf-8'); + await fs.symlink(sentinelPath, tmpPath); + + // Act: the purge should complete successfully + await purgeAllPreV2KnowledgeEntries({ memoryDir }); + + // Assert: the sentinel file was NOT overwritten + const sentinelContent = await fs.readFile(sentinelPath, 'utf-8'); + expect(sentinelContent).toBe('original-sentinel'); + + // And decisions.md was still written correctly + const updated = await fs.readFile(decisionsPath, 'utf-8'); + expect(updated).not.toContain('ADR-020'); + }); + + it('does not remove PROJECT-PATTERNS.md (v2 owns that cleanup)', async () => { + const projectPatternsPath = path.join(memoryDir, 'PROJECT-PATTERNS.md'); + await fs.writeFile(projectPatternsPath, '# Old patterns', 'utf-8'); + + const result = await purgeAllPreV2KnowledgeEntries({ memoryDir }); + + // v3 should not touch PROJECT-PATTERNS.md — it is v2's responsibility + await expect(fs.access(projectPatternsPath)).resolves.toBeUndefined(); + expect(result.files).not.toContain(projectPatternsPath); + }); + + it('is a no-op when both files have only self-learning entries', async () => { + const decisionsPath = path.join(knowledgeDir, 'decisions.md'); + const pitfallsPath = path.join(knowledgeDir, 'pitfalls.md'); + + const decisionsContent = ` + +## ADR-001: Self-learning kept + +- **Status**: accepted +- **Source**: self-learning:obs_aaa +`; + const pitfallsContent = ` + +## PF-001: Self-learning kept + +- **Status**: active +- **Source**: self-learning:obs_bbb +`; + await fs.writeFile(decisionsPath, decisionsContent, 'utf-8'); + await fs.writeFile(pitfallsPath, pitfallsContent, 'utf-8'); + + const result = await purgeAllPreV2KnowledgeEntries({ memoryDir }); + + expect(result.removed).toBe(0); + expect(result.files).toEqual([]); + // Files are unchanged + expect(await fs.readFile(decisionsPath, 'utf-8')).toBe(decisionsContent); + expect(await fs.readFile(pitfallsPath, 'utf-8')).toBe(pitfallsContent); + }); +}); diff --git a/tests/migrations.test.ts b/tests/migrations.test.ts index 74a8804..4721a42 100644 --- a/tests/migrations.test.ts +++ b/tests/migrations.test.ts @@ -124,6 +124,27 @@ describe('MIGRATIONS', () => { expect(m).toBeDefined(); expect(m?.scope).toBe('per-project'); }); + + it('contains purge-legacy-knowledge-v3 with per-project scope', () => { + const m = MIGRATIONS.find(m => m.id === 'purge-legacy-knowledge-v3'); + expect(m).toBeDefined(); + expect(m?.scope).toBe('per-project'); + expect(m?.description).toBeTruthy(); + expect(typeof m?.run).toBe('function'); + }); + + it('v3 description explains source discriminator approach', () => { + const m = MIGRATIONS.find(m => m.id === 'purge-legacy-knowledge-v3'); + expect(m?.description).toContain('pre-v2'); + expect(m?.description).toContain('self-learning'); + }); + + it('v3 is after v2 in the MIGRATIONS array (ordering preserved)', () => { + const v2Index = MIGRATIONS.findIndex(m => m.id === 'purge-legacy-knowledge-v2'); + const v3Index = MIGRATIONS.findIndex(m => m.id === 'purge-legacy-knowledge-v3'); + expect(v2Index).toBeGreaterThanOrEqual(0); + expect(v3Index).toBeGreaterThan(v2Index); + }); }); describe('runMigrations', () => { @@ -346,6 +367,42 @@ describe('runMigrations', () => { } }); + it('v3 migration runs independently even when v2 is already applied', async () => { + const fakeHome = path.join(tmpDir, 'home', '.devflow'); + + // Mark v2 (and global migrations) as already applied — v3 has NOT been applied yet + const appliedBefore = [ + ...MIGRATIONS.filter(m => m.scope === 'global').map(m => m.id), + 'purge-legacy-knowledge-v2', + ]; + await writeAppliedMigrations(fakeHome, appliedBefore); + + // Create a project with a seeded entry (no self-learning: source) + const projectRoot = path.join(tmpDir, 'project-v3-independent'); + const knowledgeDir = path.join(projectRoot, '.memory', 'knowledge'); + await fs.mkdir(knowledgeDir, { recursive: true }); + const decisionsPath = path.join(knowledgeDir, 'decisions.md'); + await fs.writeFile(decisionsPath, ` + +## ADR-003: Seeded entry lacking self-learning marker + +- **Status**: accepted +- **Source**: /code-review (seed) +`, 'utf-8'); + + const ctx = { devflowDir: fakeHome, claudeDir: tmpDir }; + const result = await runMigrations(ctx, [projectRoot]); + + // v3 should have run and succeeded + expect(result.failures).toEqual([]); + expect(result.newlyApplied).toContain('purge-legacy-knowledge-v3'); + expect(result.newlyApplied).not.toContain('purge-legacy-knowledge-v2'); // v2 was pre-applied + + // The seeded entry should be gone + const updated = await fs.readFile(decisionsPath, 'utf-8'); + expect(updated).not.toContain('ADR-003'); + }); + it('runs global migrations against devflowDir (not project root)', async () => { const fakeHome = path.join(tmpDir, 'home', '.devflow'); diff --git a/tests/resolve/knowledge-citation.test.ts b/tests/resolve/knowledge-citation.test.ts new file mode 100644 index 0000000..553f40d --- /dev/null +++ b/tests/resolve/knowledge-citation.test.ts @@ -0,0 +1,238 @@ +// tests/resolve/knowledge-citation.test.ts +// Tests for Fix 1: /resolve reads and cites project knowledge. +// +// Strategy: The filter + loader logic lives in the production module +// scripts/hooks/lib/knowledge-context.cjs; these tests import it directly +// for real coverage. The markdown structural tests verify that the instruction +// to invoke the module (or follow its algorithm) is present on every surface. +// +// Test groups: +// 1. Unit tests: filterKnowledgeContext (D-A filter) — imported from production module +// 2. Unit tests: filterKnowledgeContext — imported from production module +// 3. Structural tests: resolve.md — Step 0d presence + KNOWLEDGE_CONTEXT in Phase 4 +// (knowledge-context.cjs index invocation covered by tests/knowledge/command-adoption.test.ts) +// 4. Structural tests: resolve-teams.md — parity with base +// (knowledge-context.cjs index invocation covered by tests/knowledge/command-adoption.test.ts) +// 5. Structural tests: resolve:orch SKILL.md — Phase 1.5 parity +// (knowledge-context.cjs index invocation covered by tests/knowledge/command-adoption.test.ts) +// 6. Structural tests: resolver.md — Input Context + Apply Knowledge +// (ADR/PF citation format + hallucination guard covered by tests/knowledge/apply-knowledge-skill.test.ts) +// 7. Cross-cutting: all four surfaces reference KNOWLEDGE_CONTEXT + +import { describe, it, expect } from 'vitest'; +import * as path from 'path'; +import { createRequire } from 'module'; +import { + ACTIVE_ADR, ACTIVE_PF, DEPRECATED_ADR, DEPRECATED_PF, + SUPERSEDED_ADR, +} from '../knowledge/fixtures'; +import { loadFile, extractSection } from '../knowledge/helpers'; + +const ROOT = path.resolve(import.meta.dirname, '../..'); +const require = createRequire(import.meta.url); + +// Import the production module — this is the real implementation, not a test copy. +const { filterKnowledgeContext } = require( + path.join(ROOT, 'scripts/hooks/lib/knowledge-context.cjs') +) as { + filterKnowledgeContext: (raw: string) => string; +}; + +// --------------------------------------------------------------------------- +// Unit tests: filterKnowledgeContext (D-A filter) — production module +// --------------------------------------------------------------------------- + +describe('filterKnowledgeContext — Deprecated/Superseded filtering (D-A)', () => { + it('returns empty string when input is empty', () => { + expect(filterKnowledgeContext('')).toBe(''); + }); + + it('preserves Active ADR sections unchanged', () => { + const output = filterKnowledgeContext(ACTIVE_ADR); + expect(output).toContain('ADR-001'); + expect(output).toContain('Always return Result'); + }); + + it('removes Deprecated ADR sections', () => { + const output = filterKnowledgeContext(DEPRECATED_ADR); + expect(output).not.toContain('ADR-002'); + expect(output).not.toContain('Do the old thing'); + }); + + it('removes Superseded ADR sections', () => { + const output = filterKnowledgeContext(SUPERSEDED_ADR); + expect(output).not.toContain('ADR-003'); + }); + + it('removes Deprecated PF sections', () => { + const output = filterKnowledgeContext(DEPRECATED_PF); + expect(output).not.toContain('PF-001'); + }); + + it('keeps Active PF sections', () => { + const output = filterKnowledgeContext(ACTIVE_PF); + expect(output).toContain('PF-004'); + expect(output).toContain('Watch out for growing scripts'); + }); + + it('preserves Active sections when mixed with Deprecated sections', () => { + const input = [ACTIVE_ADR, DEPRECATED_ADR, ACTIVE_PF].join('\n'); + const output = filterKnowledgeContext(input); + expect(output).toContain('ADR-001'); + expect(output).toContain('Always return Result'); + expect(output).not.toContain('ADR-002'); + expect(output).not.toContain('Do the old thing'); + expect(output).toContain('PF-004'); + expect(output).toContain('Watch out for growing scripts'); + }); + + it('returns empty string when all sections are removed (orchestrator emits "(none)")', () => { + const output = filterKnowledgeContext(DEPRECATED_ADR); + // Empty string signals orchestrator to emit "(none)" + expect(output).toBe(''); + }); +}); + +// --------------------------------------------------------------------------- +// Structural tests: resolve.md (base command) +// --------------------------------------------------------------------------- + +describe('resolve.md — base command', () => { + const content = loadFile('plugins/devflow-resolve/commands/resolve.md'); + + it('contains Step 0d: Load Project Knowledge after Phase 0c', () => { + expect(content).toMatch(/Step 0d.*Load Project Knowledge/i); + }); + + it('Step 0d instructs passing KNOWLEDGE_CONTEXT to Phase 4 Resolvers', () => { + const step0dSection = extractSection(content, 'Step 0d', '\n### Phase 1'); + expect(step0dSection).toContain('KNOWLEDGE_CONTEXT'); + }); + + it('Step 0d emits (none) when both files are absent or empty', () => { + const step0dSection = extractSection(content, 'Step 0d', '\n### Phase 1'); + expect(step0dSection).toContain('(none)'); + }); + + it('Phase 4 Resolver spawn block includes KNOWLEDGE_CONTEXT variable', () => { + const phase4Section = extractSection(content, '### Phase 4', '### Phase 5'); + expect(phase4Section).toContain('KNOWLEDGE_CONTEXT'); + }); + + it('Phase 5 or Phase 8 mentions Knowledge Citations in resolution-summary.md (D-B)', () => { + expect(content).toContain('Knowledge Citations'); + }); + + it('Step 0d is nested inside the per-worktree Phase 0 section (multi-worktree constraint)', () => { + const phase0Start = content.indexOf('### Phase 0'); + const phase1Start = content.indexOf('### Phase 1'); + const step0dIdx = content.indexOf('Step 0d'); + expect(step0dIdx).toBeGreaterThan(phase0Start); + expect(step0dIdx).toBeLessThan(phase1Start); + }); +}); + +// --------------------------------------------------------------------------- +// Structural tests: resolve-teams.md (teams variant — must match base) +// --------------------------------------------------------------------------- + +describe('resolve-teams.md — teams variant parity', () => { + const content = loadFile('plugins/devflow-resolve/commands/resolve-teams.md'); + + it('contains Step 0d: Load Project Knowledge', () => { + expect(content).toMatch(/Step 0d.*Load Project Knowledge/i); + }); + + it('Phase 4 Resolver teammate prompt includes KNOWLEDGE_CONTEXT variable', () => { + const phase4Section = extractSection(content, '### Phase 4', '### Phase 5'); + expect(phase4Section).toContain('KNOWLEDGE_CONTEXT'); + }); + + it('mentions Knowledge Citations for resolution-summary.md (D-B)', () => { + expect(content).toContain('Knowledge Citations'); + }); +}); + +// --------------------------------------------------------------------------- +// Structural tests: resolve:orch SKILL.md (ambient mode) +// --------------------------------------------------------------------------- + +describe('resolve:orch SKILL.md — ambient mode parity', () => { + const content = loadFile('shared/skills/resolve:orch/SKILL.md'); + + it('contains Phase 1.5: Load Project Knowledge between Phase 1 and Phase 2', () => { + expect(content).toMatch(/Phase 1\.5.*Load Project Knowledge/i); + }); + + it('Phase 4 spawn block includes KNOWLEDGE_CONTEXT', () => { + const phase4Section = extractSection(content, '## Phase 4', '## Phase 5'); + expect(phase4Section).toContain('KNOWLEDGE_CONTEXT'); + }); + + it('Phase 6 (Report) mentions Knowledge Citations (D-B)', () => { + const phase6Section = extractSection(content, '## Phase 6', '## Error Handling'); + expect(phase6Section).toContain('Knowledge Citations'); + }); +}); + +// --------------------------------------------------------------------------- +// Structural tests: shared/agents/resolver.md +// --------------------------------------------------------------------------- + +describe('resolver.md — Input Context and Apply Knowledge section', () => { + const content = loadFile('shared/agents/resolver.md'); + + it('declares KNOWLEDGE_CONTEXT in Input Context section', () => { + const inputContextSection = extractSection(content, '## Input Context', '\n## '); + expect(inputContextSection).toContain('KNOWLEDGE_CONTEXT'); + }); + + it('contains Apply Knowledge section', () => { + expect(content).toMatch(/## Apply Knowledge|### Apply Knowledge/); + }); + + it('Apply Knowledge section describes citing inline in Reasoning column', () => { + const applyStart = content.search(/## Apply Knowledge|### Apply Knowledge/); + if (applyStart === -1) throw new Error('Apply Knowledge section not found in resolver.md'); + const applyAnchor = content.slice(applyStart, applyStart + 30); + const applySection = extractSection(content, applyAnchor.split('\n')[0], '\n## '); + expect(applySection).toMatch(/[Rr]easoning/); + }); + + it('KNOWLEDGE_CONTEXT is marked optional in Input Context', () => { + const inputContextSection = extractSection(content, '## Input Context', '\n## '); + const knowledgeIdx = inputContextSection.indexOf('KNOWLEDGE_CONTEXT'); + if (knowledgeIdx === -1) throw new Error('KNOWLEDGE_CONTEXT not found in Input Context section'); + const surroundingText = inputContextSection.slice( + Math.max(0, knowledgeIdx - 20), + Math.min(inputContextSection.length, knowledgeIdx + 120) + ); + expect(surroundingText).toMatch(/optional|if provided|when provided|non-empty/i); + }); +}); + +// --------------------------------------------------------------------------- +// Cross-cutting: all four surfaces reference KNOWLEDGE_CONTEXT +// --------------------------------------------------------------------------- + +describe('cross-cutting — KNOWLEDGE_CONTEXT on all four surfaces', () => { + it('resolve.md contains KNOWLEDGE_CONTEXT', () => { + const content = loadFile('plugins/devflow-resolve/commands/resolve.md'); + expect(content).toContain('KNOWLEDGE_CONTEXT'); + }); + + it('resolve-teams.md contains KNOWLEDGE_CONTEXT', () => { + const content = loadFile('plugins/devflow-resolve/commands/resolve-teams.md'); + expect(content).toContain('KNOWLEDGE_CONTEXT'); + }); + + it('resolve:orch SKILL.md contains KNOWLEDGE_CONTEXT', () => { + const content = loadFile('shared/skills/resolve:orch/SKILL.md'); + expect(content).toContain('KNOWLEDGE_CONTEXT'); + }); + + it('resolver.md contains KNOWLEDGE_CONTEXT', () => { + const content = loadFile('shared/agents/resolver.md'); + expect(content).toContain('KNOWLEDGE_CONTEXT'); + }); +}); diff --git a/tests/skill-references.test.ts b/tests/skill-references.test.ts index 4d26373..7e39674 100644 --- a/tests/skill-references.test.ts +++ b/tests/skill-references.test.ts @@ -1036,9 +1036,15 @@ describe('citation sentence propagation', () => { expect(coderSentence).toBe(canonical); }); - it('reviewer.md has byte-identical citation sentence', () => { - const canonical = extractCitationSentence(skillPath); + it('reviewer.md has citation sentence referencing KNOWLEDGE_CONTEXT index and apply-knowledge skill', () => { + // reviewer.md uses the index+Read pattern (KNOWLEDGE_CONTEXT + devflow:apply-knowledge), + // not the direct-file-read pattern used by coder. The sentence intentionally differs from + // the canonical (coder-style) sentence. const reviewerSentence = extractCitationSentence(reviewerPath); - expect(reviewerSentence).toBe(canonical); + expect(reviewerSentence.trim()).toBeTruthy(); + expect(reviewerSentence).toContain('KNOWLEDGE_CONTEXT'); + expect(reviewerSentence).toContain('devflow:apply-knowledge'); + expect(reviewerSentence).toContain('applies ADR-NNN'); + expect(reviewerSentence).toContain('avoids PF-NNN'); }); });