feat(core): portable org identity — locally-minted org_id, stamped on tenant-scoped rows (ACE-056/057) - #141
Conversation
…stamp it (ACE-056/057) 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 <noreply@anthropic.com>
…rg_id is minted 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[__<PROFILE>] 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 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Implements a “portable org identity” seam for single-tenant OSS deployments by minting a locally-generated org_id (uuid4 hex), persisting it in org.yaml, resolving it consistently at deploy/serve time, and stamping/backfilling tenant-scoped DB rows to prevent cross-deployment collisions during hosted lift/import.
Changes:
- Add
Organization.org_id(optional) and mint/adopt/persist it viabuild.ensure_org_id()+write_tree(), with read-only resolution vialoader.load_org_id()/deployment_org_id()andtools.resolved_org_id(). - Stamp/backfill DB rows from legacy
'local'to the resolved minted org id at deploy time; addorg_idtousersvia migration012_users_org_id.sql. - Add/extend end-to-end tests to assert mint-once immutability, deployment-scoped adoption, deploy↔serve agreement, and backfill/credential-resolution invariants.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_schema.py | Asserts users.org_id exists, is NOT NULL with default 'local', and is indexed. |
| tests/test_org_identity_e2e.py | New E2E coverage for mint/adopt/resolve invariants and credential-resolution regression guard. |
| tests/test_org_cache.py | Adds resolver tests and isolates AGAMI_ARTIFACTS_DIR/cache behavior. |
| tests/test_model_deploy.py | Verifies deploy stamps minted org id and backfill updates legacy 'local' rows idempotently. |
| tests/conftest.py | Clears resolved_org_id LRU cache between tests to avoid env/artifacts-dir leakage. |
| plugins/agami/skills/agami-connect/SKILL.md | Updates connect skill docs to mint org_id for the sample copy path. |
| packages/agami-core/src/tools.py | Introduces resolved_org_id() + _credential_org_id() and routes execute_sql credential selection through the sentinel mapping. |
| packages/agami-core/src/semantic_model/models.py | Adds optional Organization.org_id field for backwards-compatible loads. |
| packages/agami-core/src/semantic_model/loader.py | Adds load_org_id() and deployment_org_id() to resolve minted ids without building full models. |
| packages/agami-core/src/semantic_model/cli.py | Adds sm ensure-org-id command to mint/persist org_id for models written outside introspect/curate. |
| packages/agami-core/src/semantic_model/build.py | Adds ensure_org_id() and stamps org_id into org.yaml via the main write chokepoint. |
| packages/agami-core/src/model_deploy.py | Uses unified resolver for deploy stamp and adds _backfill_org_id() executed after migrations. |
| packages/agami-core/src/migrations/core/012_users_org_id.sql | Adds users.org_id with default 'local' and an org index for tenant scoping. |
| packages/agami-core/src/mcp_http.py | Uses the unified resolver for the single-tenant org and logs resolved org_id + source. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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: |
There was a problem hiding this comment.
Checked this — the graceful failure you're describing is already in place, so no change here.
_read_yaml does open the file unguarded, so it raises FileNotFoundError. But cli.main() has a purpose-built handler for exactly that case (cli.py:1332-1339):
except FileNotFoundError as e:
if "org.yaml" in str(e):
_print_json({"error": "no_model", "detail": str(e)})
return 3 # "no model here — run agami-connect"Verified:
$ sm ensure-org-id /tmp/dir-with-no-org-yaml
{ "error": "no_model", "detail": "[Errno 2] No such file or directory: '.../org.yaml'" }
exit code: 3
That's a clear error and a non-zero exit, using the repo's established convention (exit 3 = "no model", distinct from 1 = validation and 2 = usage) shared by every other sm subcommand. Adding a local guard would make ensure-org-id report this condition differently from its siblings, which is the opposite of what we want.
| """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()``.""" |
There was a problem hiding this comment.
Good catch — valid, and fixed in 722f211.
The docstring described the earlier design (a per-profile load_org_id read) while the implementation resolves via loader.deployment_org_id across the whole artifacts dir. It also contradicted the inline comment four lines below it.
Worth more than a tidy-up, for the reason you gave: it documented precisely the behaviour we deliberately moved away from. resolve_profile() falls back to the literal "default" whenever AGAMI_PROFILE is unset and there's no .config — exactly a deploy container, whose model lives under its datasource's name (e.g. /artifacts/northpeak_salesforce/). A per-profile read would look for /artifacts/default/org.yaml, find nothing, resolve "local", and make the feature inert on the very deployments it exists for. Someone reading the old docstring could have "corrected" the code straight back into that bug.
The docstring now states the scan and why it's deployment-scoped (every profile in a dir carries the same id, so any of them answers the question). Also corrected "one org.yaml read per process" → "one artifacts-dir scan per process".
…cts-dir scan 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 <noreply@anthropic.com>
Goal
Give every self-hosted deployment a permanent, locally-minted, globally-unique
org_id(uuid4) — minted once atagami-connect, persisted inorg.yaml, and stamped on every tenant-scoped row. Today every deployment stamps the shared constant"local", so lifting one into the paid hosted product can't tell deployments apart. This makes the data portable so it imports as exactly one clean tenant.agami-core stays single-tenant (N=1) — this is an identity seam, not multi-tenancy. Implements F14 (specs ACE-056 + ACE-057).
No-egress (the governing invariant)
The id is a local
uuid4()plus a local file write — never transmitted. It travels only the day the operator exports their own DB.tests/test_privacy_no_network.pystays green (uuid4 /Path.write_textaren't network primitives).ACE-056 — mint + resolve
Organization.org_id(Optional) — required because the model isextra="forbid()"; the default keeps pre-F14org.yamlloadable.build.ensure_org_id()— idempotent + deployment-scoped: keeps an existing id, else adopts a sibling profile's (so multi-datasource stays ONE tenant), else mints.write_tree()is the single mint chokepoint, so introspect/curate/snapshot all preserve it.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_httplogs the resolved id and its source.Resolution scans the artifacts dir (not one "active" profile) — otherwise a container with
AGAMI_PROFILEunset and the model under a named profile would silently resolve"local".ACE-057 — stamp + backfill
012_users_org_id.sql— addsorg_idtousers(indexed non-PK;usersis keyed byid/username, deliberately unlike the serving tables).feedbackstays excluded (paid/AH surface, ACE-034).model_deployafter migrations — UPDATE-moves legacy'local'rows onto the minted id 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'; idempotent on re-run. A static.sqlcan't do this (SQLite has no uuid function).Verification
uv run dev.py check— 1527 passed, ruff + gitleaks clean.uv run dev.py cover— 100% patch coverage (0 missing lines)./agami-connectmintedorg_idintoorg.yaml, andmodel_deploystamped that exact uuid onto the serving rows in Postgres — no'local'rows left.tests/test_org_identity_e2e.pyasserts mint-once immutability, deployment-scoped adoption, the deploy↔serve agreement invariant, and the legacy→localpath.Compatibility note (follow-up, not this PR)
Running the hosted repo's suite against this branch surfaced one cross-test issue:
resolved_org_id()is memoized with a zero-arglru_cache, so a consumer's tests can observe a stale org when they varyAGAMI_ORG_ID/artifacts dir in-process. Production is unaffected (env is fixed per process), and hosted's multi-tenant path never reaches it (the per-request contextvar short-circuits). Tracking separately rather than widening this PR.Out of scope
🤖 Generated with Claude Code