Skip to content

feat(escalations): operational escalations with dedup_key + slack not_in_channel#3672

Merged
bokelley merged 1 commit into
mainfrom
bokelley/operational-escalations
Apr 30, 2026
Merged

feat(escalations): operational escalations with dedup_key + slack not_in_channel#3672
bokelley merged 1 commit into
mainfrom
bokelley/operational-escalations

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Summary

Adds an operational-escalation path so actionable-but-not-page-worthy signals stay visible in the escalation queue without firing :rotating_light: System error alerts.

Closes the gap from #3664: Slack not_in_channel was previously a logger.error (paging) → I downgraded it to warn (silent), but the right answer is warn + deduplicated escalation because the bot does need to be invited (or the calling code needs to stop).

What's in here

Schema (migration 459):

  • ALTER TABLE addie_escalations ADD COLUMN dedup_key TEXT
  • Partial unique index on dedup_key where status IN ('open','acknowledged','in_progress') — same key reusable once the prior escalation closes.

createEscalation:

  • Optional dedup_key. SELECT-before-INSERT returns the existing row if one is open. Try/catch on 23505 handles the concurrent-insert race.

inviteToChannel (slack/client.ts):

  • On not_in_channel: warn log (no page) + fire-and-forget createEscalation with category='needs_human_action', priority='low', dedup_key='slack:not_in_channel:${channelId}'.
  • Caller still gets { ok: false, error }. DB blip in escalation creation does not affect Slack flow.
  • Other expected Slack codes (channel_not_found, is_archived, user_disabled, etc.) stay at warn-only — those are user-side, not operator-side.

Why escalations and not just warn

Three categories of failure now have clear semantics:

  • error — unexpected; pages on-call via #aao-errors Slack alert.
  • warn (no escalation) — third-party state we accept and shrug at (deactivated user, archived channel).
  • warn + escalation — actionable; needs human action but not on-call. Lands in the existing escalation queue/admin UI for batch triage.

Test plan

  • New integration test (escalation-dedup.test.ts) — 4 cases: fresh key inserts, repeat key returns existing row, key reuse after resolve, no-dedup_key behaves normally
  • All 31 existing escalation tests pass
  • Pre-commit (test:unit + test-dynamic-imports + typecheck) — green
  • Verify in #aao-errors after deploy: no more not_in_channel system-error alerts; escalations appear in admin queue when the bot hits the condition

🤖 Generated with Claude Code

…_in_channel

Adds an operational-escalation path for actionable-but-not-page-worthy
signals. Bot was paging on Slack 'not_in_channel' before, then was made
silent at warn — neither is right. Now: warn-level log + a deduplicated
escalation in needs_human_action so the queue stays clean and the work
stays visible.

Schema (migration 459):
- ALTER TABLE addie_escalations ADD COLUMN dedup_key TEXT
- Partial unique index on dedup_key WHERE status IN
  ('open','acknowledged','in_progress'). Lets the same key be reused
  once the prior escalation is resolved.

createEscalation:
- Accepts optional dedup_key.
- SELECT-then-INSERT with a try/catch on 23505 to handle the race
  where two callers try to insert the same key concurrently. The
  partial unique index is the safety net.

inviteToChannel (slack/client.ts):
- On not_in_channel, fire-and-forget createEscalation with
  category='needs_human_action', priority='low',
  dedup_key='slack:not_in_channel:${channelId}'. Caller still gets
  { ok: false, error } back; the bot needs to be invited (or the
  calling code needs to stop). DB blip in escalation creation does
  not affect the Slack flow.
- Other expected Slack codes (channel_not_found, is_archived,
  user_disabled, etc.) stay at warn only — those are user-side, not
  operator-side.

Tests: new integration test covers fresh-key insert, repeat-key
returns existing row, key-reuse-after-resolve, and no-dedup_key
behavior. All 31 existing escalation tests still pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley enabled auto-merge (squash) April 30, 2026 13:09
@bokelley bokelley merged commit 35860b5 into main Apr 30, 2026
14 checks passed
@bokelley bokelley deleted the bokelley/operational-escalations branch April 30, 2026 13:14
bokelley added a commit that referenced this pull request Apr 30, 2026
Extends the convention added in #3650 with the third tier that emerged
during the alert-noise cleanup:

- Tier 1: logger.error — unexpected, pages on-call
- Tier 2: logger.warn — expected third-party state we accept
- Tier 3: logger.warn + createEscalation({ category, dedup_key }) —
  actionable but not page-worthy, queued for batch triage

Docs-only. Future authors picking between error and warn for a third-party
failure now have a single reference, and the escalation tier (introduced
in #3672) is discoverable from the same JSDoc instead of being implicit
in PR history.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley added a commit that referenced this pull request May 1, 2026
…ion block)

Two migrations landed on main with prefix 459 (PR #3672 09:14 UTC,
PR #3567 22:31 UTC) without renumbering. Subsequent PRs trip:

- `No duplicate migration numbers` workflow check
- `migrate.test.ts > has no duplicate migration version numbers on disk`
- `loadMigrations` validation in `Server integration tests` and
  `Built migrations against Postgres`

Renumber the later one (`459_create_type_reclassification_log` → 462)
since 460 (identities) and 461 (measurement_capabilities, this branch)
are already taken. Filename reference in
`type-reclassification-log-db.ts` updated to match.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley added a commit that referenced this pull request May 1, 2026
…3613) (#3726)

* feat(registry): measurement-vendor discovery on /api/registry/agents (#3613)

Crawler ingests each measurement agent's get_adcp_capabilities.measurement
block (AdCP 3.x, schema #3652) and the public /api/registry/agents endpoint
gains three filters:

- metric_id=attention_units (exact, repeatable, JSONB containment)
- accreditation=MRC (exact, repeatable; verified_by_aao always false)
- q=attention (substring on metric_id; v1 scope only)

All three imply type=measurement; explicit non-measurement type returns 400.
Filtering at SQL level so no live fan-out per request. sources counts
recomputed against filtered set (sum(sources) === count invariant).

Crawler calls get_adcp_capabilities on agents that expose the tool, parses
the measurement block, and persists to new measurement_capabilities_json
JSONB column. 10s timeout. A measurement fetch failure does not fail the
whole discovery — other capability blocks still land normally.

Per security review:
- Per-field caps at write time (metrics ≤500, description ≤2000,
  metric_id ≤256, URI ≤2048, accreditations/metric ≤32). Reject — don't
  silently truncate — so failure is visible via discovery_error.
- Belt-and-braces 256KB DB CHECK on the column.
- Strip C0 controls + DEL (keep \t, \n).
- Reject <script / javascript: / data:text/html / inline event handlers
  after NFKC normalization.
- URI fields https-only in production.
- q rejects %/_ outright (substring search, not pattern); remaining ILIKE
  escaping uses ESCAPE '\\' (mirrors catalog-db.ts:353 pattern, not
  brand-db.ts which omits the explicit ESCAPE clause).

Closes #3613, closes #3614 (direct-call vs index doc folded into
docs/registry/index.mdx).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(migrations): renumber duplicate 459 → 462 (resolves CI dup-migration block)

Two migrations landed on main with prefix 459 (PR #3672 09:14 UTC,
PR #3567 22:31 UTC) without renumbering. Subsequent PRs trip:

- `No duplicate migration numbers` workflow check
- `migrate.test.ts > has no duplicate migration version numbers on disk`
- `loadMigrations` validation in `Server integration tests` and
  `Built migrations against Postgres`

Renumber the later one (`459_create_type_reclassification_log` → 462)
since 460 (identities) and 461 (measurement_capabilities, this branch)
are already taken. Filename reference in
`type-reclassification-log-db.ts` updated to match.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(measurement): clarify stripControlChars comment + add \r preservation test

Reviewer noticed the comment claimed "\r is stripped" but the regex
explicitly preserves 0x0D — exact intent was to keep all whitespace
controls (\t, \n, \r), strip everything else in C0 plus DEL. Comment
now matches code; test asserts \r survives.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant