feat(db-tools): provision_database no longer touches Postgres - #1863
Conversation
… by logical name, fail hard
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
Review complete. 🟠 2 high 💬 Inline comments (2)
The PR deletes
Reviewed commit: 2b80563 |
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR replaces the post-provisioning SQL "fixup" step with a strict app_public schema resolver and simpler provisioning flow, but removes two provisioning guarantees without in-repo replacements.
Key findings
- 🟠 Strict
app_publiclookup loses its naming-settings guarantee — context.ts:348 - 🟠 Membership-defaults fixup deleted without replacement — provision-database.ts:267
| where: { databaseId: { equalTo: databaseId }, name: { equalTo: 'app_public' } }, | ||
| }) | ||
| .execute(); | ||
|
|
||
| if (!result.ok) return undefined; | ||
| if (!result.ok) { | ||
| const detail = result.errors?.[0]?.message ?? 'unknown error'; | ||
| return { reason: `Could not query the app_public schema for database ${databaseId}: ${detail}` }; | ||
| } | ||
|
|
||
| const nodes = (result.data.schemas?.nodes ?? []).filter((s) => Boolean(s && s.id)); | ||
| const appPublic = nodes.find((s) => s.name === 'app_public'); | ||
| if (appPublic) return appPublic.id; | ||
| const pub = nodes.find((s) => s.name === 'public'); | ||
| if (pub) return pub.id; | ||
| return nodes[0]?.id; | ||
| const appPublic = (result.data.schemas?.nodes ?? []).find((s) => s?.name === 'app_public'); | ||
| if (!appPublic?.id) { | ||
| return { | ||
| reason: `Database ${databaseId} has no app_public schema. The database may not be fully provisioned yet.`, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🟠 bug · high
Strict app_public lookup loses its naming-settings guarantee
resolveSchemaId now returns schema-unresolved unless a logical schema named app_public exists (agentic/db-tools/src/context.ts:348), but this PR also deleted pg-fixups.ts, the only code that set constructive.simple_schema_names/schema_use_underscores — which, per that file's own comment, is what makes provision_blueprint create app_public instead of a hyphenated <db>-<hex>-app-public schema. No replacement exists in the provision request path (request-database.ts sends only a preset/module list). If the backend still applies the old naming default, provisioning succeeds but every tool gated on resolveProjectContext fails with schema-unresolved; previously the resolver fell back to public/any first schema and kept working. Legacy databases kept by the reprovision path (old database preserved) cannot resolve at all under the strict filter.
📋 Prompt for AI Agents
In agentic/db-tools, reconcile the strict app_public requirement in resolveSchemaId (agentic/db-tools/src/context.ts lines 341-364) with the removal of provision-database/pg-fixups.ts. The deleted fixup set constructive.simple_schema_names and constructive.schema_use_underscores so provision_blueprint created an app_public schema. Either move that guarantee into the provisioning request path (agentic/db-tools/src/provision-database/request-database.ts or a preset flag in presets.ts) or add a documented fallback in resolveSchemaId when app_public is absent (with a legacy-naming reason pointing to reprovision). Update agentic/db-tools/tests/context.test.ts to cover the chosen behavior.
| // provisioning already succeeded, so a fixup failure is a warning, not an error. | ||
| const fixup = await applySqlFixups({ databaseName, physicalDb }); | ||
|
|
||
| // Persist the binding to the project .env (upsert, preserving other keys). |
There was a problem hiding this comment.
🟠 bug · high
Membership-defaults fixup deleted without replacement
Deleting pg-fixups.ts removes the fixup that flipped app_membership_defaults/app_memberships to is_approved=true/is_verified=true, and nothing in the provision flow replaces it — provision-database.ts now goes from requestDatabaseProvision straight to .env persistence (agentic/db-tools/src/tools/provision-database.ts:267). The deleted file's own header documented the consequence: new sign-ups land unapproved/unverified and the AuthzEntityMembership policy silently denies every insert/select, so CRUD rows never persist. The tool also dropped its fixupNote success-message feedback and its 'enable membership defaults' description wording, so users get no signal that the guarantee is gone.
📋 Prompt for AI Agents
Restore the guarantee that agentic/db-tools/src/provision-database/pg-fixups.ts previously provided: after provisioning, app_membership_defaults and app_memberships must have is_approved=TRUE and is_verified=TRUE. Implement it in the provisioning backend or in the request payload built by agentic/db-tools/src/provision-database/request-database.ts (used from agentic/db-tools/src/tools/provision-database.ts around lines 249-296), since the CLI no longer connects to Postgres directly. If the backend cannot be confirmed to apply it, surface a warning note in the tool's success message/details so users know sign-in/CRUD may need a manual fixup.
1 participant
Summary
Fixes constructive-io/constructive-planning#2114 B-01.
provision_databaseused to open a directpgconnection to the physical control-plane DB (CONSTRUCTIVE_DB || 'constructive', credentials sourced frompgpm env) to flip local-dev naming settings and auto-approve memberships. On hosted/devnet there is no reachable Postgres, so that step failed — and the tool still returnedsuccess: truewith afixupNote.(a) The tool no longer touches Postgres.
provision-database/pg-fixups.ts(dynamicimport('pg'),pgpm envsourcing,ALTER DATABASE … SET constructive.simple_schema_names/schema_use_underscores,UPDATE app_membership_defaults/app_memberships) is deleted, along withapplySqlFixups,CONSTRUCTIVE_DB, andProvisionDatabaseDetails.fixupNote. Provisioning is GraphQL-only (requestDatabase+ ticket poll).(b) App scaffolding no longer auto-approves sign-ups. The bootstrapped owner is already active/approved (bootstrap sets
is_owner/is_admin). Whether later sign-ups are auto-approved/verified is the tenant owner's product setting: the owner signs in and approves members / changes membership defaults in the dashboard. The harness does not set it.Schema resolution (
context.tsresolveSchemaId):metaschema_public.schema.nameis the logical name (alwaysapp_public, inserted by the create-database trigger); the physical, possibly hashed hosted name (<db>-<hash>-app-public) isschemaName. So this match works with hashed names and no longer depends on the simple-naming settings. No other field identifies the app schema by role: every module public schema (auth_public,memberships_public, …) is alsoisPublic/EXPOSABLE, andcategoryis never set (allAPP). Thepublic/first-row fallbacks are removed because they silently picked an arbitrary schema.(c) Harness docs/skills describing the fixups live in constructive-desktop and are updated in the companion PR https://github.com/constructive-io/constructive-desktop/pull/148:
BLUEPRINT-PENDING-001rewritten (owner approves / changes defaults in the dashboard),MEMBERSHIP-DB-001and thefix-membership-defaultsSQL workaround removed, thefixupNotetool-card UI and theparsePgpmEnvtest import dropped. Nothing in this repo (agentic/, docs, skills) referencedfixupNote/applySqlFixupsbesidesprovision-database.ts.Review focus
schema-unresolvednow carries the underlying GraphQL error instead of a generic message.Link to Devin session: https://app.devin.ai/sessions/d5f0aade466d4f638cc14673e42309de
Open in Devin Desktop: https://app.devin.ai/desktop/session/d5f0aade466d4f638cc14673e42309de?variant=devin
Requested by: @pyramation