Storage reorganisation · audit & remediation

Where CAP-2031 actually stands

Ten parallel reviews across backendApi, capExpertApp, dbMigrations and the live clone1 database, each finding put to an adversarial verifier before it was allowed into this report.

Branch CAP-2031-same-bucket-migration Commits 88aa4d36c · ce4077be4 · d6a2180 Database clone1 Method 83 agents · 46 confirmed of 73 raised
27Findings fixed
6Left, and why
6,499Audit values saved from corruption
627,763Photos added to the backfill
1,337,564Bare names still in the DB

What changed — implemented, gates green

Every fix below is in the working tree, unstaged and uncommitted. The API typechecks, the layering gate passes, both roles boot — the real test for the DI changes, and one of them caught a missing StorageModule import mid-way — and the front end builds with only its 11 pre-existing errors in an unreferenced component.

What I did not change, and why

The verdict in one paragraph

Step 2 is essentially done — the shared path rule exists in both repos and is called nearly everywhere. Step 1 is half done in a way that is worse than not started: the column widening ran, the backfill did not, and the two backfill migrations are sitting on disk executable and untracked, so the next db:migrate anyone runs will apply them by accident. Step 4's machinery is built but has no rollback lever. Steps 5 and 6 do not exist yet.

The 46 confirmed findings are not 46 separate problems. They collapse into seven root causes, and one of them — a complete path being thrown away and rebuilt as a legacy path — accounts for fourteen of them on its own.

Progress against the six steps — your plan's own sequence

01

Store the full file path

Widen ran and is applied. The backfill has not run — 709,801 sticker names, 627,763 inventory photo elements and 7,799 proposal quotes are still bare names.

Half applied
02

One rule finds every file

Built in both repos. 17 hand-built locations remain in 15 front-end files and about 5 in the API, against a gate that says confirm zero. No tests cover the rule in either repo.

Nearly

Gate 1 — blocked by step 1

Not met
03

Thumbnails learn both shapes

Cloud Function, outside all three repos — nothing here can confirm it. Step 4 is already live in code, which inverts the plan's required order if the function has not been deployed.

External
04

New uploads change shape

Staging, filing and the Redis commit path all work. But there is no shape flag anywhere, so the plan's rollback — "unset the shape flag and redeploy" — has nothing to unset.

No rollback

Gate 2 — untestable without a flag

Not met
05

Copy the existing files

presigned_url_links is created and mapped, and nothing writes a row to it. There is no copier and no scheduled job.

Not started

Gate 3 — nothing to gate yet

Not met
06

Keep and watch the old folders

No legacy-read logging, no app-version report. Correctly last.

Not started

Separately, all six code deltas your plan's "where the code stands" table lists are closed — there are now zero references to useOldBucket, is_new_bucket, new_bucket_source_path or liveBucketName anywhere. The seventh row of that table is the backfill, which is step 1 above.

Blocking — three things to settle before any other work

The two backfill migrations are live, untracked, and will run by accident

I told you earlier these were commented out. They are not. require() on both returns up and down as functions, and sequelize-cli globs the migrations directory from disk, not from git — so the next db:migrate in dbMigrations applies both to clone1, including the audit-trail damage below.

All three artifacts — both migrations and database/helpers/json-file-paths.js — are untracked, so this state exists only on this machine. Commit d6a2180, whose message reads "add database migrations to store complete file paths and update JSON-encoded references", contains only the widen migration.

A blanket ezrfp-quote/ prefix rewrites the copilot audit trail

Not reversible

In 101444, the key paths equipments[].changes[].new and .old are addressed by position, and the walker never reads the sibling key field that says what the change was. Measured on clone1: 6,228 values would be prefixed, of which 8 are actually quote files.

A status becomes ezrfp-quote/Approved, a manufacturer becomes ezrfp-quote/Stryker, a zip code becomes ezrfp-quote/90210. Free-text comments are additionally split on their commas and trimmed, which down() cannot undo. 60 attachment names are misfiled into the wrong folder — they belong in ezrfp/attachments.

  • dbMigrations/database/migrations/20260902101444-store-complete-file-paths-in-json.js:98Either drop the copilot_parent_requests.action_logs entry — it is a frozen audit record, the same reasoning that already excludes email_logs.data — or teach the walker a sibling predicate so only key = 'quote' is rewritten.

