Skip to content

docs(content-drive): add backend and frontend specs for bulk file upload (#37166) - #37346

Merged
zJaaal merged 1 commit into
mainfrom
37166-content-drive-bulk-file-upload-signed
Sep 2, 2026
Merged

docs(content-drive): add backend and frontend specs for bulk file upload (#37166)#37346
zJaaal merged 1 commit into
mainfrom
37166-content-drive-bulk-file-upload-signed

Conversation

@dario-daza

@dario-daza dario-daza commented Sep 2, 2026

Copy link
Copy Markdown
Member

This PR fixes the unverified commit signing in #37300.

Spec-Kit specifications for Content Drive bulk file upload. Content Drive accepts a multi-file selection today, warns that only one file will be uploaded, and discards the rest.

This PR carries both halves of the specification, one document each, written by the developer building that half:

Spec Author Covers
specs/37166-bulk-file-upload/spec.md @dario-daza Server side: accepting a batch, creating the files, limits, outcome shape, notification
specs/37166-bulk-file-upload-frontend/spec.md @zJaaal Browser side: choosing and submitting files, reporting progress and outcome, unifying in-flight feedback

They are reviewed together because they meet at one contract and neither is complete without it. The backend spec defines it in "Contract Consumed by the Client" (C-001 to C-006); the frontend spec restates it from the consumer's side in "Contract Consumed", so the boundary is reviewable from either document. The implementation of each half lands in its own separate PR.

Backend spec: decisions recorded

Directory upload and creating a missing target folder stay out of scope, as #37166 specifies.

Frontend spec: six required stories plus one optional

  • P1 Every chosen file is uploaded, from both entry points. The upload control's file input carries no multiple attribute today, so only drag-and-drop delivers a batch; the premise that a multi-file selection already reaches the client held for the drop path only.
  • P1 The outcome is honest: counts come from the server, never from the number of files the author picked, and a partial failure names the files that failed and why.
  • P1 In-flight state is reported in one place. Content Drive announces "this is running" four different ways today, the worst of which replaces the entire listing with skeleton rows for an action fired on a single row.
  • P2 Leaving does not lose the batch, carried by the pushed completion signal (C-006), so no jobs screen is required.
  • P2 Every outcome names what ran and what it ran on. The copy for the missing cases was written and never wired.
  • P3 One operation does not block another.
  • P4, optional Stopping a run, deferred: a general task manager is the expected home for it.

Per-row in-flight marking and bulk reindex's reporting are documented as out of scope, with reasons.

Review rounds

Two rounds so far, threaded on this PR rather than restated item by item here.

Round 2 (@fabrizzio-dotCMS) changed the shape of the work in three places:

  • Resumability (backend FR-037) is either a new job-framework capability or a store this feature owns: job parameters are immutable after creation, progress is a single float, and the result is harvested only at the terminal state. It is now the first Planning Obligation, and it removed the alternative to Q6 rather than merely supporting it, since the abandoned-job sweep re-queues without consulting the retry policy.
  • The concurrent-collision window is already closed by a unique index over the lower-cased full path per host, so FR-042 is now about catching that violation and mapping it to the collision reason, not building a lock for a race the storage layer already loses on the caller's behalf. FR-042a fixes collision as case-insensitive.
  • The total-size ceiling splits into a fast refusal on the declared total and authoritative enforcement during the read (FR-013c), because content arriving in the same call is only measurable by reading it. That in turn requires the client to declare the total (frontend FR-038), or every over-ceiling batch is refused only after the author has already uploaded it.

Plus one correction spanning both documents: C-006 summarised the completion signal and the durable record as carrying "the run's counts", while FR-014 to FR-016 record counts and the per-file results with a reason each. A plan trusting the summary would have shipped a counts-only notification, leaving an author who stepped away with "27 of 30 created" and no way to learn which three failed. Corrected in both specs, with FR-020 pinning what "the outcome" means there and the frontend's FR-023 / FR-023a covering the transient and durable surfaces alike.

Proposed Changes

  • Add specs/37166-bulk-file-upload/spec.md, the backend spec: the batch-upload contract, limits, outcome shape, and notification behavior.
  • Add specs/37166-bulk-file-upload-frontend/spec.md, the frontend spec: consuming that contract, and unifying how Content Drive reports work in progress and its outcomes.
  • Address two rounds of review on both specs. Round 1 clarified the per-file "permission denied" reason in FR-016 as distinct from the target-folder permission check, and aligned the #37062 / #37063 ticket references. Round 2 is summarised above.

Checklist

  • Tests — not applicable; this PR is spec-only (no implementation code). Test strategy is defined per Spec-Kit's plan phase (/speckit-plan) once these specs are approved.
  • Translations — not applicable; no user-facing strings in this PR.
  • Security Implications Contemplated — permission enforcement (folder + per-file) and configurable limits are specified in the backend spec's FR-003, FR-004, FR-010–FR-013, FR-016; no security-relevant code changes in this PR itself.

Additional Info

This is PR 1 of the Spec-Kit flow (spec only). Approval here unblocks /speckit-plan/speckit-tasks/speckit-implement for both halves. No code changes are included.

One item is deliberately left for planning: the concrete submission format (field names, endpoint, and the shape of the handle) is not pinned in either spec, which state the boundary at behavior altitude. It must be agreed between the two halves and recorded under specs/*/contracts/, since plan.md is gitignored in this repo and an agreement recorded there would not survive.

Screenshots

Not applicable — this PR only adds specification documents, no UI or behavior changes.

This PR fixes: #37166

This PR fixes: #37166

@claude

claude Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Claude finished @dario-daza's task in 1m 6s —— View job


Spec review: bulk file upload (backend + frontend)

Both documents are spec-only (no code). I reviewed them for internal consistency, cross-document contract alignment, and completeness against the Round 2 changes described in the PR body.

Contract alignment is solid. The backend C-001…C-006 and the frontend Contract Consumed restate the same boundary and agree on every point I checked:

  • The six failure reasons map one-to-one — backend FR-016 (over size, disallowed type, collision, permission denied, staged content unavailable, unclassified) ↔ frontend FR-036.
  • The "counts and per-file results" correction landed in both: backend C-006/FR-020 ↔ frontend C-006/FR-023/FR-023a.
  • Declared-total split is consistent: frontend FR-038 (client declares total) ↔ backend FR-013c.1 (fast refusal on declared) / FR-013c.2 (authoritative during read).
  • Case-insensitive collision (backend FR-042a) is reflected in frontend FR-036.
  • Cross-references resolve correctly (e.g. backend FR-016a → frontend FR-030; frontend backend-FR citations all exist).

New Issues

  • 🟡 Medium: specs/37166-bulk-file-upload/spec.md:502Key Entities → "Batch limits" is now incomplete. It lists only "maximum file count per batch, and maximum size per file" but omits the maximum total size per batch added in Round 2 (FR-013b, Q7). Since this entity is meant to enumerate the operator-configured limits, the newest one should appear here too. Fix this →
  • 🟡 Medium: specs/37166-bulk-file-upload/spec.md:384Numbering gap: FR-035 does not exist in the backend spec. Requirements run FR-031→FR-034 (staging) then jump to FR-036 (surviving interruption). The frontend spec does have an FR-035, so a reader cross-referencing may assume backend FR-035 was dropped by accident. If the gap is deliberate (reserved), a one-line note would prevent that read; otherwise renumber. Non-blocking, but easy to fix now.

Minor

  • specs/37166-bulk-file-upload/spec.md:130 — User Story 3, Acceptance Scenario 1 still reads "notified of the outcome with its counts", the counts-only phrasing that Round 2 corrected in FR-020/C-006. It's defensible because "the outcome" is defined as counts + per-file results elsewhere, but tightening the wording here (as FR-020 was) would remove the last spot where a plan author could read "counts only."

Nothing blocking. The spec pair is unusually complete for altitude — the deliberately-deferred submission format (field names / endpoint / handle shape, left to contracts/ in planning) is called out explicitly in both documents, and the P1/P2/P3/P4 story prioritisation and out-of-scope reasoning are clear.

• branch 37166-content-drive-bulk-file-upload-signed

@zJaaal
zJaaal added this pull request to the merge queue Sep 2, 2026
Merged via the queue into main with commit 6e45ca1 Sep 2, 2026
48 checks passed
@zJaaal
zJaaal deleted the 37166-content-drive-bulk-file-upload-signed branch September 2, 2026 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Content Drive: bulk file upload (multi-file selection uploads only the first file)

2 participants