Repository navigation
feat(db-tools,graphile-meta): composed-app codegen, resumable provisions and form metadata in _meta - #1919
feat(db-tools,graphile-meta): composed-app codegen, resumable provisions and form metadata in _meta#1919yyyyaaa wants to merge 12 commits into
Conversation
…ockfile changes When packages/app already exists, run_codegen leaves its package.json and lockfile alone and runs the app's own codegen bin instead of npx with a latest pin, so a project cloned from a prepared template stays reproducible. The tool now reports its timings and exports their types.
…ng provisions A host can set provisionPollDeadlineMs for callers outside an agent tool timeout. A provision that outlasts the poll is recorded in .env with its ticket, and a later call with the same name resumes that ticket instead of requesting a second database.
MetaField gains allowedValues (from check constraints), isRequiredOnCreate, isWritableOnCreate and isWritableOnUpdate; MetaTable gains isJunction, and detected junctions get many-to-many entries named by their own keys. Only form-level facts are exposed: no SQL, constraint, trigger or smart-tag internals, because _meta is public. Descriptions are written as public copy.
|
Review complete. 🟠 1 high · 🟡 2 medium 💬 Inline comments (3)
This PR wires a resumable database-provisioning flow into
Reviewed commit: c1f8ee6 |
This comment has been minimized.
This comment has been minimized.
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
There was a problem hiding this comment.
Adds a provisioning resume/ticket state machine and .env binding persistence to agentic/db-tools, plus a form-metadata expansion (check-constraint allowed values, junction relations, node-tree parsing) in graphile/graphile-meta.
Key findings
- 🟠 Unterminated quote makes
.envedits swallow later entries — env-file.ts:39 - 🟡 Missing
ownerIdhandled inconsistently across ownership checks — provision-database.ts:231 - 🟡 Null resume poll starts a duplicate provision — request-database.ts:201
| if (QUOTES.has(value[0]) && !value.slice(1).includes(value[0])) { | ||
| while (i + 1 < lines.length) { | ||
| span.push(lines[++i]); | ||
| if (lines[i].includes(value[0])) break; | ||
| } | ||
| } |
There was a problem hiding this comment.
🟠 bug · high
Unterminated quote makes .env edits swallow later entries
envEntries treats any value starting with a quote/backtick that is not closed on the same line as a multi-line value and consumes all subsequent lines into that entry (agentic/db-tools/src/provision-database/env-file.ts:39-44), but the parser it claims to mirror does not: node:util.parseEnv('A="unterm B=2') yields {A: '"unterm', B: '2'}, ending the value at end-of-line. For a hand-edited .env with an unterminated quote, mergeEnv replacing that key rewrites the whole span as one line (deleting every later definition), removeEnvKeys drops those lines entirely, and archiveBindingKeys comments out live entries belonging to other keys. Impact: a reprovision or refresh silently destroys unrelated user content in the project .env.
📋 Prompt for AI Agents
In agentic/db-tools/src/provision-database/env-file.ts lines 37-44, tighten the multi-line span detection in envEntries to mirror node:util.parseEnv: only open a span when the closing quote appears as the last non-comment character of the value on a later line, and if no such terminator exists treat the entry as single-line (the parser ends an unterminated quoted value at end-of-line — verified: parseEnv('A="unterm\nB=2') === {A:'"unterm', B:'2'}). Add a unit test: for content 'A="unterm\nB=2\nC=3', mergeEnv(content, {A:'x'}) must preserve B=2 and C=3, and removeEnvKeys(content, ['A']) must keep B and C.
| const sessionUserId = host.account()?.userId; | ||
| if (probe.ownerId && sessionUserId && probe.ownerId !== sessionUserId) { | ||
| return fail( | ||
| `Project carries a binding (DATABASE_ID=${existingEnv.DATABASE_ID}) but that database was provisioned under a different account than the one signed in. Re-run with reprovision: true to mint a fresh database under this account (old keys archived in .env, old database untouched), then rebuild the schema (provision_blueprint + run_codegen); records do not carry over.`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🟡 security · medium
Missing ownerId handled inconsistently across ownership checks
The two ownership checks treat an absent probe.ownerId in opposite ways. The skip path fails open: a found probe with a null owner is accepted as account-owned and the current account bearer is written into the project .env (agentic/db-tools/src/tools/provision-database.ts:232), so a foreign or hand-edited binding with no owner evidence is adopted. The post-provision path fails hard: owner.ownerId !== ownerId rejects a database this account just provisioned when the probe answers with a null owner (agentic/db-tools/src/tools/provision-database.ts:364), clearing the pending ticket and telling the agent to request a new database that will hit the name-taken path. Impact: either an unverified binding is adopted, or a successful provision is abandoned.
📋 Prompt for AI Agents
In agentic/db-tools/src/tools/provision-database.ts, unify the handling of a missing ownerId across the two ownership checks. Skip path (lines 231-236): when probe.outcome === 'found' but probe.ownerId is missing, or host.account()?.userId is missing, return a fail() saying the binding's ownership could not be verified and to re-run with reprovision: true, instead of silently skipping and refreshing .env ACCESS_TOKEN. Post-provision path (line 364): change owner.outcome === 'missing' || owner.ownerId !== ownerId so an undefined ownerId does not count as a foreign account — e.g. owner.outcome === 'missing' || (owner.ownerId && owner.ownerId !== ownerId) — since the database was just provisioned under the signed-in account.
| let ticket = | ||
| resumedTicket ?? | ||
| (await executor.request({ | ||
| databaseName: args.databaseName, | ||
| subdomain: args.databaseName, | ||
| domain: args.domain, | ||
| request: args.request, | ||
| })); |
There was a problem hiding this comment.
🟡 bug · medium
Null resume poll starts a duplicate provision
When a pending ticket is resumed, requestDatabaseProvision polls it once and falls back to a brand-new executor.request if the poll returns null (agentic/db-tools/src/provision-database/request-database.ts:201-208). The backend provision job behind the old ticket is still running, so a transient empty poll answer triggers a second provision request for the same databaseName, and the new ticket id is recorded — the duplicate the resume feature exists to prevent. Impact: an orphaned in-flight provision and either a name-taken failure or a duplicate database on the next call.
📋 Prompt for AI Agents
In agentic/db-tools/src/provision-database/request-database.ts lines 190-208, guard the resume path: when args.resumeTicketId is set and the initial executor.poll returns null (or throws), do not fall through to executor.request — throw ProvisionPendingError (for transient errors) or a 'could not be read' error carrying the resumeTicketId so the caller keeps the pending record and retries, instead of minting a second provision request for the same databaseName. Add a test that a null resume poll never triggers executor.request.
The SDK FetchAdapter turns any non-2xx into 'HTTP 403: Forbidden' and drops the body, so a tenant API's ACCESS_SUSPENDED reads like any refusal. postGraphQL and statusAdapter keep extensions.code and add the status, and classifyBackendErrors sorts failures into suspended, network, server, auth, forbidden and graphql.
readSchemaModel reads every APP table in one piece, or throws: stable ids, every field with its category, normalised types, structured defaults, check-derived allowed values, key flags, foreign keys with their actions, and policies, with accessOf mapping them to shared, private or custom. The schema hash is read first.
probeDatabase returns found (with the schema hash), suspended, missing or unreachable with a cause. Only an empty answer is missing: an auth rejection, a failing backend or an error the probe doesn't know are unreachable, never a reason to reprovision. A backend without hash gets one retry with the older fields, and probeBoundDatabase probes with the host's account. resolveProjectContext adds db-suspended, the unreachable cause and context.hash. The host gains reprovisionHint, toolReprovision and persistBinding.
Provisioning moves to provisionDatabase and runs under a per-project lock. A binding is DATABASE_ID alone, probed and healed or refused. Replace mode is a compare-and-swap on the binding the caller saw and refuses a suspended one, or a live one on the host path. A ticket that reads back empty never falls back to a new request: an in-flight provision of the same name is found with listOwn first, and a completed one is only reported with name-taken, for the host to adopt. Past the attempt cap the call fails provision-stuck until the host abandons the ticket. The host saves the binding before .env is written, and failures carry a code. The tool's schema is unchanged; toolReprovision: false makes its reprovision refuse.
…nd key tools add_records and create_api_key read the app APIs through statusAdapter, so a deleted or suspended database answers database-unavailable instead of a bare HTTP 403. run_codegen no longer retries a refused introspection, and when the app API itself answers ACCESS_SUSPENDED it returns code: 'database-unavailable'.
…private The backend fills sprt_table, sprt_schema and membership_type into every app-membership policy it stores, so readSchemaModel read every table the desktop built as custom access. Those keys no longer count as narrowing the policy; a membership type other than the app's still does.
1 participant
Backend and tooling changes behind Constructive Desktop's fast app path: db-tools works on an app composed from a template, and
_metagains the form-level facts a generated form needs.db-tools
packages/appalready exists,run_codegenleaves itspackage.jsonand lockfile alone.ensureComposedInstallinstalls it from its own lockfile (--frozen-lockfile --prefer-offline, and a no-op when a finished install is already there), andrun_codegenruns the app's own codegen bin instead ofnpxwith alatestpin, so a project cloned from a prepared template stays reproducible. The tool reports its timings and exports their types.provisionPollDeadlineMswhen the caller sits outside an agent tool timeout. A provision that outlasts the poll is recorded in.envwith its ticket, and a later call with the same name resumes that ticket instead of requesting a second database.provisionDatabaseruns under a per-project lock and treatsDATABASE_IDalone as a binding. A ticket that reads back empty is looked up again by name and never re-requested. Replace mode is a compare-and-swap on the database it replaces, andadoptbinds your own database when its name is already taken. Failures carry acode, such asdb-missing,db-suspended,binding-liveorname-takenwithheldBy.probeBoundDatabasereturns one of four states:found(with the schemahash),suspended,missing, orunreachablewith a cause. An unrecognised error is never reported as missing.resolveProjectContextgainsdb-suspendedand the hash.readSchemaModelreads the app tables with stable ids. For each column it reads the normalised type, whether it's required, the default and the allowed values. It also reads foreign keys with their delete actions, and the access the policies amount to:shared,privateorcustom. The read is all or nothing.toolReprovision: falsestops the agent'sreprovision: true.persistBindingsaves the binding before.envis written.reprovisionHintchanges the advice in messages.ACCESS_SUSPENDEDstays visible.add_records,run_codegenandcreate_api_keyreturncode: 'database-unavailable'instead of a generic 403, andrun_codegenno longer retries one.graphile-meta
MetaFieldgains:allowedValues, from check constraintsisRequiredOnCreate,isWritableOnCreateandisWritableOnUpdateMetaTablegainsisJunction, and detected junctions get many-to-many entries named by their own keys._metais public, so it adds only form-level facts: no SQL, constraint, trigger or smart-tag internals, and the descriptions are written as public copy._metacaptured before the change on two local-stack databases.{ _meta { tables { name isJunction fields { name allowedValues isRequiredOnCreate isWritableOnUpdate } } } }Release
ensureComposedInstall, the probe, the schema model and the provision guard, and bumps to^0.11.0.ci/pins.jsonand theplatform-graphql-serverimage strings). Clients ask for the new fields only when the backend serves them, so nothing breaks before the bump.🤖 Generated with Claude Code
https://claude.ai/code/session_01TzSs1SgpRVHH81mRAK3ecK