feat(webhook): add incident.io webhook flow with parallel reconciler path

[sarahjanenewnham]

Summary

Adds a new POST /webhooks/incident-io endpoint and a parallel reconciler
entry point (Reconciler.ProcessIncidentEvent) for handling incident.io
webhook events. The new flow bypasses the SCM ticketing pipeline and the
RepoURL gate entirely — the controller does not require a repository
or merge-request lifecycle for these events. The endpoint is intended
for triage and similar use cases where the agent reads context and
posts to chat rather than shipping code.

The new flow is intentionally implemented in parallel to the existing
ticketing flow rather than refactoring both behind a common interface.
A future change will lift ProcessTicket and ProcessIncidentEvent
behind a shared use-case abstraction; this PR introduces the second
consumer first so that abstraction can be designed against two real
consumers rather than speculatively.

Scope of this PR — receiver only; dispatch deferred

This PR ships the receiver half of the new flow:

• HTTP endpoint, signature verification, payload parsing, error handling
• Type definitions and the IncidentHandler interface
• Configuration schema (webhook.incident_io and incident_triage
  blocks)
• Server wiring (WithIncidentConfig, WithIncidentHandler options,
  the new route in NewServer)
• The controller adapter in cmd/osmia/main.go
• Documentation, CHANGELOG entries, and tests

It does not ship a working dispatch path. Reconciler.ProcessIncidentEvent
is a structured-log no-op that returns nil, which causes the webhook
server to respond 200 OK without spawning an agent Job. The dispatch
logic (engine selection, TaskRun creation, K8s Job launch, stream
reader) arrives in a follow-up change paired with the operator-side
wiring — specifically, a classifier skill and the helm values that
mount it as a ConfigMap and set incident_triage.append_system_prompt.
Running the agent without a configured classifier would produce
unstructured output, so deferring dispatch until that wiring exists
is the safer state to ship.

What's added

• POST /webhooks/incident-io — typed handler for
  public_incident.incident_created_v2 and
  public_incident.incident_status_updated_v2. Verifies Svix
  signatures, parses the event-namespaced wrapper key, and dispatches
  to a configured IncidentHandler.
• Two new YAML config blocks: webhook.incident_io (sibling of
  webhook.generic, currently auth_mode: svix only) and a top-level
  incident_triage block carrying the engine name (default
  claude-code) and an append_system_prompt override that, once
  dispatch is restored, will be applied per-call to the engine's
  EngineConfig.
• Reconciler.ProcessIncidentEvent — currently a structured-log
  no-op as described above. Future changes will restore the dispatch
  body.
• Shared HTTP helpers on Server: readRequestBody and
  verifySvixSignature. The svix branch of handleGeneric now uses
  these too.
• Documentation: new "incident.io" section in
  docs/api/webhooks.md covering the endpoint, authentication,
  supported events, dispatch configuration, and example payload, with
  a "Dispatch is currently disabled" admonition. Also backfills the
  previously-undocumented auth_mode: svix row in the Generic HTTP
  authentication table.

Behavioural change for existing deployments

Without the new YAML blocks, the new endpoint and reconciler entry
point are inert; existing deployments are unaffected.

One observable change in handleGeneric's svix branch logging: the
message "invalid generic webhook svix signature" is now
"invalid svix signature" with source=generic as a structured slog
field, and similarly for the "not prepared" error message. Anyone
alerting on the literal old strings should migrate to the structured
field. HTTP behaviour (status codes, response bodies) is unchanged —
the existing TestHandleGeneric_* tests pass without modification.

Known limitation (future)

When the dispatch path is restored, completed incident TaskRuns will
trigger handleJobComplete, which calls ticketing.MarkComplete with
the incident ID as tr.TicketID. The configured ticketing backend
will not recognise the ID and will log a "not found" error. Logged
but non-fatal. The clean fix requires a use-case-aware completion-
handler dispatch, which the upcoming abstraction will provide. Not
relevant in this PR because the dispatch path is disabled.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update
• Plugin addition

Checklist

• Code follows the project's code style (gofmt, go vet)
• Tests added for new functionality across types, parser, handler,
  and config parsing / validation; the controller-side dispatch
  tests will be added alongside the dispatch path itself in the
  follow-up change
• All tests pass locally (go test ./...)
• CHANGELOG entries added under "Unreleased"
• Documentation updated: new "incident.io" section in
  docs/api/webhooks.md

Related Issues

N/A

Summary by CodeRabbit

• New Features

  • Added POST /webhooks/incident-io endpoint (receives and verifies incident.io events; deliveries are acknowledged and logged; dispatch to agent jobs is currently disabled)
  • New incident triage configuration (engine selection and optional system prompt)

• Improvements

  • Slack notifications now update in-place and reuse threads to reduce noise
  • Generic webhook Svix signature failures now produce clearer, structured logs

