maint/1691 outbound gateway internal notification blocked by w - #214
Closed
DevNexsler wants to merge 58 commits into
Closed
DevNexsler wants to merge 58 commits into
DevNexsler wants to merge 58 commits into
Conversation
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.
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>
Author
|
Opened against wrong repo (fork parent) by automation — superseded by a broader internal fix. Please disregard. |
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.
Ticket #1691
Fix outbound gateway authorization for internal Cliq notifications independent of inbound wake provider. Recognize Zillow system sender domains and normalize
escalationintent tomanual_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.mdOn merge: verify this ticket's acceptance boxes against production (not tests), tick them, then move the ticket to
tickets/done/and setstatus: done.A merged PR is not a closed ticket — an unmerged PR is not a shipped fix.