Skip to content

feat(storage): make the reconciler lane the only physical-bucket minting driver - #1783

Merged
pyramation merged 3 commits into
mainfrom
feat/storage-single-minting-driver
Aug 28, 2026
Merged

feat(storage): make the reconciler lane the only physical-bucket minting driver#1783
pyramation merged 3 commits into
mainfrom
feat/storage-single-minting-driver

Conversation

@pyramation

@pyramation pyramation commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Phase 1 of the storage convergence (constructive-io/constructive-planning#1891): the reconciler lane is the only thing that mints a physical bucket; everything in this repo reads.

Three drivers could create a physical bucket. Two of them lived here and are removed:

  • first-upload lazy mintingprovisionAndRecordPhysicalBucket() minted a name from a deployment resolveBucketName policy, called the provisioner, and wrote physical_name from inside a presign/upload request. That is why "whoever uploads first" was the de-facto provisioner.
  • the provisionBucket mutation — a second, complete provisioning implementation that also swallowed its failures, returning { success: false, error } from a catch that only logged.

What remains is a read: the recorded physical_name is the only physical coordinate, and NULL means not reconciled yet.

-const physicalName = bucket.physical_name === null
-  ? await provisionAndRecordPhysicalBucket(options, withPgClient, storageConfig, databaseId, bucket, allowedOrigins)
-  : bucket.physical_name;
+const physicalName = assertBucketReconciled(bucket, databaseId);   // throws StorageBucketNotReconciledError

StorageBucketNotReconciledError carries code: 'STORAGE_BUCKET_NOT_RECONCILED' and retryable: true (also in extensions, so it reaches the client) — one typed error, one construction site, no provider call and no database write on the failure path.

provisionBucket becomes "reconcile now"

The mutation still resolves the bucket row under RLS, then enqueues the same job the generated INSERT trigger enqueues instead of provisioning again. Nothing catches; an enqueue failure is a GraphQL error.

The payload stops claiming a synchronous provisioning result it no longer performs:

type ProvisionBucketPayload {
  bucketId: UUID!
  bucketKey: String!
  physicalName: String   # null while reconciliation is pending
  jobId: UUID!
}

The enqueue shape is keyed off the storage module's stored entity_field — never the scope name, per compute/AGENTS.md — which is exactly one of NULL (global scopes), 'database_id' (database scope), or 'owner_id' (entity scopes):

entity_field payload extra add_job args
NULL {id, scope}
'database_id' {database_id, id, scope} db_id, entity_id, organization_id => NULL, entity_type
'owner_id' {id, owner_id, scope} entity_id, organization_id => <get_org_fn>(entity_type, entity_id), entity_type

get_org_fn is resolved at runtime through metaschema.resolve_entity_context_by_field(...), the same resolver data_job_trigger uses, so the entity-scope attribution matches the trigger's rather than being dropped.

This mirroring of the generated trigger is a deliberate, commented wart. The durable fix is to generate a callable enqueue function beside the trigger so the trigger, this mutation and the constructive-db backfill share one implementation — noted for follow-up, out of scope here.

Deployment naming policy is gone

mintPhysicalBucketName(prefix, databaseId, bucketKey) and the resolveBucketName / ensureBucketProvisioned options are deleted from @constructive-io/bucket-provisioner, graphile-presigned-url-plugin, graphile-bucket-provisioner-plugin and graphile-settings (including createBucketNameResolver, createEnsureBucketProvisioned, getBucketNamePrefix and the bucket-provisioner-resolver module). physicalBucketName(identity) — derived from the bucket row's own scope/database/key rather than deployment config — is the surviving policy, and only the reconciler consults it. CDN_BUCKET_NAME remains the connection default s3.bucket, which was never a tenant bucket.

BucketProvisionerPluginOptions had nothing left in it, so the plugin and its preset now take no options.

Companion PR

The other half is in constructive-io/constructive-db: the reconciler switches to physicalBucketName, provision_mode (lazy/eager) is removed so every bucket INSERT enqueues, and existing rows with a NULL physical_name are backfilled onto the job lane. Deployments that relied on lazy minting are covered by that backfill — without it, uploads to those rows would start returning STORAGE_BUCKET_NOT_RECONCILED.

Testing

pnpm lint clean; unit suites green for all four touched packages. Coverage added for: an upload against an unreconciled bucket rejecting with STORAGE_BUCKET_NOT_RECONCILED and making no provisioner/S3 call; the enqueue payload and add_job arguments for all three entity_field shapes (including entity scope with and without an organization resolver); and a failing enqueue propagating instead of being swallowed.

Proof

An upload against a bucket whose physical_name is still NULL rejects with the typed error and touches neither the provisioner nor S3 — the two named cases are rejects an unreconciled bucket without calling S3 or provisioning and reports an unreconciled bucket with a retryable typed error:

upload against an unreconciled bucket

The other half of the claim is only observable against a live database, so it is proved in the companion PR (constructive-io/constructive-db#3542): a bucket row inserted by psql — no GraphQL — reaches a non-NULL physical_name through the job lane and a real MinIO, the provisionBucket mutation's add_job statement produces a byte-identical job, and a provisioning failure propagates with the bucket id and scope named instead of being logged and swallowed.

the mutation enqueue and the loud failure, run against a live database

Link to Devin session: https://app.devin.ai/sessions/67bb2f8f1a0e48b48517fac54f069f1c
Open in Devin Desktop: https://app.devin.ai/desktop/session/67bb2f8f1a0e48b48517fac54f069f1c?variant=devin
Requested by: @pyramation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

🤖 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

@blacksmith-sh

This comment has been minimized.

@pyramation
pyramation merged commit f603688 into main Aug 28, 2026
20 checks passed
@pyramation
pyramation deleted the feat/storage-single-minting-driver branch August 28, 2026 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant