feat(#108): memory distillation pipeline — scraper, filter, distiller, CLI, and CI workflow

[alexosugo]

Summary

Adds an automated pipeline that grows agent-memory/ from merged cht-core PRs — no more hand-writing knowledge entries. It runs daily (or on demand), looks at each merged PR, decides whether it's worth capturing, and uses an LLM to draft a structured note for a human to review.

Flow: scrape PR → filter (keep or skip) → distill into a draft → human reviews

cht-core PR ──► Scraper ──► Filter ──► Distiller ──► _pending/ draft ──► human review
              (gh CLI)   (rules+LLM)  (LLM)

Each stage is its own module, independently tested, and runnable from one CLI or the CI workflow.

How each stage works

• Scraper (scraper.ts) — uses the gh CLI to gather everything about a PR: title, labels, author, file list, full diff, linked issues, and reviewer comments. No LLM, just data assembly.
• Filter (filter.ts) — decides keep vs. skip in two passes: fast deterministic rules first (skip chore/test/dependency-only or trivial PRs; keep code changes that have a linked issue), then an LLM judges anything ambiguous. Every skip is logged to _skipped.ndjson with a reason so the rules can be audited and tuned.
• Distiller (distiller.ts) — sends the kept PR to an LLM and writes a structured draft to agent-memory/_pending/<domain>/<pr>.md, complete with provenance frontmatter (source_pr, source_sha, confidence, etc.). Drafts that fail schema validation are logged to _skipped.ndjson instead of being written.

How to run it

• run-pipeline CLI (run-pipeline.ts) — wires all three stages. Flags: --pr <n> (one PR), --since <hours> (recent merges, default 24), --repo <owner/repo> (default medic/cht-core). Exits non-zero if any PR fails.
• CI workflow (.github/workflows/run-pipeline.yml) — runs daily at 06:00 UTC over the last 24h, plus a manual workflow_dispatch trigger. It commits new drafts and _skipped.ndjson updates back with [skip ci].
• open-review-pr (open-review-pr.ts) — the human side: opens each _pending/ draft in the editor, validates on save, and moves approved drafts to their final agent-memory/domains/ home.

Config

Set in .env (see .env.example): OPENROUTER_API_KEY, TRIAGE_MODEL, DISTILL_MODEL, GH_TOKEN.

What else is in here

• Shared pieces: schema-utils.ts (AJV validator + frontmatter helpers), src/types/pipeline.ts (all pipeline types), src/constants/, and agent-memory/schema.json + TEMPLATE.md defining the note format.
• Tests: ~2,100 lines of unit tests across four spec files (scraper, filter, distiller, open-review-pr) covering gh mocking, every filter branch, LLM happy/invalid paths, and the review loop. 265 passing.
• SonarCloud: Fixed an S5852 ReDoS in the lockfile-matching regex ([^/]* instead of .*). The remaining S4036 "PATH" hotspots are execFileSync('gh', …) calls — reviewed and accepted: no shell is used, args are arrays, and they only run in controlled CI; a hardcoded path would break cross-platform portability.

Test plan

• npm test — all unit tests pass
• npm run smoke-test — full pipeline against three live PRs (11057, 11022, 11077); expect at least one distill and one skip
• npm run run-pipeline -- --pr 11057 — produces a valid draft in _pending/
• npm run run-pipeline -- --since 48 — runs clean; _skipped.ndjson grows for skips
• workflow_dispatch with pr_number=11057 — workflow commits a draft back to the branch
• npm run open-review-pr — opens, validates, and moves a draft to the right domain

Closes #108 and lays the foundation for #101

[alexosugo]

SonarCloud security hotspots (PR #109) — review summary

S5852 — ReDoS in LOCKFILE_PATTERN (filter.ts:23): FIXED (commit on this branch).
Replaced the unbounded .* with [^/]* in the lockfile alternation so the wildcard can no longer overlap the (?:^|/) anchor. This bounds backtracking to a single path segment — a 50k-char pathological non-match now resolves in ~0.07ms — while preserving match behavior across root-level and nested *.lock paths. Verified by the existing filter test suite (265 passing) plus edge-case checks.

S4036 ×6 — "PATH variable" on gh calls (scraper.ts:54,94,126,164,198, run-pipeline.ts:74): reviewed, accepted as-is.
These are all execFileSync('gh', [...args]):

• execFileSync with an args array uses no shell, so command/argument injection is not possible.
• The residual $PATH-hijack risk is purely environmental; these scripts run only in controlled CI (and local dev), not against untrusted PATH.
• Hardcoding an absolute path is not portable (gh lives at /opt/homebrew/bin, /usr/local/bin, or /usr/bin depending on platform) and the child-process mocks in the test suite would mask a wrong path, so the fix would be both fragile and untestable.

These 6 are LOW-probability hotspots; assessed as acceptable risk for CI tooling rather than altered.

[sugat009]

Review (request changes): memory distillation pipeline

Reviewed across security, the pipeline stages, schema/review-loop, and tests, with a QA manual-run pass to confirm findings empirically. Solid PR with good security fundamentals. The blocking items are narrow: one cross-PR schema reconciliation plus CI/conflict housekeeping; everything else is non-blocking follow-ups.

Verified strong: no shell injection (execFileSync with array args), no path traversal (z.enum domain + slugified filename), the S5852 ReDoS fix is real, provenance fields are not spoofable, the CI workflow uses the safe input pattern (env: indirection, not ${{ }} inside run:), and the human-review gate is solid: nothing reaches agent-memory/domains/ without passing AJV re-validation in open-review-pr, a human-initiated --apply, and a human-reviewed promotion PR.

Requested changes

1. Settle on one frontmatter contract. agent-memory/schema.json conflicts with #87 (confirmed via git merge-tree). All 30 committed agent-memory files already use #87's camelCase shape (id, category, issueNumber, issueUrl, lastUpdated, summary, services, techStack); they only pass this PR's snake_case schema via a lastUpdated to last_updated alias in normalizeFrontmatter, and the rich fields go unchecked. Suggest aligning on #87's shape (it matches the files and allows extra properties, so the provenance fields still fit) and updating the distiller to emit it. Worth coordinating before either PR merges, to avoid a schema conflict/regression.

2. Housekeeping (these block merge today). The SonarCloud os.tmpdir() hotspots in smoke-test.ts are the only failing check (fix with fs.mkdtempSync or mark them safe), and there is a .gitignore merge conflict to resolve.

question (non-blocking): OpenRouter / non-Claude stack

This pipeline reaches its LLM through OpenRouter (ChatOpenAI + baseURL: https://openrouter.ai/api/v1 in filter.ts:152 / distiller.ts:68), with a ChatAnthropic fallback. That is the first place cht-agent routes through a third-party gateway and pulls in @langchain/openai + an OPENROUTER_API_KEY; everywhere else we standardize on Claude via the src/llm LLMProvider abstraction. Could you add a short rationale in the PR/README?

• The code defaults are Claude-via-OpenRouter (anthropic/claude-haiku-4-5, anthropic/claude-sonnet-4-5), where OpenRouter is just a proxy in front of Claude, but .env.example recommends open models (google/gemma-3-4b-it, deepseek/deepseek-chat-v3-0324). Which is the intended default?
• If open models are the intent, a note that agent-memory drafts would be authored by non-Anthropic models. The cht-core PR content is public, so this is a quality and consistency call, not a data-privacy one.

Not asking to change the code here, just capturing the decision so the divergence from the Claude-only stack is intentional.

Secondary (non-blocking follow-ups)

• Validate drafts before writing. distillPR hand-assembles the YAML frontmatter by string interpolation (title: ${draft.title} unquoted) and writes with no parse/AJV check, which contradicts the documented "schema-validate or log-skip". Not a memory-safety risk, since open-review-pr re-validates before promotion and a human gates the PR, but the code does not match its docs, and broken drafts pile up in committed _pending/ and re-spam _skipped.ndjson (never cleaned). Fix: serialize with js-yaml dump() and validate (parse + AJV) before writing, routing failures to _skipped.ndjson. (validate-schema also skips _pending while the cron commits it.)
• scraper --paginate review merge corrupts bodies containing ] [ and crashes on empty pages (scraper.ts:259); use --slurp or parse per page.
• open-review-pr promotion is not isolated per-domain: one domain's failure aborts the rest and leaves an orphan pushed branch.
• filter LLM tests stub @langchain/anthropic, but the primary path is OpenRouter (ChatOpenAI); the full suite passes only because a sibling spec clears OPENROUTER_API_KEY first (cross-spec env leakage), QA confirmed.
• smaller: --since <non-numeric> exits 0 as success; unguarded r.user.login throws on deleted-account reviews; source_pr link builds .../cht-core#42 instead of /pull/42; and npm run smoke-test is in the test plan but not in package.json.

Nits

• LOCKFILE_PATTERN (filter.ts:25) matches any *.lock basename, so a PR touching an unrelated .lock could be mis-skipped.
• A failed issue fetch still returns a linked-issue object (scraper.ts), so a 404'd reference can flip a skip into a distill.
• slugify (distiller.ts:287) yields an empty slug for symbol-only or non-Latin titles (42-.md), and a re-run overwrites the prior draft (no existence check).
• The draft title is interpolated into the PR-body markdown unescaped (open-review-pr.ts:79).
• runLlmTriage's try/catch (filter.ts:228) is redundant; the real triageFn already catches internally.
• JSON.parse of metadata/reviews (scraper.ts:240) sits outside the ScraperError wrap, leaking a raw SyntaxError.
• A touchesMultipleServices test (filter.spec.ts:171) is mislabeled and its inline comment contradicts the assertion.

Thanks, this is a genuinely useful pipeline; the asks are cross-PR schema coordination plus CI housekeeping, not a redesign.

[alexosugo]

Thanks for the thorough review. Both blocking items are addressed (commits 9bcb508, 6098573), plus the "validate before writing" follow-up and the SonarCloud housekeeping.

1. Frontmatter contract reconciled with #87 (camelCase). Adopted #87's converged schema.json and schema-utils.ts verbatim, so the two PRs now share one contract and won't conflict when either merges. The committed agent-memory files validate natively (the lastUpdated → last_updated alias is gone), and the distiller now emits camelCase frontmatter including the now-required issueNumber, issueUrl, services, and techStack — derived from the first linked issue, falling back to the PR number when no issue is closed.

2. Housekeeping. The os.tmpdir() hotspots in smoke-test.ts are fixed with fs.mkdtempSync (random name, 0700 perms). The .gitignore conflict was already resolved by the earlier main merge (GitHub reports MERGEABLE). I also reset _skipped.ndjson to empty, dropping accumulated dev-run noise — this covers your "never cleaned" note.

Folded in "validate drafts before writing": distillPR now serializes frontmatter with js-yaml dump() (correct quoting — fixes the unquoted-title bug) and AJV-validates every draft before writing. Invalid drafts route to _skipped.ndjson as flag-for-human instead of landing in _pending/.

SonarCloud (the remaining red check). The mkdtempSync fix cleared the temp-dir hotspots. What's left is a different set — S4036 ("PATH variable") on the gh/git execFileSync calls in the scraper and pipeline runner. Invoking these tools by name via PATH is the standard approach for CI and local dev; the only code-level "fix" is hardcoding absolute binary paths, which breaks portability. The correct resolution is to mark those hotspots Safe in the SonarCloud UI (needs project access). This check is informational — main has no required status checks, so it doesn't block merge.

OpenRouter / LLM provider question. Addressed in the README: the OpenRouter integration is there to keep the pipeline provider-agnostic rather than locked to a single vendor. There's no mandated default — whoever runs the pipeline picks the model. (The Claude defaults in code are just one example.)

One item surfaced, needs a call on #87's / #79's side. Under #87's strict CHTService enum, 4 already-committed contacts files from #79 fail validation because they use services: shared-libs / config. This isn't introduced here and isn't in #109's CI path, but it's a real #87↔#79 data divergence: either the enum grows to include those values, or the 4 files are corrected. I left another PR's committed data untouched.

Nits — all addressed (b3a89f0): LOCKFILE_PATTERN narrowed to known lockfile names (no longer skips unrelated *.lock files, and now fully literal — no ReDoS surface); unresolvable linked issues are dropped instead of returned as empty stubs (so a 404'd reference can't flip a skip into a distill); JSON.parse of metadata/reviews is wrapped in ScraperError; slugify falls back to untitled for symbol-only/non-Latin titles; draft titles are Markdown-escaped in the PR body; the redundant double error-catch in the triage path is collapsed to one boundary; and the mislabeled filter test is fixed.

Secondary follow-ups 1–3 — addressed (30bda57): the scraper now fetches reviews with --paginate --slurp and flattens the array-of-pages instead of string-stitching [...][...] (no longer corrupts bodies containing ] [ or breaks on empty pages); per-domain promotion is isolated (a domain failure is recorded and the rest still run, and an orphan remote branch is cleaned up if PR creation fails after push); and the filter tests now stub both LLM providers and clear both API keys per test (removing the cross-spec env leakage), with an explicit OpenRouter primary-path test added.

Secondary follow-ups 4–7 — addressed (17ab417): run-pipeline now rejects a non-numeric/non-positive --since (or --pr) with a clear error instead of silently processing nothing; reviews from deleted GitHub accounts (user: null) fall back to the ghost author rather than throwing; source_pr references resolve to working /pull/<n> URLs instead of a dead repo-home anchor; and the smoke-test npm script is now defined. The CLI entry point is also guarded so its orchestration is unit-tested (100% coverage on that module).

All review items — both blocking fixes, the seven nits, and follow-ups 1–7 — are now resolved.

[sugat009]

Approving. Re-reviewed at head ceea34d; every requested change and finding from the prior review is resolved, verified in code (not just from the reply):

• Frontmatter contract converged with #87 (schema.json + schema-utils.ts are now byte-identical). The distiller emits camelCase lastUpdated plus the required id/issueNumber/issueUrl/services/techStack, serializes via js-yaml dump(), and AJV-validates before writing (invalid drafts route to flag-for-human, never _pending).
• Scraper: --paginate --slurp (no more [...][...] string-stitch), literal lockfile pattern, ScraperError-wrapped parse, unresolvable issues dropped, deleted-account guard.
• run-pipeline: --since/--pr reject non-numeric, partially-numeric, decimal, non-positive, and oversized values.
• open-review-pr: per-domain isolation + orphan-branch cleanup, markdown-escaped titles, correct /pull/ link.
• Filter/tests: both providers stubbed and both keys cleared with an explicit OpenRouter primary-path test (the cross-spec env leak is gone; the suite passes even with poisoned keys), mislabeled test fixed, new run-pipeline spec.
• Housekeeping/docs: mkdtempSync + a smoke-test script, slugify untitled fallback, README OpenRouter rationale.

Thanks for the thorough turnaround, including the deferred follow-ups and the extra input-validation hardening.

Two non-blocking follow-ups (not gating this approval):

1. agent-memory/_skipped.ndjson is committed with ~7 dev-run noise lines (the reset did not stick); worth emptying or gitignoring so it does not accumulate.
2. The distiller emits 6 H2 body sections, but #87's validate-schema body check requires 8 (## Testing, ## Related Issues), so a promoted draft would fail the full-corpus validator. Forward-looking convergence item, bounded by the human promotion gate.

(SonarCloud Code Analysis is advisory now; the remaining S4036 gh/git PATH hotspots are legitimate execFileSync calls and can be marked Safe when convenient.)

[Hareet]

You guys have caught a lot of the same issues I had when running it (i.e. making sure schema's matched and included all fields such as related categories, sub-issues etc.)

Some feedback, but I'm not sure if you want to address it in this PR or next ones:

• Claude Code CLI factory (allows claude p to populate this)
• The info below was already captured during the LLM call, and I found it very helpful to insert into the context doc, in case we want to deterministically re-organize domains or specific sub-issues.

For PR 10804:

Domain Rationale

Fit: strong

The change operates entirely on the contact profile page, the contact view-model generator, and the child-contact hierarchy display — squarely contact lookup and presentation. No sync, permission, or config concerns are involved.