fix(#136): memory pipeline hardening + issue relinking (#135)

[alexosugo]

Description

Two related pieces of work on the memory distillation pipeline:

#135 — Issue resolution and relinking: replaces the buggy pr.prNumber-as-issue-number aliasing with proper GitHub issue resolution via closingIssuesReferences + title scope + body keywords, plus a one-off relink-issues tool to repair previously mislinked drafts.

#136 — Memory pipeline hardening (first-run post-mortem follow-through):

• CI guard rejecting mislinked/slug-contradicting drafts
• Cross-domain dedup (dedupeByIssueId) collapsing backport/multi-PR duplicates into a single canonical draft
• Prompt hardening (CONSTRAINTS block) and HTML boilerplate stripping
• Adaptive issue-body budget for near-empty PRs
• Confidence gradient replacing hardcoded 'medium'
• Per-run reconciliation report
• Unambiguous slug separator between conventional-commit type and issue number
• Fix for dedupeByIssueId's sort comparator: returned NaN (non-deterministic ordering) when two drafts for the same issue both lacked source_pr — flagged by roborev review #223, fixed with a stable path tiebreaker, regression tests added

#135
#136

Code review checklist

• Readable: Concise, well named, follows the style guide.
• Tested: Unit tests added/updated (dedup, distiller, gh-classify, issue-linkage, reconcile, relink-issues, run-pipeline, scraper)
• Backwards compatible: Works with existing data; includes a one-off relink-issues repair tool for previously mislinked drafts.

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

[Hareet]

Ah! I didn't realize there was a merge order dependency with #129 . Can you rebase with main?

[Hareet]

Some quick fixes and let's get this merged in. We will want to run the dedup on the memory/promote-* PRs when those are rebased and before they are merged.

Ran the suite locally on Node 22: build clean, 1057 tests passing (both new spec files execute in full), lint scoped to the PR's files clean.

Blockers:

• issue (blocking): AGENTS.md is unrelated boilerplate that contradicts repo policy. It is the stock beads/bd onboard template: it mandates git push ("Work is NOT complete until git push succeeds", "NEVER stop before pushing", "If push fails, resolve and retry until it succeeds"), git pull --rebase, and bd sync; says "Do NOT use external issue trackers" (we track in GitHub issues); and references docs/QUICKSTART.md and .beads/, neither of which exists in this repo. A checked-in AGENTS.md is exactly the file coding agents treat as authoritative, so this would actively instruct agents to violate the repo's no-commit/no-push constraints. Please drop it (along with the .dolt/ / *.db .gitignore lines that only make sense with beads) or replace it with a real project-specific AGENTS.md.
• issue: slugIssueNumber over-matches descriptive numbers, so the CI guard can reject valid drafts. /^\d+-[a-z]+-?(\d+)-/ (dedup.ts:85) accepts ANY word-then-number: 10043-fix-500-errors-on-login.md → 500, 9200-support-2-level-hierarchy.md → 2 (reproduced on Node 22). When that spurious number differs from the frontmatter issueNumber, ciGuardReason flags a correct draft as mislinked and open-review-pr drops it from promotion. relink-issues.ts already solves this with a conventional-commit type list (fix|feat|perf|…) — reuse that (or the shared module) so the two parsers cannot diverge.

Smaller:

• dedupeByIssueId (dedup.ts:153): drafts with a missing/non-scalar id all bucket under the shared '' key and get collapsed as duplicates of each other, silently deleting one. Unreachable in-pipeline today (schema validation requires id upstream), but the function is exported with no guard and the branch is untested.
• Cross-domain canonical selection is lowest-PR-number only; domainFit is never consulted, so a weak-fit lower-PR draft wins the domain over a strong-fit higher-PR one. Worth a tiebreaker or at least a doc note.
• reconcile's CI-guard/dedup buckets are structurally zero at the only wired call site: run-pipeline never invokes open-review-pr, and open-review-pr (which writes the CI guard: / duplicate of reasons) never calls reconcile. Either print a reconciliation at the end of open-review-pr too, or trim those buckets from the report.
• reconcile throws on a valid-JSON skip line without a string reason (entry.reason.includes, reconcile.ts:50) — parseSkipLogLine casts JSON.parse without shape-checking, and because reportOutcome runs unconditionally, one corrupt audit line turns a fully successful batch into exit(1). One typeof guard plus a test.
• When dedup adds source_prs[], rewriteFrontmatterOnDisk persists the fully normalized frontmatter, not a targeted insertion — key order/date quoting can reflow beyond the intended change. Cosmetic; it re-validates.
• nitpick: stray // ponytail: marker at run-pipeline.ts:484.
• nitpick: comment density — dedup.ts is ~48% comment lines and reconcile.ts ~57%, with @example blocks restating one-line bodies; this repo keeps comments minimal.

[alexosugo]

Updates

• Made duplicate-draft promotion retry-safe: pending drafts are removed only after the canonical review PR is created.
• Applied consolidated source_prs metadata only to the promoted copy, keeping pending drafts retryable if promotion fails.
• Scoped reconciliation summaries to records written during the current invocation.
• Preserved the source repository for provenance and related-issue links, including CHT Interoperability drafts.
• Added explicit, unambiguous issue tokens for Conventional Commit titles, including breaking changes.

Verification

• Focused regression coverage passes.
• Lint and TypeScript build pass.
• A live distillation of merged CHT Core PR #11331 produced a schema-valid draft with fully grounded file references.

[alexosugo]

@Hareet this is ready for another review.

[sugat009]

Substantial work, and the core design is right. resolveRealIssue classifying a number before trusting it, with one hop from PR to sole closing issue, is the correct shape, and flagForHuman on zero resolved issues is the correct failure mode. Checked against cht-core PR 9039, whose title says fix(#9024) where 9024 is an unrelated PR closing nothing: the chain drops the ref and flags for a human instead of mislinking. issueEqualsSourcePr is structurally right too, catching the pre-R1 shape by construction rather than by pattern.

One blocking problem: the file name and the guard disagree by design, so the guard rejects drafts where resolution did its most valuable work. The rest are gaps, plus a merge-order note for #127.

Two comments are anchored a little away from the code they discuss (the scraper.ts base-branch gate and discoverDraftsByDomain), because those lines fall outside this diff's hunks; the real line numbers are in the comment text.

[sugat009]

issue (blocking): the file name embeds the issue number from the raw PR title (issueToken(pr.prTitle)); frontmatter.issueNumber comes from resolution. ciGuardReason rejects when they disagree (dedup.ts:72-76). Disagreeing with a wrong title is what resolution is for, so the guard fires on its own successes.

Two triggers: a title token that is a PR closing one issue, where the hop returns a different number (the main thing R1 adds); and a closing ref pointing at X while the title says Y, where the closing ref wins on authority (issue-linkage.ts:68-73). The second is the human-typo case, for example a PR correctly linking #9023 with #9024 in the title.

Simulated against the real FILENAME_TOKEN_RE:

9039-issue-9024-assert-couchdb-version   slug=9024  resolved=9023  -> REJECTED
10043-issue-10036-add-personqualifier    slug=10036 resolved=10036 -> pass
4278-sms-gateway-api-tests               slug=null  resolved=3738  -> pass

Only the already-correct title passes cleanly.

Fix that keeps the signal instead of hiding it: name the file from the resolved issueNumber, and record a title-versus-resolved mismatch as a warning for a human rather than a rejection. The guard keeps issueEqualsSourcePr, and the slug check becomes what it should be, a detector of stale files on disk.

[alexosugo]

Fixed in 235ca7d. The filename now derives from the resolved frontmatter.issueNumber (issueToken on the raw title is deleted), so the name can never contradict the frontmatter it was written with. A title-versus-resolution mismatch is recorded as a decision: 'warn' audit entry for a human instead of a rejection — the 9039 shape (fix(#9024) resolving to 9023) now writes 9039-issue-9023-*.md plus a warning, covered by a regression test. issueEqualsSourcePr stays a rejection, and the slug check is now purely a stale-file detector.

[alexosugo]

Validated against your exact simulation table, running the built code (plus two live gh cases):

9039-issue-9024-* on disk, resolved 9023  -> rejected as STALE FILE:
    "filename slug implies issue #9024 but frontmatter issueNumber is 9023"
fresh distill of the same shape           -> writes 9039-issue-9023-*.md; guard passes its own output
10043-issue-10036-*, resolved 10036       -> pass
4278-sms-gateway-api-tests, resolved 3738 -> pass (no token parsed)

The human-typo shape (title fix(#9024), closing ref 9023) now produces 9039-issue-9023-fix-9024-assert-couchdb-version.md plus the audit entry {"decision":"warn","reason":"title says #9024 but resolution chose #9023 — verify linkage"} — recorded, not rejected.

Live cht-core PR 9039 (scraped via gh): resolution drops the 9024 ref, linkedIssues is empty, and distillPR returns flag-for-human ("PR closes no tracked issue") — no mislink.

[sugat009]

issue (blocking): the body calls this a CI guard, but this is its only call site and no workflow invokes it; unit_tests.yml and run-pipeline.yml are unchanged. It runs only locally, so a mislinked draft still reaches a promote PR by any other path. Wire it into a workflow, or rename it in the body.

[alexosugo]

Wired in CI now (second commit of this batch). A new npm run check-pending script validates every agent-memory/_pending draft against schema.json and ciGuardReason, and unit_tests.yml runs it on every PR — so a mislinked or stale-named draft fails the PR that carries it, regardless of the path it took to get there.

[sugat009]

issue (non-blocking): nothing records or gates on the base branch. baseRefName is zero-hit across src/, and the merge gate at :262-264 reads mergedAt alone. A PR merged into a later-abandoned feature branch still distils as shipped.

Live case: agent-memory/domains/data-sync/issues/10767-feat10663-add-ui-extension-to-service-worker.md is on main describing an appendUiExtensions hook that never reached master (cht-core PR 10767 merged into 10224-ui-extensions, abandoned; the symbol is zero-hit on master). cht-core PR 10432 is the same shape. gh already returns baseRefName in the same query as this call. PR #145 does not cover this either, so it is unguarded on both sides.

[alexosugo]

Fixed in a8deae2. The scraper now requests baseRefName in the same gh pr view call and rejects a PR whose base is not the repo's default branch (resolved once per repo via gh repo view, cached, failing open if the lookup fails). The 10767/10432-shape drafts already on main need a one-off cleanup — tracking that in a follow-up issue.

[alexosugo]

Validated live against both PRs you named, via the real gh CLI:

scrapePR(10767) -> ScraperError: PR #10767 merged into non-default branch '10224-ui-extensions' (default is 'master')
scrapePR(10432) -> ScraperError: PR #10432 merged into non-default branch '10140_previous-month-targets' (default is 'master')

Neither reaches distillation any more. The already-promoted 10767-shape drafts on main are tracked in #155.

[sugat009]

issue (non-blocking): the dedup is genuinely cross-domain, as documented. But discoverDraftsByDomain (:42-58) reads only the current _pending batch, never the promoted corpus, so a new draft colliding with a landed one is invisible.

Live case: cht-core issue 8074 is claimed by forms-and-reports/8759-feat8074-* on PR #122 and by contacts/8074-* on main. An id-uniqueness check against the corpus would catch it.

[alexosugo]

Agreed this is a real gap — filed as #153 rather than growing this PR further: build the id set from the promoted corpus (agent-memory/domains/**) before promotion and flag colliding pending drafts. The 8074 double-claim is recorded there as the live case.

[sugat009]

issue (non-blocking): backport cherry-picks and multi-PR epics collapse on the same key, but deleting a copy is safe while deleting one part of an epic loses content, and the survivor still advertises source_prs as if it covered all of them. Distinguish them, or refuse to auto-delete when a group's PRs have different diffs.

[alexosugo]

Agreed — the discriminator between a safe-to-delete backport and a content-bearing epic member needs a design decision, so filed as #154: auto-collapse only groups that look like backports (titles matching modulo the trailing (#NNNN) suffix / cherry-pick marker), flag mixed groups for a human. Kept out of this PR to avoid changing dedup semantics under review.

[sugat009]

issue (non-blocking): a dedup group whose members all lack source_pr writes a valueless source_prs: key: schema-invalid YAML that reddens CI on the promote PR it just created.

[alexosugo]

Fixed in b8dddfd. source_prs is only set when the list is non-empty, so an all-source_pr-less group no longer writes a valueless key; the drop-log reason also omits the empty provenance suffix. Regression test added.

[sugat009]

nitpick: stray // ponytail: marker, still present since Hareet flagged it in July. Also a doubled /** opener at open-review-pr.ts:351-352, and dedup.ts:10-14 docstrings still describe the pre-review canonical-selection rule.

suggestion (non-blocking): this PR and #127 both restructure processSinglePR and runPipeline, with five conflicting hunks, and the mechanical resolution drops a feature. Merge this first and rebase #127 on top. Happy to re-review the rebase.

[alexosugo]

All three cleaned up in b8dddfd: the // ponytail: marker is gone, the doubled /** opener is fixed, and the dedup.ts docstrings now describe the domainFit-first canonical rule and the slug check's stale-file role. On merge order: agreed — this merges first and #127 rebases on top; thanks for offering to re-review the rebase.

[alexosugo]

@sugat009 thanks for the thorough review — every item is addressed and validated, and all CI checks (including the SonarCloud gate) are green on e2fd52f.

Blocking:

• Filename vs guard: draft filenames now derive from the resolved issueNumber; a title-versus-resolution mismatch is a warn audit entry, not a rejection. Validated against your simulation table and live cht-core PR 9039 — details on the thread.
• CI wiring: npm run check-pending now runs ciGuardReason + schema validation over _pending in unit_tests.yml on every PR.

Non-blocking:

• Base-branch gate added to the scraper; live-validated against 10767 and 10432 (both now rejected). Corpus cleanup for already-promoted drafts: Clean up promoted drafts distilled from PRs merged into abandoned feature branches #155.
• Corpus-wide id dedup: Dedup pending drafts against the promoted corpus, not just the current batch #153. Epic-vs-backport discrimination: dedupeByIssueId: distinguish multi-PR epics from backport cherry-picks #154. Both kept out of this PR to avoid changing dedup semantics under review.
• Empty source_prs no longer written; nitpicks (marker, doubled /**, stale docstrings) cleaned.

Merge order as you suggested: this PR first, #127 rebases on top. Ready for another look when you have time.

[sugat009]

Re-review at e2fd52f, full pass over all 23 changed files. This time with the pipeline model corrected: _pending/ is the staging area, and open-review-pr promotes drafts into domains/<domain>/issues/.

Retractions first. My earlier review was built against 0e5cd96 and I posted it hours after you had already pushed fixes. Four of those seven findings were fixed before I posted:

• filename now derives from the resolved issue number, not the title token (235ca7d)
• pending-draft guard wired into CI (6a7307c)
• PRs merged into non-default branches are excluded (a8deae2)
• source_prs omitted when empty (b8dddfd)

On the first of those: you shipped the warn-don't-reject shape instead of rejecting a title/issue mismatch. That is the better design and it is what we wanted. Sorry for the wasted round.

What is left. Two blockers in this PR's own code, both inline. Separately, three blockers already on main that this PR does not cause, but that stop its new CI guard from ever receiving input. I list those here because they change how you should read the guard, not because they are yours to fix in this PR.

Blockers already on main (not caused by this PR)

None of the three lives in a file this PR touches, so they are here rather than inline.

1. .husky/pre-commit fails the nightly commit step, and the hook does run in CI. package.json:14 is "postinstall": "husky install", and npm ci runs postinstall. husky 8.0.3's install() skips only when HUSKY=0, or when git or .git is unavailable, neither of which applies on a runner. .husky/pre-commit:4-9 then rejects any commit while the branch is master or main. The job log of run 31673806379 says, verbatim: > husky install / husky - Git hooks installed, then at step 7 You can't commit directly to master/main branch / husky - pre-commit hook exited with code 1 (error).

The clean way to state the scale: across the workflow's entire history, all 50 runs since it landed in c3b4bf6 on 2026-06-24, the scheduled pipeline has never once committed a draft. All 26 failed runs died at "Commit knowledge drafts" on this hook. All 24 green runs found 0 PRs and staged nothing, so the commit was skipped and the push was a no-op.

Smallest fix: env: { HUSKY: '0' } on that step (run-pipeline.yml:57). Keep the hook, it is useful locally.

2. Disabling the hook only moves the failure to git push (run-pipeline.yml:63). Ruleset 15146469 ("Main Branch Rule") is active on ~DEFAULT_BRANCH, has an empty bypass_actors, and carries a pull_request rule. Classic protection is off (branches/main/protection returns 404), which is why main looks unprotected. The other ruleset you will see in the list, 10342826 ("Main"), is disabled and not relevant.

This is not just configured, it is demonstrably evaluated on ref updates to refs/heads/main: the rule suite for the last such update (3642933719, b2571b9 to 041f132) evaluated pull_request, required_status_checks, non_fast_forward and deletion. It passed, because that update arrived through a merged PR. A direct push would not satisfy the pull_request rule. The same ruleset also requires the "Build, lint, and test" and "Lint PR title" checks, and the pipeline commits with [skip ci], so those two contexts would never report on a directly pushed commit either. I should be straight that no run has ever reached git push with a real commit, so this is the expected outcome rather than an observed one.

3. The nightly has no LLM key, so it cannot produce a draft. In run 31673806379, two of the three PRs died at the filter stage with filter: flag-for-human - LLM triage unavailable: no API key set (OPENROUTER_API_KEY or ANTHROPIC_API_KEY), and the third (#11319) passed the deterministic filter (filter: distill - Shared library change affecting multiple consumers), reached distilling..., and died there with distill: flag-for-human - Distill LLM unavailable: no API key set. repos/medic/cht-agent/actions/secrets returns total_count: 0, and the 3 org secrets visible to this repo are unrelated (BrowserStack, build history, Docker Hub), so secrets.OPENROUTER_API_KEY resolves to empty.

Across all 50 runs I checked every job log (none expired): 8 PRs reached the distill stage, all 8 flagged for human on the missing key, and 0 drafts were ever written. Note distiller.ts supports ANTHROPIC_API_KEY and the error message advertises it, but the workflow wires only OPENROUTER_API_KEY, so an Anthropic-only secret would still leave CI keyless.

Those three together mean agent-memory/_pending/ has never held a committed .md on main (only 8 .gitkeep files). So check-pending validates an empty directory today. The guard itself is correct. It just has no input yet, and it cannot reach the three mislink defects already sitting in domains/. That was the agreed scope, so this is fine.

Open questions

I am not confident enough to assert these, so I am asking rather than filing them.

1. Release branches. The gate treats any non-default base as unshipped. cht-core PR 11137 (fix(#11128): adds limit to Nouveau indexed strings) merged into 5.1.x on 2026-06-04; its merge commit f8f58ce is contained in tags 5.1.3, 5.1.3-beta.1 and 5.1.3-beta.2 and never reached master. That is real released work the gate excludes. Intended, or should 5.1.x-style release branches be allowed?
2. Long-lived feature branches. 10 of the newest 100 merged cht-core PRs went into 10707-dmp-2026-enhance-bikram-sambat-support-in-cht, whose umbrella PR 11211 is still open. The sharper case is one that already landed: the 10224-ui-extensions umbrella merged to master via PR 11050 on 2026-06-23 and its branch is deleted, yet PRs 11105 and 10909 still report baseRefName: 10224-ui-extensions, so the gate rejects them permanently even though their work is on master. Acceptable, or should the corpus pick such PRs up once the umbrella lands?
3. Epic semantics. Is dropping non-canonical epic members intended, or should a genuine multi-PR epic keep one entry per PR? See the inline comment on dedup.ts:129.
4. _skipped.ndjson truncation. This PR empties the file (7 lines to 0 bytes). All 7 removed rows are prNumber: 1 with reason "LLM triage skipped", so I read it as deliberate cleanup. Confirming, because #127 made the same truncation in ba6ef06 and then reverted it in 766f008.
5. --repo and the schema. run-pipeline accepts any --repo owner/repo and buildFrontmatter derives id/issueUrl from it, while schema.json permits only medic/cht-core and medic/cht-interoperability. Should the CLI reject other repos up front?
6. Merge order with #127. This is not a risk, it is already a fact: git merge-tree on the two heads conflicts today, with src/scripts/run-pipeline.ts the only conflicted file and 5 conflict regions, one of them exactly the auditOffset/BatchState initialization in runPipeline. #127 changes that file by +58/-18. Whoever merges second must re-verify that the run-scoping of _skipped.ndjson survives the resolution.
7. Is a direct push to main the intended design at all? Ruleset 15146469 requires 1 approving review, and this repo already has open-review-pr.ts. Routing the nightly through a PR would make both the hook and the ruleset non-issues.

[sugat009]

issue (blocking): The exclusion is signalled as an error, so one correctly excluded PR fails the whole nightly and discards the drafts the same run produced.

assertMergedIntoDefaultBranch throws ScraperError, the same channel as a gh failure or a corrupt JSON parse. The message matches neither isRateLimitError nor isAuthError, so recordWorkerError does state.failures++ (run-pipeline.ts:403) and reportOutcome does process.exit(1) (:488). The "Run pipeline" step has no continue-on-error, and neither "Validate distilled drafts" nor "Commit knowledge drafts" carries if: always(), so both are skipped. Every draft written for the other PRs that night dies with the runner. Nothing reaches _skipped.ndjson on this path, so the exclusion never lands in the audit log it is meant for.

This is not hypothetical. 13 of the newest 100 merged cht-core PRs have a non-master base, and two fall inside recent nightly windows: #11237 (merged 2026-08-10T13:36:29Z) and #11320 (2026-08-11T06:01:08Z). getRecentlyMergedPRs passes no --base, so they do enter the batch.

A second cost, on the manual path rather than CI: because the throw writes neither a draft nor an audit row, getProcessedPRs (run-pipeline.ts:266-271, the union of _pending drafts and audit-log entries) never records the PR as handled, so --resume re-attempts every excluded PR. Indefinitely with --last N, and for the whole lookback window with --since. The scheduled workflow never passes --resume, so this one is about manual and container runs.

The commit message says "skip PRs merged into non-default branches" and skip is the right behaviour. Two ways to get it, your call:

• In the filter (my preference). Add baseRefName?: string to ScrapedPR, carry it through scrapePR, drop the call at :305, and add a rule to checkSkipRules. filterPR then writes the audit row via writeSkipLog (filter.ts:272-278) and returns {decision: 'skip', reason} (:279), so state.failures is untouched, the exit code stays 0, --resume becomes idempotent for free, and reconcile counts it under otherFlags. (checkSkipRules itself, filter.ts:90-101, only returns a reason string; the write is filterPR's.)
• Keep the early exit. Add PrExcludedError extends ScraperError, throw that here, and handle it in processBatchItem before recordWorkerError, writing the skip row with a scraper: prefix (mirroring open-review-pr.ts:162). This avoids the wasted pr diff / reviews / issue-hydration calls for an excluded PR.

Either way scraper.spec.ts:166-169 needs retargeting, and a run-pipeline.spec.ts case asserting failures === 0 plus one appended skip row would lock it in. While you are here: PR #N is not merged (:303) is the same category of legitimate exclusion.

[sugat009]

issue (blocking): A dropped duplicate's distilled content is deleted with no copy kept anywhere, so a genuine multi-PR epic loses the dropped PRs' work permanently.

To be precise about what does survive: :361 writes an audit row naming the dropped file and its canonical, and dedup.ts:138 merges the dropped members' source_pr refs into the canonical's source_prs. So provenance survives. What is lost is the dropped draft's body and the rest of its frontmatter: summary, tags, services, techStack, related workflows, confidence. Nothing merges those, and stageDrafts copies only the kept entries.

Ground truth that this is a real shape, not a theoretical one: cht-core issue 6543 is a 4-PR epic on master, all four titled feat(#6543), so all four resolve to cht-core-6543:

PR	commit	files	area
9093	1e8264a1d	19	webapp breadcrumbs / extract-lineage
9094	4fdcb59b5	7	webapp contacts
9099	ce76404b0	7	webapp target-aggregates
9126	2fdddd071	28	api controllers + routing + authorization, shared-libs/contacts, shared-libs/user-management, ddocs

I computed the file sets: they are effectively disjoint, overlapping on exactly one file, tests/e2e/default/targets/target-aggregates.wdio-spec.js, shared by 9099 and 9126.

Two honest qualifications. Only #9126 distils deterministically, via isSharedLibsWithMultiService, because it touches both shared-libs/ and api/. None of the four carries a label, so #9093, #9094 and #9099 fall through to Stage 3 LLM triage and I cannot derive their outcome from primary sources. And the canonical is chosen on domainFit before lowest source PR, so #9093 wins only if the surviving members tie on domainFit. If triage rated #9126 strong and #9093 weak, #9126 survives instead.

The mechanism holds whichever way triage lands: whenever two or more members of one epic distil, all but one are deleted body-and-all. Worse today, stage 1 has never committed a _pending draft, so these are untracked files on disk and unlinkSync is unrecoverable.

Suggested: move dropped drafts to agent-memory/_pending/_collapsed/<domain>/, or copy them onto the promote branch so a reviewer can merge content by hand. At minimum gate the delete on the file being tracked in git, and record the dropped draft's title in the audit reason.

[sugat009]

question (blocking on the answer, not the code): Epics and backports are keyed identically, and members are dropped rather than merged.

The module docstring (:11-13) names backport cherry-picks, multi-PR epics and independent domain promotions together. For a backport cluster, collapsing is exactly right. On the 10792 cluster in the corpus today (three drafts, all in data-sync), this code keeps 10793, and cht-core confirms that is the master fix (97e3d45, an ancestor of origin/master), while 10798 (5.0.x) and 10799 (5.1.x) are release-branch backports outside master's ancestry. Correct member, no argument.

For an epic it is the opposite. The comparator reads only the integer from source_pr (:131-133), so the tiebreak is lowest-numbered, i.e. earliest-opened, not earliest-merged and not mainline-vs-backport. That usually selects the least representative fragment of an epic. And the survivor's source_prs then asserts provenance over content that was deleted rather than merged.

Two things follow, both yours to decide:

• Is dropping epic members intended, or should an epic keep one entry per PR (or be flagged for a human to merge)? A cheap discriminator is whether members' relatedFiles overlap: a backport cluster's do, an epic's do not.
• If the tiebreak's intent is "prefer the mainline fix over backports", PR number cannot express that: a backport opened before the master PR would win. Should it prefer the draft whose source PR merged into the default branch instead?

Related: dedup.spec.ts:161-171 is titled "collapses a multi-PR epic" but uses 10792/10793/10798/10799, which cht-core shows is a backport cluster (one Type: Bug issue, one master fix, two release-branch backports). The fixture also places them in tasks-and-targets, while the three real drafts are in data-sync. Was an epic fixture intended?

[sugat009]

issue (non-blocking): The promote commit never records the _pending deletions, so promotion is not idempotent.

git add (:459) receives only the domains/ copies from stageDrafts, and git commit (:460-461) has no -a. The _pending originals are removed by fs.unlinkSync at :473 with no subsequent git add. The only git add agent-memory/_pending/ anywhere in the tree is run-pipeline.yml:61, which belongs to the pipeline workflow and runs in a different job.

promoteDomain's add/commit/push/unlink shape is pre-existing on main, unchanged here apart from the frontmatterByPath threading. I am flagging it because this PR adds a second deleter, finalizeDedupDrops (:352-366, unlink at :360), which is new, so the uncommitted-deletion surface doubles.

The consequence is conditional, so stating it carefully: it needs _pending drafts to actually be tracked on main, which the CI path at run-pipeline.yml:61 would do but which has never yet happened. Once it does, a merged promote PR leaves the draft in _pending on main, any clone reflecting origin/main rediscovers it, and re-promotion stages nothing if every draft in that domain is already byte-identical on main. Then git commit exits 1 and promoteDomainSafely records failed, so that domain never drains. If even one draft in the domain is new or differs, the commit succeeds and the stale one is simply re-proposed. Note the recorded error reads Command failed: git commit -m feat(memory): promote ...; git's own "nothing to commit" text goes to stdout and is not captured.

Two notes so this is not overstated. I checked whether this already caused the 10792 triple and it did not: all three files arrived in the single commit fdf4af2, so that is three PRs resolving to one issue in one run. Second, :473 sits inside the try whose catch (:476-478) does if (pushed) deleteRemoteBranch(...), so an unlinkSync failure after gh pr create would delete the branch of the PR just created. Also pre-existing, also worth a guard.

Cleanest fix: stage the _pending paths as deletions on the promote branch so one PR both adds domains/ and drains _pending, and drop :473.

[sugat009]

issue (non-blocking): The buckets match free text, and one of them counts successes as failures.

bucket() keys on reason.includes('CI guard:') (:30) and reason.includes('duplicate of') (:31). Two consequences:

• formatReconciliation labels the total "N flagged/skipped". A decision: 'warn' row is written for a draft that was successfully created (distiller.ts:437), so a good run reports created drafts inside that flagged total. Suggest excluding warn, giving it its own bucket, or renaming the label.
• The LLM's own triage reason reaches the same log verbatim, when the triage decision is not distill (filter.ts:302), so a model that writes the words "duplicate of" in prose would be counted under dedupCollapses. Keying on decision plus a structured field would be sturdier than substring matching.

Also worth knowing: at run-pipeline's call site neither the CI guard nor dedup runs, so those two buckets have no producer there and read as zero except via exactly that free-text collision. The line is only meaningful from open-review-pr.

[sugat009]

issue (non-blocking): The title-vs-resolution warning never reaches a human-facing surface.

The mechanism is tested, so this is not about correctness: distiller.spec.ts asserts a warn entry exists and that its reason includes both #9024 and #9023, with a negative test for the agreeing case. The gap is downstream. The warning goes only to _skipped.ndjson, and nothing surfaces it to the person reviewing the draft: not the draft frontmatter, not buildPRBody, not the run summary except as an anonymous otherFlags count.

Suggest carrying it into the draft frontmatter (e.g. linkage_warning:) so buildPRBody can list it on the promote PR. That is where a human can actually act on it.

One aside on that test's fixture, since it names the 9039 shape in a comment: it stubs linkedIssues: [{ number: 9023 }]. The real cht-core PR 9039 never reaches that state. It has no closing refs, its title token #9024 is itself a PR whose own closing refs are empty, and the body's bare #9023 has no fixes/closes keyword, so linkedIssues comes back empty and distillPR returns flag-for-human at :502-503. The fixture is a fine unit test of the warning, but 9039 is not a live example of it, which is worth knowing if that shape is ever used to argue coverage.

[sugat009]

issue (non-blocking): A transient gh failure is cached as a permanent null, silently disabling the gate for the rest of the process.

catch { name = null } then defaultBranchCache.set(repo, name). Failing open on one lookup is the right call and the docstring says so. Caching that failure is not: one rate-limited gh repo view turns the base-branch gate off for every remaining PR in the batch, with no log line, and nothing resets the cache between PRs.

Suggest caching only successful lookups, plus a console.warn on failure.

[sugat009]

nitpick (non-blocking): DRAFT_SHAPE was not tightened along with the inline block.

:227 says relatedFiles and entities must come only from the Files changed list, and the inline spec carries that for both fields (:214 and :221). But DRAFT_SHAPE (:88-109) still describes entities as "<file or module path>" at :99 and relatedFiles loosely at :106, and createStructuredCliChain(draftSchema, DRAFT_SHAPE) (:141) sends it on the claude-cli path. structured-cli.ts:52-55 appends the shape after the whole buildPrompt output, so the loose wording is the last field-level spec the model sees, with only the provider's own JSON-only trailer (claude-cli.ts:368) behind it.

Scoping this so it does not over-reach: the broader wording in schema.json:175 and TEMPLATE.md:98 looks deliberate, not stale, because reconcile.ts:12-13 and :52-54 (added in this PR) explicitly allow entities to name a module or concept rather than a literal path, and hallucinationRate is reporting rather than a gate. DRAFT_SHAPE is the one clear divergence.

[sugat009]

note (non-blocking): Right guard, right place. One correction to how I described its reach, and one real gap.

I want to be accurate about the reach, because it is better than I first thought: check-pending scans the checked-out working tree (check-pending.ts:18, :63-67, relative to REPO_ROOT), not the PR diff. actions/checkout@v6 on a pull_request event checks out the merge ref, so any draft sitting in _pending/ on main is present in every PR's tree and is inspected. Promote PRs are covered too, since promoteDomain only unlinks the originals locally after the PR exists.

The real gap is timing: this workflow has no push trigger, and the pipeline's commit carries [skip ci] (run-pipeline.yml:62). So the guard never runs on the commit that adds a draft. A bad draft would fail the CI of the next unrelated PR instead of failing at the point of landing. Note a push trigger alone would not close that, because [skip ci] would still suppress it. Was [skip ci] deliberate there?

Also, check-pending currently scans a directory holding 8 .gitkeep files and zero drafts, so the step passes trivially today. Neither point is a defect in this PR.

[sugat009]

note (non-blocking): Consolidated smaller items, anchored here because the first few concern this function. Not worth their own threads.

• scraper.spec.ts:178 (stub :179-181, sole assertion :182) cannot fail. Its guard throw is swallowed by defaultBranch's own catch (scraper.ts:268-270) and the fail-open at :285. I checked by mutation: deleting scraper.ts:283 (if (!base) return;) leaves the suite at 1081 passing, unchanged from baseline, so the test does not detect the regression it names.
• stripBoilerplate: both production call sites are untested. Neutering either scraper.ts:90 (issue body) or :357 (PR body) leaves the suite at 1081 passing. Only the function itself is covered (scraper.spec.ts:1023-1040). It also removes comments with no separator, so adjacent words glue together, and scraper.spec.ts:1039 locks that in by asserting '<!-- a -->Keep this<!-- b -->and this<!-- c -->' becomes 'Keep thisand this'.
• The ~246-char figure in this rationale (:29) does not match the issue it cites. The leading PHI comment in cht-core #10912 measures 343 chars, and the bug-report template has been 343 at every revision that carries it. The rationale otherwise stands: against ISSUE_BODY_LIMIT = 500 a 343-char boilerplate comment really does push the root-cause sentence out of budget. Where does 246 come from?
• Only review bodies currently reach the distiller prompt un-stripped (distiller.ts:166-170, interpolated at :200), so that is the live prompt-contamination path and worth stripping. Issue comments reach no prompt at all: LinkedIssue.comments is written at scraper.ts:89-90 and read nowhere in src/, since buildPrompt uses only i.body. Is that field meant to be dead? If it is meant to feed the prompt, it needs both wiring and stripping.
• pipeline.ts:38: was repo?: string left optional deliberately to avoid touching ScrapedPR fixtures? scrapePR always sets it and distiller.ts:316 falls back with ??.
• scraper.ts:265 carries the NOSONAR S4036 marker, and it is one of six call sites in this file that shell out to gh: five direct execFileSync (:76, :159, :198, :236, :265) and one through the ghExec wrapper (:87). Fine if the Sonar gate is new-code-only, otherwise the siblings will surface eventually.
• uniqueBranchName (open-review-pr.ts:207) probes only local refs, and executeApply fetches only origin main (:531), so remote-tracking refs are stale too. A promote from a fresh clone, or one whose local branch was pruned, can collide with a branch that already exists on origin. No workflow runs open-review-pr today, so this is a local hazard for now.
• rewriteFrontmatterOnDisk can silently no-op, because hasFrontmatter (schema-utils.ts:50-53) strips a leading BOM while the rewrite regex at :295 is ^-anchored and does not, so a BOM-prefixed draft is admitted and rewritten to itself. And on a draft that already carries source_prs it injects a duplicate key, which is not cosmetic: I verified against the installed js-yaml 4.1.1 that it throws duplicated mapping key, and gray-matter 4.0.3 propagates it as a YAMLException, so the promoted file becomes unparseable. Nothing in the pipeline writes source_prs into _pending today, so this needs a hand-authored draft, and stageDrafts triggers on source_prs !== undefined alone (:423), with no dedup collapse required. Can a draft ever arrive there already carrying the key?
• A promotion that fails at stageDrafts/git add/git commit (open-review-pr.ts:458-461) leaves copies under agent-memory/domains/ in the operator's tree, staged rather than untracked if git add succeeded. If push or gh pr create fails instead, the copies are already committed and the residue is an abandoned local branch. Nothing cleans up either: the catch (:476-478) and the finally (:537-540) touch neither domains/ nor the local branch.
• source_prs behaves correctly (every non-null ref in the group is kept), but there is a coverage gap and a provenance oddity: when a strong-fit draft with no source_pr of its own wins the sort, it is stamped with a source_prs list that does not include itself. dedup.spec.ts:193-201 is the only mixed-group test and it asserts nothing about source_prs; its canonical does carry a source_pr, so the canonical-without-source_pr case is untested anywhere in the file.
• dedup.ts:19 imports its filename grammar from relink-issues.ts, so the CI guard at unit_tests.yml:34-35 now depends on a repair script. relink-issues.ts has an npm script (package.json:25) but appears in no workflow. Consider moving FILENAME_TOKEN_RE to a shared module.
• The new <pr>-issue-<n>-<slug>.md convention is a third grammar. On main the 110 drafts split 52 <number>-<slug>.md and 58 <pr>-<type><issue>-<slug>.md, with 0 in the new shape, while TEMPLATE.md:7 still documents <issue-number>-<short-slug>.md.
• docs/memory-seeding-runbook.md is stale on four points, three of them new here: :151-152 (force overwrite / same output path), :160 (_skipped.ndjson described as skip/flag-for-human only, now also warn), and :7-9 (merged PRs, now default-branch-only). The pre-existing one is :96 (a "2g memory limit" vs the compose default of 6g). :161 omitting npm run check-pending is an omission rather than a contradiction.
• #146 is half addressed. Done: the polluting call site (filter.spec.ts:229/:237) and the purge of all 7 fake prNumber: 1 rows. Not done: logPath on the four option-less filterPR calls (:190, :203, :216, the three DISTILL: cases, plus :245), and the NODE_ENV=test guard. Also the PR references #146 nowhere, so it will not auto-close. Do you want the remainder here or left on the issue?
• The exclusion message interpolates a branch name into a string that recordWorkerError later keyword-matches for rate limits. Currently harmless: 0 of cht-core's 916 remote branch names collide, and only 5 of the 14 keywords are reachable from a refname at all, since refnames cannot contain spaces. The realistic path is a stacked PR whose branch is slugged from an issue like #10705 or #11090, where ...-rate-limiter-... matches /rate[\s_-]?limit/. Brittle by construction rather than broken.
• Scope: 30 commits, 23 changed files, and 11 commits with no ticket reference. The PR body declares two issues, #135 and #136. Was the #135 relink-issues work meant to ship here, or split?