Skip to content

WS-29 H2 (waves 1+2): nine gateway packages converted to tenant_session — 111 classified sites remain - #417

Merged
vjvarada merged 16 commits into
mainfrom
claude/paca-research-task-management-a1f6zd
Aug 10, 2026
Merged

WS-29 H2 (waves 1+2): nine gateway packages converted to tenant_session — 111 classified sites remain#417
vjvarada merged 16 commits into
mainfrom
claude/paca-research-task-management-a1f6zd

Conversation

@vjvarada

Copy link
Copy Markdown
Contributor

Completes the request-handler half of H2 (saas_multitenancy_handover.md), on top of #416's central binding + Projects slice. Seven parallel slice-agents across two waves, integrated with recomputed ratchets, followed by an adversarial diff review (D29 loop) whose two confirmed findings are fixed in the final commit.

Converted (this PR)

Package Converted Left (classified)
routes/email 98 27 — webhook, OAuth callback, scheduler hooks, BackgroundTask jobs
routes/notes 61 33 — transcription pipeline, bot poller, copilot tasks, one bot-token route
routes/tasks 73 6 — broker handler, sync scheduler, nightly rollover sweep
routes/whatsapp 38 14 — Meta webhook, bridge-secret pushes, post-sync hooks, enrichment loop
routes/workflows 26 17 — cron loop, engine, dual-use run lifecycle, hook-token route
routes/apps 32 6 — orchestrator-invoked execute_app_action path, broker publish-review, audit helper
routes/crm 28 3 — auto-lead job, Zoho sync engine, broker handler
routes/people 4 0
routes/admin 23 0

H2_BASELINE_ELSEWHERE banked stepwise 494 → 111. Every remaining site carries an in-place # H4: (background consumer) or # H4/H6: (service-identity route) marker naming its tenant-derivation source — the classification IS the deliverable for H4/H6. Ratchets: zero-remainder packages pinned by the parametrized test_converted_packages_stay_converted (+ H2_EXEMPT_FILES with reasons); whatsapp/apps pinned at exact per-file counts; the elsewhere baseline only ratchets down with a bank-your-progress guard.

The adversarial review's two catches (fixed here, both invisible to every hermetic suite)

  1. Auto-lead would have died today: the background mail path (process_new_mail_create_lead) calls the now-converted create_record with no ambient tenant → TenantUnbound on every message → _consider counts it failed → the watermark never advances. Fixed the H4 way, early: the mailbox owner's organization is resolved from their app_user row and bound explicitly around the call. A new AST ratchet (test_exempt_files_do_not_reach_converted_sessions_unreviewed) now catches this whole class mechanically — the exemption markers are file-scoped but the call graph is not, so any exempt file calling a tenant_session-opening function needs a reviewed edge in H2_EXEMPT_CALL_EDGES.
  2. A mid-block commit inside three email handlers: _store_ai_draft(db, …) defaulted commit=True under the tenant wrapper — ending the transaction and dropping the GUC for everything after it (partial commits today; zero-row tails under RLS phase 4). Now commit=False at the one call site whose callers all hold wrapper sessions.

Conversion discipline (uniform across slices)

