Skip to content

maint/1691 outbound gateway internal notification blocked by w - #214

Closed
DevNexsler wants to merge 58 commits into
crystaldba:mainfrom
DevNexsler:maint/1691-outbound-gateway-internal-notification-blocked-by-w
Closed

DevNexsler wants to merge 58 commits into
crystaldba:mainfrom
DevNexsler:maint/1691-outbound-gateway-internal-notification-blocked-by-w

Conversation

@DevNexsler

@DevNexsler DevNexsler commented Aug 27, 2026

Copy link
Copy Markdown

Ticket #1691

Fix outbound gateway authorization for internal Cliq notifications independent of inbound wake provider. Recognize Zillow system sender domains and normalize escalation intent to manual_review_alert.

Tests: 661 passed, 14 skipped, 1 xfailed; isolated candidate qualification passed.


Maint ticket: #1691 — Maint-Manager tickets/review/1691-outbound-gateway-internal-notification-blocked-by-w.md
On merge: verify this ticket's acceptance boxes against production (not tests), tick them, then move the ticket to tickets/done/ and set status: done.
A merged PR is not a closed ticket — an unmerged PR is not a shipped fix.

Adds an MCP tool that fuzzy-searches across messages, participants,
channels, and transcripts in a single call. Uses ILIKE filtering
(GIN-index-accelerated) with similarity() ranking. Supports OR/AND
modes and validates minimum keyword length.
First-class MCP tool wrapping the migration-026 outbound intent lock SQL
functions so agents stop hand-writing lock SQL through psql/asyncpg (measured:
~6% of all agent tokens, 152 friction events/week — see Comm-Data-Store
docs/proposals/outbound-lock-tool.md).

- One `outbound_lock` tool, op = acquire | complete | release | check.
- Writable connection lives in the tool (unrestricted driver); message ids /
  proxy emails travel as JSON args, never inline SQL -> kills the terminal
  security-scanner false positives.
- Strict JSON in/out.
- is_duplicate_send_evidence = (completed_at set AND request_ref set),
  independent of released_at — encoded in every response so the
  released-vs-completed misread (2026-07-01 near-miss) is structurally
  impossible.
- Pure evidence-semantics helpers + a driver-injected op runner in
  outbound_lock.py (unit tested, 12 cases); live acquire->block->complete->
  post-block-with-evidence->check->release round-trip verified against the DB.

Wraps the existing concurrency-safe functions unchanged; no schema change.
DevNexsler and others added 28 commits July 17, 2026 11:14
Real wake envelopes never carry a top-level provider key, so provider
derivation fell through to message_source (tenantcloud_api) and every
production TenantCloud action failed the provider gate. Real wakeup
event sources are tenantcloud_api and tenantcloud_claim; the bare
"tenantcloud" value never occurs in production and now rejects.

Verified against prod: 29 tenantcloud_api / 47 tenantcloud_claim wake
events, zero with source "tenantcloud".

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… 118

Rederive the Step 4 persistence layer from the real schema
(migrations/118_tenantcloud_outbound_gateway_operations.sql) instead of
invented column names and literals:

- evidence_kind literal is exactly "verified_provider_readback"
- p_observation carries exactly the six keys the acceptance guard requires
  (canonical_observed_state, operation, provider_object_id,
  target_reference, readback_timestamp, readback_verified); the facade's
  own opaque evidence_hash travels as a seventh adapter-side key and is
  stripped out before the SQL call, forwarded unmodified as p_evidence_hash
- recovery reads evidence from outbound_action_attempts (the actual
  durable location -- outbound_actions gets no new columns from 118),
  fetched via a follow-up query keyed on attempt_number = attempt_count
- evidence_reference is always provider_request_ref, no silent fallback;
  raises explicitly if provider_request_ref is missing
- enqueue-time arguments are enriched with desired_state/target_reference/
  idempotency_key for the four TenantCloud operations only, derived via a
  new shared tenantcloud_shared module (also hoists the previously
  duplicated TENANTCLOUD_OPERATIONS/evidence-key-set constants out of
  service.py/store.py/adapters/tenantcloud.py); non-TenantCloud arguments
  are persisted byte-for-byte unchanged
