Repository navigation
SLA policy matrix: per-priority × tier clocks, first-response stamping, escalation that reassigns #595
Description
Activity
- added 7 commits that reference this issue
on Aug 10, 2026 Ruling (maintainer, 2026-08-11, PM chat, verbatim: 「接受你的全部建议」): implement, minimal version. This is not capability expansion —
sla_due_date,case_sla_monitorandcrm_account.tierare all shipped surfaces, and measurement shows 3 of 4 priorities get no SLA clock at all (declared-but-unenforced). Scope of the minimal version: the priority × tier matrix (config constant pinned by a test) + first-response stamping on all real touchpoints + the calendar-hours assumption stated explicitly. The escalation-reassignment half is simplified or deferred (platform dot-walk limitation) — the dev states which in the PR; if deferred, it gets its own follow-up card, not silence.Queued (M). Serial constraint: this card and #596 both land in
case.hook.ts— #595 dispatches first, #596 follows in a later round.
Generated by Claude Code
Claim: PM loop round 3 (hotcrm whole-repo seat)
Session:session_01NM6o28jmBgsyTRQHutn7LC
Branch:claude/issue-595-sla-policy-matrix
Worktree: dedicated cloud session (fresh clone)
Domain: hotcrm whole-repo seat
File surface:src/objects/case.hook.ts,src/actions/global.actions.ts(first-response stamping),src/data/service.seed.ts(seed SLA values consistent with the matrix), case tests (test/case-first-response.test.ts+ a new matrix pin), language packs if new strings appear
Container & model: M,mode:cloud(dedicated session),model: claude-opus-5
Serial constraints cleared: #596 is deliberately held for a later round (samecase.hook.ts); in-flight #1066 (rc.6 upgrade) touchespackage.json/lockfiles/src/flows/forecast-snapshot.flow.ts/five unrelated test files — disjoint from this surface; #1017 dispatched in the same batch touchessrc/objects/quote.object.tsonly — disjoint.Path correction found by the pre-dispatch premise check (the issue body's paths are stale, the facts are not): the SLA hook is
src/objects/case.hook.ts:60, notsrc/hooks/case.hook.ts:58. Verified onorigin/main: the critical-only 4h rule stands (priority === 'critical' && !input.sla_due_date),crm_account.tieris declared (account.object.ts:344, four options) and read by nothing outsidesrc/views/account.view.ts, and theservice_managerposition exists (src/sharing/positions.ts:24).
Generated by Claude Code
{ "issue": 595, "status": "done", "branch": "claude/issue-595-sla-policy-matrix", "pr": "https://github.com/objectstack-ai/hotcrm/pull/1071", "premise_still_valid": true, "summary": "Implemented the minimal version. (1) Priority x account-tier SLA matrix (16 cells, calendar hours) stamps sla_due_date for EVERY priority; the critical row is flat at 4h across all tiers so the change is a strict superset of the old critical-only rule (differentiating that row by tier would have LOOSENED the deadline on non-strategic critical cases, i.e. taken a clock away from work that already had one — the ruling's stated goal, 'superset', is only literally true this way; strategic/critical is 4h as instructed). Tier is resolved via ctx.api from crm_account.tier; unreadable/blank/unknown tier falls back to the smb column (the field's own default), because inventing a tighter deadline out of a permission error would manufacture breaches. Unknown priority still gets no clock. The lookup is skipped entirely when a row does not vary by tier, so the critical path is byte-for-byte the old behaviour. (2) first_response_date moved out of the log_call/log_meeting action body into event_activity_bubble (src/objects/event.hook.ts), which fires on any crm_event entering 'held' with related_to_case — one writer covering every outbound touchpoint including events entered directly, replacing the old 'CONVENTION: remember to stamp this too' comment. Status transitions remain deliberately excluded (documented, previously-rejected non-goal: an agent can move a case to in_progress and investigate for an hour while the customer hears nothing); booked-but-not-held meetings likewise. (3) Calendar-hours assumption stated in _case-sla.ts, in the hook body beside the numbers, on the sla_due_date field, in src/docs/crm_service.md and across the SLA/cases/setup/FAQ/glossary pages in all three locales (15 content files) — those pages carried detailed 'only Critical has a clock' claims this change makes false. Also switched the offset from setHours(getHours()+n) to elapsed milliseconds: local calendar arithmetic turns '+4 hours' into 3 or 5 across a DST transition, and a 168h clock crosses one twice a year. (4) Seeded case due dates now derived from the matrix + the seeded account's tier (daysAgo(n) + duration('Nh')) instead of hand-typed; is_sla_violated left alone deliberately — it is case_sla_monitor's field to write. Matrix lives in two copies by necessity (sandboxed hook bodies cannot import a module constant); the pin test drives the shipped handler so neither copy can move alone. ESCALATION REASSIGNMENT: DEFERRED, filed as #1070 — the blocker is not the dot-walk (a hook can do what the flow cannot) but three separate decisions: crm_case.allowTransfer is a permission-model widening, the write re-enters the record-change trigger surface that has looped in this exact file before, and an unstaffed service_manager pool needs a defined no-op plus demo staffing.", "tests": "All five local gates green, re-run against rc.6 after `git fetch origin main && git merge origin/main` pulled in #1066 (rc.5->rc.6) and #1068 and deps were reinstalled from the merged lockfile: `pnpm build` exit 0; `pnpm validate` exit 0; `pnpm lint` exit 0; `pnpm hygiene` exit 0; `pnpm typecheck` exit 0; `pnpm test` 82 files, 1939 passed / 1 skipped / 0 failed. New test/case-sla-matrix.test.ts (32 cases) pins all 16 cells by driving the shipped handler — PROVEN to fail on a cell change, twice: editing high x smb 8->12 in the hook only gave 2 failures ('high×smb: expected 8h, got 12.000h', 'denied read: expected 8h, got 12.000h'); editing it in BOTH copies gave 2 failures including 'constant cell high×smb: expected 12 to be 8' from the longhand expectation table (deliberately not derived from the constant under test); reverted, 32 passed. Acceptance criterion 2 written as a real runtime test, not reasoning: test/flow-scheduled.test.ts 'case_sla_monitor — non-critical breaches (#595)' winds the clock back to creation, runs the SHIPPED case_sla_defaults handler to stamp the deadline, then runs the SHIPPED case_sla_monitor flow through the real AutomationEngine, over high x strategic / high x enterprise / medium x mid_market / low x smb, asserting is_sla_violated, is_escalated, status escalated, a non-empty escalation_reason and exactly one owner notification — plus the negative case (a High case inside its window is untouched). The repo's own guard test/hook-write-shape.test.ts caught the new update() call site ('11 sites but this file exercises 10'); the added case runs the new hook code through the real QuickJS sandbox. test/seed-consistency.test.ts re-derives all 38 seeded due dates from the matrix. test/case-first-response.test.ts rewritten for the hook path, with a guard that fails if either action body re-grows its own stamp. test/hooks-runtime-service.test.ts's 'does not give a non-critical case an SLA' — the assertion that encoded the defect — now asserts the opposite.", "open_questions": [], "out_of_scope_findings": ["filed as #1070: escalation still does not reassign — the deferred half of this card, with the transfer-grant / re-entrancy / empty-pool analysis and acceptance criteria"] }
Generated by Claude Code
ACCEPT — PR #1071 reviewed, CI 9/9 green, queued to land.
The dev corrected the ruling, and the correction is right — recording it in the open. The ruling said "keep 4h-critical as the strategic-tier cell so behavior is a superset of today's". Differentiating the rest of the critical row by tier would have loosened the deadline on non-strategic critical cases — taking a clock away from work that already had one, which is the opposite of a superset. The shipped matrix holds the whole critical row flat at 4h, and
src/objects/_case-sla.tsrecords why. Read the module comment, not the ruling, as the authority on that row.Verified rather than taken from the report:
- Reverse verification of the matrix pin, run twice: editing
high × smb8→12 in the hook alone fails 2 cases; editing it in both copies still fails, via a longhand expectation table deliberately not derived from the constant under test. The two-copy duplication is a platform constraint (sandboxed hook bodies cannot import a module constant) and is mitigated the only way it can be — the pin drives the shipped handler, so neither copy can move alone. - Acceptance criterion 2 is a real runtime test, not reasoning:
case_sla_monitordriven through the actualAutomationEngineover four non-critical priority×tier combinations, asserting the breach stamp, escalation, reason and exactly one notification, plus the negative case. - Escalation does not reassign: route an escalated case to the service_manager pool #1070 exists and carries the deferred escalation half with its three real blockers (the
allowTransferpermission widening, afterUpdate re-entrancy on a file that has looped before, and the unstaffed-pool no-op) — the ruling's "if deferred, it gets its own card, not silence" is satisfied.
File surface exceeded the claim, with reasons accepted:
src/objects/event.hook.ts(first-response stamping moved out of the two action bodies into one writer oncrm_event → held, which is what "all real touchpoints" required — and removes a "CONVENTION: remember to stamp this too" comment, i.e. a missing-producer shape),src/objects/case.object.ts, a newsrc/objects/_case-sla.ts, and 15 content files across three locales that carried "only Critical has a clock" claims this change makes false. Leaving those docs would have shipped a lie; the expansion is the right call, and it was declared rather than quietly taken.Two things worth keeping beyond this card: the DST fix (
setHours(getHours()+n)→ elapsed milliseconds — local calendar arithmetic turns "+4 hours" into 3 or 5 across a transition, and a 168h clock crosses two a year), and that the repo's owntest/hook-write-shape.test.tsguard caught the newupdate()call site unprompted.Gates were re-run against rc.6 after merging main mid-flight (82 files, 1939 passed).
Generated by Claude Code
- Reverse verification of the matrix pin, run twice: editing
Batch-2 (business completeness) item from the 2026-08-02 review.
Problems
case.hook.ts:58:criticalpriority ⇒sla_due_date = now + 4h, hardcoded. High/medium/low cases get no SLA due date at all, socase_sla_monitorcan never fire for them.crm_account.tier(strategic/enterprise/mid_market/smb) exists and is the obvious SLA driver — nothing reads it.first_response_dateis stamped only bylog_call/log_meeting(global.actions.ts:315); a case answered any other way never records a first response.case-escalation.flow.ts:62documents that flow templates cannot dot-walkowner.manager, so an "escalated" case stays with the same overloaded agent and only notifies its owner.Scope
sla_due_datefor every priority; keep 4h-critical as the strategic-tier cell so behavior is a superset of today's.first_response_dateon the first outbound touch of any kind (call, meeting, email send, first agent status transition) — not just calls/meetings.service_managerposition pool (the flat-position substitute for the missing manager chain — same technique aslead_auto_assign), in addition to notifying.Acceptance
sla_due_dateconsistent with the matrix.case_sla_monitorfires on non-critical breaches in a runtime test.