Every site classified BEFORE converting: user-identity request handler (convert — the central binding from #416 has the tenant) vs. service-identity route (system:internal binds nothing — leave) vs. background consumer (ambient inheritance forbidden by the runbook — leave). Explicit db.commit() calls removed (the wrapper commits on clean exit; mid-block commits drop the GUC), with honest two-block restructures where local-commit-before-provider-sync ordering matters (tasks, email helpers, apps publish). Hermetic fakes re-pointed to asynccontextmanager seams with commit-on-clean-exit; committed == 0 "writes nothing" proxies replaced by statement/row assertions.

Verification

  • Full unit suite on the final tree: 5807 passed (49 skips are the documented needs-live-Postgres checks); ruff CI gate clean.
  • Live-Postgres smokes (R8), per wave, on the merged tree: one converted read per package under a bound GUC on a migrated scratch cluster (email/whatsapp/tasks wave 1; crm/people/admin/workflows/apps wave 2); notes proven on its slice's own full cluster (create/list/patch meeting + note read); the tasks slice additionally demonstrated FORCE-RLS org-A/org-B row isolation with a non-superuser role; unbound sessions raise TenantUnbound everywhere.
  • The P0 finding was runtime-reproduced (no DB needed) before fixing, and the fixed path is covered hermetically (fakes now seed/answer the owner-org lookup).

Not in this PR

  • H4: explicit tenants for the 111 marked sites (jobs, consumers, service-identity routes) — the markers name each site's tenant source.
  • H5 (Redis prefix conversion), H6 (identity cutover), H3 (applying the generated RLS policies — gated on H2+H4 per the runbook's cliff warning).

🤖 Generated with Claude Code

https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W


Generated by Claude Code

claude added 16 commits August 10, 2026 06:58
61 of the package's 94 get_db() sites move to the tenant-bound seam:
every user-identity request handler now runs `async with _tenant_session()
as db:` (commit-on-clean-exit; explicit db.commit() lines deleted), with
the alias exported from routes/notes/core.py exactly as routes/projects
does it.

The 33 sites that stay on get_db() are each marked in place:

- H4 background consumers: pipeline.run_transcription, summaries.
  generate_notes/enqueue_summary, dispatch's auto_dispatch/_dispatch/_mark/
  _audit/_dispatch_document, copilot's _persist/_deep_context, meeting_bot's
  _refresh_bot/_poll_bot/_ingest_recording, live_session.end,
  live_speakers.apply_live_names, glossary.glossary_prompt,
  speaker_id.infer_speaker_names — all reachable from spawned tasks or the
  poller where no ambient tenant exists; each names the row to derive from.
- H4/H6 service identity: live.live_wanted (meeting-bot worker calls
  /live/wanted under MEETING_BOT_TOKEN; system identity binds no tenant).
- Dual-use settings/copilot_agenda/copilot_context helpers consumed by the
  copilot orchestrator task stay unbound with the same markers.

Tests: _install_db in test_notes_owner_scoping.py now patches the module's
_tenant_session (asynccontextmanager, commit-on-clean-exit) alongside
_get_db, so both seam shapes stay hermetically testable per module.
H2_BASELINE_ELSEWHERE banked 494 → 433.

Verified against a real scratch Postgres 16 per the handover's warning:
unbound → TenantUnbound; bound → create/list/patch/read commit under the
GUC (set_config('app.tenant_id', …)); patch_meeting's read-after-write
holds in the single wrapper transaction.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
All 52 get_db() sites classified before touching any (this package is
ingestion-heavy, and a wrong CONVERT is a runtime 500):

- 38 request-handler sites (Depends(get_current_user), tenant bound
  centrally by _with_resolved_access) are now
  `async with _tenant_session() as db:` — the per-package alias in
  whatsapp/core.py IS acb_common.db.tenant_session, mirroring
  projects/core.py; identity asserted by the existing seam test.
  Explicit commits removed (the wrapper commits on clean exit; a
  mid-block commit would end the transaction and drop the GUC), the
  bootstrap/update commit-then-reread shapes now read their own writes
  in the one transaction, and the saved-replies inner excepts keep
  their 409 mapping inside the block with the wrapper owning rollback.

- 7 service-identity sites STAY on _get_db with `# H4/H6` markers: the
  Meta webhook pair (signature-authed, no member session) and the Go
  bridge's five shared-secret push routes (ingest/reclassify/labels/
  avatars/paired — grep of whatsapp_bridge/gateway.go is the evidence).
  system:internal binds NO tenant, so tenant_session() there would 500
  every inbound batch; H4/H6 derives the tenant from the wa_accounts
  row instead. bridge.py carries both seams deliberately: its /connect
  and /status routes are user-facing and converted.

- 7 background-consumer sites STAY with `# H4` markers: the enrichment
  scheduler pair, the post-sync hooks (intent.process_new_messages,
  replyzero.classify_chats), the two scheduled enrichment passes
  (transcribe_pending, summarize_stale_groups) and the Action Broker
  broadcast handler — inheriting an ambient tenant is exactly what the
  runbook forbids for jobs.

- Ratchets (test_db_engine_seam.py): H2_BASELINE_ELSEWHERE banked
  494 → 456 (measured), and a new pin holds routes/whatsapp at exactly
  its named remainder file-by-file, so a new unbound handler fails the
  build and a retired site must shrink the dict.

- Hermetic seams re-pointed: test_whatsapp_calls/_connect now patch
  _tenant_session with an @asynccontextmanager fake that commits on
  clean exit (the _projects_fakes.bind_db shape); the closed-is-True
  resource proxy became a committed==1 assertion.

Verification: the 27 test_whatsapp_*.py files + test_db_engine_seam.py
pass by name (335 tests); ruff F821/F601/F602/F502/F7/B006 clean on all
touched files; live smoke on scratch Postgres 16 with the repo
migrations applied — unbound converted handler raises TenantUnbound,
bound GET /whatsapp/accounts answers through the real
set_config('app.tenant_id',…) round-trip, converted DELETE commits via
the wrapper, and the GUC does not leak to a fresh session.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
Baseline conflict resolved by recomputing: 494 - 61 (notes) - 38
(whatsapp) = 395; both slices' provenance comments kept.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
The largest remaining H2 slice: 98 of routes/email's 125 get_db() sites are
now `async with _tenant_session() as db:` — the per-package alias IS
acb_common.db.tenant_session (email/core.py re-exports it beside the thin
`_get_db` wrapper the seam test already exempts; notes/dispatch and the
27 B/C-class sites still import that wrapper).

Every site was classified before conversion:
- A (converted, 98): member-identity request handlers plus helpers called
  only from them (chat context builder, retry_failed_executions,
  restore_provider_labels) — the central binding has bound the tenant.
- B (left, 2): service-identity routes — the OAuth provider callback
  (trust = HMAC-signed state) and the Microsoft Graph webhook. Marked with
  the row each would derive its tenant from (app_user via the signed
  state's user; email_accounts via the subscription).
- C (left, 25): scheduler post-sync hooks, BackgroundTask jobs,
  create_task paths (_compose_assist_run's streaming leg), and helpers
  with mixed route+background callers (sweep_uncategorized,
  apply_thread_status_correction, _maybe_send_follow_up_reminders).
  Each carries an `# H4:` marker naming why.

Mid-block commits removed per the runbook (the wrapper commits on clean
exit; a mid-block commit ends the transaction and drops the GUC). Four
shared helpers that committed on a caller-passed session were the real
traps and are restructured honestly:
- core.hydrate_message_body no longer commits (cache write; callers own
  the boundary),
- senders._apply_newsletter_status and messages._hydrate_attachments and
  contacts._remember_contact lose their commits (only converted callers,
  and BackgroundTasks run post-response, i.e. after the exit commit),
- drafting._store_ai_draft grows commit=False for its converted caller
  while background callers keep committing.
rule_policies keeps its defensive rollback (documented: nothing touches
the DB after it, so the dropped GUC is moot).

Tests: tests/unit/_email_fakes.py::bind_db mirrors the seam shape
(commit-on-clean-exit, entry counter replacing get_db.assert_not_awaited);
~90 patch sites across 27 email test files re-pointed; the reclassify
fixture doubles BOTH seams (its job stays on get_db until H4); the
download-attachment "commits so rotated creds land" pin now asserts the
tenant seam owns that boundary. H2_BASELINE_ELSEWHERE banked 494 → 396.

Verification: all 96 tests/unit/test_email_*.py files + seam/tenancy pins
green (1167 tests); ruff F821,F601,F602,F502,F7,B006 clean. NOT yet
smoked against live Postgres — the runbook asks one live smoke per
package; cheapest converted read is GET /email/senders/categories.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
Baseline recomputed: 494 - 61 (notes) - 38 (whatsapp) - 98 (email) = 297.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
73 of the package's 79 get_db() sites — every user-identity request
handler — now acquire sessions as 'async with _tenant_session() as db:'
where _tenant_session IS acb_common.db.tenant_session (aliased in
core.py, imported by every submodule by name, mirroring the Projects
exemplar). The tenant comes from the request context bound centrally in
_with_resolved_access; no call site passes one.

The 6 sites that stay on get_db() are background consumers, each with an
H4 comment naming why ambient inheritance is forbidden for them:
broker_handlers.py (1, action-broker execute), scheduler.py (3,
asyncio.create_task sync loops), calendar.py (2, the nightly rollover
sweep — cross-tenant by construction).

Restructures where a mid-block commit would have dropped the tenant GUC
(never a naive swap):
- commit-then-push handlers (patch_item, merge_into, archive, bulk-
  archive, organize, delegate) become TWO tenant blocks: the local edit
  commits on the first block's clean exit, the best-effort provider
  back-sync runs in a second — same "the edit is saved before upstream"
  contract as before.
- _push_patch_upstream / _push_pending_item / the calendar pending-plan
  helpers no longer commit; the owning handler block does (which also
  makes the *-today apply atomic).
- _refresh_schema no longer commits or calls _reconcile_people; handlers
  run both inside tenant blocks, the (unbound, H4) scheduler commits
  explicitly and sequences the reconcile itself.
- accounts/settings handlers that must be COMMITTED before the scheduler
  re-reads on its own session (sync_enabled / background_sync toggles,
  put_day_state's echo) commit by block boundary, then act.
- _sync_account / embed_pending_people keep their trailing commit and
  are documented as sole-occupant-of-a-block helpers (commit is their
  last statement, wrapper exit is an empty no-op).

Tests: the tasks hermetic seams (test_tasks_people_scoping,
test_people_write) patch an @asynccontextmanager _tenant_session with
commit-on-clean-exit; test_tasks_gtd's commit-before-push source pin now
asserts the two-block shape. H2_BASELINE_ELSEWHERE banked 494 → 421.

Live scratch-Postgres smoke (Postgres 16, FORCE RLS on gtd_items/
gtd_waiting/gtd_projects/task_accounts, non-superuser role): unbound
handler raises TenantUnbound; capture_item writes under the GUC with the
row stamped by the bound org; the same read handlers return the row
under org A and zero rows under org B; delete_item's tombstone persists
via the wrapper commit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
Baseline recomputed: 494 - 61 - 38 - 98 - 73 = 224. All four wave-1
slices now merged (email/notes/tasks/whatsapp).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
26 of 43 get_db() sites converted to `async with _tenant_session() as db:`
(crud 7, publish 4, modules 6, runs 5, copilot 2, catalog 1, search 1) —
every member-reached handler in the package. The 17 that remain are the
unattended half, each H4-annotated in place: service.py (13 — run lifecycle,
engine module loads, F13 programmatic entries; start_run/resume_run are
dual-use and must not inherit an ambient tenant), scheduler.py (2 — the cron
loop), triggers.py (1 — event sink), hooks.py (1 — hook_token service
identity; tenant derives from the workflow row's organization).

core.py gains the `_tenant_session` alias of the shared seam
(gateway.db.tenant_session); submodules import it by name. update_workflow's
mid-block commit is gone — its re-reads now see the same transaction's own
writes. Hermetic fakes re-pointed to @asynccontextmanager patches with
commit-on-clean-exit. H2_BASELINE_ELSEWHERE banked 494 → 468.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
Baseline recomputed: 224 - 26 = 198.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
32 of 38 get_db() sites in gateway/routes/apps now acquire sessions via
`async with _tenant_session() as db:` — the package alias of
acb_common.db.tenant_session, exposed from _common.py and imported by name
per submodule (the seam test_db_engine_seam.py asserts identity on).
Explicit commits are gone: the wrapper owns the one commit on clean exit,
so patch_app's re-read moved inside the same transaction and
sync_workspace_to_store / _remember_tool_grant no longer commit themselves
(their callers' blocks do).

Six sites stay on the unbound seam, each with an H4 marker naming the
tenant source (the app row's organization):
- actions.py (4): execute_app_action + _run_storage_* are dual-audience —
  the HTTP route AND the orchestrator's in-process agent tools
  (orchestrator/app_tools.py) call them with no request and no bound
  tenant; ambient inheritance there is what the H2 runbook forbids.
- _common.py (1): record_app_audit is reached from that same agent path;
  under the ambient seam its except-all would silently drop audit rows.
- tools.py (1): _apply_publish_review is a broker-invoked action handler
  running on proposal approval, outside the publishing request.

Ratchet banked: H2_BASELINE_ELSEWHERE 494 -> 462, plus a per-file exact-
count pin (H2_APPS_EXEMPT_SITES + test_routes_apps_is_converted_and_
stays_converted) so the six exemptions can neither grow nor silently
linger once retired. Hermetic fakes re-pointed to asynccontextmanager
patches with commit-on-clean-exit (test_app_grants / test_app_tools /
test_app_runtime_activity); test_app_actions keeps its _get_db patches
because actions.py deliberately stays.

NOT verified against live Postgres in this environment (no DB available);
the handover requires one live smoke per package — cheapest converted
read: GET /apps/pins.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
Baseline recomputed: 198 - 32 = 166; whatsapp and apps remainder pin
tests both kept.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
The previous merge commit shipped with one stray '<<<<<<< HEAD' line in
test_db_engine_seam.py (a piped test run masked the failure). Ratchet
and apps suites green again (116 passed).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
… to tenant_session (slice)

55 of the three packages' 58 get_db() sites now acquire their session
through the tenant-bound seam (`async with _tenant_session() as db:`),
with the seam aliased in each package's central module (crm/core.py,
admin/_common.py, people/core.py) exactly as routes/projects did:

- crm: activities (4), admin (8), deal_contacts (3), import_zoho (1),
  pipeline (2), records (5), reports (4), stage_metadata (1) — all
  member-reached request handlers (the CRM agent reaches them over HTTP
  with the acting member's identity, so the central binding covers it).
  Three files stay on the unbound seam BY DECISION, each annotated and
  named in H2_EXEMPT_FILES: auto_lead (background, email scheduler hook),
  sync_zoho (scheduled engine, service identity, per-phase commits),
  broker_handlers (approval-time handler; tenant belongs to the proposal
  payload, never the approver's ambient binding) — H4 owns all three.
- people: directory (4) — all request handlers; core.py's dead
  `get_session` wrapper replaced by the shared-seam alias and the module
  added to the seam-identity parametrize list.
- admin: members (8), roles (5), groups (6), access_requests (3), me (1)
  — since S1-1 admin resolves the CALLER's org from app_user, the same
  source the central bind_tenant reads; the explicit organization_id
  predicates stay as defense in depth. `_common.get_db` remains exported
  only for routes/agent_skills.py (not this package's to convert).

Handlers' explicit commits are gone (the wrapper commits on clean exit);
post-commit re-reads now read their own uncommitted writes in the same
transaction. Hermetic fakes swap in via @asynccontextmanager patches with
commit-on-clean-exit, so one-transaction contracts stay observable;
`committed == 0` proxies on read-only successes became statement
assertions, and the purge route's one-commit structural test now pins the
`async with` shape instead of a literal db.commit().

Ratchets: H2_BASELINE_ELSEWHERE 494 → 439 (banked); the projects-only
converted-package test is now parametrized over all four converted
packages, with a stale-exemption check.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
Ratchet composition: kept the whatsapp/apps exact-count pin tests AND
the slice's generalisations (H2_CONVERTED_PACKAGES parametrized zero
check now covering projects/crm/people/admin, plus the stale-exemption
check). Baseline recomputed: 166 - 55 = 111.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
…remain

Handover H2 box and WS-29 board row updated to the post-wave state.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
… + mid-block commit; cross-file edge ratchet

P0: auto_lead._create_lead calls the H2-converted create_record from the
background mail path, where no ambient tenant exists — every message
raised TenantUnbound, _consider counted it failed, and the watermark
never advanced (auto-lead dead behind a WARNING). Fixed the H4 way, done
early because it was a today-failure: the mailbox owner's organization
is resolved from their app_user row on the caller's unbound session and
bound explicitly around the create_record call, released after.

P1: actions.py passed the enclosing tenant session to _store_ai_draft
with the default commit=True — a mid-block commit that ends the
transaction and drops the tenant GUC for everything after it (partial
commits today; zero-row reads under RLS phase 4). Now commit=False; all
three _apply_rule_actions callers hold wrapper sessions.

New ratchet: test_exempt_files_do_not_reach_converted_sessions_unreviewed
— no H4-exempt file may call a tenant_session-opening function without a
reviewed edge in H2_EXEMPT_CALL_EDGES (AST calls, so marker comments
can't false-positive). The exemption markers are file-scoped but the
call graph is not; this is the mechanical catch for the P0 class, plus a
stale-edge companion test. CRM fake taught the app_user lower()=lower()
lookup arm; auto-lead tests seed the owner's app_user row.

Full suite: 5807 passed. Both findings from the D29 adversarial diff
review; neither was visible to any hermetic suite.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
@vjvarada
vjvarada merged commit 43e07ac into main Aug 10, 2026
9 checks passed
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.

2 participants