The largest photo column is in neither backfill

Coverage gap

camp_inventory_items.images holds 627,763 bare-name elements across 312,152 rows — more elements than any other column in the schema. 101443 never mentions the table; 101444 covers only its additional_details JSON column. Every one of those photos would be left behind by a backfill that reports success.

  • dbMigrations/database/migrations/20260902101443-store-complete-file-paths.js — COLUMNSSixteen columns are covered. This one, a varchar[] of inventory photos, is not. Order matters here more than anywhere: the first element is the preview shown on hundreds of thousands of records.

Root causes — 46 findings, seven causes

The complete path is thrown away and a legacy path rebuilt in its place

14 sites · broken now

The upload returns where the object really went. The caller discards that return value, keeps only the bare file name, and reconstructs a path from a legacy BucketFolder. The object is in the new tree; the recorded path points into the old one; the read 404s. This is the exact failure your plan names as the original cause of the whole project — "the upload step already works out the complete location and hands it back, the rest of the system just discards it."

  • backendApi/src/shared/queue/mail/mail.service.ts:789Large-export emails. Four sites (789, 841/858, 889/899, 933/943) write to doc/capture/export/ and mail a link to capture/export/. Every export over the size limit sends a dead link today.
  • backendApi/src/primary/modules/developers-tab/developers-tab.service.ts:22Consumables-catalog import probes the legacy folder before committing, so the endpoint rejects every upload — unconditionally broken, not just post-cutover.
  • backendApi/src/shared/modules/document-manager/document-manager.service.ts:2735Facility-data import writes document_manager.file_path as a path the object never occupied, and its commit can never match.
  • backendApi/src/primary/modules/task-queue/vendor-offer-transaction.service.ts:88Offline PA upload strips its own staged key, which no-ops commitUploads, then re-prefixes pa-documents/ at three separate sites.
  • capExpertApp/…/match-equipment-model.component.ts:1473Sticker upload sends a bare name, so the commit is a no-op and the sticker never renders.
  • capExpertApp/…/update-sticker-image.component.ts:87Sticker replace, same shape — the picture stays in img/temp/ and the record points nowhere.
  • capExpertApp/…/consumables-catalog-import-modal.component.ts:123Import modals send a bare name to endpoints that re-prefix a legacy folder.

A folder prefix is added to a value that is already a complete path

Do-not #5

Your plan lists this among the seven ways to lose something permanently. Five sites still do it, and one of them physically moves the object backwards out of the new tree.

  • backendApi/src/import/queue/attachment/attachment.service.ts:1027Document-manager "move to folder" deletes the new-structure object and rewrites it into a legacy folder under its display name. This one destroys, it does not merely mis-record.
  • backendApi/src/shared/modules/document-manager/document-manager.service.ts:2498fileImports prefixes a legacy folder onto names already in the new tree — biomed, asset-registry, service-contract, warranty and service-history flows, five at once.
  • backendApi/…/copilot/copilot-utilities.service.ts:374Prefixes ezrfp-warranty/ onto a proposal value that already holds a complete path.

Modules that hand out an upload link but never file the result

Strands files in temp/

Your plan is explicit that there is no safety net: "each of the 24 places that hands out an upload link has to hand the id back on save". These do not, so their objects stay under img/temp/ or doc/temp/ and the row records an address holding nothing.

  • backendApi/…/denovo-projects/denovo-projects.service.ts:356Project documents: the submit path has no commit at all, and the update path commits fileName where it should commit filePath.
  • backendApi/…/users/accounts/account.service.ts:452Account create files logos and tag sheets but leaves purchasing-formulary documents staged.
  • backendApi/…/inventory-divestiture.service.ts:393Divestiture auction-check upload is never committed and is recorded under the legacy folder.
  • capExpertApp/…/shipping.component.ts:145Shipping-quote logo has no filing path at all — and its target is account-scoped for what is a global setting, so it could never leave temp/ even if it were filed.

The commit can throw after the record is already durable

Violates "a failed move must never fail a save"

Your plan states the trade plainly: losing someone's record to protect a file copy is the wrong way round. Two paths still get it wrong.

  • backendApi/src/shared/modules/storage/storage.service.ts:326The CAP-number lookup sits outside the try/catch, and RedisService.get is not fail-open — so a Redis fault after the row is written throws back into the caller.
  • backendApi/src/shared/modules/storage/storage.service.ts:375noteFiled runs inside fileStagedObject's try, so a Redis fault after a successful copy loses the record of where the file went — the copy happened, nothing knows it.
  • backendApi/src/shared/modules/redis/redis.service.ts:159Documents itself as "never throws" and then throws on a command failure.

Server-generated files still land on legacy flat keys

~20 of 24 sites

Exports, signed PDFs, QR codes and mail attachments skip staging by design — your plan says so. But they also skip the shared rule: they pass no upload target, so generatedPathFor silently writes them into the old structure. When the account is unknown it does the same thing rather than failing, which is the "silent default" the plan warns about.

  • backendApi/src/shared/modules/storage/storage.service.ts:196generatedPathFor falls back to the old structure instead of refusing. Four of the eight server-generated kinds name destinations no call site can reach.
  • backendApi/src/shared/modules/storage/storage.service.ts:911uploadFileFromPath and save() have no route to the shared rule at all; 19 of 24 uploadBufferFile sites pass no target.
  • backendApi/…/qr-codes/qr-codes.service.ts:307QR PNGs still go to assets/QR; UploadModule.QrCode and its img/qr destination are dead code.

The array migration changed types the code still reads as strings

Silent logic inversion

Five columns became varchar[]. An empty array is truthy in JavaScript where an empty string was not, so a guard that used to mean "no warranty" now means "has one".

  • backendApi/…/copilot/copilot-utilities.service.ts:337The warranty guard is inverted by the string → string[] change.
  • backendApi/…/copilot/copilot-utilities.service.ts:393warranties.file is varchar(255) and was never widened, yet is fed 'ezrfp-warranty/' plus a complete path, joined per document — an overflow, not a mis-record.

Two methods that no longer exist are still being called

Runtime crash, typechecks clean

noImplicitAny: false in the import role's tsconfig hides it, so the build is green and the endpoint dies on the first request.

  • backendApi/src/import/modules/thread.ts:103Dispatches two StorageService methods removed in the split — live crash on POST /remi-service-history.

Database ground truth — measured on clone1, not inferred

ColumnType todayRows / elementsStill bareIn backfill?
camp_inventory_items.imagesvarchar[]627,763627,763No — gap
inventory_sticker_details.sticker_namevarchar(256) NOT NULL709,801709,801Yes
copilot_launch_proposals.quotetext8,2717,799Yes
equipment_models.filestext116,0475,030Yes
equipment_models.imagestext116,0474,496Yes · JSON skipped
copilot_launch_proposals.warranty_documentvarchar[]647647Yes
copilot_requests.attachmentsvarchar[]662662Yes
equipment_categories.default_imagevarchar(512)3,532105Yes
warranties.filevarchar(255)4114Yes — not widened
documents.file_pathvarchar(256) NOT NULL20,2690Excluded — already paths
transactions.invoice_namevarchar(255)1,0551,026Excluded — two folders

Migrations are applied through 20260902101442-widen-file-path-columns.js. The tracking table is migrations, not SequelizeMeta. Exactly one row anywhere still points at a staged object — a single equipment_models.images value under img/temp/assets/ — so the stranding is real but has barely been exercised.

Your plan's fourth open question, "measure the real storage", is still open, and two columns the backfill prefixes were never widened. That is the same gap seen from both ends.

What I propose to do first

  1. Make the migrations safe.Neutralise 101443 and 101444 so an accidental db:migrate cannot fire them, and get all three files into git so they are reviewable. Nothing is run against the database.
  2. Fix the action_logs prefix and add the missing column.Restrict the JSON walker to the quote key, and add camp_inventory_items.images to the column backfill with order preserved.
  3. Stop discarding the complete path.The fourteen sites above, API and front end together. This is the single change that closes the most findings, and every one of them is a live 404.
  4. Remove the remaining double-prefixes.Starting with attachment.service.ts:1027, which moves objects backwards out of the new tree.
  5. Make the commit path fail-open.A Redis fault after a durable write must never reach the caller.
  6. Wire the four unfiled modules.DeNovo documents, account formularies, divestiture checks, shipping-quote logo.
  7. Fix thread.ts:103.One live crash, two lines.

The shape flag, the step-5 copier and the step-6 logging are new construction rather than repair, and belong after this list rather than inside it.