fix(observability): canonicalise child-logger entity id under entityNumber

[chrisleekr]

What

Resolves #175. The canonical createChildLogger helper (src/logger.ts) defines the {deliveryId, owner, repo, entityNumber} correlation contract and is already used by app.ts, core/context.ts, and daemon/job-executor.ts — but the webhook event handlers bypassed it, hand-rolling logger.child with a drifting entity field name (issueNumber in issues/issue-comment, prNumber in pull-request/review-comment). An operator who knows a PR/issue number could not grep one field name to reconstruct a request end-to-end.

Changes

• Widen createChildLogger to accept the 4 required canonical fields plus arbitrary extra bindings (& Record<string, unknown>), so handlers route through the single source of truth while keeping per-handler context (event, label, senderLogin). Existing 4-field callers are unaffected (verified typecheck).
• Migrate 4 handlers (issues, pull-request, issue-comment, review-comment) to createChildLogger, emitting canonical entityNumber.
• Add entityNumber to review.ts and review-thread.ts per-request log lines (they previously emitted no entity id), so every webhook handler with a per-request line is greppable by entity.
• workflow-executor.ts: add a flat entityNumber alongside the nested target envelope (target retained).
• Docs: observability.md lede + a new entityNumber row in the Common log fields table (documents the surviving adjacent layers: job-payload prNumber/issueNumber and scoped-rail snake_case pr_number/issue_number).
• Tests: test/utils/logger.test.ts asserts createChildLogger().bindings() carries entityNumber, passes extras through, and that the drifted issueNumber/prNumber names do not leak back.

Verification

• bun run typecheck, eslint . (0 errors), prettier --check, check:docs-citations, check:no-em-dashes → clean.
• bun test test/utils/logger.test.ts → 16 pass (2 new). Webhook/workflow-executor tests pass per-file; the combined-run 4 fail/4 error are a pre-existing cross-file test-isolation issue (identical count on clean main, verified by stash).

Scope decisions (judgement calls)

• No prNumber/issueNumber compatibility mirror. Verified grep-clean — nothing consumes those field names (no docs, dashboards, or alerts). A mirror would be tech debt + a needless follow-up. This also makes the proposed isPR discriminator unnecessary.
• installationId descoped → tracked as feat(observability): emit installationId on per-request child loggers for per-installation quota triage #177. It is a separate correlation enhancement and interacts with installation-guard ordering; not the field-name-drift bug.
• Job-queue Zod schema (ws-messages.ts/job-queue.ts/job-dispatcher.ts) untouched — the prNumber/issueNumber there are the job-payload contract, a different layer not cited by the issue.
• Known-deferred (non-defects): two early-return no-op log lines (issues.ts, pull-request.ts) and one internal-error fallback (pull-request.ts repos.getCommit failed, retains prNumber) are not request-correlation lines; left as-is, can fold into feat(observability): emit installationId on per-request child loggers for per-installation quota triage #177 for full uniformity.

Senior-review gate: 3 iterations, converged clean (CodeRabbit unavailable this run — org out of credits — so senior-review is the quality gate).

🤖 Generated with Claude Code

Summary by CodeRabbit

• Chores
  • Improved structured logging infrastructure with enhanced entity tracking capabilities. Updated logging configuration across webhook event handlers and background operations to include unified identifier fields alongside existing metadata. These changes strengthen overall system observability, improve event correlation for debugging and monitoring, and enable better data collection for system health reporting and analysis.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 74ebba16-88f1-4284-b034-6b5fb3c71928

📥 Commits

Reviewing files that changed from the base of the PR and between 1d1bc3b and 72cda6f.

📒 Files selected for processing (10)

• docs/operate/observability.md
• src/daemon/workflow-executor.ts
• src/logger.ts
• src/webhook/events/issue-comment.ts
• src/webhook/events/issues.ts
• src/webhook/events/pull-request.ts
• src/webhook/events/review-comment.ts
• src/webhook/events/review-thread.ts
• src/webhook/events/review.ts
• test/utils/logger.test.ts

---

📝 Walkthrough

Walkthrough

This PR establishes a canonical entityNumber field for per-entity log correlation across webhook handlers and the daemon executor. The createChildLogger signature is extended to accept required correlation identifiers plus arbitrary additional properties, webhook handlers migrate to use createChildLogger and rename drifted field names to entityNumber, the daemon workflow executor adds entityNumber to its logging context, and observability documentation is updated to explain the new tracking capability.

Changes

Entity Number Logging Correlation

Layer / File(s)	Summary
Logger correlation contract and tests src/logger.ts, test/utils/logger.test.ts	createChildLogger signature extended to accept required fields (deliveryId, owner, repo, entityNumber) plus Record<string, unknown> for additional bindings; doc comment expanded to describe the correlation contract. Unit tests verify canonical entityNumber binding, absence of drifted field names, and preservation of arbitrary extras.
Webhook handler migrations to entityNumber src/webhook/events/issue-comment.ts, src/webhook/events/issues.ts, src/webhook/events/pull-request.ts, src/webhook/events/review-comment.ts, src/webhook/events/review-thread.ts, src/webhook/events/review.ts	Five webhook event handlers migrate from logger.child to createChildLogger and rename entity identifier fields from handler-specific names (issueNumber, prNumber) to canonical entityNumber; imports and logger construction calls updated consistently.
Daemon workflow executor entity tracking src/daemon/workflow-executor.ts	Structured logging context in executeWorkflowRun extended with top-level entityNumber field (copied from context.entityNumber) alongside existing target envelope to support per-entity searching.
Observability documentation docs/operate/observability.md	Documentation updated to describe entityNumber as canonical issue/PR identifier emitted by webhook handlers and daemon executor for entity-scoped aggregate reporting; new table row distinguishes entityNumber from handler-specific (prNumber, issueNumber) and scoped-rail (pr_number, issue_number) naming variants.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

• #177: Complementary change requesting installationId addition to the same createChildLogger bindings for installation-scoped filtering; both address per-request logger correlation standardization.

Suggested labels

type: docs 📋

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Linked Issues check	❓ Inconclusive	The PR addresses most core requirements from #175 but partially deviates on installationId (deferred) and compatibility mirrors (not implemented).	Verify whether deferring installationId to #177 and omitting compatibility mirrors are acceptable trade-offs, or whether these are blocking requirements for #175 closure.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: canonicalizing the entity identifier field across loggers to use a single 'entityNumber' field.
Out of Scope Changes check	✅ Passed	All changes are directly related to #175: canonicalizing entityNumber across handlers, extending createChildLogger, updating docs, and adding tests.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

Canonicalizes the per-request entity identifier across webhook handlers and the workflow executor under a single entityNumber field so operators can grep one field name to reconstruct a request end-to-end, resolving the field-name drift described in issue #175.

Changes:

• Widen createChildLogger to accept the four canonical fields plus arbitrary extra bindings, and route 4 webhook handlers (issues, pull-request, issue-comment, review-comment) through it, replacing issueNumber/prNumber with entityNumber. Also add entityNumber on review.ts / review-thread.ts log lines and a flat entityNumber (alongside nested target) in workflow-executor.ts.
• Update docs/operate/observability.md lede and Common log fields table with the entityNumber row.
• Add tests in test/utils/logger.test.ts for the bindings contract.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/logger.ts	Widens createChildLogger signature with & Record<string, unknown> and expands the contract docstring.
src/webhook/events/issues.ts	Routes labeled-handler through createChildLogger and renames issueNumber → entityNumber.
src/webhook/events/pull-request.ts	Same migration for pull_request.labeled (prNumber → entityNumber).
src/webhook/events/issue-comment.ts	Migration plus comment noting PR comments share the canonical field.
src/webhook/events/review-comment.ts	Migration and rename for review-comment handler.
src/webhook/events/review.ts	Adds entityNumber to the per-request log line.
src/webhook/events/review-thread.ts	Adds entityNumber to the per-request log line.
src/daemon/workflow-executor.ts	Adds flat entityNumber from context.entityNumber next to nested target.
docs/operate/observability.md	Updates lede and Common log fields table with entityNumber row.
test/utils/logger.test.ts	New tests for canonical bindings and extras passthrough.