fix(core): dispatch detailed bugbot review comments to agents - #1334

- Rewrite the @aoagents/ao-core changeset to describe only what feat/1226
  contributes: event enrichment with schemaVersion: 2, issueTitle
  persistence, executeReaction refactor, maybeDispatchCIFailureDetails,
  and bugbot-comments enrichment. Drop the false claims about adding
  spawn-target and format-automated-comments (those modules came from
  upstream PRs OrchestratorInc#1330 and OrchestratorInc#1334).
- Delete stop-kill-all-resilience.md — its claims (kill-all loop,
  fd-safety in tryAcquire, allSessionPrefixes unify) are no longer
  in the branch after the rebase took upstream's canonical stop logic.

Co-Authored-By: Claude Opus 4.7 <[email protected]>

* fix: serialize ao start and stop numbered orchestrators (#1306)

* fix: restore dead orchestrators on start (#1306)

* fix: harden startup lock handling (#1306)

* feat(core): enrich events with PR title, description, and URL

Adds PR and issue context to all event payloads sent to notifiers.
External consumers (Telegram, Discord, n8n) can now display meaningful
information without making additional API calls.

Changes:
- Add buildEventContext() helper to extract PR/issue context from session
- Enrich all createEvent() calls with context data (pr, issueId, issueTitle, branch)
- Store issueTitle in session metadata during spawn
- Add issueTitle field to SessionMetadata interface
- Update executeReaction() to accept session for context access
- Add tests for event enrichment

The context includes:
- pr: { url, title, number, branch } when PR exists
- issueId: issue identifier
- issueTitle: issue title (from tracker during spawn)
- branch: session branch name

Events before PR creation gracefully omit PR fields (pr: null).
Existing webhook consumers that ignore unknown fields are unaffected.

Closes #1226

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* fix(core): address review comments for event enrichment

- Add issueTitle to readMetadata/writeMetadata for proper persistence
- Create ReactionSessionContext type for type-safe system events
- Replace unsafe `as unknown as Session` cast with proper union type
- Add end-to-end test verifying issueTitle persistence during spawn

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* fix(cli): include lock file path in startup lock error message

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address review feedback — fd safety, kill-all resilience, issueTitle restore

- Restore try/catch/finally in tryAcquire for fd leak prevention
- Wrap stop command's sm.list()+kill in try/catch so dashboard shutdown
  always runs even on session listing failure
- Add per-iteration error handling in kill-all loop with partial failure
  reporting (spinner.warn for mixed results)
- Unify allSessionPrefixes derivation between start and stop commands
- Propagate issueTitle through archive restore path
- Add clarifying comments on agentInfo.summary fallback and intentional
  prNumber/prUrl duplication in event data
- Add ora warn mock for stop tests
- Update changeset to minor (event enrichment is a feature) and add CLI
  changeset for stop resilience
- Add tests for kill-all error mid-loop and issueTitle archive restore

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address harsh-batheja review — title/summary split, lock grace, context namespace

- Separate PR title from agent summary in EventContext: title is null
  until enrichment cache populates; summary is a distinct field so
  webhook consumers never confuse the task summary for a PR title.
- Restore UNPARSEABLE_LOCK_GRACE_MS (5s mtime grace) and
  isStaleUnparseableLock lost during merge conflict resolution —
  prevents lockfile-steal race when process A just created the file
  but hasn't written metadata yet.
- Fix orchestrator sort: extract numeric suffix instead of
  localeCompare so -10 sorts after -2, not before.
- Namespace context under data.context instead of spreading into data
  to prevent field collisions with reaction-specific keys.
- Add schemaVersion: 2 to all enriched events so consumers can
  migrate away from top-level prNumber/prUrl (kept for compat,
  marked for removal in v3).
- Update event enrichment tests for nested context structure.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: resolve merge conflicts — use formatReviewCommentsMessage, fix batch enrichment mock

- Replace formatAutomatedCommentsMessage with upstream's formatReviewCommentsMessage
  for automated review comment dispatch (fixes type mismatch with ReviewComment[])
- Make createMockSCM's enrichSessionsPRBatch dynamically resolve from individual
  method mocks so test overrides (e.g. getPRState("closed")) propagate correctly
- Add explicit enrichSessionsPRBatch to merge-conflict-tracking test to avoid
  unexpected getMergeability calls from the dynamic mock
- Remove all debug console.log statements added during troubleshooting

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: wire up maybeDispatchCIFailureDetails and record dispatch hash on transition

The function was defined but never called after merge conflict resolution
dropped the call site. Added it back to the Promise.allSettled alongside
maybeDispatchReviewBacklog and maybeDispatchMergeConflicts.

Also updated the transition-reaction early-return to record the dispatch
hash, since the transition path now enriches the CI message with detailed
check info from the batch cache — preventing duplicate sends on subsequent
polls.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test: remove stale duplicate test from rebase

Removes the orphaned numbered-orchestrator restoration test left over
from feat/1226 history. Upstream's canonical model test (same name,
expects "app-orchestrator" via ensureOrchestrator) supersedes it.

Co-Authored-By: Claude Opus 4.7 <[email protected]>

* docs: remove misleading CLI CHANGELOG entry

The "Restore the most recently active dead orchestrator on ao start"
entry described upstream's ensureOrchestrator behavior (#1487), not
work contributed by this PR.

Co-Authored-By: Claude Opus 4.7 <[email protected]>

* docs(changeset): correct scope and drop stale CLI changeset

- Rewrite the @aoagents/ao-core changeset to describe only what feat/1226
  contributes: event enrichment with schemaVersion: 2, issueTitle
  persistence, executeReaction refactor, maybeDispatchCIFailureDetails,
  and bugbot-comments enrichment. Drop the false claims about adding
  spawn-target and format-automated-comments (those modules came from
  upstream PRs #1330 and #1334).
- Delete stop-kill-all-resilience.md — its claims (kill-all loop,
  fd-safety in tryAcquire, allSessionPrefixes unify) are no longer
  in the branch after the rebase took upstream's canonical stop logic.

Co-Authored-By: Claude Opus 4.7 <[email protected]>

---------

Co-authored-by: Claude Opus 4.5 <[email protected]>

…tratorInc#1334)

* fix(core): dispatch detailed bugbot review comments to agents

The bugbot-comments reaction previously sent a generic "fix the issues"
message, leaving the agent to discover comments on its own. The typical
agent flow — fetch `GET /pulls/{pr}/comments` (first page only) and
filter for a bugbot marker — misses new comments when the response is
paginated, and trusts a stale `gh pr checks` status.

Now we format the already-fetched `AutomatedComment[]` into a detailed
message that lists each comment (severity, path:line, bot name, excerpt,
URL) and embeds explicit API-level guidance so the agent can verify via
`/reviews` + `/reviews/{id}/comments` + paginated `/pulls/{pr}/comments`
(checking `in_reply_to_id` for replies) instead of the naive first-page
scan.

Closes OrchestratorInc#895

* refactor(core): extract bugbot comment formatter, address review

Follow-up to OrchestratorInc#895 review:

- Extract formatAutomatedCommentsMessage to a module-level helper
  (packages/core/src/format-automated-comments.ts) so it is directly
  unit-testable rather than closure-scoped inside createLifecycleManager.
- Accept optional PRInfo and interpolate owner/repo/number into the
  verification guidance when dispatched from the lifecycle; fall back
  to OWNER/REPO/PR placeholders otherwise.
- Drop the dead `if (c.url)` guard (url is required on AutomatedComment).
- Append `…` when the first-line excerpt is truncated at 160 chars.
- Simplify the config.ts default message to a short fallback and note
  inline that the lifecycle dispatcher injects the rich listing.
- Add 11 focused unit tests for the formatter (interpolation, ellipsis,
  first-line extraction, missing path/line, severity rendering, ordering).

* fix(core): address review feedback on bugbot detail dispatch

Resolves the 6 comments from @illegalcall on OrchestratorInc#1334:

- H: Sentinel-gate the default-message override — export
  DEFAULT_BUGBOT_COMMENTS_MESSAGE from config.ts and only replace the
  reaction message when it matches the sentinel. User-customized
  messages in project YAML are left untouched.
- H: Add --paginate to step 2 of the verification guidance. Step 2 was
  previously missing --paginate, reintroducing the exact pagination
  failure mode OrchestratorInc#895 is meant to fix.
- H: Prompt-injection mitigation — strip backticks from comment body
  excerpts, wrap each excerpt in a code span so the content cannot
  break out, and add an "untrusted third-party data" preamble telling
  the agent not to treat excerpts as instructions.
- M: Preserve line number when c.line === 0 (file-level or 0-indexed
  tools). Was previously treated as falsy and dropped.
- M: Skip leading blank lines in excerpt extraction; strip leading
  markdown heading markers (### Title) and whole-line bold/italic
  wrappers before truncating.
- L: Correct the "reply resolves the thread" wording — replying alone
  does not resolve a review thread on GitHub; resolution is a separate
  "Resolve conversation" action.

Also adds a patch-level changeset for @aoagents/ao-core and expands
the test suite to 811 tests (was 803) covering sentinel override,
custom-message passthrough, line===0, backtick escaping, heading
stripping, and step-2 pagination regression.

…tratorInc#1326)

* fix: serialize ao start and stop numbered orchestrators (OrchestratorInc#1306)

* fix: restore dead orchestrators on start (OrchestratorInc#1306)

* fix: harden startup lock handling (OrchestratorInc#1306)

* feat(core): enrich events with PR title, description, and URL

Adds PR and issue context to all event payloads sent to notifiers.
External consumers (Telegram, Discord, n8n) can now display meaningful
information without making additional API calls.

Changes:
- Add buildEventContext() helper to extract PR/issue context from session
- Enrich all createEvent() calls with context data (pr, issueId, issueTitle, branch)
- Store issueTitle in session metadata during spawn
- Add issueTitle field to SessionMetadata interface
- Update executeReaction() to accept session for context access
- Add tests for event enrichment

The context includes:
- pr: { url, title, number, branch } when PR exists
- issueId: issue identifier
- issueTitle: issue title (from tracker during spawn)
- branch: session branch name

Events before PR creation gracefully omit PR fields (pr: null).
Existing webhook consumers that ignore unknown fields are unaffected.

Closes OrchestratorInc#1226

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* fix(core): address review comments for event enrichment

- Add issueTitle to readMetadata/writeMetadata for proper persistence
- Create ReactionSessionContext type for type-safe system events
- Replace unsafe `as unknown as Session` cast with proper union type
- Add end-to-end test verifying issueTitle persistence during spawn

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* fix(cli): include lock file path in startup lock error message

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address review feedback — fd safety, kill-all resilience, issueTitle restore

- Restore try/catch/finally in tryAcquire for fd leak prevention
- Wrap stop command's sm.list()+kill in try/catch so dashboard shutdown
  always runs even on session listing failure
- Add per-iteration error handling in kill-all loop with partial failure
  reporting (spinner.warn for mixed results)
- Unify allSessionPrefixes derivation between start and stop commands
- Propagate issueTitle through archive restore path
- Add clarifying comments on agentInfo.summary fallback and intentional
  prNumber/prUrl duplication in event data
- Add ora warn mock for stop tests
- Update changeset to minor (event enrichment is a feature) and add CLI
  changeset for stop resilience
- Add tests for kill-all error mid-loop and issueTitle archive restore

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address harsh-batheja review — title/summary split, lock grace, context namespace

- Separate PR title from agent summary in EventContext: title is null
  until enrichment cache populates; summary is a distinct field so
  webhook consumers never confuse the task summary for a PR title.
- Restore UNPARSEABLE_LOCK_GRACE_MS (5s mtime grace) and
  isStaleUnparseableLock lost during merge conflict resolution —
  prevents lockfile-steal race when process A just created the file
  but hasn't written metadata yet.
- Fix orchestrator sort: extract numeric suffix instead of
  localeCompare so -10 sorts after -2, not before.
- Namespace context under data.context instead of spreading into data
  to prevent field collisions with reaction-specific keys.
- Add schemaVersion: 2 to all enriched events so consumers can
  migrate away from top-level prNumber/prUrl (kept for compat,
  marked for removal in v3).
- Update event enrichment tests for nested context structure.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: resolve merge conflicts — use formatReviewCommentsMessage, fix batch enrichment mock

- Replace formatAutomatedCommentsMessage with upstream's formatReviewCommentsMessage
  for automated review comment dispatch (fixes type mismatch with ReviewComment[])
- Make createMockSCM's enrichSessionsPRBatch dynamically resolve from individual
  method mocks so test overrides (e.g. getPRState("closed")) propagate correctly
- Add explicit enrichSessionsPRBatch to merge-conflict-tracking test to avoid
  unexpected getMergeability calls from the dynamic mock
- Remove all debug console.log statements added during troubleshooting

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: wire up maybeDispatchCIFailureDetails and record dispatch hash on transition

The function was defined but never called after merge conflict resolution
dropped the call site. Added it back to the Promise.allSettled alongside
maybeDispatchReviewBacklog and maybeDispatchMergeConflicts.

Also updated the transition-reaction early-return to record the dispatch
hash, since the transition path now enriches the CI message with detailed
check info from the batch cache — preventing duplicate sends on subsequent
polls.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test: remove stale duplicate test from rebase

Removes the orphaned numbered-orchestrator restoration test left over
from feat/1226 history. Upstream's canonical model test (same name,
expects "app-orchestrator" via ensureOrchestrator) supersedes it.

Co-Authored-By: Claude Opus 4.7 <[email protected]>

* docs: remove misleading CLI CHANGELOG entry

The "Restore the most recently active dead orchestrator on ao start"
entry described upstream's ensureOrchestrator behavior (OrchestratorInc#1487), not
work contributed by this PR.

Co-Authored-By: Claude Opus 4.7 <[email protected]>

* docs(changeset): correct scope and drop stale CLI changeset

- Rewrite the @aoagents/ao-core changeset to describe only what feat/1226
  contributes: event enrichment with schemaVersion: 2, issueTitle
  persistence, executeReaction refactor, maybeDispatchCIFailureDetails,
  and bugbot-comments enrichment. Drop the false claims about adding
  spawn-target and format-automated-comments (those modules came from
  upstream PRs OrchestratorInc#1330 and OrchestratorInc#1334).
- Delete stop-kill-all-resilience.md — its claims (kill-all loop,
  fd-safety in tryAcquire, allSessionPrefixes unify) are no longer
  in the branch after the rebase took upstream's canonical stop logic.

Co-Authored-By: Claude Opus 4.7 <[email protected]>

---------

Co-authored-by: Claude Opus 4.5 <[email protected]>