refactor(storage): one naming policy, one record-once write, three doors

[pyramation]

Summary

Phase 1 of https://github.com/constructive-io/constructive-planning/issues/1901: make the physical bucket name come from exactly one function, and delete the door that had its own naming policy.

mintPhysicalBucketName already lives in @constructive-io/bucket-provisioner (#1779), but the bucket-provisioner plugin still had a private naming ladder behind it:

-  if (options.resolveBucketName) return options.resolveBucketName(bucketKey, databaseId);
-  if (options.bucketNamePrefix)  return `${options.bucketNamePrefix}-${bucketKey}`;
-  return bucketKey;                      // every tenant's "default" -> one S3 bucket
+  if (!options.resolveBucketName) throw new Error('STORAGE_BUCKET_NAME_POLICY_MISSING: ...');
+  return options.resolveBucketName(databaseId, bucketKey);

The bare-key branch is the tenant-collision bug just removed from the constructive-db reconciler: physical_name is durable, so a colliding name is stamped on the row permanently. Naming is a deployment policy — absent policy now throws, matching the presigned path's error.

The autoProvision resolver wrap is gone (create hook + CORS hook + autoProvision option). It was a second eager path — no retries, catch → log.error → mutation still reports success, its own WHERE key = $1 LIMIT 1 re-lookup of the row it just created — and it was already switched off in the only wiring (autoProvision: false in constructive-preset). Eager provisioning is the DB trigger → storage:provision_bucket job; lazy is first upload; explicit is the mutation. BucketProvisioner.updateCors stays; allowed_origins → S3 CORS reconciliation moves onto the job lane in phase 3 rather than living in a resolver wrap that minted a name for a bucket that may not exist (reasoning).

Two same-policy resolvers with mirrored argument order (createBucketNameResolver (databaseId, bucketKey) vs createProvisionerBucketNameResolver (bucketKey, databaseId), correct only by test) collapse into one; BucketNameResolver in both plugins is now (databaseId, bucketKey), so the compiler proves there is no caller left on the old order.

The physical_name IS NULL guard is the "recorded name is authoritative" invariant, so it is written once, in graphile-storage-registry (environment-free, already a dependency of the presigned plugin, no cycle). It takes the query runner rather than choosing a lane, because the two callers legitimately differ: the upload path must run under jwt.claims.database_id for the buckets table's catalog-sync trigger, the mutation path runs in the system lane.

recordPhysicalName(query, bucketsQualifiedName, bucketId, physicalName)
// UPDATE <buckets> SET physical_name = $1 WHERE id = $2 AND physical_name IS NULL

Net −1400 lines, mostly tests for the deleted hook. No behavior change for any configured deployment: the removed hook was disabled, and the naming algorithm is untouched, so existing physical_name values stay valid.

Link to Devin session: https://app.devin.ai/sessions/967dd6f667004819ae392597f090a3cd
Requested by: @pyramation

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[tenki-reviewer]

Review complete. 🟠 1 high

💬 Inline comments (1)

• 🟠 provisionBucket record write omits database_id claim — plugin.ts:438

🧹 Nitpicks (1) — 🟢 1 low

• 🟢 resolveBucketName optional in type but required at runtime (types.ts:56) — The public BucketProvisionerPluginOptions.resolveBucketName is declared optional (resolveBucketName?: BucketNameResolver at graphile/graphile-bucket-provisioner-plugin/src/types.ts:56), yet plugin.ts's resolveBucketName() helper throws STORAGE_BUCKET_NAME_POLICY_MISSING whenever it is absent (graphile/graphile-bucket-provisioner-plugin/src/plugin.ts:219-226).

---

This PR introduces a new graphile-bucket-provisioner-plugin that exposes a provisionBucket mutation to create S3 buckets, record their physical names, and configure CORS, plus wiring into the constructive preset and reworked presigned-URL bucket resolution and storage-registry physical-bucket handling.

Files	Change
graphile-bucket-provisioner-plugin/src/*	Adds the plugin, preset, types, and index that define and register the provisionBucket mutation and its option plumbing.
graphile-presigned-url-plugin/src/physical-bucket.ts, graphile-storage-registry/src/*	Refactors physical-bucket resolution and adds the tenant database_id claim when recording bucket names.
graphile-settings/src/*	Wires the new plugin into the constructive preset and updates the presigned-url resolver.
__tests__/*, README.md, package.json	Adds tests for the plugin, preset, and types, and documents the new option surface.

_{Reviewed commit: 4772936}

[tenki-reviewer]

Adds a graphile-bucket-provisioner-plugin that provisions S3 buckets via a GraphQL mutation, wires it into the constructive preset, and reworks presigned-URL bucket resolution and storage-registry physical-bucket handling.

Key findings

• 🟠 provisionBucket record write omits database_id claim — plugin.ts:438

[tenki-reviewer]

🟠 bug · high

provisionBucket record write omits database_id claim

The provisionBucket mutation records physical_name on the bucket row via withPgClient(null, ...) with no JWT claim (graphile/graphile-bucket-provisioner-plugin/src/plugin.ts:438), while the presigned-url path in this same PR documents that the buckets table's catalog-sync trigger calls jwt_private.current_database_id() and raises DATABASE_CLAIM_REQUIRED otherwise, so it wraps the identical write in withRequestPgClient with the claim (graphile/graphile-presigned-url-plugin/src/physical-bucket.ts:144). The mutation's record write therefore fails, the catch returns success:false, and physical_name is never persisted so every later call re-mints and re-provisions the bucket.

📋 Prompt for AI Agents

In graphile/graphile-bucket-provisioner-plugin/src/plugin.ts around lines 438-445, change the withPgClient(null, (client) => recordPhysicalBucketName(...)) call to carry the tenant database_id claim. Use withRequestPgClient(withPgClient, { 'jwt.claims.database_id': databaseId }, (client) => recordPhysicalBucketName((query) => runQuery(client, query.text, query.values), bucketsTable, bucket.id, result.bucketName)) (importing withRequestPgClient from the shared request-pg-client helper), matching graphile/graphile-presigned-url-plugin/src/physical-bucket.ts:144-154. Without the claim the buckets table's catalog-sync trigger raises DATABASE_CLAIM_REQUIRED, the write fails, and the mutation returns success:false with physical_name never persisted.