Skip to content

Common Webhook — Future brief

A running list of "what I'd do differently" observations about the common webhook system. Sandy's running notes, not factual claims about Avoca's plans. Updated as Sandy works each migration.

How to read this

This page contains opinions, not facts. The factual mechanics are in architecture.md and legacy-comparison.md. Use this page to (1) understand what improvements I think are worth advocating for, (2) accumulate new observations as I do migrations, (3) draft PR-ready arguments before raising changes with the Avoca team.

Unsolicited Opinion

The tension this page preserves: I'm new to this codebase and biased toward seeing improvement opportunities. The "leaving a lot on the table" feeling is real, but my mandate is to execute migrations fast in the existing brittle shape. This page is where that improvement-thinking accumulates without slowing down individual migrations. When an observation gathers enough weight (across multiple migrations, with concrete cost evidence) it gets promoted to a flagpole-ready argument.


Convention for new entries

Every observation has four fields:

  • Problem — the concrete thing observed in the codebase
  • Why it matters — cost evidence: friction during migrations, drift risk, observability gap, etc.
  • Future shape — proposed direction (not a full design)
  • Statusjust-an-observation / runnable-up-the-flagpole / actively-discussed / actioned / rejected

When status promotes to runnable-up-the-flagpole, a PR-ready paragraph gets added under § PR-ready arguments.


Observations seeded from PR AvocaAI/avoca-next#10249

The 12 observations below were collected during the initial code-read while preparing this section. They're the starting set — entries 13+ get added as Sandy does subsequent migrations.

obs-1 — Three coexisting CRM-dispatch patterns in HomeServicesWorkflowRunTriager

Problem. HomeServicesWorkflowRunTriager.executeOutcome has three different patterns for CRM-specific behavior (ALBIWARE outer + single-outcome, HCP-incoming outer + internal type switch, FOUR_SEASONS per-case inline), plus an implicit fourth (SERVICE_TITAN default → generic stage which has its own inline ACCULYNX branches). A reader has to know about all four to predict what runs for any (CRM, outcome) pair.

Why it matters. Each new CRM migration requires deciding which pattern to use, then replicating it. Reviewers can't grep for one signature to find all CRM-specific code paths.

Future shape. Each CRM declares (a) its vertical, (b) its supported outcome types as a set, (c) its handler module. The triager becomes a lookup table: CRM_REGISTRY[crm][outcomeType]. Unsupported types are data, not switch defaults. Sandy: this is probably the highest-value structural improvement.

Status. just-an-observation — needs at least one more migration's worth of evidence before promoting.

obs-2 — Adapter pattern duplicates leaf-function call sites

Problem. hcpJobBooker is called from two places (legacy hcpWorkflow and new run-booking-hcp.ts) with slightly different but equivalent argument constructions. Both reach the same leaf. The "adaptation" is identity-ish in most of the call surface.

Why it matters. Drift risk: a fix in one path doesn't propagate. Copy-paste tax: every migration adds another callsite.

Future shape. Leaf functions take a canonical BookingContext (or RescheduleContext, etc.) struct. Both legacy webhook (during sunset) and new triager (going forward) construct that shape from their respective inputs. The adapter package becomes one-line forwarders.

Status. just-an-observation — only meaningful AFTER multiple migrations confirm the call-shape similarity.

obs-3 — Classification happens twice in the legacy path

Problem. Legacy: LLM extracts is_cancellation / is_rescheduling flags inside the per-CRM monolith. Common: LLM classifies once via determineWorkflowOutcomes, leaves execute pre-classified rows.

Why it matters. Single classification is a real architectural improvement the migration already delivers. Worth flagging in PR descriptions.

Future shape. Already implemented. This obs is a positive note — celebrate the win in writing.

Status. actioned (by the migration itself, not as a separate improvement).

obs-4 — Silent error swallowing in post-call dispatch

Problem. Both legacy and new paths wrap postCallProcess, notifier, oncallV2 in try/catch and log-and-continue. The new path is strictly better (per-stage catch instead of throw-past). But if notifier breaks for all migrated teams at once, the only signal is Datadog noise.

Why it matters. No per-stage success field on outcome_results. Fleet-wide regressions take longer to detect than they should.

Future shape. Add notifier_succeeded, postcall_processed, oncall_dispatched boolean columns (or a JSON stage_results) on outcome_results. Surface fleet-wide success rates in Datadog. Alert on regression.

Status. just-an-observation — needs a regression incident before it has urgency.

obs-5 — Per-CRM outcome support is ad-hoc

Problem. HCP's "doesn't support eta / job-notes / etc." is encoded as switch cases that return {skipped: true}. AutoOps's vertical has its own determineAutoServiceWorkflowOutcomes with a different outcome vocabulary entirely.

Why it matters. New outcome types added upstream don't propagate cleanly. CRM capabilities are scattered across switch cases instead of being declared somewhere queryable.

Future shape. CRM declares its capabilities in a manifest: HCP_CAPABILITIES = { outcomes: ['booking', 'message', 'rescheduling', 'cancellation'] }. determineWorkflowOutcomes reads the manifest and only classifies into supported types. The skipped-because-unsupported branches go away.

Status. just-an-observation — pairs naturally with obs-1.

obs-6 — Vertical/CRM matrix is sparse

Problem. Of 5 specialized verticals, 4 support exactly one CRM. Only HOME_SERVICES is multi-CRM.

Why it matters. The vertical abstraction adds a layer of indirection that doesn't pay for itself when 4/5 verticals are single-CRM. The matrix could collapse to a flat (CRM, outcome-set) lookup.

Future shape. Verticals exist because the action vocabulary differs. But (vertical, CRM, outcome-set) triples could collapse to (CRM, outcome-set) with vertical inferred. Especially if obs-5's manifest lands.

Status. just-an-observation — the inverse case (a new vertical with multiple CRMs) would invalidate this direction.

obs-7 — No shared result contract

Problem. Each booking-* package defines its own *BookingResult, *CallContext, *WorkflowResult types. The triager's WorkflowTriageResult is a contract but its results: unknown[] is anything-goes. Downstream consumers duck-type.

Why it matters. Adding a new downstream consumer (a per-call dashboard, a metrics extractor, a callback) means writing CRM-specific code paths instead of querying a canonical shape.

Future shape. CrmActionResult discriminated union (booked | rescheduled | cancelled | skipped | failed) with common fields (jobId?, customerId?, error?). All CRMs return it. The legacy *WorkflowResult types become CRM-specific extensions of the discriminated union.

Status. just-an-observation — relatively low cost / low blast radius, could be done incrementally.

obs-8 — common/webhook.ts vs common/workflow.ts name collision

Problem. pages/api/responder/common/webhook.ts is a LEGACY ServiceTitan-only single-shot route. pages/api/responder/common/workflow.ts is the NEW unified entry. Same directory, adjacent file names, opposite meanings. A grep for "common/webhook" produces misleading results.

Why it matters. Onboarding friction. Code-review confusion. Risk that someone routes a new team at the legacy route by mistake.

Future shape. Rename or delete the legacy one. If any team's serverUrl still hits it, that team is itself a migration candidate.

Status. runnable-up-the-flagpole — small change, immediate clarity win. See PR-ready argument below.

obs-9 — ACCULYNX is half-migrated

Problem. Legacy webhook exists at pages/api/responder/non-st-crm/acculynx/webhook.ts. lib/non-st-workflow/acculynx/run/handler.ts exists. But NO booking-acculynx/ package — instead, ACCULYNX has inline if (crm === 'ACCULYNX') branches in booking/booking-triage.ts (the generic ServiceTitan-shaped pipeline).

Why it matters. A third state beyond fully-legacy and fully-migrated. When someone says "migrate ACCULYNX," the answer is "ACCULYNX is partly migrated already and the existing branches probably cover the booking path." The mental model has to bend to accommodate this.

Future shape. Pick one: either remove the inline branches and write a proper booking-acculynx/ package, OR accept the inline-branches shape as canonical and sunset only the legacy webhook. Document the choice as the "ACCULYNX pattern" if accepted.

Status. just-an-observation — won't have urgency until ACCULYNX is on the migration backlog explicitly.

obs-10 — OASIS lives in both worlds

Problem. Legacy webhook at pages/api/responder/non-st-crm/oasis/webhook.ts exists AND booking-oasis/ package exists AND FlooringWorkflowRunTriager dispatches to oasis. The legacy oasis has no run/handler.ts (so probably only handles assistant-request).

Why it matters. Mental-model confusion. Risk of routing changes breaking one path without the other.

Future shape. Confirm with team why oasis is dual-state. Likely some message types still route via legacy while others go through common. Document the split if intentional; consolidate if accidental.

Status. just-an-observation — open question, not yet an improvement argument.

obs-11 — Hardcoded CRM key in legacy getVoiceAgent(team, 'hcp')

Problem. The string 'hcp' is wired into the URL path AND into the getVoiceAgent call inside the legacy webhook. The common webhook resolves the CRM from typedConfig instead.

Why it matters. When sunsetting a legacy webhook, hardcoded literals are easy to miss. Need to grep for the literal before deletion.

Future shape. Migration playbook should include "grep for the legacy CRM-key literal before sunsetting the legacy webhook" as a step.

Status. actioned — added to migration-playbook off-ramp section as a check.

obs-12 — No per-CRM tests in the new packages (except autoops)

Problem. PR #10249 adds zero tests. The HCP PR's "Test Plan" section in the description is blank. Hamming is the testing surface for voice but there's no automated regression net for the migration. A migration that breaks runHcpBookingWorkflow will be caught by Datadog errors or a real call going wrong, not by CI.

Why it matters. Fleet-wide regressions take longer to catch. New CRMs add their own untested code. The pattern signals "tests are optional" which compounds.

Future shape. Per-package contract tests against mock CRM clients. At minimum, booking-{crm} writes the expected outcome_results payload shape, and unsupported outcomes return the expected skipped result. Propose this in the next migration PR's description and see if it sticks.

Status. runnable-up-the-flagpole — small cost (1-2 tests per package), demonstrable value (catches the obvious regression class).


PR-ready arguments

Short paragraphs Sandy can drop into Slack or PR comments to advocate for specific changes.

For obs-8 (common/webhook.ts rename)

Quick suggestion: rename or delete pages/api/responder/common/webhook.ts. It's the legacy ST single-shot route, but its file path is one character off from common/workflow.ts (the unified webhook). A grep for "common/webhook" produces misleading results, and the side-by-side names make code review harder than it needs to be. If any team's server.url still hits the legacy route, that team is itself a migration candidate; if no teams hit it, it's deletable. Either way, the name collision is gratuitous.

For obs-12 (per-package contract tests)

Proposal: add a minimal contract test per booking-{crm}/ package. The goal isn't full coverage; it's catching the obvious regression class — "the dispatch block returns the expected outcome shape" and "unsupported outcomes return {skipped: true, reason: ...}". Two tests per package, runs in CI. Right now Hamming + manual smoke + Datadog are the safety net, which works for catching a regression in production but means we don't catch them at PR time. As CRMs accumulate, missing per-package tests compounds.


Working notes (Sandy adds here as migrations happen)

DateMigrationObservation addedStatus
2026-05-12HCP (PR #10249) reviewobs-1 through obs-12 seededvarious