From 55d6208dae69e81e9b6ac9fd0aa8b56514e6744f Mon Sep 17 00:00:00 2001 From: vishal-agami Date: Tue, 21 Jul 2026 18:59:05 +0530 Subject: [PATCH 1/3] =?UTF-8?q?feat(core):=20portable=20org=20identity=20?= =?UTF-8?q?=E2=80=94=20mint=20a=20locally-minted=20org=5Fid=20and=20stamp?= =?UTF-8?q?=20it=20(ACE-056/057)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F14. Every self-hosted deployment stamped its tenant-scoped rows with the shared constant "local", so exporting one into the hosted multi-tenant product could not tell deployments apart. Give each deployment a permanent, globally-unique org_id (uuid4) minted ONCE at agami-connect and persisted in org.yaml, then stamp it on every tenant-scoped row — so the data is portable and imports as exactly one clean tenant. agami-core stays single-tenant (N=1); this is an identity seam, not multi-tenancy. No network egress: the id is a local uuid4 plus a local file write, never transmitted. It travels only when the operator exports their own DB. test_privacy_no_network.py stays green. ACE-056 — mint + resolve: - Organization gains `org_id` (Optional, so pre-F14 org.yaml still loads under extra="forbid"). - build.ensure_org_id(): idempotent + deployment-scoped — keeps an existing id, else adopts a sibling profile's (multi-datasource stays ONE tenant), else mints uuid4. write_tree() is the single mint chokepoint, so introspect/curate/snapshot all preserve the id. - loader.load_org_id() / deployment_org_id(): read-only, never raise on a missing/legacy file. - tools.resolved_org_id(): AGAMI_ORG_ID -> org.yaml -> "local". The SAME resolver backs the deploy stamp (model_deploy._default_org) and the serve path, so writes and reads can't disagree. mcp_http logs the resolved id + its source. ACE-057 — stamp + backfill: - 012_users_org_id.sql adds org_id to `users` (indexed non-PK; users is keyed by id/username). - model_deploy backfills legacy 'local' rows onto the minted id after migrations. It UPDATE-moves rows rather than re-seeding, because the redeploy DELETE is (org_id, datasource)-scoped and would otherwise orphan them; runtime tables are append-only and only fixable this way. No-op when the resolved id is still 'local', and idempotent on re-run. Verified end-to-end through the real paths: /agami-connect minted the id into org.yaml, and model_deploy stamped that exact uuid onto the serving rows in Postgres. Co-Authored-By: Claude Opus 4.8 --- packages/agami-core/src/mcp_http.py | 17 ++- .../src/migrations/core/012_users_org_id.sql | 18 +++ packages/agami-core/src/model_deploy.py | 44 ++++++- .../agami-core/src/semantic_model/build.py | 24 ++++ packages/agami-core/src/semantic_model/cli.py | 25 ++++ .../agami-core/src/semantic_model/loader.py | 39 +++++++ .../agami-core/src/semantic_model/models.py | 5 + packages/agami-core/src/tools.py | 27 ++++- plugins/agami/skills/agami-connect/SKILL.md | 2 +- tests/conftest.py | 2 + tests/test_model_deploy.py | 53 +++++++++ tests/test_org_cache.py | 26 ++++- tests/test_org_identity_e2e.py | 107 ++++++++++++++++++ tests/test_schema.py | 18 +++ 14 files changed, 394 insertions(+), 13 deletions(-) create mode 100644 packages/agami-core/src/migrations/core/012_users_org_id.sql create mode 100644 tests/test_org_identity_e2e.py diff --git a/packages/agami-core/src/mcp_http.py b/packages/agami-core/src/mcp_http.py index cae3a682..b5c2ee46 100644 --- a/packages/agami-core/src/mcp_http.py +++ b/packages/agami-core/src/mcp_http.py @@ -51,6 +51,7 @@ _current_org_ctx, bootstrap_paths, record_tool_call, + resolved_org_id, server_version, set_injected_executor, ) @@ -104,10 +105,18 @@ def _build_auth_provider() -> AuthProvider: def _build_org_resolver() -> SingleTenantOrgResolver: - """The OSS default tenancy: single-tenant, one configured org (id from AGAMI_ORG_ID, default - "local"). Multi-tenant is a future change at the *schema* layer (rows key on datasource, not - (org, datasource)) plus an authz check — not a resolver swap, so the seam lives here now.""" - org_id = os.environ.get("AGAMI_ORG_ID", "").strip() or "local" + """The OSS default tenancy: single-tenant, one configured org. The id is the F14 minted uuid + resolved by `tools.resolved_org_id()` (AGAMI_ORG_ID env -> org.yaml -> "local") — the SAME + resolution the deploy stamp uses, so writes and reads agree. Multi-tenant is a future change at + the *schema* layer plus an authz check — not a resolver swap, so the seam lives here now.""" + org_id = resolved_org_id() + # Load-bearing for a hand-rolled DB-only deploy: log the resolved id + where it came from, so an + # operator can see "org_id=local (default)" and realize org.yaml/AGAMI_ORG_ID isn't reaching the + # server (see F14's documented residual risk). + source = "env" if os.environ.get("AGAMI_ORG_ID", "").strip() else ( + "default" if org_id == "local" else "org.yaml" + ) + _log.info("single-tenant org_id=%s (source=%s)", org_id, source) return SingleTenantOrgResolver(Org(id=org_id)) diff --git a/packages/agami-core/src/migrations/core/012_users_org_id.sql b/packages/agami-core/src/migrations/core/012_users_org_id.sql new file mode 100644 index 00000000..2951bd65 --- /dev/null +++ b/packages/agami-core/src/migrations/core/012_users_org_id.sql @@ -0,0 +1,18 @@ +-- Tenant-scope the users table (F14 / ACE-057). The serving + runtime tables already carry `org_id` +-- (added earlier); `users` is the last per-customer table that didn't. Adding it here lets an +-- authorized-user roster ride along when a self-hosted deployment is lifted into hosted as one tenant. +-- +-- Plain ADD COLUMN with a constant default is portable across SQLite + Postgres (no table rebuild) — +-- same shape as 007_user_names.sql. It DEFAULTs to the 'local' sentinel so existing rows land on it; +-- the minted uuid isn't known here (a static .sql migration can't generate one — SQLite has no uuid +-- function, and run_migrations applies static SQL only), so model_deploy runs a code backfill right +-- after migrations that moves every 'local' row (here and in the serving/runtime tables) onto the +-- resolved org_id. +-- +-- Unlike the serving tables, `org_id` is NOT part of the primary key here: `users.id` (uuid4) is the +-- PK and logins resolve by the global `UNIQUE(username)` — correct for single-tenant (N=1). An indexed +-- non-PK column is the right shape; `username` stays globally unique (a per-org unique is a paid +-- multi-tenant concern, out of scope). + +ALTER TABLE users ADD COLUMN org_id TEXT NOT NULL DEFAULT 'local'; +CREATE INDEX idx_users_org ON users (org_id); diff --git a/packages/agami-core/src/model_deploy.py b/packages/agami-core/src/model_deploy.py index 36d0b2aa..b4f52cda 100644 --- a/packages/agami-core/src/model_deploy.py +++ b/packages/agami-core/src/model_deploy.py @@ -10,7 +10,6 @@ from __future__ import annotations -import os import sys from pathlib import Path @@ -20,10 +19,41 @@ def _default_org() -> str: - """The org to deploy under when the caller names none. A CLI has no request, so this is exactly - what the server's read path falls back to (AGAMI_ORG_ID / 'local') — the two MUST agree, or the - model is written under one org and read under another and the server sees no model.""" - return os.environ.get("AGAMI_ORG_ID") or "local" + """The org to deploy under when the caller names none. A CLI has no request, so it calls the SAME + resolver the server's read path uses (`tools.resolved_org_id`: AGAMI_ORG_ID -> the minted uuid in + org.yaml -> 'local') — the two MUST agree, or the model is written under one org and read under + another and the server sees no model (F14 / ACE-056).""" + from tools import resolved_org_id # lazy: keeps the deploy CLI's import surface small + + return resolved_org_id() + + +# Every tenant-scoped table carrying `org_id` — the serving model (9), the append-only runtime logs (2), +# and the user roster (1, added by 012). The backfill moves legacy 'local' rows across all of them. +_BACKFILL_TABLES = ( + "datasource_model", "subject_area", "model_table", "metric", "entity", + "relationship", "prompt_example", "memory", "model_version", # serving + "query_executions", "tool_calls", # runtime (append-only) + "users", # auth roster +) + + +def _backfill_org_id(store: Store, org_id: str) -> None: + """Move rows written under the legacy 'local' sentinel onto the resolved minted `org_id` + (F14 / ACE-057). Runs once at boot, right after migrations, so an EXISTING deployment that ran + under 'local' before this feature adopts its minted id instead of orphaning those rows. + + Why an UPDATE-move and not a re-seed: `model_store.write_organization`'s redeploy DELETE is scoped + to (org_id, datasource), so re-deploying under a NEW org_id would leave the old 'local' serving + rows behind (doubled); the runtime tables are append-only and can only be corrected by an UPDATE. + Idempotent + safe: a no-op when the target is still 'local' (a pre-F14 / un-minted deployment), and + `WHERE org_id='local'` matches zero rows once moved, so re-runs do nothing. Never touches + `username`, so the users UNIQUE index can't trip.""" + if org_id == "local": + return + for tbl in _BACKFILL_TABLES: + store.execute(f"UPDATE {tbl} SET org_id = ? WHERE org_id = 'local'", (org_id,)) + store.commit() def deploy_one(store: Store, datasource: str, profile_dir: Path, org_id: str | None = None) -> None: @@ -108,6 +138,10 @@ def main(argv: list[str] | None = None) -> int: # server (whose lifespan auto-migrates) is up, so the schema may not be applied yet. # run_migrations is idempotent, so the later lifespan pass is a harmless no-op. store.run_migrations() + # Move any legacy 'local' rows onto this deployment's minted org_id BEFORE deploying — so the + # redeploy's (org_id, datasource)-scoped overwrite lines up with the just-moved serving rows + # instead of orphaning them. No-op on a fresh or un-minted ('local') deployment. + _backfill_org_id(store, _default_org()) artifacts_dir = agami_paths.artifacts_dir() if not artifacts_dir.is_dir(): # clean exit, not an uncaught FileNotFoundError from iterdir() store.close() diff --git a/packages/agami-core/src/semantic_model/build.py b/packages/agami-core/src/semantic_model/build.py index cf1cfcfa..d7fc1d37 100644 --- a/packages/agami-core/src/semantic_model/build.py +++ b/packages/agami-core/src/semantic_model/build.py @@ -478,6 +478,26 @@ class WriteReport: files_written: list[str] = field(default_factory=list) +def ensure_org_id(out: Path, existing: Optional[str] = None) -> str: + """Idempotent, deployment-scoped mint of the org identity (F14 / ACE-056). Returns, in order: + ``existing`` (an id the caller already resolved); the org_id already persisted at ``out/org.yaml`` + (preserved across re-introspect — the immutability guarantee); an id already minted for a SIBLING + profile in the same artifacts dir (``out.parent``) — so a company with several datasources stays + ONE tenant rather than fragmenting; else a freshly minted ``uuid4().hex``. Pure-local — a uuid4 + plus (later) a file write, no network egress; a random uuid4 is globally unique with no + coordinator, the only option under no-egress.""" + from uuid import uuid4 # local generation only — no egress (F14 invariant) + + from . import loader + + return ( + existing + or loader.load_org_id(out) + or loader.deployment_org_id(out.parent) # adopt the deployment's id (deployment-scoped) + or uuid4().hex + ) + + def write_tree( org: Organization, out: Path, @@ -498,7 +518,11 @@ def write(rel: str, content: str) -> None: p.parent.mkdir(parents=True, exist_ok=True) p.write_text(content, encoding="utf-8") + # F14: the deployment identity is the first key. `ensure_org_id` reads the *previous* org.yaml + # (about to be overwritten) so a re-introspect/curate/snapshot preserves the minted id rather + # than re-minting — the immutability guarantee, centralized on the one write chokepoint. write("org.yaml", _dump({ + "org_id": ensure_org_id(out, org.org_id), "organization": org.organization, "version": org.version, "description": org.description, diff --git a/packages/agami-core/src/semantic_model/cli.py b/packages/agami-core/src/semantic_model/cli.py index 2d262fbf..2c44cef2 100644 --- a/packages/agami-core/src/semantic_model/cli.py +++ b/packages/agami-core/src/semantic_model/cli.py @@ -86,6 +86,27 @@ def cmd_snapshot(args) -> int: return 0 if h else 1 +def cmd_ensure_org_id(args) -> int: + """Mint + persist the deployment org_id into /org.yaml if absent (F14 / ACE-056), then + print it. Idempotent — an already-minted id is returned unchanged. The introspect/curate paths + mint inline via write_tree; this command is for paths that write a model WITHOUT them — e.g. + agami-connect's sample copy (6A) drops a prebuilt model that carries no id. Deployment-scoped: + adopts a sibling profile's id if one exists, so multi-datasource stays one tenant.""" + from pathlib import Path + + from . import build + root = Path(args.root) + org_path = root / "org.yaml" + doc = L._read_yaml(org_path) or {} + existing = doc.get("org_id") + oid = build.ensure_org_id(root, existing or None) + if oid != existing: + doc["org_id"] = oid + org_path.write_text(build._dump(doc), encoding="utf-8") + print(oid) + return 0 + + def cmd_context(args) -> int: org = L.load_organization(args.root) out = L.get_table_context( @@ -1052,6 +1073,10 @@ def build_parser() -> argparse.ArgumentParser: sp.add_argument("root") sp.set_defaults(func=cmd_snapshot) + sp = sub.add_parser("ensure-org-id", help="mint + persist the deployment org_id into org.yaml if absent (e.g. after copying a sample model)") + sp.add_argument("root") + sp.set_defaults(func=cmd_ensure_org_id) + sp = sub.add_parser("context", help="assemble get_table_context") sp.add_argument("root") sp.add_argument("--area", default=None) diff --git a/packages/agami-core/src/semantic_model/loader.py b/packages/agami-core/src/semantic_model/loader.py index 38027b8a..863c7467 100644 --- a/packages/agami-core/src/semantic_model/loader.py +++ b/packages/agami-core/src/semantic_model/loader.py @@ -114,6 +114,7 @@ def load_organization(root: str | Path, *, include_rejected: bool = False) -> Or subject_areas.append(_load_subject_area(sa_dir, include_rejected=include_rejected)) org = Organization( + org_id=org_doc.get("org_id"), # F14: minted uuid4 (None for pre-F14 files) organization=org_doc.get("organization", root.name), version=org_doc.get("version", 1), description=org_doc.get("description", ""), @@ -128,6 +129,42 @@ def load_organization(root: str | Path, *, include_rejected: bool = False) -> Or return org +def load_org_id(root: str | Path) -> str | None: + """Return the minted org_id recorded in ``/org.yaml``, or ``None`` if the file is absent + or predates F14 (no ``org_id`` key). This is the serve-time identity read (F14 / ACE-056). + + Deliberately read-only and lenient — unlike ``load_organization`` it never raises on a missing + file — so the single-tenant resolver can fall through to its ``"local"`` default. Reads only the + top-level key; it does NOT build the full pydantic model (identity resolution runs per process, + not per query, so it stays cheap).""" + org_path = Path(root) / "org.yaml" + if not org_path.exists(): + return None + doc = _read_yaml(org_path) or {} + oid = doc.get("org_id") + return oid or None + + +def deployment_org_id(artifacts_dir: str | Path) -> str | None: + """The deployment-wide org identity: the first ``org_id`` found across ``/*/org.yaml``, + or ``None`` if no profile carries one (F14 / ACE-056, the *deployment-scoped* rule). + + A deployment is ONE tenant even with several datasource profiles, so the minted id is shared across + every profile's ``org.yaml``. Resolving by scanning the artifacts dir (rather than one 'active' + profile) means the deploy stamp and the serve resolver agree even when ``AGAMI_PROFILE`` is unset and + the real model lives under a named profile (e.g. ``northpeak_salesforce``, not ``default``). Read-only; + never raises. Profiles are expected to agree; the first (sorted) is returned deterministically.""" + root = Path(artifacts_dir) + if not root.is_dir(): + return None + for prof in sorted(root.iterdir()): + if prof.is_dir(): + oid = load_org_id(prof) + if oid: + return oid + return None + + def _rejected(obj) -> bool: return getattr(obj, "review_state", None) == "rejected" @@ -603,6 +640,8 @@ def list_prompt_examples(root: str | Path, area: str, __all__ = [ "load_organization", + "load_org_id", + "deployment_org_id", "collect_default_filters", "get_table_index", "get_table_context", diff --git a/packages/agami-core/src/semantic_model/models.py b/packages/agami-core/src/semantic_model/models.py index d97bd676..c4fe88d0 100644 --- a/packages/agami-core/src/semantic_model/models.py +++ b/packages/agami-core/src/semantic_model/models.py @@ -632,6 +632,11 @@ def defined_table(self, name: str) -> Optional[Table]: class Organization(_Base): + # Locally-minted, globally-unique deployment identity (F14 / ACE-056): a uuid4 hex minted ONCE at + # agami-connect and persisted in org.yaml, then immutable. Optional/None so pre-F14 org.yaml files + # (which have no org_id key) still load under the model's `extra="forbid"` policy. Never transmitted; + # it only travels the day the operator exports their own DB — see F14's no-egress invariant. + org_id: Optional[str] = None organization: str version: int = 1 description: str = "" diff --git a/packages/agami-core/src/tools.py b/packages/agami-core/src/tools.py index 5bf37d16..838cb0f8 100644 --- a/packages/agami-core/src/tools.py +++ b/packages/agami-core/src/tools.py @@ -21,6 +21,7 @@ from __future__ import annotations import csv +import functools import io import json import os @@ -279,10 +280,32 @@ def _model_version(profile: str) -> str | None: _current_org_ctx: ContextVar[str | None] = ContextVar("agami_current_org_id", default=None) +@functools.lru_cache(maxsize=None) +def resolved_org_id() -> str: + """The single-tenant deployment org id, resolved once per process (F14 / ACE-056). Precedence: + ``AGAMI_ORG_ID`` env override -> the minted uuid in the active profile's ``org.yaml`` + (``loader.load_org_id``) -> ``"local"``. The SAME function backs both the deploy-time stamp + (``model_deploy._default_org``) and the serve-time resolver, so a deployment writes and reads its + rows under one identical id. Memoized: at most one ``org.yaml`` read per process (this sits on the + per-request path via ``_current_org_id``). Tests that vary env/profile must call ``.cache_clear()``.""" + env = os.environ.get("AGAMI_ORG_ID", "").strip() + if env: + return env + try: + from semantic_model import loader as L # lazy: keeps tools import light + avoids a cycle + + # Deployment-scoped: scan the whole artifacts dir (not one 'active' profile) so a deploy with + # AGAMI_PROFILE unset and the model under a named profile still finds the minted id. + oid = L.deployment_org_id(resolve_artifacts_dir()) + except Exception: + oid = None # missing/legacy org.yaml or absent model deps -> single-tenant default + return oid or "local" + + def _current_org_id() -> str: """The org id to scope this process's model cache by: the request's resolved org when the HTTP server - set it (per-request under a multi-tenant resolver), else AGAMI_ORG_ID / 'local' (single-tenant / stdio).""" - return _current_org_ctx.get() or os.environ.get("AGAMI_ORG_ID") or "local" + set it (per-request under a multi-tenant resolver), else the process-wide minted/`local` id.""" + return _current_org_ctx.get() or resolved_org_id() # Public alias: a consumer's tool handler needs to know the org its call resolved to (to scope its own diff --git a/plugins/agami/skills/agami-connect/SKILL.md b/plugins/agami/skills/agami-connect/SKILL.md index 84a662d9..e4500001 100644 --- a/plugins/agami/skills/agami-connect/SKILL.md +++ b/plugins/agami/skills/agami-connect/SKILL.md @@ -371,7 +371,7 @@ If `/agami-example/org.yaml` already exists, the sample is alread | `Just let me query it (Recommended)` | **6A — copy** | | `Build the model from scratch so I can see it work` | **6B — rebuild live (~5–10 min)** | - - **6A (copy, < 1 min):** `mkdir -p "/agami-example"` then `cp -R "$AGAMI_PLUGIN_ROOT/samples/store/model/." "/agami-example/"`. **Validate it loads here**: `bash "$AGAMI_PLUGIN_ROOT/scripts/sm" validate "/agami-example"`. If it fails, surface the errors and stop — never leave a half-wired profile. Then **stamp a model_version** (a *copy* doesn't go through introspect/curate, so nothing auto-stamps it): `bash "$AGAMI_PLUGIN_ROOT/scripts/sm" snapshot "/agami-example"` — best-effort, so the answer receipt shows a version rather than `null`. (We stamp at copy time instead of committing a static `.snapshots/` so it always matches the model's actual content; 6B gets one automatically from introspect.) + - **6A (copy, < 1 min):** `mkdir -p "/agami-example"` then `cp -R "$AGAMI_PLUGIN_ROOT/samples/store/model/." "/agami-example/"`. **Validate it loads here**: `bash "$AGAMI_PLUGIN_ROOT/scripts/sm" validate "/agami-example"`. If it fails, surface the errors and stop — never leave a half-wired profile. Then **mint the deployment `org_id`** (a *copy* carries no id — introspect mints inline, a copy bypasses it): `bash "$AGAMI_PLUGIN_ROOT/scripts/sm" ensure-org-id "/agami-example"` — idempotent, writes a locally-minted uuid into `org.yaml` so the sample deployment has a portable identity like any other. Then **stamp a model_version** (a *copy* doesn't go through introspect/curate, so nothing auto-stamps it): `bash "$AGAMI_PLUGIN_ROOT/scripts/sm" snapshot "/agami-example"` — best-effort, so the answer receipt shows a version rather than `null`. (We stamp at copy time instead of committing a static `.snapshots/` so it always matches the model's actual content; 6B gets one automatically from introspect.) - **6B (rebuild live — "watch it build"):** ignore the committed `model/` and run the **normal Phases 1→2** against the `agami-example` profile (`--db-type sqlite`) — the same introspect → enrich → seed pipeline a real onboarding uses, just pointed at the sample SQLite file. It takes a few minutes (the non-default option). **Don't mention tokens, cost, or billing** — surface time (~5–10 min), not scary money words. - **Sample carve-outs — the dataset is small + curated, so DON'T prompt (build silently over ALL tables):** skip the [Phase 1.6](#16--discover--prune-the-table-list-cheap-first-pass) **prune** page, skip the **org-description** prompt (Phase 2f / 0a), and skip the **doc/metrics intake** (Phase 1's "do you have a data dictionary / dbt repo?"). These prompts exist for a real unknown DB; for the sample they're noise. Introspect + enrich every sample table without asking. - **Clear the pre-seed gate before seeding (the silent build has no human to sign off).** Enrichment lands the sample's metrics/entities `unreviewed`, so `seed-examples` would refuse with `preseed_review_pending`. Because the sample is curated and trusted, **auto-approve the queue as a system signer** right before seeding — the silent path clears its own gate: diff --git a/tests/conftest.py b/tests/conftest.py index 0405364f..ae7ba4c3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -22,9 +22,11 @@ def _reset_org_cache(): return tools._ORG_CACHE.clear() tools._current_org_ctx.set(None) + tools.resolved_org_id.cache_clear() # F14: memoized org-id resolver; clear so env/profile changes take yield tools._ORG_CACHE.clear() tools._current_org_ctx.set(None) + tools.resolved_org_id.cache_clear() @pytest.fixture(autouse=True) diff --git a/tests/test_model_deploy.py b/tests/test_model_deploy.py index 991a3747..e1904fcb 100644 --- a/tests/test_model_deploy.py +++ b/tests/test_model_deploy.py @@ -207,3 +207,56 @@ def test_main_errors_naming_a_missing_datasource(tmp_path, monkeypatch): monkeypatch.setenv("AGAMI_ARTIFACTS_DIR", str(arts)) rc = model_deploy.main(["nope"]) # no nope/org.yaml assert rc == 1 + + +# --- F14 / ACE-056 + ACE-057: minted org_id stamping + backfill -------------------------------- + +def test_deploy_stamps_minted_org_id(tmp_path, monkeypatch): + # A model whose org.yaml carries a minted org_id: deploy resolves it (via _default_org -> + # tools.resolved_org_id over the artifacts dir) and stamps serving rows with it, not 'local'. + import tools + + arts = tmp_path / "artifacts" + _write_model(arts, "demo") + p = arts / "demo" / "org.yaml" + p.write_text("org_id: mintedcafe\n" + p.read_text()) + monkeypatch.setenv("AGAMI_ARTIFACTS_DIR", str(arts)) + monkeypatch.delenv("AGAMI_ORG_ID", raising=False) + tools.resolved_org_id.cache_clear() + + store = _store(tmp_path) + model_deploy.deploy_one(store, "demo", arts / "demo") # org_id=None -> resolves the minted id + orgs = {r["org_id"] for r in store.query("SELECT DISTINCT org_id FROM datasource_model")} + store.close() + assert orgs == {"mintedcafe"} + + +def test_backfill_moves_local_rows_idempotently(tmp_path): + store = _store(tmp_path) + store.execute("INSERT INTO users (id, username, password_hash, created) VALUES ('u1','a@x','h','t')") + store.execute("INSERT INTO datasource_model (org_id, datasource, doc) VALUES ('local','ds','{}')") + store.execute("INSERT INTO tool_calls (id, ts, org_id, tool_name) VALUES ('t1','t','local','execute_sql')") + store.commit() + + model_deploy._backfill_org_id(store, "the-uuid") + + def orgs(t): + return {r["org_id"] for r in store.query(f"SELECT org_id FROM {t}")} + + assert orgs("users") == {"the-uuid"} + assert orgs("datasource_model") == {"the-uuid"} + assert orgs("tool_calls") == {"the-uuid"} + # idempotent: re-run matches zero 'local' rows, nothing changes + model_deploy._backfill_org_id(store, "the-uuid") + assert orgs("users") == {"the-uuid"} + store.close() + + +def test_backfill_noop_when_target_is_local(tmp_path): + # A pre-F14 / un-minted deployment resolves 'local' -> the backfill must leave 'local' rows alone. + store = _store(tmp_path) + store.execute("INSERT INTO datasource_model (org_id, datasource, doc) VALUES ('local','ds','{}')") + store.commit() + model_deploy._backfill_org_id(store, "local") + assert {r["org_id"] for r in store.query("SELECT org_id FROM datasource_model")} == {"local"} + store.close() diff --git a/tests/test_org_cache.py b/tests/test_org_cache.py index 152e415e..78331e4e 100644 --- a/tests/test_org_cache.py +++ b/tests/test_org_cache.py @@ -70,7 +70,31 @@ def fake_load(profile): assert tools.get_cached_org("sales") is a -def test_default_org_id_falls_back_to_local(monkeypatch): +def test_default_org_id_falls_back_to_local(monkeypatch, tmp_path): + # No env override and no org.yaml anywhere in the artifacts dir -> single-tenant 'local' default. monkeypatch.delenv("AGAMI_ORG_ID", raising=False) + monkeypatch.setenv("AGAMI_ARTIFACTS_DIR", str(tmp_path)) # empty -> deployment_org_id() is None + tools.resolved_org_id.cache_clear() tools._current_org_ctx.set(None) assert tools._current_org_id() == "local" + + +def test_resolved_org_id_reads_minted_uuid_from_org_yaml(monkeypatch, tmp_path): + # F14: with no env override, the id is the uuid minted into a profile's org.yaml (deployment-scoped + # scan of the artifacts dir — the active profile need not be named). + (tmp_path / "acme").mkdir() + (tmp_path / "acme" / "org.yaml").write_text("org_id: abc123def\norganization: Acme\n") + monkeypatch.delenv("AGAMI_ORG_ID", raising=False) + monkeypatch.setenv("AGAMI_ARTIFACTS_DIR", str(tmp_path)) + tools.resolved_org_id.cache_clear() + assert tools.resolved_org_id() == "abc123def" + + +def test_env_override_beats_org_yaml(monkeypatch, tmp_path): + # AGAMI_ORG_ID always wins (explicit operator/multi-tenant override). + (tmp_path / "acme").mkdir() + (tmp_path / "acme" / "org.yaml").write_text("org_id: from-file\norganization: Acme\n") + monkeypatch.setenv("AGAMI_ARTIFACTS_DIR", str(tmp_path)) + monkeypatch.setenv("AGAMI_ORG_ID", "from-env") + tools.resolved_org_id.cache_clear() + assert tools.resolved_org_id() == "from-env" diff --git a/tests/test_org_identity_e2e.py b/tests/test_org_identity_e2e.py new file mode 100644 index 00000000..b4882f9e --- /dev/null +++ b/tests/test_org_identity_e2e.py @@ -0,0 +1,107 @@ +"""F14 / ACE-056–057 end-to-end: the deployment mints a portable org identity, keeps it immutable, +and the deploy-stamp + serve-resolve agree on it — the invariant the whole feature rests on. + +No network is touched (the id is a local uuid4 + a file write); `tests/test_privacy_no_network.py` +is the separate static gate that proves no egress primitive was introduced. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import pytest + +pytest.importorskip("pydantic") +pytest.importorskip("yaml") + +PKG_SRC = Path(__file__).resolve().parent.parent / "packages" / "agami-core" / "src" +if str(PKG_SRC) not in sys.path: + sys.path.insert(0, str(PKG_SRC)) + +import tools # noqa: E402 +from semantic_model import build, loader # noqa: E402 +from semantic_model.models import Organization # noqa: E402 + +_HEX = set("0123456789abcdef") + + +def _minimal_org(name: str = "acme") -> Organization: + return Organization(organization=name) + + +def test_write_tree_mints_uuid4_and_is_immutable(tmp_path): + prof = tmp_path / "acme" + build.write_tree(_minimal_org(), prof) + + minted = loader.load_org_id(prof) + assert minted and len(minted) == 32 and set(minted) <= _HEX # a uuid4 hex, minted locally + + # load -> mutate -> rewrite: the id is preserved, never re-minted (immutability guarantee). + org = loader.load_organization(prof) + assert org.org_id == minted + org.description = "edited" + build.write_tree(org, prof) + assert loader.load_org_id(prof) == minted + + +def test_deployment_scoped_second_profile_adopts_the_id(tmp_path): + # A company with several datasources is ONE tenant: a new profile adopts the deployment's id + # rather than minting a second one. + build.write_tree(_minimal_org("acme"), tmp_path / "sales") + deployment_id = loader.load_org_id(tmp_path / "sales") + + build.write_tree(_minimal_org("acme"), tmp_path / "support") + assert loader.load_org_id(tmp_path / "support") == deployment_id + assert loader.deployment_org_id(tmp_path) == deployment_id + + +def test_deploy_and_serve_resolve_the_same_id(tmp_path, monkeypatch): + # The load-bearing invariant: the deploy-time stamp (_default_org) and the serve-time resolver + # (resolved_org_id) call the SAME function over the SAME artifacts dir, so they can't disagree — + # even with AGAMI_PROFILE unset and the model under a named profile (not 'default'). + import model_deploy + + build.write_tree(_minimal_org("acme"), tmp_path / "northpeak_salesforce") + minted = loader.load_org_id(tmp_path / "northpeak_salesforce") + + monkeypatch.setenv("AGAMI_ARTIFACTS_DIR", str(tmp_path)) + monkeypatch.delenv("AGAMI_ORG_ID", raising=False) + monkeypatch.delenv("AGAMI_PROFILE", raising=False) + tools.resolved_org_id.cache_clear() + + assert model_deploy._default_org() == minted # deploy stamps this + assert tools.resolved_org_id() == minted # serve resolves this + assert tools._current_org_id() == minted # ...and the per-request path agrees + + +def test_ensure_org_id_cli_mints_into_copied_model(tmp_path, capsys): + # The sample-copy path (agami-connect 6A) drops a prebuilt org.yaml with no id; `sm ensure-org-id` + # mints one into it. Idempotent: a second run prints the SAME id and doesn't rewrite. + from semantic_model import cli + + prof = tmp_path / "agami-example" + prof.mkdir() + (prof / "org.yaml").write_text("organization: Acme Store\nversion: 1\n") + + assert cli.main(["ensure-org-id", str(prof)]) == 0 + minted = capsys.readouterr().out.strip() + assert minted and len(minted) == 32 and set(minted) <= _HEX + assert loader.load_org_id(prof) == minted # persisted into org.yaml + + assert cli.main(["ensure-org-id", str(prof)]) == 0 + assert capsys.readouterr().out.strip() == minted # idempotent — same id, no re-mint + + +def test_legacy_profile_without_org_id_resolves_local(tmp_path, monkeypatch): + # A pre-F14 org.yaml (no org_id key) still loads and resolves to the 'local' sentinel — no crash, + # no forced mint at serve time (serve is read-only; minting happens only at connect/build). + (tmp_path / "old").mkdir() + (tmp_path / "old" / "org.yaml").write_text("organization: legacy\nversion: 1\n") + assert loader.load_org_id(tmp_path / "old") is None + assert loader.load_organization(tmp_path / "old").org_id is None + + monkeypatch.setenv("AGAMI_ARTIFACTS_DIR", str(tmp_path)) + monkeypatch.delenv("AGAMI_ORG_ID", raising=False) + tools.resolved_org_id.cache_clear() + assert tools.resolved_org_id() == "local" diff --git a/tests/test_schema.py b/tests/test_schema.py index 53c85b57..824ac866 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -70,6 +70,24 @@ def test_users_table_is_flat_no_role_column(): s.close() +def test_users_has_org_id_column(): + # F14 / ACE-057: the users roster is tenant-scoped like the serving/runtime tables (migration 012), + # so an authorized-user set rides along when a deployment is lifted into hosted. org_id is an indexed + # NON-PK column here (users PK is `id`, login is by UNIQUE username) — deliberately unlike the + # serving tables where org_id leads the PK. + s = Store.connect("sqlite://") + ran = s.run_migrations() + assert "012_users_org_id.sql" in ran + info = {r["name"]: r for r in s.query("PRAGMA table_info(users)")} + assert "org_id" in info + assert info["org_id"]["notnull"] == 1 + assert info["org_id"]["dflt_value"] in ("'local'", "local") # SQLite quotes the default literal + assert info["org_id"]["pk"] == 0 # NOT part of the primary key + indexes = {r["name"] for r in s.query("PRAGMA index_list(users)")} + assert "idx_users_org" in indexes + s.close() + + def test_users_password_hash_nullable_and_email_indexed(): # OIDC users have no password, and we look them up by email — so password_hash is nullable and # email is indexed after the passwordless migration. From b59f22e3dec2bf6dbd38a1fa297a7305b50dbebf Mon Sep 17 00:00:00 2001 From: vishal-agami Date: Tue, 21 Jul 2026 19:22:55 +0530 Subject: [PATCH 2/3] fix(core): keep the org-less DATASOURCE_URL channel working once an org_id is minted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Regression found validating F14 against the real deploy flows. `tools` passes the resolved org straight into `execute_guarded`, which uses it to SELECT CREDENTIALS. `execute_sql` is fail-closed: the shared, org-less DATASOURCE_URL[__] vars are only honoured when the org is the single-tenant sentinel "local". F14 changes that value to a minted uuid — so every deployment using the documented env credential channel (the agami-deploy bundle, any container self-host) would have lost its warehouse the moment an id was minted, failing every query with "No warehouse credentials". The sentinel was standing in for "this is the single-tenant deployment", and F14 invalidated that assumption without changing the intent. `_credential_org_id()` restores it: the deployment's OWN resolved id maps back to "local" for credential lookup, while an explicitly-named tenant (AGAMI_ORG_ID, or one a multi-tenant resolver picked per request) is passed through unchanged and stays fail-closed — so one tenant still can't borrow another's warehouse. Row stamping is untouched: `_current_org_id()` (and every model_store write) still carries the minted uuid. Only the credential selector is mapped. Three tests pin the behaviour, including the exact regression: minted org + DATASOURCE_URL resolves, while a named AGAMI_ORG_ID and a per-request tenant both stay fail-closed. Co-Authored-By: Claude Opus 4.8 --- packages/agami-core/src/tools.py | 21 +++++++++++++- tests/test_org_identity_e2e.py | 50 ++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/packages/agami-core/src/tools.py b/packages/agami-core/src/tools.py index 838cb0f8..43b27abb 100644 --- a/packages/agami-core/src/tools.py +++ b/packages/agami-core/src/tools.py @@ -313,6 +313,25 @@ def _current_org_id() -> str: current_org_id = _current_org_id +def _credential_org_id() -> str: + """The org that selects WAREHOUSE CREDENTIALS — deliberately NOT always the row-scoping org. + + `execute_sql` is fail-closed: a NAMED tenant never falls back to the shared, org-less + `DATASOURCE_URL[__]` vars, so one tenant can't silently borrow another's warehouse. That + rule keys on the single-tenant sentinel `"local"`. F14 makes a single-tenant deployment's org a + minted uuid — it is still ONE deployment using ITS OWN credentials — so the deployment's own id must + keep behaving like the sentinel here, or every existing `DATASOURCE_URL`-based deploy would lose its + credentials the moment an id is minted. + + So: the deployment's own resolved id (no `AGAMI_ORG_ID` naming a tenant) maps to `"local"`; anything + else — an explicitly-named `AGAMI_ORG_ID`, or a tenant a multi-tenant resolver picked per request — + is passed through unchanged and stays fail-closed.""" + org = _current_org_id() + if not os.environ.get("AGAMI_ORG_ID", "").strip() and org == resolved_org_id(): + return "local" + return org + + # Per-process semantic-model cache (ACE-045). The long-lived server loads the whole model 2-3x per query # (_resolve_units + _resolve_receipt) and re-loads it every query; caching serves it warm across queries and # users. Keyed (org_id, datasource, model_version): org-scoped so a multi-tenant server never serves one org's @@ -1002,7 +1021,7 @@ def _run_in_process( cap_token = execute_sql._max_rows_override.set(max_rows) try: result = execute_sql.execute_guarded( - sql, profile, area, executor=executor, org_id=_current_org_id() + sql, profile, area, executor=executor, org_id=_credential_org_id() ) except execute_sql.GuardRefused as refusal: # A read-only refusal (envelope present) is already caught by tool_execute_sql's upstream diff --git a/tests/test_org_identity_e2e.py b/tests/test_org_identity_e2e.py index b4882f9e..85e93e33 100644 --- a/tests/test_org_identity_e2e.py +++ b/tests/test_org_identity_e2e.py @@ -105,3 +105,53 @@ def test_legacy_profile_without_org_id_resolves_local(tmp_path, monkeypatch): monkeypatch.delenv("AGAMI_ORG_ID", raising=False) tools.resolved_org_id.cache_clear() assert tools.resolved_org_id() == "local" + + +# --- F14 regression guard: minting an org must NOT break credential resolution ------------------- +# execute_sql is fail-closed — a NAMED tenant never falls back to the shared org-less +# DATASOURCE_URL[__] vars. That rule keyed on the literal "local", so minting a uuid for a +# single-tenant deployment would have silently cut every DATASOURCE_URL-based deploy off from its +# warehouse. `_credential_org_id` maps the deployment's OWN id back to the sentinel; a genuinely +# named tenant still passes through fail-closed. + +def test_minted_org_still_resolves_the_orgless_datasource_url(tmp_path, monkeypatch): + build.write_tree(_minimal_org(), tmp_path / "np") + minted = loader.load_org_id(tmp_path / "np") + monkeypatch.setenv("AGAMI_ARTIFACTS_DIR", str(tmp_path)) + monkeypatch.delenv("AGAMI_ORG_ID", raising=False) + tools.resolved_org_id.cache_clear() + tools._current_org_ctx.set(None) + + assert tools._current_org_id() == minted # rows are stamped with the minted uuid … + assert tools._credential_org_id() == "local" # … but credentials use the single-tenant channel + + import execute_sql + monkeypatch.setenv("DATASOURCE_URL__NP", "postgresql://u:p@h:5432/np") + assert execute_sql._env_datasource_dsn("np", tools._credential_org_id()) is not None + + +def test_named_tenant_stays_fail_closed(tmp_path, monkeypatch): + # An explicitly-named AGAMI_ORG_ID is a real tenant: it must NOT borrow the org-less DSN. + monkeypatch.setenv("AGAMI_ARTIFACTS_DIR", str(tmp_path)) + monkeypatch.setenv("AGAMI_ORG_ID", "acme") + tools.resolved_org_id.cache_clear() + tools._current_org_ctx.set(None) + assert tools._credential_org_id() == "acme" + + import execute_sql + monkeypatch.setenv("DATASOURCE_URL__NP", "postgresql://u:p@h:5432/np") + assert execute_sql._env_datasource_dsn("np", tools._credential_org_id()) is None # fail-closed + + +def test_per_request_tenant_stays_fail_closed(tmp_path, monkeypatch): + # A tenant chosen per-request by a multi-tenant resolver (the contextvar) is also fail-closed, + # even though the deployment itself has a minted id. + build.write_tree(_minimal_org(), tmp_path / "np") + monkeypatch.setenv("AGAMI_ARTIFACTS_DIR", str(tmp_path)) + monkeypatch.delenv("AGAMI_ORG_ID", raising=False) + tools.resolved_org_id.cache_clear() + tools._current_org_ctx.set("tenant-b") + try: + assert tools._credential_org_id() == "tenant-b" + finally: + tools._current_org_ctx.set(None) From 722f211848c8874245fdcc8f3f4609ed882f9aab Mon Sep 17 00:00:00 2001 From: vishal-agami Date: Wed, 22 Jul 2026 12:07:41 +0530 Subject: [PATCH 3/3] docs(core): correct the resolved_org_id docstring to match the artifacts-dir scan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review on #141. The docstring still described the earlier design — "the minted uuid in the active profile's org.yaml (loader.load_org_id)" — while the implementation resolves via loader.deployment_org_id over the whole artifacts dir. It also contradicted the inline comment four lines below it. Stale here is worse than untidy: it documents precisely the behaviour we moved away from, so a reader could "fix" the code back to a per-profile read and silently reintroduce the bug — resolve_profile() falls back to the literal "default" whenever AGAMI_PROFILE is unset and there is no .config (a deploy container), which finds no org.yaml and resolves "local", making the feature inert on the very deployments it exists for. The docstring now states the scan, and why it is deployment-scoped. Also corrects "one org.yaml read per process" -> "one artifacts-dir scan per process". Co-Authored-By: Claude Opus 4.8 --- packages/agami-core/src/tools.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/agami-core/src/tools.py b/packages/agami-core/src/tools.py index 43b27abb..5ee504ac 100644 --- a/packages/agami-core/src/tools.py +++ b/packages/agami-core/src/tools.py @@ -283,11 +283,20 @@ def _model_version(profile: str) -> str | None: @functools.lru_cache(maxsize=None) def resolved_org_id() -> str: """The single-tenant deployment org id, resolved once per process (F14 / ACE-056). Precedence: - ``AGAMI_ORG_ID`` env override -> the minted uuid in the active profile's ``org.yaml`` - (``loader.load_org_id``) -> ``"local"``. The SAME function backs both the deploy-time stamp + ``AGAMI_ORG_ID`` env override -> the minted uuid found by scanning the artifacts dir + (``loader.deployment_org_id``) -> ``"local"``. The SAME function backs both the deploy-time stamp (``model_deploy._default_org``) and the serve-time resolver, so a deployment writes and reads its - rows under one identical id. Memoized: at most one ``org.yaml`` read per process (this sits on the - per-request path via ``_current_org_id``). Tests that vary env/profile must call ``.cache_clear()``.""" + rows under one identical id. + + It scans the artifacts dir rather than reading one 'active' profile deliberately. The id is + DEPLOYMENT-scoped — every profile in the dir carries the same one — so any of them answers the + question, while ``resolve_profile()`` would fall back to the literal ``"default"`` whenever + ``AGAMI_PROFILE`` is unset and there is no ``.config`` (exactly a deploy container, whose model + lives under its datasource's name). That would find no ``org.yaml``, silently resolve ``"local"``, + and make the whole feature inert on the deployments it exists for. + + Memoized: at most one artifacts-dir scan per process (this sits on the per-request path via + ``_current_org_id``). Tests that vary env/profile must call ``.cache_clear()``.""" env = os.environ.get("AGAMI_ORG_ID", "").strip() if env: return env