- store test fixtures now mirror migration 118's actual shapes instead of
  hand-invented columns

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
OutboundActionRecord.execute_request() rebuilds an ExecuteRequest from
action.arguments on every reconcile()/resume() call. Since create_or_load()
now persists TenantCloud arguments enriched with desired_state/
target_reference/idempotency_key (for migration 118's acceptance guard),
and every ArgumentModel is a StrictModel with extra="forbid", every such
rebuild raised ValidationError uncaught out of reconcile()/execute() --
breaking precisely the crash-recovery path where the strict evidence gate
returns None and reconciliation is supposed to take over.

Add strip_tenantcloud_persisted_argument_keys() (inverse of
tenantcloud_persisted_arguments()) in the shared module and apply it in
execute_request() before validating. Non-TenantCloud arguments are
unaffected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…Web-Usage mount

Adds a regression test showing scripts/tenantcloud_auth.py's module-level
web_usage_runner_control import resolves via WEB_USAGE_WORKSPACE pointed at
an arbitrary directory containing only that one file -- the shape the
container actually has (single-file mount, not the whole host workspace) --
rather than only the developer's default host path. Companion to the
Comm-Data-Store docker-compose.yaml fix that actually wires the mount + env
var into the running containers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tbound-gateway

Add the TenantCloud provider adapter and runtime assembly: strict typed
argument models, trusted server-derived context with claim/owner/family
validation, verified-readback evidence persistence matching the CDS
migration-118 guard, bounded reconciliation with no blind second write
after ambiguity, and fail-closed runtime wiring behind a pinned
https://api.tenantcloud.com origin.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eview fixes

- Provider used for enabled_operations_by_provider/enabled_intents_by_provider
  gating is now "tenantcloud" for the four TenantCloud operations regardless
  of the wake's message shape. _provider() previously fell through to the
  wake's message_source, so a TenantCloud execute on a non-TenantCloud-shaped
  wake (e.g. an email-origin wake) was rejected by the allowlist gate -- the
  same wake-shape coupling this feature removes, surviving one layer up.
- canonical_context/canonical_scope no longer bake a Python None into the
  literal string "None" for tenantcloud_claim_id when a wake has no claim
  linkage; store.py's lock_intent and tenantcloud_shared's idempotency key
  read this value directly into persisted, immutable identity strings.
- Restored suggest_targets()-based coverage for the wake self-consistency
  checks (source/provider/claim linkage/state/owner/family, entity id
  format/overflow/alias-conflict, thread-channel agreement) that used to
  raise via the removed execute-blocking gate and are now return {} instead;
  the per-operation "required ids for this operation" cases don't translate
  since suggest_targets is operation-agnostic.
- Restored the cross-operation target-smuggling test for the three id
  fields still illegal on LeadStatusArguments (lead_id is now legitimate
  there; thread_id/request_id/property_id/unit_id are not).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Agents choose the outbound target. TenantCloud operations now carry
thread_id/lead_id/property_id+unit_id/request_id as typed arguments,
validated for format only. Target derivation is demoted from an
enforcement gate to an advisory 'suggest' verb on the same tool.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
suggest_targets() (the advisory-only TenantCloud target hint) was still
gating on the retired enforcement rules: tenantcloud_api ownership,
claimed/running state, and event-source/message-source/channel-type
agreement. In production 0 of 142 claims are tenantcloud_api-owned and
only 22/142 are active, so suggest returned {} for nearly every real
wake. It also only ever read tenantcloud_event_claims.entity_ids, which
0 of 142 real claims populate -- the ids actually live in
entity_scope_key's `prefix:id` shape.

Removes the ownership/state/source/channel gating (suggest is read-only
and advisory, so a legacy-owned or completed claim's hint is exactly as
safe as an active one) while keeping the checks that prevent emitting a
wrong id: claim-identity agreement and family-correct keys for
entity_ids, and prefix/parse validation for entity_scope_key. Adds
entity_scope_key to WakeEventRecord/the wake-event query and a parser
covering the real production prefix distribution (lead, tenantcloud:lead,
tc-lead, tenantcloud-lead, tenantcloud-prospect, tenantcloud:maintenance-
request; property slugs and phone numbers contribute nothing).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
An execute whose write was rejected before the provider ever processed
it (authentication_unavailable) is already classified
DEFINITIVE_NON_ACCEPTANCE+retryable=True by _from_execution and, via
service.py's existing generic routing, retries cleanly. But
_from_reconciliation only ever reached that same provably-safe-to-retry
conclusion for the two TenantCloud *status* operations (via
definitive_absence_detail); for the two *create* operations
(message.send, maintenance.create), definitive_absence_detail is None,
so a reconcile-time authentication_unavailable result fell through to
the generic ambiguous bucket and could ride the ambiguous-create path
toward reconcile/manual_review -- even though a reconciliation read
that cannot authenticate proves nothing was written, exactly like the
invoke-time case.

Adds the same authentication_unavailable check to _from_reconciliation,
ahead of and independent from definitive_absence_detail, so it applies
uniformly across all four TenantCloud operations. service.py itself did
not need to change: _finish_observation already promotes any
retryable DEFINITIVE_NON_ACCEPTANCE observation to RETRY_READY
regardless of whether it arrived via dispatch or reconcile, and
ALLOWED_TRANSITIONS already permits RECONCILING -> RETRY_READY --
retryable already reaches that decision point correctly for every
disposition the adapter hands it. The ambiguous-create and
non-retryable-rejection cases are pinned unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two defects found by the live production canary: suggest returned {} for
every real wake (it still gated on the removed ownership rules and read
provider ids from entity_ids, which production never populates -- they
live in entity_scope_key), and a reconcile whose own read could not
authenticate escalated the two create operations toward manual review
instead of retrying a request that provably never dispatched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rolls the TenantCloud target-derivation pattern (Task 1) to the remaining
four providers. Each of email.send, quo.sms.send, cliq.channel.post,
cliq.chat.post, calendar.create, calendar.update, and calendar.delete now
carries a required agent-supplied target field (to_address, to_phone,
channel_or_chat_id, calendar_id) instead of having its recipient/channel
derived from wake data. _target() reads the value straight off
request.arguments and always returns verified=True, matching the
TenantCloud precedent. The retired wake-side derivation (proxy/direct/
participant email resolution, phone resolution) moves behind
suggest_targets() as an advisory hint rather than being deleted, so an
agent that doesn't know an id can still ask what the wake implies.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Agents name the outbound target for every provider. email/quo/cliq/calendar
now carry to_address/to_phone/channel_or_chat_id/calendar_id as required
typed arguments, validated for format only. Recipient derivation moves
behind the advisory suggest verb. Quo's real send destination
(recipient_phone) comes from the agent too, closing the shared-line
wrong-recipient hazard.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CalendarAdapter._parse looked for the event under data.content, but
Agent Email's request_status envelope carries it under data.event.
The old code silently fell through to regexing the human-formatted
status text, whose embedded result is JSON-encoded with literal \n
instead of real newlines, so the UID/URL regexes never matched and a
successfully created event was reported as
calendar_reconciliation_inconclusive.

Read the event object directly out of the structured payload (walking
nested objects the way CliqAdapter._parse already does) and only fall
back to the text/regex path when no structured event is present.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CalendarAdapter reported calendar_reconciliation_inconclusive for events the
provider had actually created. Agent Email's async queue returns the event as
data.event in the structured payload, but _parse looked for data.content and
fell back to regexing the status text, whose newlines are JSON-escaped. Read
the event fields directly, keeping the text path as a fallback.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
calendar.update and calendar.delete were the last two outbound-gateway
operations still deriving their event identity solely from the wake
payload, which production data shows is always empty (zero raw_events
rows carry calendar_event_url), making both operations unreachable.

Split CalendarDescriptionArguments into CalendarCreateArguments (unchanged
shape) and CalendarUpdateArguments (calendar_id, event_url, etag,
event_uid, description), and extended CalendarDeleteArguments with the
same event_url/etag/event_uid fields. event_uid defaults to the CalDAV
event_url's basename when omitted. context.py now prefers the
agent-supplied event_url/etag/event_uid, falling back to the wake-derived
values only when the agent omits them.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
calendar.update and calendar.delete took the event identity only from the
wake payload, which no production wake carries -- both were unreachable.
Take event_url/etag (and an optional event_uid, derived from the URL
basename otherwise) from agent arguments, keeping the wake values as a
fallback. Completes the agent-supplied-targets migration.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A delete payload carries {deleted:true,event_url} and no event object, so
the uid-seeking walk missed it and successful deletions settled as unknown.
Accept the deletion confirmation, deriving the uid from the URL basename.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TenantCloud lead status updates latched dead on 2026-08-10. Three leads
(2413486, 2417344, 2417650) stranded, every attempt
tenantcloud_auth_rejected_before_dispatch, and the only thing that cleared it
was restarting the container.

Cause is a lifetime mismatch, not a bug in the facade. The Comm-Data-Store
AuthRefreshBudget is SCAN-LOCAL by design (see that repo's
docs/superpowers/specs/2026-07-21-tenantcloud-inactive-runner-refresh-design.md):
it permits exactly one token refresh, then caches that token for the budget's
lifetime so a short batch does not re-read a frozen token per endpoint.
Correct for a scan; wrong for a daemon.

_build_tenantcloud_adapter() constructed one TenantCloudClient inside
build_runtime() and the adapter reused it for the container's whole life, so
"scan-local" became "forever":

    Firefox lapses once -> facade spends its single refresh -> caches that
    token -> serves it on every later call, never re-reading localStorage.
    Ten minutes later the cached token is dead, the budget is spent, and
    every write fails until someone restarts the container.

That shape explains the evidence: 13 successes then permanent failure (a latch,
not degradation), no code or config change at the boundary, RestartCount=0 with
40+ hours uptime, and a live get_token returning ttl -132.

Fix: share `control` and `auth` (stateless with respect to token lifetime) and
build the CLIENT per operation, restoring the lifetime the budget assumes. A
single operation still uses one facade, so the pre-write readback and the write
stay in the same scan and the one-refresh anti-storm reservation still holds
within a dispatch.

TenantCloudAdapter now takes `mutations_factory` only. The instance
constructor is deleted rather than deprecated: accepting a ready-made facade is
exactly how the latch was created, and an API that still permits it relies on
every future caller remembering. The 16 existing call sites pass
`mutations_factory=lambda: double`.

Six tests. The load-bearing one asserts on the real builder and fails for the
right reason -- with the old wiring it reports "got 1 client for 2 operations",
not a TypeError. Verified red/green by toggling only server.py with the adapter
held constant, including a simulated regression where the factory closes over a
shared instance. Three tests guard the design that must NOT be relaxed:
one facade per operation (no double auth per dispatch), doubles supplied
through the factory, and no instance constructor.

No Comm-Data-Store change. AuthRefreshBudget behaves exactly as specified.

663 passed, 9 skipped, 1 xfailed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Dockerfile does `ADD . /app`, and there was no .dockerignore, so whatever
sat in the tree a human happened to build from went into the release artifact.
The image running in production carries 22 stale .pyc files from a worktree's
__pycache__ that the main checkout never had. Nothing was broken by it -- all
51 .py files compare byte-identical against a fresh build from main -- but it
meant two images built from the same commit did not compare equal, which is
exactly the property you want when checking what is deployed.

Ignores bytecode, VCS and tool caches. Leaves .venv alone on purpose: the host
checkout has one, it is currently copied in and then reconciled by `uv sync
--frozen --no-dev`, and excluding it changes what the builder resolves. That
is worth doing, but as its own verified change.

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

Copy link
Copy Markdown
Author

Opened against wrong repo (fork parent) by automation — superseded by a broader internal fix. Please disregard.

@DevNexsler DevNexsler closed this Aug 27, 2026
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