• Documentation

  • Webhook API and auth modes (svix) documented and exemplified

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 55222943-2bf4-4da8-bb60-70ca6f6b751c

📥 Commits

Reviewing files that changed from the base of the PR and between e6963d2 and 0812cb7.

📒 Files selected for processing (3)

• docs/api/webhooks.md
• internal/webhook/incident.go
• internal/webhook/incident_test.go

✅ Files skipped from review due to trivial changes (1)

• docs/api/webhooks.md

🚧 Files skipped from review as they are similar to previous changes (2)

• internal/webhook/incident_test.go
• internal/webhook/incident.go

---

Walkthrough

Adds an incident.io webhook receiver: new YAML config and validation, Svix verifier preparation and centralized verification, typed event parsing, HTTP handler POST /webhooks/incident-io, shared request helpers, application wiring, tests/docs/changelog; reconciler entrypoint logs events and currently does not dispatch agent jobs.

Changes

incident.io Webhook Integration

Layer / File(s)	Summary
Configuration & Validation internal/config/config.go, internal/config/validate.go, internal/config/config_test.go	Adds webhook.incident_io and incident_triage YAML structures; validation enforces auth_mode: svix and required secret; config loader tests added.
Webhook Models & Parsing internal/webhook/incident.go	Introduces IncidentEvent, typed schemas, event constants (public_incident.incident_created_v2, public_incident.incident_status_updated_v2), IncidentIOConfig.Prepare() for Svix verifier setup, and ParseIncidentEvent() for typed payload parsing and validation.
Webhook Server Integration internal/webhook/server.go, internal/webhook/incident.go	Adds server options WithIncidentConfig and WithIncidentHandler, registers POST /webhooks/incident-io, implements handleIncidentIO, and extracts readRequestBody and verifySvixSignature helpers.
Generic Webhook Refactoring internal/webhook/generic.go	Generic handler now reuses readRequestBody and verifySvixSignature for Svix verification and body reading.
Application Wiring cmd/osmia/main.go	Conditional registration of incident.io config and incidentAdapter that forwards parsed events to reconciler.ProcessIncidentEvent().
Controller Reconciler internal/controller/incident.go	ProcessIncidentEvent() reduced to structured logging of received incident events and returns nil; prior dispatch/Job creation logic removed.
Config Tests internal/config/config_test.go	Added tests: TestLoad_IncidentIOWebhookConfig and TestLoad_IncidentIOWebhookValidation.
Webhook Parsing & Server Tests internal/webhook/incident_test.go	Added ParseIncidentEvent unit tests and extensive HTTP tests covering Svix signatures, malformed payloads, handler error paths, and IncidentIOConfig.Prepare validation.
Controller Tests internal/controller/incident_test.go	Added TestProcessIncidentEvent_DispatchDisabled asserting no Jobs created and no errors returned.
API Documentation docs/api/webhooks.md	Documents /webhooks/incident-io, Svix-only auth configuration, supported event types, triage config, examples, and adds svix to generic auth modes.
Changelog CHANGELOG.md	Records generic webhook verification logging change, Slack notification behavior changes, and incident.io receiver dispatch-disabled note.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• unitaryai/osmia#39: Adds Svix authentication support to webhook package; both PRs extend Svix verifier preparation and verification helper logic.

Suggested reviewers

• nikoul
• mattcamp

Poem

🐰 A rabbit sniffs the log tonight,
Svix-signed whispers parsed just right,
Handlers listen, adapters hum,
Events arrive — no jobs to run,
Carrots clap for tests that bite.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 48.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately describes the primary change: adding incident.io webhook support with a parallel reconciler path, matching the main objectives documented in the PR.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/incident-io-webhook-flow

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

internal/webhook/incident_test.go (1)

90-152: ⚡ Quick win

Make the parser happy-path coverage table-driven.

These two success-path tests exercise the same ParseIncidentEvent contract with different fixtures. Folding them into one table with t.Run will keep the suite consistent and make the next supported incident event cheaper to add.

As per coding guidelines, "Use table-driven tests with subtests (t.Run) in Go unit tests".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/webhook/incident_test.go` around lines 90 - 152, Combine the two
near-duplicate tests TestParseIncidentEvent_CreatedV2 and
TestParseIncidentEvent_StatusUpdatedV2 into a single table-driven test (e.g.,
TestParseIncidentEvent_Table) that iterates over cases containing name, payload
(incidentCreatedV2Payload / incidentStatusUpdatedV2Payload) and expected
assertions, and use t.Run for each case; call ParseIncidentEvent inside the loop
and perform the same assertions previously in each test (EventType, Incident
fields, Message/NewStatus/PreviousStatus, and Raw JSON for the created case)
while branching on case-specific expectations so adding new fixtures becomes a
new table entry.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/webhook/incident.go`:
- Around line 253-261: The EventIncidentStatusUpdatedV2 branch must reject
payloads missing required transition fields; after unmarshalling into
incidentStatusUpdatedBody in the EventIncidentStatusUpdatedV2 case, validate
that w.NewStatus and w.PreviousStatus are present (non-zero/empty) and return a
descriptive error (e.g., fmt.Errorf("decoding %s body: missing new_status" or
"missing previous_status")) instead of proceeding to set
evt.Incident/Message/NewStatus/PreviousStatus; update the case that handles
EventIncidentStatusUpdatedV2 to perform these checks and fail fast on malformed
payloads so only complete incident_status_updated_v2 events are accepted.

---

Nitpick comments:
In `@internal/webhook/incident_test.go`:
- Around line 90-152: Combine the two near-duplicate tests
TestParseIncidentEvent_CreatedV2 and TestParseIncidentEvent_StatusUpdatedV2 into
a single table-driven test (e.g., TestParseIncidentEvent_Table) that iterates
over cases containing name, payload (incidentCreatedV2Payload /
incidentStatusUpdatedV2Payload) and expected assertions, and use t.Run for each
case; call ParseIncidentEvent inside the loop and perform the same assertions
previously in each test (EventType, Incident fields,
Message/NewStatus/PreviousStatus, and Raw JSON for the created case) while
branching on case-specific expectations so adding new fixtures becomes a new
table entry.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b9a9455e-910a-4cd3-975d-a2fc30cc729c

📥 Commits

Reviewing files that changed from the base of the PR and between 8704d1b and 1dbfaf5.

📒 Files selected for processing (12)

• CHANGELOG.md
• cmd/osmia/main.go
• docs/api/webhooks.md
• internal/config/config.go
• internal/config/config_test.go
• internal/config/validate.go
• internal/controller/incident.go
• internal/controller/incident_test.go
• internal/webhook/generic.go
• internal/webhook/incident.go
• internal/webhook/incident_test.go
• internal/webhook/server.go

[sarahjanenewnham]

⚙️ Automated reply from Claude Code (running locally as the PR author's assistant). The author has reviewed and approved this response before posting.

Thanks for the review. Quick response on the two findings.

✅ Actionable: reject incomplete incident_status_updated_v2 payloads

Fixed in c3badcf. ParseIncidentEvent now returns a descriptive error when either new_status or previous_status is missing on this event type, matching the existing empty-incident.id guard. Two test cases added covering the new error paths.

⚡ Nitpick: fold TestParseIncidentEvent_CreatedV2 and _StatusUpdatedV2 into a table-driven test

Acknowledged but not addressing in this PR. The two tests have meaningfully different assertion shapes:

• _CreatedV2 asserts on the full IncidentV2 field set, exercises the Raw JSON round-trip, and explicitly checks that Message / NewStatus / PreviousStatus are zero-valued.
• _StatusUpdatedV2 asserts on the wrapped body — Message, NewStatus.Category, PreviousStatus.Category, plus that Severity / IncidentType are nil (because the fixture omits them).

A table-driven version would either need an assertEvent func(*testing.T, IncidentEvent) per case (which is approximately the same line count as two named tests, with worse stack traces on failure) or branching on EventType inside the loop (reduces clarity). The error-path coverage in TestParseIncidentEvent_Errors already uses the table-driven pattern where it fits cleanly — same-shape assertions across many inputs.

If we add a third event type whose happy-path assertions are shape-compatible with one of the existing two, folding becomes worthwhile. Until then I'd rather keep two named tests.

⚠️ Pre-merge warning: docstring coverage 34.48%

Investigated — every exported type, function, method, and Server field added in this PR carries a doc comment. The 34.48% appears to come from CodeRabbit counting individual struct fields and unexported test helpers. Existing Osmia types (e.g. ticketing.Ticket, engine.Task, taskrun.TaskRun) follow the same one-comment-per-type convention without per-field docs, so adding them here would be inconsistent with the codebase's style. Treating the warning as a CodeRabbit default rather than an Osmia bar.

Ready for human review.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/webhook/incident.go`:
- Around line 64-80: Prepare() can leave a stale i.svixWebhook verifier when a
later validation fails; clear the previous verifier on error so we fail closed.
In IncidentIOConfig.Prepare(), reset i.svixWebhook = nil on any validation error
return (or clear it at the start of Prepare()) and only set i.svixWebhook to the
new value after svix.NewWebhook succeeds; ensure all error branches (invalid
svix secret, empty auth_mode, unsupported auth_mode) clear the existing verifier
before returning.
- Around line 258-266: The code currently only checks w.NewStatus and
w.PreviousStatus for nil but accepts empty objects; update the validation in the
parsing/validation block that returns IncidentEvent to also ensure the nested
status ID fields are populated (e.g., check w.NewStatus.Id and
w.PreviousStatus.Id or equivalent identifier fields) and return an error with
eventType if either ID is empty/zero; this prevents accepting "{}" objects and
ensures IncidentEvent is only created when both status objects contain valid
IDs.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ea9a24b0-e131-4eea-b8a7-708a1b1f023e

📥 Commits

Reviewing files that changed from the base of the PR and between 1dbfaf5 and c3badcf.

📒 Files selected for processing (2)

• internal/webhook/incident.go
• internal/webhook/incident_test.go

✅ Files skipped from review due to trivial changes (1)

• internal/webhook/incident_test.go

[sarahjanenewnham]

⚙️ Automated reply from Claude Code (running locally as the PR author's assistant). The author has reviewed and approved this update before posting.

Heads up — scope of this PR has narrowed since opening, with the author's sign-off:

Pushed in e6963d2: the dispatch path inside Reconciler.ProcessIncidentEvent is removed for this PR. The method becomes a structured-log no-op that returns nil; the webhook still receives, verifies, parses, and acknowledges every delivery with 200 OK, but no agent Job is spawned.

Why: running the agent without a configured classifier skill produces unstructured output. The classifier skill (a markdown file in a downstream consumer repo) and the helm-side wiring that mounts it as a ConfigMap and sets incident_triage.append_system_prompt aren't ready yet. Merging dispatch before they exist would put the agent in a worse state than the no-op. The dispatch logic will be restored in a follow-up Osmia PR paired with that operator-side wiring.

What's unchanged: receiver-side infrastructure (route, signature verification, parser, types, config schema, adapter, server wiring, docs, CHANGELOG) all stays. That's the immediate value of this PR — incident.io webhook subscriptions can be configured against /webhooks/incident-io against any deployment that takes this in.

What's gone: ~330 LOC of dispatch path code in internal/controller/incident.go (engine selection, TaskRun creation, Job launch, stream reader) and the corresponding tests. PR description has been updated to reflect the receiver-only scope.

CodeRabbit's previous actionable finding (reject incomplete incident_status_updated_v2 payloads, c3badcf) is unaffected — it's about parser-side validation and stays in.

Ready for human re-review.

[coderabbitai]

🧹 Nitpick comments (1)

docs/api/webhooks.md (1)

336-338: 💤 Low value

Add language specifier to fenced code block.

The static analysis tool flagged this code block for missing a language specifier. Adding one improves rendering consistency and silences the linter.

📝 Suggested fix

-```
+```http
 POST /webhooks/incident-io

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @docs/api/webhooks.md around lines 336 - 338, Add the missing language
specifier to the fenced code block that contains the HTTP example "POST
/webhooks/incident-io" so the linter stops flagging it; replace the
triple-backtick fence before that line with a language-tagged fence (e.g.,

rendering in the docs.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@docs/api/webhooks.md`:
- Around line 336-338: Add the missing language specifier to the fenced code
block that contains the HTTP example "POST /webhooks/incident-io" so the linter
stops flagging it; replace the triple-backtick fence before that line with a
language-tagged fence (e.g., ```http) and keep the closing triple-backticks
as-is to ensure consistent rendering in the docs.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 44248b3b-fa26-461f-bd76-dd54f079ab4a

📥 Commits

Reviewing files that changed from the base of the PR and between c3badcf and e6963d2.

📒 Files selected for processing (4)

• CHANGELOG.md
• docs/api/webhooks.md
• internal/controller/incident.go
• internal/controller/incident_test.go

[sarahjanenewnham]

⚙️ Automated reply from Claude Code (running locally as the PR author's assistant). The author has reviewed and approved this update before posting.

Applied all three remaining findings in 0812cb7:

✅ internal/webhook/incident.go:80 — Fail closed when Prepare() validation fails

IncidentIOConfig.Prepare() now clears i.svixWebhook = nil at the start, so any error branch leaves the config in a fail-closed state. Added TestIncidentIOConfig_PrepareFailsClosed asserting that a successful Prepare followed by a failed re-prepare leaves the verifier nil.

✅ internal/webhook/incident.go:266 — Reject empty status objects as malformed input

The status-updated branch now also requires non-empty new_status.id and previous_status.id, mirroring the existing Incident.ID == "" guard for the created-v2 path. Two new test cases (empty new_status object, empty previous_status object) cover the {} deserialisation edge case the previous nil-only check missed.

✅ docs/api/webhooks.md:336-338 — Add language specifier to fenced code block

Added http language tag to the POST /webhooks/incident-io fenced block.

The earlier inline comment about rejecting incomplete incident_status_updated_v2 payloads was already addressed in c3badcf; flagging in case the comment thread looks unresolved.

Ready for human re-review.