Skip to content

feat(lifecycle): link authoritative PR merges and expose claim-to-merge timelines (#1203) - #1296

Merged
joryirving merged 10 commits into
mainfrom
courier/misospace/dispatch/issue-1203
Oct 20, 2026
Merged

joryirving merged 10 commits into
mainfrom
courier/misospace/dispatch/issue-1203

Conversation

@itsmiso-ai

@itsmiso-ai itsmiso-ai commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Builds on the durable IssueLifecycleEvent stream from #1202 and delivers the source-neutral lifecycle read contract for #1183/#1184: authoritative PR-merge linkage (multi-PR, explicit provenance), a typed timeline export, and a bounded operator-only backfill. Closes #1203.

  • One merge-write path. recordPrMergedEvent + canonical prMergedDedupeKey/prMergeStamp in src/lib/issue-lifecycle.ts. Reconcile records one pr_merged per linked merged PR with GitHub's mergedAt, mergeCommitSha/headSha, and a linkSource (body-keyword, branch-name).
  • No title inference. PR titles are not consulted for close or link evidence (feat(lifecycle): link authoritative PR merges and expose claim-to-merge timelines #1203, review-required): a merged PR whose only correlation to an issue is its title neither closes it nor records a merge for it. Only body keywords and branch-name patterns qualify; when a caller supplies no evidence list at all, the close-driving PR is still recorded with linkSource: "reconcile-close-trigger" (the observed action, never a text guess).
  • Merge evidence decoupled from the action loop. recordLinkedMergeEvidence runs a bounded pass inside the same reconcile run over every linked number in the fetched merged-PR window (no extra GitHub calls): a linked merge records whether the issue is open or closed and whether the close action executed. Rows the close path wrote converge on the same canonical dedupe key (upsert is a no-op on conflict) — repeated reconciliation yields exactly one event per PR. Merges outside the window remain the operator-only backfill's territory.
  • Link evidence independent of the close decision. Branch-name evidence is collected for every merged PR, not only for one whose own number happens to be free in the close-decision map: GitHub shares the issue/PR number space, so a PR whose number collides with an issue number already in that map still records its branch link, and the merge-evidence pass no longer misses that issue's merge. The close decision itself stays first-PR-wins per issue number.
  • Read contract dispatch.issue-lifecycle-timeline/v1: pure projection src/lib/issue-lifecycle-read.ts + GET /api/issues/[issueId]/lifecycle — first/subsequent claims, done/close/reopen milestones, all linked merged PRs, claimToDoneMs/claimToMergeMs in milliseconds with per-timestamp provenance (observed / backfilled / unknown). Missing first claim is { at: null, provenance: "unknown" } — never zero, never issue-creation time. A close without an observed merge is closed-unmerged, never "shipped". delivery follows the latest close/reopen ordering — the latest close is the later of the last observed close and the issue cache's closedAt (the cache counts only for a closed issue) — so a close after a reopen reports closed-unmerged again, while closedAt keeps reporting the first close milestone. Timeline is invariant to event input order; duplicate api:*-vs-webhook observations collapse with a note. Read is bounded (500-event window, truncated flag).
  • Bounded backfill POST /api/issues/[issueId]/lifecycle (operator auth only; agent bearer 403): derives pr_merged solely from verifiable records — an AgentRun attached to the issue naming a PR URL, correlated with that PR's persisted GitHub merge time (GithubPullRequest.mergedAt). Handles the html-URL vs API-URL shape split. Never fabricates claim evidence, skips PRs without an authoritative merge time, idempotent across replays.
    • The backfill's own read is bounded: it loads only the PR rows for the numbers the issue's runs touch, and takes the repo's merged-PR total from an aggregate count() instead of a row scan — no unbounded row set is materialized. recorded / skippedUnmerged / skippedNoLink keep their documented meanings; skippedNoLink is the repo's merged-PR total minus the merges this issue recorded.
    • The precondition the backfill cannot bypass: a merge time must already be persisted on the PR row. The automation sync currently upserts open PRs only, and an open PR never carries a merge, so a row it wrote holds no mergedAt and a PR that later merges is not updated either — the backfill recovers a merge only where the merge time is already in the table. fix(automation-sync): persist merged pull requests so the lifecycle backfill has an authoritative merge time #1313 tracks closing that sync gap; the reconcile path is unaffected because it reads merged_at straight from GitHub.
  • Docs: docs/issue-lifecycle-events.md gains the read-contract and merge-linkage sections, the reconciler's bounded merged-PR window, and the operator recovery path for merges outside it.

Invariants this change keeps (and the other paths enforcing the same rule)

Invariant Other code paths for the same state How each is kept
pr_merged only ever from an authoritative merge observation — never a worker report, never a title inference tasks/report / report-close.ts (writes completed/attempt_failed only); sync/webhook (recordSyncedIssueEvents writes no pr_merged) Untouched writers record no merge events; both new writers require a persisted/observed GitHub mergedAt. Title-derived matches produce no close and no link evidence.
One physical merge = one row, across observers and restarts pre-#1203 reconcile rows keyed reconciler|<repo>|<n>|pr_merged|<pr>|<raw merged_at>|"unknown" prMergeStamp canonicalizes .000Z → Z so a Date and GitHub's second-precision string of one instant produce byte-identical pre-#1203 keys; reconcile (close path and decoupled pass) and backfill share recordPrMergedEvent. Tests pin the exact legacy key string and Date↔string convergence.
Status transitions flow through the single #1202 contract move, status, groom, claim/unclaim, groomer, webhook Reconcile keeps recordStatusTransition and the unchanged issue_closed/status_changed dedupe keys; the read path, backfill, and merge-evidence pass write no status events.
Reconcile's close decision tracks explicit links only reconcileIssue fed by mergedFixingIssues mergedFixingIssues is body-keyword/branch-name only; title-only matches cannot close. The branch claim still feeds the close decision under the same first-PR-wins-per-issue-number rule, now that the collision no longer suppresses it.
Unknown ≠ zero ≠ estimated claim evidence in claim/unclaim routes Projection emits nulls with provenance; backfill writes only pr_merged with backfilled: true and never reads startedAt/createdAt; tests assert zero claim rows from backfill.
Operator-only mutation; bounded reads admission-override (same rejectAgentCaller gate); lane route bearer auth POST uses rejectAgentCaller + rate limit (tested 403); GET uses authorizeRequest and take-bounded findMany; the backfill's PR read is scoped to the touched PR numbers.

Local validation (at head d789ae1)

  • npx vitest run src/lib/issue-lifecycle-read.test.ts src/app/api/issues/reconcile/route.test.ts src/lib/issue-lifecycle.test.ts src/lib/lifecycle-backfill.test.ts "src/app/api/issues/[issueId]/lifecycle/route.test.ts" — 5 files, 106 passed.
  • npm test — 194 files passed / 4 skipped; 4791 tests passed / 24 skipped / 1 todo (vitest). The 4 skipped files are the Postgres-backed integration suites, which need a live database and run in CI.
  • npm run typecheck — exit 0.
  • npm run lint — 0 errors (1 pre-existing warning in src/app/login/page.tsx, untouched).

CI was green on the previous head e1dd146 (all 13 checks). CI on this head (d789ae1) is pending — not claimed green.

Two findings from @joryirving's review of e1dd146 — the delivery derivation for close → reopen → close, and the branch-evidence collision with the close-decision map — are implemented at b82754e, with the follow-ups at d789ae1 (the issue cache's closedAt counts toward the latest close only for a closed issue, the delivery ladder is state-based when no close evidence exists, the close-after-reopen note stays silent for a merged delivery, and the reconciler's widened close reach in the collision case is stated and pinned by a test). An independent same-model review of that delta returned GO-with-minors; all five minors it raised are fixed. Same-model review limitation noted — no model-family diversity available.

Known limits (deliberate, documented)

  • Merge recording is bounded by the fetched merged-PR window: reconcile fetches each repo's open PRs and its most recently updated closed PRs (100 per page — up to 200 open, 100 closed), and merged PRs come from the closed side, since an open PR never carries a merge. A merge outside that window is never seen by reconcile and is therefore never recorded by reconcile.
  • To recover merges for older issues, run POST /api/issues/{issueId}/lifecycle for that issue. Both preconditions must hold: an AgentRun attached to the issue whose touchedIssueUrls name the PR, and that PR's persisted GithubPullRequest.mergedAt — which is only there if it was persisted before the PR dropped out of the sync's open-PR listing (fix(automation-sync): persist merged pull requests so the lifecycle backfill has an authoritative merge time #1313).
  • The 500-event window: beyond it the response carries truncated: true; full export is follow-up work.
  • mergedAtRaw preserves present-but-unparseable source stamps instead of dropping provenance.

Handoff

Settled decisions: shared reconciler\|… merge-key namespace (historical, deliberate); title-derived matches removed from the close decision per review (a deliberate #1202-behavior change, demanded and now implemented); merge evidence decoupled from close execution and open-state gating per review; issue-cache fallback reads keep provenance: "observed"; the backfill's PR read is scoped to the touched PR numbers with the repo merged total taken as an aggregate count, preserving the response contract; delivery follows the latest close/reopen ordering while closedAt keeps the first close milestone; branch-name link evidence is collected independently of the close-decision map, with the close decision unchanged in kind (first-PR-wins per issue number).

Remaining external verification: CI on this head (d789ae1) — green on e1dd146, pending here and not claimed green — and @joryirving's re-review of the two findings addressed at b82754e/d789ae1.

Courier added 2 commits October 18, 2026 10:13
…ge timelines (#1203)

Record pr_merged through one shared write path (recordPrMergedEvent) with a
canonical dedupe identity shared by every merge observer: reconcile now
records one merge event per linked merged PR with authoritative mergedAt,
head/merge-commit shas, and linkSource provenance instead of first-PR-wins
with number-only metadata.

Add the source-neutral read contract
(dispatch.issue-lifecycle-timeline/v1): a bounded projection and
GET /api/issues/[issueId]/lifecycle exposing first/subsequent claims,
done/close/reopen milestones, all linked merged PRs, and claim-to-done /
claim-to-merge intervals in milliseconds with explicit provenance. A missing
first claim is unknown, never zero; a close without an observed merge is
closed-unmerged, never shipped.

Add a bounded, idempotent, operator-only merge backfill
(POST /api/issues/[issueId]/lifecycle) deriving pr_merged solely from a run
record naming the PR plus that PR's persisted GitHub merge time; it never
fabricates claim evidence.
…R evidence, honest link provenance (#1203)

- prMergeStamp canonicalizes the identity so a Date and GitHub's
  second-precision string form of one merge produce the same dedupe key;
  reconcile and the backfill can no longer split one physical merge onto
  two pr_merged rows.
- Wire mergedPrLinkEvidence into recordReconciledAction: production
  reconcile now records one merge event per explicitly linked PR.
- Drop title-derived link evidence entirely; the close-driver PR records
  with reconcile-close-trigger provenance instead.
- Present-but-unparseable mergedAt stays visible as metadata.mergedAtRaw.

@its-saffron its-saffron Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI Automated Review

Full PR review.

Analysis engine: MiniMax-M3.1-Flash-Preview (anthropic) — primary route · pr-reviewer-action v3.5.0

Findings (2 major)

Severity Location Finding
⚠️ Major src/app/api/issues/reconcile/route.ts:271 Title-only matches still enter the close decision and are subsequently persisted as authoritative issue-linked merges, violating PR 1203's prohibition on inferring links from PR titles.
⚠️ Major src/app/api/issues/reconcile/route.ts:396 Multi-PR evidence is only persisted while the issue remains open, so a linked PR merged after the first PR closes the issue is never recorded.

Request changes: the new lifecycle API and bounded backfill are internally consistent, but reconcile still treats a title-derived association as an authoritative issue link and cannot record additional PR merges after the first merge closes the issue. Both behaviors contradict PR 1203's explicit linkage and multi-PR acceptance requirements.

Decisive findings

  1. Title inference still creates authoritative merge linkage. extractFixingIssueNumbers(pr.body ?? pr.title ?? null) puts title-only matches into mergedFixingIssues, which drives reconcileIssue's close action. That action then feeds mergedPr into recordPrMergedEvent as reconcile-close-trigger. Relabeling a title-derived association as the observed close trigger does not prevent the title inference; it only hides its origin. The linked issue explicitly says “Do not infer links from PR titles.” Title-only PRs must not drive the close or be recorded as issue-linked merge evidence.

  2. A later linked PR is never recorded after the issue has closed. The route builds merge evidence for every currently fetched linked PR, but its reconciliation loop still skips every issue whose current GitHub state is not open. Thus, if PR PR 7 closes an issue and PR PR 9 is linked and merged later, subsequent runs never call recordReconciledAction for that issue and PR 9 never enters the lifecycle stream. This misses PR 1203's explicit multi-PR and late/out-of-order merge behavior. Recording multi-PR evidence only while several PRs happen to be visible during the same open-state run is insufficient.

Linked Issue Fit

  • Authoritative merge time: implemented by prMergeTimestamp, and the backfill correctly requires persisted GithubPullRequest.mergedAt.
  • Missing claim remains unknown: implemented and directly tested.
  • Closed-unmerged is not shipped: implemented and tested.
  • Reopen/reclaim preserve milestones: implemented and tested.
  • Typed, bounded read contract: implemented with a 500-event query window and truncated output.
  • Backfill uses verifiable records: implemented through issue-attached AgentRun URLs and persisted PR merge timestamps; it does not synthesize claims.
  • Do not infer links from PR titles: violated by the title-to-close-to-reconcile-close-trigger path.
  • Multiple linked PRs and late/out-of-order merges: violated after the issue reaches closed state.
  • Idempotency across observers/restarts: the canonical second-precision merge stamp and database unique dedupe key support replay when observers provide the same instant.

Claim Falsification

Violated: 2 of 2 PR-body claims. The pure projection, backfill, provenance, and same-instant replay tests largely hold, but each high-level claim is contradicted by a concrete production path: title-only linkage remains authoritative, and reconcile does not capture every linked merge over the issue lifecycle.

Standards Compliance

  • API responses are JSON with appropriate 401/403/404/500 handling.
  • authorizeRequest and rejectAgentCaller enforce authenticated reads and operator-only mutation.
  • The backfill does not read or persist secrets.
  • The change follows the existing Prisma and lifecycle-module organization.
  • Lint, Typecheck, Build, Tests, and the database checks are reported successful at the reviewed head.

Requirement Coverage

  • Lint/typecheck blocks CI; must pass: satisfied by the successful Lint and Typecheck results for 1c8b9a5, with no contrary implementation finding.
  • DISPATCH_AGENT_TOKEN and GITHUB_TOKEN must never be logged, echoed, or persisted: satisfied within the available diff and inspected lifecycle/backfill paths; no changed code reads, logs, or stores either value.

Tool Harness Findings

Native inspection confirmed that recordPrMergedEvent is the shared recorder, backfill writes use it, GithubPullRequest.mergedAt is the persisted authoritative merge source, operator mutation rejects bearer callers, and the lifecycle dedupe key has a database unique index. Inspection also confirmed the title fallback and the issue.state !== "open" skip that produce the two findings. The 24-call investigation budget was exhausted before all potentially related callers and test integrations could be reviewed.

Unknowns or Needs Verification

  • smoke is still pending in the supplied CI results; this review does not infer that it failed.
  • Linked example GitHub URLs could not be fetched, but they are fixture URLs rather than required upstream behavioral evidence.
  • Additional historical lifecycle writers beyond the searched call sites were not exhaustively reviewed before the tool budget was exhausted; the two reported defects are directly established from the production paths read above.

@joryirving joryirving left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

REQUEST CHANGES on exact head 1c8b9a5. Verified two correctness defects in the code, consistent with the automated review; all completed exact-head CI checks are green but smoke is queued. CI green does not address these behavioral gaps.

  1. Title-only association becomes an authoritative merge link (major). src/app/api/issues/reconcile/route.ts:267-276 uses extractFixingIssueNumbers(pr.body ?? pr.title ?? null) to fill mergedFixingIssues, while bodyKeywordNumbers intentionally excludes title matches from mergedPrLinkEvidence. However reconcileIssue(...mergedFixingIssues) still closes the issue for a title-only match, and recordReconciledAction:90-109 promotes the close driver into a pr_merged event with linkSource: "reconcile-close-trigger". This is still title inference, merely relabeled as an observed action, contrary to #1203's explicit no-title-link requirement. Remove title-only matching from the authoritative close/link path (or separate legacy close policy from trusted linkage without writing a pr_merged claim). Add a no-body, title-"Fixes #42" merged PR regression: no inferred authoritative pr_merged for #42 and no incorrect closure if not explicitly linked.

  2. Later linked PR merges are invisible once an issue is closed (major). src/app/api/issues/reconcile/route.ts:335-336 skips non-open issues. The only mergedPrLinkEvidence write is inside recordReconciledAction, and that function writes pr_merged only for a successful close_issue action. After PR #7 closes issue #42, a second explicitly linked PR #9 merged on a later run produces no event; even several linked merges in the same run are lost unless that run executes a successful close. The POST backfill is a manual, issue-specific path requiring an issue-attached AgentRun.touchedIssueUrls reference, not a complete substitute. Decouple read-only authoritative merge evidence recording from issue-close actions and open-state gating, retaining bounded scans, idempotency keys and no GitHub mutation of closed issues. Cover sequential linked PR merges (#7 closes first; #9 merges later), out-of-order observations, and repeated reconciles yielding exactly one event per PR.

Please push a new head with regression tests for both. Preserve the bounded timeline API, unknown provenance, and existing status-transition semantics.

@its-saffron

its-saffron Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

🔄 This PR fix item has been requeued and is active again — no human attention needed.

Note: requeue from queue page

Updated automatically by Dispatch on 2026-10-20T14:23:24.405Z

@its-saffron its-saffron Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI Automated Review

Full PR review.

Analysis engine: glm-5.3-flash (openai) — fallback (primary failed) · pr-reviewer-action v3.6.0

Findings (2 major, 1 minor)

Severity Location Finding
⚠️ Major src/app/api/issues/reconcile/route.ts:90 A title-only matched PR that drives a close is still recorded as an authoritative pr_merged event (reconcile-close-trigger), contrary to PR 1203's no-title-link requirement.
⚠️ Major src/app/api/issues/reconcile/route.ts:335 Merge evidence recording is coupled to close actions and open-state gating, so linked PRs merging after the issue closes produce no pr_merged event.
Minor src/app/api/issues/reconcile/route.test.ts:330 No regression test for a no-body, title-'Fixes PR 42' merged PR asserting no inferred pr_merged and no incorrect closure.

Request changes. The outstanding change request (review 5563081926) identifies two correctness defects that the current head still exhibits; the diff explicitly embraces the first rather than fixing it.

  1. Title-only match still yields an authoritative pr_merged (major). src/app/api/issues/reconcile/route.ts still fills mergedFixingIssues from extractFixingIssueNumbers(pr.body ?? pr.title ?? null), and recordReconciledAction now records the close-driver PR as pr_merged with linkSource: "reconcile-close-trigger" even when mergedPrs is null ("A close whose only link was a title match therefore still gets its (authoritative) merge recorded"). Issue PR 1203 says "Do not infer links from PR titles"; relabeling the title-driven close as an observed merge link does not satisfy that, and the new test only asserts the label is not title-keyword — it does not cover a no-body, title-"Fixes PR 42" PR as the reviewer requested.
  2. Later linked merges after close are still invisible (major). mergedPrLinkEvidence is only consumed via recordReconciledAction, which writes pr_merged only on a successful close, and reconcile still skips non-open issues; a second linked PR merging after the issue closes records nothing. The POST backfill requires an issue-attached AgentRun and is not a substitute.

The read contract, bounded window, unknown-provenance handling, and operator-only backfill look well built and well tested; the defects are confined to the reconcile write path.

Linked Issue Fit: acceptance criteria for multi-PR/late-merge timelines are met on the read side, but the no-title-link and multi-PR write criteria are not fully met per above.

Unknowns or Needs Verification: the truncated diff hides the reconcile open-state skip and the tail of issue-lifecycle-read.ts; the second defect's line numbers are from the reviewer and should be re-confirmed at head, though the diff's structure supports the concern.

Outstanding Human Change Requests

  • @​joryirving's change request (1c8b9a5, head moved since) is not shown addressed at this head; it needs the reviewer's own re-review.

@joryirving joryirving left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Request changes on exact head e4f43d4bfd0f. CI, Security Audit, Build Dispatch Image, PR Smoke, and AI PR Review all pass, but two major lifecycle-correctness findings from the prior review remain in this head.

Major 1: title-only matches still close issues and become authoritative pr_merged links

src/app/api/issues/reconcile/route.ts still calls extractFixingIssueNumbers(pr.body ?? pr.title ?? null) when building mergedFixingIssues. When a PR has no body and its title says Fixes #42, that title-derived match enters the map used by reconcileIssue() to decide whether to close issue 42. bodyKeywordNumbers correctly excludes that match from mergedPrLinkEvidence, but when the close succeeds, recordReconciledAction() falls back to the close-driving mergedPr and writes it as pr_merged with linkSource: "reconcile-close-trigger".

Renaming the provenance does not make a title inference authoritative. The linked #1203 contract explicitly says title-derived matches are not links. A title-only match must not drive the authoritative close/link path.

Required: remove title-only matching from the close decision and merge-link evidence path. Add a route-level regression with a merged PR whose body is null/empty and title is Fixes #42; assert it neither closes issue 42 nor emits a pr_merged event for it.

Major 2: merge evidence is still coupled to a successful close action and open-issue processing

mergedPrLinkEvidence is only consumed by recordReconciledAction(), which is called from the action loop. That function writes pr_merged events only inside the successful close_issue branch. The outer reconcile loop also skips every issue whose current GitHub state is not open. Consequently, after PR #7 closes issue #42, a later explicitly linked PR #9 can merge and subsequent reconciliation will never record #9's merge. Even for an open issue, a linked merge is lost if that reconciliation pass has no successful close action.

The documented backfill is not equivalent to continuous reconciliation: it requires an AgentRun attached to the issue whose touchedIssueUrls names the PR. Explicit body-keyword/branch evidence for a later merge does not guarantee that such a run record exists. This contradicts #1203's multi-PR requirement and the goal of recording every linked authoritative merge.

Required: decouple idempotent pr_merged event recording from issue-close execution and open-state gating. Reconcile the bounded linked-merge evidence for already-closed issues too, without mutating their GitHub status; keep the shared canonical dedupe key so repeated observations upsert one event. Add regression coverage for sequential merges (#7 closes the issue, #9 merges later), a closed issue, and repeated reconciliation producing exactly one event per PR.

These are behavioral gaps, not CI/traceability gaps. Please push fixes and request re-review on the new head.

…oncile (#1203)

Review finding on #1296: reconcile only iterates open issues, so a linked
PR that merges after its issue closed was never recorded and the timeline
lost its authoritative merge — violating #1203's late/out-of-order merge
acceptance. A bounded late-merge pass inside the same reconcile run now
records pr_merged for linked numbers the open loop did not process, via
the shared recordPrMergedEvent path with no close/status writes. Evidence
is re-derived from the already-fetched merged-PR window (no extra GitHub
calls) and replays idempotently onto the canonical merge dedupe key.
@itsmiso-ai

Copy link
Copy Markdown
Contributor Author

Responding to the two majors in the AI review at head 1c8b9a5:

1. "Title inference still creates authoritative merge linkage" — not changed; rationale below.

The title fallback feeding the close decision (extractFixingIssueNumbers(pr.body ?? pr.title ?? null) → mergedFixingIssues) predates this PR — it is #1202's reconcile behavior, present unchanged in the parent commit. This PR deliberately did not alter reconcile's close-decision behavior (documented in the invariant table: "The widened evidence (mergedPrLinkEvidence) never feeds reconcileIssue; only lifecycle recording widened. Existing #1202 reconcile tests pass unmodified") and a test pins it (route.test.ts: "never infers a link from a title: a PR without explicit evidence records only the honest close-trigger provenance").

#1203's "Do not infer links from PR titles" governs link evidence, and link evidence now excludes title matches entirely — only body-keyword and branch-name qualify. The close-trigger merge event records an authoritative merge (the PR really merged) with reconcile-close-trigger provenance that names the actual mechanism — the observed close — rather than claiming a text match. The alternatives both lose information: dropping the event hides a real merge behind a closed-unmerged timeline; removing title matching from the close decision silently changes production reconcile behavior (issues reconcile closes today would stop closing) and is out of #1203's scope. If the maintainer wants title-driven closes removed from reconcile, that is a deliberate #1202-behavior change worth its own issue — happy to file or implement it on request.

2. "A later linked PR is never recorded after the issue has closed" — fixed in 253c2a9.

fetchIssues returns open issues only, so the open-issue loop indeed never saw late merges. The route now runs a bounded late-merge pass (recordClosedIssueMergeEvidence) inside the same reconcile run: it re-derives links from the already-fetched merged-PR window (fetchPullRequests / fetchClosedPullRequests, maxItems 100 — no extra GitHub calls), batch-loads cached issue rows for linked numbers the open loop did not process, and records one pr_merged per linked merged PR through the shared recordPrMergedEvent path — merges only, no close/status writes, no invented transitions. Idempotent across replays via the canonical merge dedupe key; per-observation failure containment matches recordReconciledAction. Covered by new helper-level and POST-level tests (single row, provenance, null statuses, replay idempotence, repo scoping, processed-number skip). Merges outside the fetched PR window remain the backfill's territory, as documented.

@its-saffron its-saffron Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI Automated Review

Full PR review.

Analysis engine: glm-5.3-flash (openai) — fallback (primary failed) · pr-reviewer-action v3.6.0

Findings (1 major, 1 minor, 1 info)

Severity Location Finding
⚠️ Major src/app/api/issues/reconcile/route.ts:331 Title-derived matches still feed the close decision (`pr.body ?? pr.title`) and the close-trigger PR is still recorded as an authoritative `pr_merged` event, contrary to the outstanding human change request and PR 1203's no-title-links rule.
Minor src/app/api/issues/reconcile/route.test.ts:314 The requested regression (merged PR with null body and title `Fixes #42` asserting no close and no pr_merged) is not present.
Info src/app/api/issues/reconcile/route.ts:538 Merges outside the fetched PR window (maxItems 100) are only recoverable via the operator backfill; confirm downstream PR 1183/PR 1184 consumers accept that bound.

Recommendation: request changes. The outstanding human change request (review 5577078043) is half-addressed: the late-merge gap is fixed with solid tests, but title-only matching still drives the close decision and the close-trigger PR is still written as an authoritative pr_merged row, which the reviewer explicitly required removed.

Change-by-change findings

  • Major (not addressed): src/app/api/issues/reconcile/route.ts still calls extractFixingIssueNumbers(pr.body ?? pr.title ?? null) into mergedFixingIssues, and recordReconciledAction still records the close-driving PR as pr_merged with linkSource: "reconcile-close-trigger" when no explicit evidence exists. The requested regression (merged PR with null body, title Fixes #42, asserting no close and no pr_merged) is absent. The PR description argues this is pre-existing PR 1202 behavior and that the provenance label is honest; that is a defensible design position, but the human reviewer decides whether their request stands, and it has not been withdrawn or satisfied.
  • Addressed: late merges after close are now recorded via recordClosedIssueMergeEvidence (reconcile/route.ts), re-deriving links from the already-fetched PR window with no extra GitHub calls, no status writes, and replay idempotence; covered by helper-level and POST-level tests including repo scoping and processed-number skip.

The read contract (GET /api/issues/[issueId]/lifecycle), operator-only backfill (agent bearer 403, rate-limited, verifiable records only), and docs match PR 1203's acceptance criteria: unknown first claim stays unknown, closed-unmerged is never "shipped", intervals are milliseconds with provenance, and the read is bounded at 500 events.

Outstanding human change requests

  • joryirving (5577078043), Major 1 — not addressed (see above).
  • joryirving (5577078043), Major 2 — addressed by recordClosedIssueMergeEvidence and its tests.

Unknowns or Needs Verification

  • Merges outside the fetched PR window (maxItems 100) remain backfill-only; confirm that window is acceptable for PR 1183/PR 1184 consumers.
  • CI (Tests, Typecheck, Lint, Database integration) passes at head 253c2a9, but the title-match regression the reviewer asked for does not exist in the diff.

Outstanding Human Change Requests

  • @​joryirving's change request (e4f43d4, head moved since) is not shown addressed at this head; it needs the reviewer's own re-review.

Courier added 2 commits October 19, 2026 23:10
…m close execution (#1203)

Review-required changes on #1296:

- Major 1: PR titles no longer feed mergedFixingIssues. A merged PR whose
  only correlation to an issue is its title neither closes the issue nor
  records a pr_merged event for it; only body keywords and branch-name
  patterns are link/close evidence (#1203).
- Major 2: recordClosedIssueMergeEvidence becomes recordLinkedMergeEvidence
  and drops the processed-issue-number skip: pr_merged recording is
  decoupled from close execution and open-state gating. The pass records
  every linked merged PR in the fetched window whether the issue is open
  or closed and whether the close action executed; rows the close path
  wrote converge on the same canonical dedupe key (upsert is a no-op on
  conflict).
- Regressions: title-only PR closes/records nothing (route level);
  merge recorded even when the close action fails (route level); one
  event per PR across sequential merges and repeated reconciliation.
@itsmiso-ai

Copy link
Copy Markdown
Contributor Author

Both majors from the re-review (head e4f43d4) are now implemented at head 4d5e078 (fix(reconcile): no title-inferred closes; decouple merge evidence from close execution (#1203)), superseding the earlier on-thread rationale for Major 1:

Major 1 — title-only matches no longer drive close or link (implemented as required).
route.ts now builds mergedFixingIssues from extractFixingIssueNumbers(pr.body ?? null) only — the ?? pr.title fallback is gone. A merged PR whose body is null/empty and whose title says "Fixes #42" neither closes issue 42 nor records a pr_merged event for it; only body keywords (body-keyword) and branch-name patterns (branch-name) qualify as close and link evidence. Route-level regression added: "never closes or records a merge from a title-only match" asserts issuesClosed: 0, zero lifecycle rows, and no executed actions for exactly that PR shape. The earlier defense (title-driven closes as pre-existing #1202 behavior) is withdrawn — the title-driven close path is removed in this PR's scope, per this review.

Major 2 — merge evidence decoupled from close execution and open-state gating.
recordClosedIssueMergeEvidence is now recordLinkedMergeEvidence: the processed-issue-number skip is removed, so the bounded pass records every linked merged PR in the fetched window whether the issue is open or closed and whether the close action executed. Rows the close path already wrote carry the same canonical prMergedDedupeKey and converge (recordLifecycleEvent upserts with update: {}), so repeated reconciliation produces exactly one event per PR. Regressions added: "records the merge even when the close action fails" (route level: failed close → pr_merged recorded, no issue_closed), "converges to one pr_merged per PR when the close succeeds in the same run" (route level: both writers active → exactly one row, status context preserved), and "records exactly one event per PR across sequential merges and repeated reconciliation" (helper level: #7 then #9, two passes, two rows).

Also added on review: a warn when a linked number has no cached issue row so dropped evidence is visible instead of silent.

Local validation at 4d5e078: reconcile route tests 24 passed; npm test 4774 passed / 24 skipped / 1 todo; npm run typecheck exit 0; npm run lint 0 errors (1 pre-existing warning in src/app/login/page.tsx, untouched). CI on this head is pending — not claimed green. Re-review requested on the new head.

@itsmiso-ai
itsmiso-ai requested a review from joryirving October 19, 2026 23:27
its-saffron[bot]
its-saffron Bot previously approved these changes Oct 19, 2026

@its-saffron its-saffron Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI Automated Review

Full PR review.

Analysis engine: glm-5.3-flash (openai) — fallback (primary failed) · pr-reviewer-action v3.6.0

Findings (1 minor, 1 info)

Severity Location Finding
Minor src/app/api/issues/[issueId]/lifecycle/route.ts:152 The backfill's githubPullRequest.findMany loads every PR for the repo without a take bound; consider limiting to merged PRs or a window.
Info Merges outside the fetched 100-PR window are intentionally left to the operator backfill; confirm operators know to run it for older issues.

Recommendation: approve. Both outstanding human change requests are resolved at head 4d5e078 with route-level regressions in the diff.

  • Major 1 (title-only matches): src/app/api/issues/reconcile/route.ts now builds mergedFixingIssues from extractFixingIssueNumbers(pr.body ?? null) only — the ?? pr.title fallback is gone — and the new test "never closes or records a merge from a title-only match" asserts issuesClosed: 0, zero lifecycle rows, and no executed actions for a body-null, title-Fixes #42 PR.
  • Major 2 (merge evidence coupled to close/open state): recordLinkedMergeEvidence runs after the action loop over every linked number in the fetched window, records merges regardless of close success or issue state, and converges on the shared prMergedDedupeKey. Regressions cover failed close, same-run convergence, and replay idempotence.

The implementation matches PR 1203's acceptance criteria: no title inference, no worker-report merges, unknown first claim stays { at: null, provenance: "unknown" }, closed-unmerged is never "shipped", multi-PR and out-of-order fixtures are covered, and the read path is bounded (500 events, truncated flag). The backfill is operator-only (agent bearer 403, tested) and derives merges only from persisted GithubPullRequest.mergedAt.

Linked Issue Fit

Satisfies PR 1203's implementation and acceptance items as evidenced by the new tests in issue-lifecycle-read.test.ts, lifecycle-backfill.test.ts, and the reconcile route tests; docs updated in docs/issue-lifecycle-events.md.

Unknowns or Needs Verification

  • CI rows shown are for head 4d5e078 and all pass; the author's own local counts are consistent with that. No gaps blocking merge.
  • POST backfill loads all PRs for the repo without a take bound — minor, noted in findings.

Outstanding Human Change Requests

  • @​joryirving's change request (e4f43d4, head moved since) is not shown addressed at this head; it needs the reviewer's own re-review.

…overy (#1203)

The operator backfill loaded every PR row of the repo with no bound. It now
loads only the rows for the PR numbers the issue's runs touch, and takes the
repo's merged-PR total from an aggregate count, so no unbounded row set is
materialized. recorded / skippedUnmerged / skippedNoLink keep their documented
meanings: skippedNoLink is the repo's merged-PR total minus the merges this
issue recorded.

Documents the reconciler's bounded merged-PR window (100 most recently updated
closed PRs; merged PRs come from the closed side) and the operator path for
merges outside it, including the precondition the backfill cannot bypass — a
merge time already persisted on the PR row (#1313 tracks the sync gap that
leaves it unpersisted for PRs the sync saw while open).
@itsmiso-ai

Copy link
Copy Markdown
Contributor Author

Addresses the two findings left open by the last AI review on 4d5e078 (minor: unbounded backfill PR read; info: the 100-PR merge window and the operator recovery path), at head e1dd146.

Minor — the backfill's PR read is bounded. POST /api/issues/[issueId]/lifecycle loaded every PR row of the repo with no bound. It now loads only the rows for the PR numbers the issue's runs touch (touchedPullRequestNumbers, derived with the existing parsePullRequestReference, restricted to this repo), and takes the repo's merged-PR total from an aggregate githubPullRequest.count({ mergedAt: { not: null } }) instead of a row scan — no unbounded row set is materialized. The response contract is unchanged: recorded and skippedUnmerged keep their meanings, and skippedNoLink (merged PRs of the repo no run of this issue touched) is now the repo's merged-PR total minus the merges this issue recorded — the same number the old repo-wide scan produced.

Info — the window and the operator path are documented. docs/issue-lifecycle-events.md now states the reconciler's bound (each repo's open PRs plus its 100 most recently updated closed PRs; merged PRs come from the closed side, since an open PR never carries a merge), that a merge outside that window is never recorded by reconcile, and that recovering it means running POST /api/issues/{issueId}/lifecycle for that issue with both preconditions — an AgentRun naming the PR and that PR's persisted GithubPullRequest.mergedAt.

One precondition the docs now state plainly (and #1313 tracks): the automation sync upserts open PRs only, and an open PR never carries a merge, so a row it wrote holds no mergedAt and a PR that later merges is never updated — the backfill recovers a merge only where the merge time is already persisted. The reconcile path is unaffected; it reads merged_at straight from GitHub. The sync gap itself is out of this PR's scope and is filed as #1313.

Local validation at e1dd146: npx vitest run src/lib/lifecycle-backfill.test.ts "src/app/api/issues/[issueId]/lifecycle/route.test.ts" → 2 files, 31 passed. npm test → 194 files passed / 4 skipped; 4783 tests passed / 24 skipped / 1 todo. npm run typecheck → exit 0. npm run lint → 0 errors (1 pre-existing warning in src/app/login/page.tsx, untouched).

Review of this delta: an independent same-model reviewer returned GO-with-minors — a docs over-promise about the backfill's reachable coverage and two missing regression cases. Both are fixed in this commit (the docs now state the persistence precondition; the new cases pin a rejected-row count and a scoped-but-unmerged row). Same-model limitation noted: no model-family diversity was available for that review.

Remaining external verification: CI on e1dd146 (green on the previous head 4d5e078), and @joryirving's re-review — its two majors were implemented at 4d5e078 and are re-requested on this head.

its-saffron[bot]
its-saffron Bot previously approved these changes Oct 20, 2026

@its-saffron its-saffron Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI Automated Review

Full PR review.

Analysis engine: glm-5.3-flash (openai) — fallback (primary failed) · pr-reviewer-action v3.6.0

Findings (1 info)

Severity Location Finding
Info src/lib/lifecycle-backfill.ts Backfill coverage depends on the PR 1313 sync gap (open-PR-only upserts never persist mergedAt); confirm the documented operator-recovery limitation is acceptable before relying on backfill for older merges.

Recommendation: approve. Both majors from the outstanding human change request are implemented at this head and pinned by route-level regressions; CI (typecheck, tests, coverage, database integration, migrations, lint) is green on e1dd146.

Change-by-change findings

  • Major 1 (title-only matches): the close/link path now uses extractFixingIssueNumbers(pr.body ?? null) — the ?? pr.title fallback is gone — and the regression "never closes or records a merge from a title-only match" asserts zero closes, zero lifecycle rows, and no executed actions for a body-less PR titled Fixes #42.
  • Major 2 (merge evidence coupled to close): recordLinkedMergeEvidence runs a bounded pass over every linked number in the fetched merged-PR window regardless of issue state or close outcome; regressions cover merge recorded on failed close, convergence to one pr_merged per PR when the close also succeeds, and idempotent replay across sequential merges.
  • The backfill's PR read is bounded to numbers the issue's runs touch, with the repo merged-PR total from an aggregate count(); the sync-gap precondition (open-PR-only upserts never carry mergedAt) is documented and tracked as PR 1313, out of scope here.

Human review dispositions: both requests from joryirving are addressed at this head (see human_review_dispositions); the human reviewer decides whether they still stand.

Sources: PR diff (src/app/api/issues/reconcile/route.ts, route.test.ts, src/lib/lifecycle-backfill.ts, src/app/api/issues/[issueId]/lifecycle/route.ts), linked issue PR 1203, PR description, CI check results on e1dd146.

Outstanding Human Change Requests

  • @​joryirving's change request (e4f43d4, head moved since) is not shown addressed at this head; it needs the reviewer's own re-review.

@itsmiso-ai

Copy link
Copy Markdown
Contributor Author

Courier: already addressed (run courier-3c38bf48fc3d63f0c3d00645351ef931)

PR #1296 (misospace/dispatch) is already complete and published: head e1dd146 on courier/misospace/dispatch/issue-1203 is both local HEAD and the PR head (verified via GitHub API; PR open, non-draft). All assigned review feedback is addressed: joryirving's two CHANGES_REQUESTED majors were implemented at 4d5e078 (title-only matches removed from close/link; merge evidence decoupled from close execution via recordLinkedMergeEvidence) and the follow-up minor (bounded backfill PR read) at e1dd146, each with on-thread responses; the latest AI review at this head is APPROVED with only an info-level item tracked as issue #1313 (out of scope, filed). Coordinator-owned local validation at this worktree/revision: npm run typecheck exit 0; npm run lint 0 errors (1 pre-existing warning in src/app/login/page.tsx, untouched); npx vitest run of the 5 changed-file test files (lifecycle-backfill, lifecycle route, issue-lifecycle-read, issue-lifecycle, reconcile route) -> 100 passed; npm test -> 4783 passed / 24 skipped / 1 todo. No commits or pushes were needed. Remaining external verification explicitly left pending: CI on e1dd146 (previous head 4d5e078 was green; this head's checks pending, not claimed green) and joryirving's human re-review.

@joryirving joryirving left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

REQUEST CHANGES on exact head e1dd146fbe252a41f88182f9602b96648a618818. The two prior major findings are addressed: title-only matches no longer drive closure/linkage, and the bounded merge-evidence pass is decoupled from close execution and issue-open state. All five reported workflows pass. I found two remaining correctness gaps in the new contract:

  1. The timeline reports delivery: "unknown" for a closed-unmerged issue that was closed, reopened, and then closed again. closedAt intentionally preserves the first close milestone, but delivery compares that first close to the last reopen instead of considering a subsequent close. Keep the first milestone if that is the contract, but derive current delivery from the latest close/reopen ordering; add a close → reopen → close regression expecting closed-unmerged when the current issue state is closed and no PR merged.
  2. Branch-name merge evidence is conditionally skipped when mergedFixingIssues already has a key equal to pr.number. That map is keyed by issue number, so an unrelated body reference to issue #42 can suppress a merged PR #42's valid branch link to issue #99. The new multi-PR evidence pass then misses #99's merge entirely. Branch evidence collection should not be gated on a key collision in the close-decision map; keep first-PR-wins for closure separately and add a collision regression.

Both are behavioral correctness issues in the timeline/linkage this PR introduces, so I would fix them before merging.

Comment thread src/lib/issue-lifecycle-read.ts
Comment thread src/app/api/issues/reconcile/route.ts Outdated
Courier added 3 commits October 20, 2026 03:29
…ce ungated (#1203)

Addresses the two open findings on PR #1296 (exact head e1dd146):

- delivery now follows the latest close/reopen ordering, so close ->
  reopen -> close reports closed-unmerged again instead of unknown.
  `closedAt` still reports the first close milestone.
- branch-name merge evidence is collected independently of the
  close-decision map, so a PR whose own number collides with an issue
  number in that map still records its branch link. The close decision
  stays first-PR-wins per issue number.
…each (#1203)

Follow-up to the review findings on PR #1296:

- The issue cache's closedAt now counts toward the latest close only when
  the issue is closed, the same gate the `closedAt` milestone uses, so
  delivery never certifies a close the milestone refuses to.
- The delivery ladder is state-based when there is no close evidence at
  all, so a reopen without a close no longer turns a closed issue
  unknown.
- The close-after-reopen note stays silent for a merged delivery.
- The reconciler's widened close reach in the branch-collision case is
  stated in the route comment and pinned by a test.
@itsmiso-ai

Copy link
Copy Markdown
Contributor Author

Run handoff — head d789ae1 (branch courier/misospace/dispatch/issue-1203, PR open, non-draft)

Took over PR #1296 and closed the two open findings from @joryirving's review of e1dd146.

What changed

  1. delivery follows the latest close/reopen ordering (src/lib/issue-lifecycle-read.ts). It previously compared the first close milestone against the last reopen, so close → reopen → close reported unknown instead of closed-unmerged. The latest close is now the later of the last issue_closed event and the issue cache's closedAt — the cache counts only for a closed issue, the same gate the closedAt milestone already used, so delivery never certifies a close the milestone refuses to. A reopen after the latest close undoes the close (the present state decides); a close after a reopen closes the item again. closedAt still reports the first close milestone; the close-after-reopen case is called out in notes.
  2. Branch-name link evidence no longer collides with the close-decision map (src/app/api/issues/reconcile/route.ts). The branch fallback was gated on mergedFixingIssues.has(pr.number) — that map is keyed by issue number, so the gate conflated GitHub's shared issue/PR number space: when another merged PR's body had already put issue Fix Board filters resetting and remove stale project-label filtering #42 in the map, PR Fix Board filters resetting and remove stale project-label filtering #42's own branch link (head ref issue-99) was skipped entirely and issue fix: v0.2 migration blockers — queue fields, status endpoint, move auth, claim behavior #99's merge went unrecorded. Branch evidence is now collected for every merged PR; the close decision keeps first-PR-wins per issue number and link-source precedence is unchanged (a body keyword for the same issue+PR stays body-keyword). The collision case now also widens the reconciler's close reach — the branch claim still feeds the close decision, so the branch-linked issue is closed by the same branch heuristic it was already subject to outside the collision case. That is stated in the route comment and pinned by a test.

Regressions added — src/lib/issue-lifecycle-read.test.ts: close → reopen → close reports closed-unmerged with the first close milestone intact; the issue cache's closedAt counts as the latest close when the second close has no event; the cache cannot certify a close for an open issue; the close-after-reopen note stays silent for a merged delivery. src/app/api/issues/reconcile/route.test.ts: the branch link survives the collision (a pr_merged for issue #99 with linkSource: "branch-name", issue #42 keeping body-keyword), and the collision case closes the branch-linked issue. All new cases fail against e1dd146.

Local validation at d789ae1 (coordinator-run, worktree /workspace)

Command Result Exit
npx vitest run src/lib/issue-lifecycle-read.test.ts src/app/api/issues/reconcile/route.test.ts src/lib/issue-lifecycle.test.ts src/lib/lifecycle-backfill.test.ts "src/app/api/issues/[issueId]/lifecycle/route.test.ts" 5 files, 106 tests passed 0
npm test 194 files passed / 4 skipped; 4791 tests passed / 24 skipped / 1 todo 0
npm run typecheck clean 0
npm run lint 0 errors (1 pre-existing warning in src/app/login/page.tsx, untouched) 0

The 4 skipped files are the Postgres-backed integration suites: they need a live database, which this environment has none of, so they are CI-only here.

Review of this delta — an independent same-model reviewer returned GO-with-minors (no blocker, no major). All five minors it raised are fixed at d789ae1: the cache/state gate asymmetry, the note firing on a merged delivery, the unstated close-reach widening (comment + test), the untested cache contribution to the latest close, and the docs clause naming the cache's role. Same-model limitation noted — no model-family diversity available on this run.

Remaining external verification (explicitly pending)

  • CI on d789ae1 — green on the previous head e1dd146 (all 13 checks); this head's checks were still queued when this run ended, so they are not claimed green.
  • @joryirving's re-review of the two findings addressed above. Both threads are answered on-thread and resolved; please re-open them if the fix does not match your intent.

Nothing here was merged — the PR is left open for operator verification and human review.

@its-saffron its-saffron Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI Automated Review

Full PR review.

Analysis engine: glm-5.3-flash (openai) — fallback (primary failed) · pr-reviewer-action v3.6.0

Findings (1 info)

Severity Location Finding
Info src/lib/lifecycle-backfill.ts Backfill coverage depends on PR 1313 (sync persisting mergedAt for closed PRs); confirm the tracked follow-up lands before relying on backfill for legacy issues.

Approve. Both findings in joryirving's change request are resolved at head d789ae1 with pinned regressions: the branch-link collision fix is visible in the reconcile route diff and covered by the new collision tests in src/app/api/issues/reconcile/route.test.ts (branch link survives a PR/issue number collision; branch-linked issue still closed), and the delivery fix is specified in the PR description (latest close = later of last observed close and the cache's closedAt, cache gated on closed state) with regressions added in src/lib/issue-lifecycle-read.test.ts. CI is green on this head (tests, typecheck, lint, migrations, build, database integration). The known sync gap — open-PR-only upserts mean GithubPullRequest.mergedAt may be missing for older merges, limiting backfill coverage — is documented in docs/issue-lifecycle-events.md and tracked as PR 1313; it is a pre-existing limitation, not a defect this PR introduces.

Outstanding Human Change Requests

  • @​joryirving's change request (e1dd146, head moved since) is not shown addressed at this head; it needs the reviewer's own re-review.

@itsmiso-ai

Copy link
Copy Markdown
Contributor Author

Courier: already addressed (run courier-811388e2b0cba6868633fc2417a65c24)

PR #1296 (misospace/dispatch) is open, non-draft, head d789ae1 on courier/misospace/dispatch/issue-1203 == local HEAD (clean tree). Both of joryirving's change-request findings are fixed at b82754e/d789ae1 with pinned regressions and on-thread replies; both review threads are resolved. AI reviewer (its-saffron[bot]) APPROVED at this exact head; its single info finding (backfill coverage depends on #1313 sync persisting mergedAt) is pre-existing, documented in docs/issue-lifecycle-events.md and the PR body, and tracked externally. All 13 CI checks are green at d789ae1 (Tests, Database integration, Typecheck, Lint, Database migrations, Build, Workflow lint, Coverage, review, Docker Build, Docker Build (MCP), npm audit, smoke). Fresh local validation this run at d789ae1 after npm ci (exit 0): npx vitest run of the 5 lifecycle suites (issue-lifecycle-read, reconcile route, issue-lifecycle, lifecycle-backfill, lifecycle route) -> 5 files / 106 tests passed; npm run typecheck -> exit 0; npm run lint -> 0 errors (1 pre-existing warning in src/app/login/page.tsx, untouched). Remaining blocker is external only: joryirving's CHANGES_REQUESTED review requires the reviewer's own re-review to dismiss (mergeable_state blocked); no code work is outstanding for this run.

@joryirving joryirving left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-reviewed exact head d789ae1 after prior REQUEST_CHANGES. Both blocking correctness issues are fixed: the lifecycle projector preserves the original first-close milestone but derives current delivery from the most recent close/reopen order (including the cache-only second close), and merged-PR branch evidence is now collected independently of close-decision-map key collisions. Dedicated close→reopen→close and PR-number/issue-number collision regressions cover the formerly broken outcomes. Title-only matches remain excluded; first-PR-wins close selection and idempotent event recording remain. All required workflows passed on this head. No remaining blocking findings.

@joryirving
joryirving merged commit 9f7ac6a into main Oct 20, 2026
13 checks passed
@joryirving
joryirving deleted the courier/misospace/dispatch/issue-1203 branch October 20, 2026 14:37
@its-miso its-miso Bot mentioned this pull request Oct 20, 2026
joryirving pushed a commit that referenced this pull request Oct 20, 2026
…s an authoritative merge time (#1320)

* fix(automation-sync): persist merged PRs so lifecycle backfill has merge times (#1313)

The automation sync sourced PR rows only from the state=open listing, so
every GithubPullRequest row it wrote carried mergedAt: null, and a PR that
later merged dropped out of the listing without ever being updated. The
lifecycle backfill added in #1296 correlates a run's touched PR against the
persisted mergedAt and therefore found no merge evidence for any PR the
sync had seen.

Also fetch the recently updated closed listing (one added call per repo per
sync, degrading gracefully like packages on failure) and upsert the rows
that carry merged_at, reusing the existing upsert write path. A PR that
merges between the two concurrent listings is upserted once with the merged
row winning, never written back to mergedAt: null.

* test: restore full @/lib/github mocks so route tests exercise the real automation sync (#1313)

The scheduled/automation sync route tests mock @/lib/github wholesale; the
new unconditional closed-PR fetch would resolve to a missing export and
fail the sync inside a catch path, so the happy-path tests passed without
ever running a successful sync. Add the missing mock to both factories and
pin the scheduled route's automation result to {synced: 1, failed: 0}.
Also pin state: closed on the both-listings upsert so 'merged row wins' is
fully asserted.

---------

Co-authored-by: Courier <courier@localhost>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(lifecycle): link authoritative PR merges and expose claim-to-merge timelines

2 participants