feat(case): SLA policy matrix per priority × account tier, single first-response writer (#595) - #1071
Merged
Merged
Conversation
…st-response writer (#595) The app's whole SLA logic was one line — `critical` ⇒ now + 4h — so High, Medium and Low cases got no `sla_due_date` at all. `case_sla_monitor` selects cases whose due date is in the past, and a blank date never is, so three of the four priorities could not breach: not because the sweep excluded them, but because they had no deadline to miss. `crm_account.tier`, the obvious driver, was read by nothing outside the account view. `case_sla_defaults` now stamps every case with a recognised priority from a sixteen-cell priority × tier table. The critical row is flat at 4h so the change is a strict superset of the old rule — nothing loses a clock, three priorities gain one — and no cell is looser than the per-priority target the docs already published. An unreadable or unclassified account falls back to the `smb` column rather than inventing a tighter deadline out of a permission error. The hours are CALENDAR hours: there is no business-hours calendar on this platform, and that is now stated in `_case-sla.ts`, beside the numbers in the hook, on the field, and across the SLA / cases / setup / FAQ / glossary pages in all three doc locales. The offset is added as elapsed milliseconds rather than `setHours(getHours() + n)`, which does local calendar arithmetic and silently turns "+4 hours" into 3 or 5 across a DST transition. `crm_case.first_response_date` moves from the log_call / log_meeting action body — which only stamped for those two buttons — to `event_activity_bubble`, which already fires on a `crm_event` entering `held` and already resolves `related_to_case`. One writer, every path. A status change and a merely booked meeting are still deliberately not first responses. Seeded case due dates are derived from the matrix and the seeded account's tier instead of hand-typed, and re-derived by test/seed-consistency.test.ts. Escalation reassignment is deferred, with a follow-up issue. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DDicE5RSpuckkUeUSrqXN6
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
huangyiirene
marked this pull request as ready for review
August 11, 2026 06:48
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #595
Description
The app's entire SLA logic was one line —
critical⇒sla_due_date = now + 4h— so High, Medium and Low cases got no due date at all. That was not cosmetic:case_sla_monitorselects cases whosesla_due_dateis in the past, and a blank date never is, so three of the four priorities could not breach — not because the sweep excluded them, but because they had no deadline to miss.crm_account.tier, the obvious driver, was read by nothing outsidesrc/views/account.view.ts.This implements the minimal version ruled on in the issue: the matrix, first-response stamping on every real touchpoint, and the calendar-hours assumption stated out loud. Escalation reassignment is deferred — see below.
Type of Change
Related Issues
Fixes #595
Related to #1070 (the deferred escalation-reassignment half, filed by this PR)
Changes Made
The matrix.
case_sla_defaultsstampssla_due_dateon every case with a recognised priority, from a sixteen-cell table of hours:ctx.api.smbcolumn, which is the tier field's owndefault. Erring loose is deliberate — a deadline invented tighter out of a permission error would manufacture breaches, and anonymous web-to-case (which can create a case and read nothing else) is the ordinary example. An unrecognised priority still gets no due date rather than a guessed one.src/objects/_case-sla.ts(imported by the seed generator, which needs real numbers at authoring time) and a hand-copied mirror inside the hook body, because L2 hook bodies run body-only in the QuickJS sandbox and a module constant arrives there asundefined. Same forced duplication as thepriority_rankmaps.setHours(getHours() + n)— the latter does local calendar arithmetic, so on a DST-observing host "+4 hours" silently becomes 3 or 5, and a 168-hour clock crosses a transition twice a year by construction.Calendar hours, stated explicitly (scope item 3). There is no business-hours calendar on this platform. It is now said in
_case-sla.ts, in the hook body beside the numbers, on thesla_due_datefield, insrc/docs/crm_service.md, and across the SLA / cases / setup / FAQ / glossary pages in all three doc locales (en, zh-Hans, zh-Hant) — 15 content files, because those pages carried detailed "only Critical has a clock behind it" claims that this change makes false.test/case-sla-matrix.test.tsasserts the statement is present in both source files, so it cannot be deleted quietly.First response (scope item 2).
crm_case.first_response_datewas stamped from thelog_call/log_meetingaction body, so it was written only when an interaction was recorded through those two buttons; an event created any other way left the metric null, under a comment asking every future author to remember to stamp it too. The stamp moves toevent_activity_bubble(src/objects/event.hook.ts), which already fires on exactly the right condition — acrm_eventon its transition intoheld— and already resolvesrelated_to_case. Both actions still stamp the case, because their body writes the event this hook watches; they just no longer each carry a copy of the rule. One writer, every path, including interactions entered straight onto the case's activity list, which previously stamped nothing. Two writers racing on a "first" timestamp is how it becomes a "last" one, so the old copy is removed rather than kept as a belt-and-braces duplicate.Two things are still deliberately not a first response, and the issue's scope note lists one of them as a candidate: a status change (an agent can move a case to In Progress and investigate for an hour while the customer hears nothing — a status-derived number reports a response that never happened; this was a documented, rejected non-goal in #575 B2 and remains one) and a meeting merely booked, which is
plannedrather than held.Seeds (acceptance criterion 1). Seeded case
sla_due_datevalues were eight hand-typed numbers picked to look plausible. They are now derived —created_date + matrix(priority, tier), with the tier read off the account seed itself rather than copied into a second table — expressed asdaysAgo(n) + duration('Nh')(verifiedduration()arithmetic evaluates in this CEL engine;daysAgo()is a UTC midnight, so the hour offset has to ride on top of it).test/seed-consistency.test.tsre-derives all 38 of them.is_sla_violatedis deliberately not re-derived: it iscase_sla_monitor's field to write, and the seed only pre-sets it where the demo wants a breach visible before the first sweep. The test does check that every pre-set breach is one the sweep's own definition would produce.Escalation reassignment: DEFERRED, filed as #1070
Deferred, not simplified. The blocker is not the flow-template dot-walk — a hook can do what the flow cannot, and
case_status_side_effectsalready runs on the escalation transition withctx.apiin hand. It is three separate decisions that each belong to their own change: writing another user'sowner_idis a transfer and needscrm_case.allowTransferon the agent profile (a permission-model widening, not a hook change); the write lands in anafterUpdatehook on the record that just fired the record-change trigger surface, which has produced an escalation loop in this exact file before; and an unstaffedservice_managerpool needs a defined no-op plus demo staffing to be demonstrable at all. Bolting that onto a card already touching the hook, the event hook, an action body, the seeds and 17 doc files would have made both halves harder to review. #1070 carries the full analysis and acceptance criteria.Testing
pnpm test) — 82 files, 1939 passed, 1 skipped, 0 failedpnpm lint) — exit 0pnpm build) — exit 0pnpm validate— exit 0 ·pnpm typecheck— exit 0 ·pnpm hygiene— exit 0Verified against rc.6. The clone built on rc.5;
git fetch origin main && git merge origin/mainbrought in #1066 (rc.5 → rc.6) and #1068, dependencies were reinstalled from the merged lockfile, and every gate above was re-run on rc.6. Nofilter: {}and no$regexare authored anywhere in this change (the only new metadata is a hook body and seed values).The matrix pin fails when a cell changes — proven, not asserted
test/case-sla-matrix.test.tspins all sixteen cells by driving the shipped handler, and separately asserts the exported constant, so neither copy of the table can move alone. Both failure modes were reproduced by temporarily editinghigh × smbfrom 8 to 12 and reverted:high×smb: expected 8h, got 12.000handdenied read: expected 8h, got 12.000hconstant cell high×smb: expected 12 to be 8(the longhand expectation table, which is deliberately not derived from the constant under test) plus the same behavioural failureAcceptance criterion 2, as a runtime test rather than reasoning
test/flow-scheduled.test.tsgainscase_sla_monitor — non-critical breaches (#595), which runs both halves for real: the shippedcase_sla_defaultshandler stamps the deadline with the clock wound back to the moment of creation, then the shippedcase_sla_monitorflow sweeps it through the realAutomationEngine. Parameterised overhigh × strategic,high × enterprise,medium × mid_marketandlow × smb, each assertingis_sla_violated,is_escalated,status: escalated, a non-emptyescalation_reasonand exactly one owner notification. No hand-written date anywhere in the test — the matrix's own offset is what puts the case past due, so the test stays honest if a cell changes. The negative half (a High case still inside its window is left alone) is asserted too.Other test changes
test/case-first-response.test.tsrewritten for the hook path, keeping the metadata assertions and the rejected-non-goal cases (booked meeting, cancelled/no-show, already-held event, event on another object), and adding a guard that fails if either action body re-grows its ownfirst_response_datewrite.test/hook-write-shape.test.ts— the repo's own guard caught the newupdate()call site immediately (11 sites but this file exercises 10); a case was added, which also runs the new hook code through the real QuickJS sandbox and proves the derived write reaches the engine in(document, options)shape.test/hooks-runtime-service.test.ts—does not give a non-critical case an SLAwas the assertion encoding the defect; it now asserts the opposite, with a note explaining why it flipped.Checklist
.changeset/case-sla-policy-matrix.md(minor)Additional Notes
Behaviour change worth flagging at review. Priorities that used to sit silently now escalate on time — which is the point of the card, but it means
Escalated Caseswill fill with more than Critical work, and the demo dataset's older open cases are past due on arrival (a 30-day-old open case has missed any deadline in this table; that is what a breach is, and the hourly sweep is the thing that notices). The docs now say this in the when escalation triggers section rather than leaving it to be discovered.Not touched, per the dispatch constraints:
package.json/ lockfiles (beyond what theorigin/mainmerge brought in),src/objects/quote.object.ts,e2e/**.Generated by Claude Code