docs(content-drive): add backend and frontend specs for bulk file upload (#37166) - #37300
Conversation
Spec-Kit specification for the server side of 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. Scope is the backend only; the browser half of #37166 is delivered separately, so the spec carries an explicit "Contract Consumed by the Client" section as the boundary between the two. Decisions recorded in the spec: - This feature defines the shared batch-outcome contract (counts + per-file results); #37062 and #37063 consume it rather than the reverse, so this work is not blocked by an unbuilt ticket. - A configurable maximum file count per batch, refused at submission, and a configurable maximum size per file, recorded as a per-file failure that leaves the batch running. No batch-total size limit. - The submitter is notified on any terminal state, both pushed and durably recorded, following the bulk refresh precedent (#36845 / #37131). This keeps the outcome discoverable after navigation without requiring a client-side jobs screen. Directory upload and creating a missing target folder stay out of scope, as #37166 specifies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Claude finished @zJaaal's task in 1m 34s —— View job Spec review — #37166 Content Drive bulk file upload
This PR adds two specification documents only — no code. There are no bugs, security issues, or convention violations to flag against a runnable surface. The review below is at the altitude that applies: internal consistency of the specs and whether the prior findings landed in the committed text. Prior findings — recheckedResolved in the current text:
Existing
New Issues
AssessmentThe substantive gaps from both reviewers — the outcome-contract ownership, the staged-bytes hole, interruption/idempotency, and the collision race — are genuinely closed in the text, and the decisions are recorded with their reasoning rather than asserted. The one item that changes how the PR reads (F6's cross-reference framing) is the only thing I'd want corrected before the gate closes; the two new notes above can ride into I can't approve PRs or submit formal GitHub reviews (security restriction) — this is review feedback only. Approval to unblock |
Adds the browser-side specification for #37166, alongside the backend spec already on this branch. The two meet at the backend spec's "Contract Consumed by the Client" (C-001 ... C-006), restated in the frontend spec from the consumer's side so the boundary is reviewable from either document. No server behaviour is re-specified, and the backend spec is unchanged. Six required user stories plus one optional: - P1 Every chosen file is uploaded, from both entry points. The upload control's file input has no `multiple` attribute today, so only drag-and-drop delivers a batch; the ticket's premise that 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. - P1 In-flight state is reported in one place. Content Drive currently announces "this is running" four different ways, the worst of which replaces the entire listing with skeleton rows for an action fired on one 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. FR-024 was widened so an unrecognised terminal state degrades to an error rather than a green success, which is what makes deferring it safe. Per-row in-flight marking and bulk reindex's reporting are documented as out of scope, with reasons. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Clarify that FR-016's per-file "permission denied" reason is a narrower check than the target-folder permission refused at submission (FR-003) — it surfaces via the reused single-file path (FR-006) on a collision with an asset the author can't edit, or a content type they lack permission to create. Also align the frontend spec's reuse-consumer references (FR-032, SC-008) with the backend spec, which already cites both #37062 and #37063. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fabrizzio-dotCMS
left a comment
fabrizzio-dotCMS
left a comment
There was a problem hiding this comment.
Spec review — #37166 Content Drive bulk file upload
Reviewed both spec documents, grounded against origin/main.
Verdict
Both documents are unusually strong at the altitude Spec-Kit asks for: behaviour-first, no
implementation leakage, priorities argued rather than asserted, and a genuinely reviewable
Contract Consumed by the Client seam between the two halves. Structure is compliant — every
mandatory template section is present in both, plus the dotCMS Legacy Considerations override,
and ADR consultation is correctly deferred to /speckit-plan rather than pre-empted.
Five substantive findings, one of which is a factual conflict with code already on main.
None is a reason to reject the approach; all are things I would want fixed or written down
before the gate closes, because each one becomes expensive to discover during /speckit-plan.
Findings
1. FR-018 / SC-006 — the "shared batch-outcome contract" already exists on main
Backend spec, FR-018 and SC-006. The spec says this feature defines the batch-outcome shape
(counts + per-file results + reason/message) as a shared contract for #37062 / #37063 to adopt.
It already exists, shipped by the very PR the spec names as its precedent two sections later
(#37131 / #36845). BulkRefreshContentletsProcessor#getResultMetadata
(dotCMS/src/main/java/com/dotcms/jobs/business/processor/impl/BulkRefreshContentletsProcessor.java:358-374)
emits exactly:
total · processed · successCount · failedCount · skippedCount · results[]
with BulkRefreshItemStatus { SUCCESS, FAILED, SKIPPED } and an immutable per-item record
(AbstractBulkRefreshItemResult).
So FR-018 should read generalize / extract the shape already established by bulk refresh,
not define it. That matters: as written, a plan author is licensed to invent a second shape,
which is the exact outcome FR-018 exists to prevent.
The generalization is real work and worth naming, because the existing shape does not yet carry
what FR-016 and SC-006 need. Three concrete deltas:
- No machine-readable reason code.
AbstractBulkRefreshItemResulthasOptional<String> errorMessage()and nothing else. FR-016 requires both a machine-readable reason and a
human-readable message, distinguishing at least five cases. This is an additive change to a
shipped contract. - Keying. The existing item result is keyed by
identifier()+List<String> inodes().
A file being uploaded has neither — it does not exist until the run creates it — and a folder
path (SC-006's verification case) has neither either. A generic key is the substance of FR-018. - Naming drift. The shipped counter is
failedCount; issue #37166 and the surrounding
discussion sayfailCount. Pick one now, in the spec, or the two halves will disagree.
Direction of dependency. Issue #37166 states the opposite of the spec's Q1 — "Depends on
#37062 for the job-result contract (successCount / failCount / results), which the folder
endpoints define first." I checked: #37062 and #37063 are both OPEN and unbuilt, so the
spec's Q1 reasoning is sound and I would keep the reversal. But it is a decision taken against
the issue's stated dependency, so the issue needs amending — otherwise #37062's author defines a
third shape in good faith.
2. Nothing in the backend spec covers the bytes between the 202 and the processing
This is the largest genuine gap. The spec answers the submission immediately (FR-002) and creates
the files later (FR-005), and says nothing about where the uploaded bytes live in between, how
long they may live there, or who cleans them up.
That is not a hypothetical. The framework entry point the issue proposes reusing —
POST /v1/jobs/{queueName}/upload — stages every uploaded file through TempFileAPI.createTempFile
(JobQueueHelper.handleUploadIfPresent, :321-329). Four consequences:
TEMP_RESOURCE_MAX_AGE_SECONDSdefaults to 1800s (TempFileAPI.java:315, 30 minutes).
A large batch queued behind other jobs can outlive its own staged files. No FR covers staged-file
lifetime, and the failure mode — a batch that reports per-file failures for files the author
supplied correctly — is exactly the "barely better than today" outcome User Story 2 is written
to prevent.TEMP_RESOURCE_MAX_FILE_SIZEdoes apply on that path (TempFileAPI.java:145, default
-1= unlimited). The spec's Assumption that it "governs a different path" is true today —
today's Content Drive upload goesDotUploadFileService.uploadFileByBaseType→
newContentletByBaseType→ multipart workflow fire, no temp resource — but it stops being true
the moment the batch is built on the job framework's upload entry point. FR-011's new per-file
limit then needs a stated relationship to the existing one: same knob, or which wins.- Q2 records "no batch-total size limit." Combined with a configurable count limit and no
staged-file cleanup requirement, that is unbounded bytes on the shared assets volume from a
single authenticated back-office user. Constitution III (Security by Default) is the frame; I
would want at least one FR for cleanup on any terminal state, and an explicit stance on total
bytes even if the stance stays "none, deliberately." - Good news worth recording in the spec, not just knowing: staging lives under
ConfigUtils.getAssetTempPath()=<assets>/tmp_upload, which is cluster-shared, so a job
picked up on another node still sees its files. AndTempFileAPIalready contains the
client-supplied filename with an absolute-path check, so filename path traversal is handled by
the reused path — a point in FR-006's favour that currently goes unstated.
3. The generic job upload endpoint accepts exactly one file today
JobParams declares a single @FormDataParam("file") InputStream fileInputStream plus one
FormDataContentDisposition. POST /v1/jobs/{queueName}/upload therefore cannot carry a batch as
it stands.
The spec is transport-agnostic and that is correct at this altitude — I am not asking it to name
an endpoint. But FR-001 ("accept a batch of several files in one submission") cannot be satisfied
by the entry point issue #37166 proposes without either changing JobParams (a shared type, so a
change with other consumers) or adding a resource that does its own multipart handling. That is a
fork in the road with different blast radii, and it should be an explicit /speckit-plan
obligation in the spec rather than something the implementer meets on day one.
4. FR-006 — "the same path the existing single-file upload uses" is ambiguous at the wrong altitude
FR-006 and Acceptance Scenario 1.3 both lean hard on this phrase, and Legacy Considerations makes
it the justification for not introducing a second creation path. It is doing a lot of work for a
phrase that has two readings.
Today "the path" is a REST call: DotUploadFileService.uploadFileByBaseType
(core-web/libs/data-access/src/lib/dot-upload-file/dot-upload-file.service.ts:143-157) →
newContentletByBaseType → a multipart workflow fire. A job processor cannot re-enter REST, so
the literal reading is unavailable. The intended reading is presumably the contentlet-creation and
workflow-fire layer beneath the resource.
Say which. Either name the layer, or state the intent as the requirement — identical content-type
resolution, permission enforcement and workflow behaviour, with the reuse point chosen in the
plan — and let FR-006 be about the observable equivalence rather than about sharing a call site.
5. SC-003 is not a bar the implementation can fail
SC-003: A batch of 50 files is not measurably slower per file than 50 sequential single-file
uploads, and does not serialize behind a per-file search-index wait.
The baseline is today's behaviour, and today's behaviour passes indexPolicy: 'WAIT_FOR' on every
upload (dot-content-drive-shell.component.ts:1001) — which is precisely the pathology FR-008
exists to remove. Measuring the fix against the unfixed baseline makes the criterion true by
construction.
Give it something falsifiable: an absolute per-file target, or a comparison against 50 sequential
uploads with indexing deferred, or a throughput floor. The second clause ("does not serialize
behind a per-file index wait") is already good — it is the first clause that is free.
Medium
6. Two specs in one PR, against a gate that says one
The documented flow is PR 1 = spec.md alone, approved not merged. This PR carries two. The
title, body and checklist all say backend-only, so a reviewer clicking Approve is also approving
527 lines of frontend spec they were not told they were reviewing.
The framing compounds it: the frontend spec (:26-29) tells the reader the server half "is
delivered separately by another developer and is specified in specs/37166-bulk-file-upload/spec.md
(PR #37300)" — pointing at the PR they are already reading, for a file already in the diff.
The bundling is defensible on the merits — the two halves share a contract and reviewing them
together is genuinely better than reviewing them apart. But then say that, and drop the "delivered
separately (PR #37300)" pointer. Either split into two PRs with correct cross-references, or keep
them together and make both documents describe the arrangement that actually exists. (The bot
raised this too; I agree with it, and it is the one finding that changes how the PR should land
rather than what it should say.)
7. Frontend User Story 5 defers an explicit acceptance criterion to a ticket that does not exist
Issue #37166 lists, under Frontend, "The indicator offers cancellation for a cancellable run."
The frontend spec demotes the whole stopping group (User Story 5, FR-018 … FR-020) to P4,
optional, deferred to "the anticipated task manager."
The argument is one of the better-written passages in either document, and FR-024 is a real safety
net rather than a hand-wave: an unrecognised terminal state degrades to an error, not to a false
success. I would accept the deferral on the merits.
What I would not accept is that it currently lives only as a paragraph. Backend FR-025 … FR-028
build a cancellation capability with no dated consumer, and C-004 is explicitly "not consumed in
this pass." Deferring an issue's explicit AC needs the issue amended and a follow-up ticket filed
with the deferred scope attached — otherwise the task manager stays "anticipated" indefinitely and
C-004 quietly becomes dead server surface.
8. Test strategy: nothing is skipped, but nothing says so either
Both specs push all test definition to /speckit-plan, which is consistent with the flow, and the
per-story Independent Test lines are well-formed and genuinely independent — better than most.
But Constitution V requires that a skipped test type be named and justified, and neither
document names a single test type, skipped or otherwise. Issue #37166, by contrast, enumerates the
coverage it expects (integration: many files, mixed partial failure, rejected extension, oversized
file, permission denied, mid-batch cancellation, progress; Jest: multi-file success, partial
failure, cancellation, single-file unchanged, no in-flight toast, grid stays rendered, two
concurrent runs).
One line per spec closes it: which layers will be exercised (unit / integration / Postman / Karate
/ e2e) and which will not, with the reason. That statement is cheap now and is the thing
/speckit-tasks inherits.
9. No stated limit on concurrent batches per submitter
FR-010 caps files per batch. Nothing caps batches. One author can queue many maximum-size batches
back to back, which pairs badly with finding 2's unbounded staged bytes. Probably a one-line
Assumption ("out of scope; the job queue's own concurrency governs it") rather than a new FR — but
it should be a decision rather than an omission.
Claims verified against origin/main
Read, not executed. Line numbers from the working tree; where they differ from issue #37166 the
drift is line movement only, and the code matches.
| Claim | Verdict | Evidence |
|---|---|---|
| Multi-file selection warns and uploads only the first | ✅ | dot-content-drive-shell.component.ts:939 branches files.length > 1 → uploadFiles (:955, toast) → uploadFile (:970) → uploadByBaseType(files[0], …) (:981) |
content-drive.multiple-files-warning exists |
✅ | Language.properties:7220 (issue said :7224) |
indexPolicy: 'WAIT_FOR' on every upload |
✅ | dot-content-drive-shell.component.ts:1001 |
Hidden file input carries no multiple |
✅ | dot-content-drive-shell.component.html:175 |
actionExecution is one slot; the guard is portlet-global |
✅ | withActionExecution.ts:30, 58; guard store.actionExecution() at :132, :198, :259 |
| Context-menu workflow action blanks the grid | ✅ | dot-folder-list-context-menu.component.ts:275, :293 — setStatus(DotContentDriveStatus.LOADING) |
Indicator label bound with [innerHTML] |
✅ | dot-content-drive-toolbar.component.html:96 |
Folder create shows a hardcoded English summary: 'Success' |
✅ | dot-content-drive-dialog-folder.component.ts:291 |
| Both folder error paths leak the raw server message | ✅ | same file, :309 and :342 — detail: error.message |
| Orphan workflow copy keys | ✅ | Language.properties:7142 (workflow-in-progress), :7143 (-detail), :7145 (workflow-executed-detail) |
| Bulk refresh precedent does push and durable record, best-effort | ✅ | BulkRefreshCompletionListener.java:116 (systemEventsAPI.pushAsync), :163 (notificationAPI.generateNotification), failures logged at :119, :123; user resolved and warned on failure at :106. FR-019 … FR-023 map onto this one-for-one |
| Job framework supports queue / cancel / progress / monitor | ✅ | processor/Queue.java, processor/Cancellable.java; JobQueueResource /{jobId}/status:273, /cancel:325, /monitor:814 — supports backend FR-024 … FR-028 and frontend FR-032 |
TEMP_RESOURCE_MAX_FILE_SIZE defaults to unlimited |
✅ | TempFileAPI.java:145, Config.getLongProperty(…, -1L) |
| …and "governs a different path" | ||
| #37062 / #37063 unbuilt, so Q1's premise holds | ✅ | both OPEN; no PR other than #37300 references them |
| Batch-outcome contract does not exist yet (FR-018) | ❌ | it does — see finding 1 |
| Generic job upload endpoint accepts a batch | ❌ | single @FormDataParam("file") in JobParams.java:18-22 — see finding 3 |
What is good, and worth keeping through the plan
- The
Contract Consumed by the Clientsection is the right invention. Two developers, two
PRs, one seam — and the seam is written down from both sides with matching identifiers
(backend C-001 … C-006, frontend C-001 … C-006, including the deliberate note that C-004 is not
consumed this pass). This is the part of the spec most likely to save the project a week. Not settled at this altitude(frontend:431-434) — recording the unpinned submission
format as a planning obligation rather than discovering it in review is exactly right, and the
three findings above should join it there rather than becoming new requirements.- FR-017 and FR-022 (counts are the server's; submitted count is never a stand-in) and their
frontend mirrors FR-022 / FR-024 — this is the original defect generalized into a rule, which is
what a spec is for. - User Story 3's priority argument on the backend side: P1 rather than P2, justified by the
asynchrony the feature itself introduces, and grounded in an in-repo precedent instead of
invention. - Constitution posture: I (new work in
com.dotcms, reuses rather than rewrites), IV (FR-029 /
FR-030 cover@Schemaaccuracy and regeneratingopenapi.yaml), and the ADR rule (consultation
deferred to plan, no ADR authored) are all respected. II is satisfied implicitly by FR-013 —
fine at this altitude.
Suggested resolution
- Rewrite FR-018 as generalize the bulk refresh outcome shape, name the three deltas
(reason code, generic key,failedCountnaming), and retarget SC-006 at the shipped type. - Add the staged-bytes requirements: lifetime, cleanup on terminal state, the relationship
between FR-011 andTEMP_RESOURCE_MAX_FILE_SIZE, and a stated position on total bytes. - Record "the generic job upload entry point is single-file today" as a plan obligation.
- Disambiguate FR-006 to the observable equivalence, not the call site.
- Give SC-003 a falsifiable baseline.
- Reconcile the two-specs-one-PR framing, either way.
- File the follow-up for deferred cancellation, and amend #37166's ACs to match.
- One line per spec on test types covered and skipped.
Findings 1–3 are the ones I would want answered in the thread before approval; the rest can ride
into /speckit-plan.
|
Two findings I saw before planning: No idempotency on resubmission after a dropped connection, risk of duplicate assets. And no defined behavior for a job in-flight when the server crashes or restarts. Does it survive, fail, or just orphan with no notification? Secondary, worth a check in planning rather than a spec rewrite: batch size is only bounded indirectly (count × per-file max). So concurrent batches racing on the same folder have no defined collision winner, and FR-016's raw error message vs FR-030's resolved copy should explicitly say the raw one is log-only. Otherwise solid, no open clarifications, requirements testable, BE/FE contracts agree. Approve with comments! Pending answers on the two blockers. |
zJaaal
changed the title
…ile-selection-uploads-only-the-first-file
…oad work (#37166) The frontend spec mentioned the permission only in an acceptance scenario, an edge case and the Legacy section. Nothing required the client to keep honoring it, which is a gap worth closing given this feature edits the upload control and the drop path directly, and the gate lives on both. Content Drive already gates every route into an upload on CAN_ADD_CHILDREN and offers each one disabled *with the reason* rather than hiding it: the upload control, the create menu, the drop zone, and a re-check on the drop target before anything is sent. The site root resolves its permission separately, having no folder to carry it. Adds FR-034 stating that gate must survive unchanged, two acceptance scenarios on User Story 1, and SC-010. As with the batch-size limit, the client-side gate is an affordance and the server stays the point of enforcement (backend FR-003, FR-004). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…cator stays quiet (#37166) Two gaps found while sweeping the spec. **The task manager is not hypothetical.** #33331, "[EPIC] Task manager UI to manage large uploads and moves", already owns it, and already claims cancel: "From this task manager UI, we can also enable the user to cancel individual file uploads, or cancel the whole upload or move." The spec said "anticipated as future work" and named nothing. Deferring User Story 5 is therefore alignment with an existing epic rather than a judgment call this spec made, so all six references now cite #33331. **Nothing said what the indicator should announce.** It is a polite live region today, and this feature adds determinate progress, an item name, and concurrent runs to it. A live region that re-announces on every progress tick is hostile to a screen reader user, and the spec was silent on it. FR-035 limits announcements to the events worth hearing - a run starting, ending, or being stopped - and requires a progress value to be available on request rather than pushed. SC-011 makes it verifiable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The out-of-scope note pointed at an unfiled follow-up. It is now #37322, so the reference names it instead of gesturing at it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Applies the substantive findings from the spec review on #37300. FR-018 said this feature defines the batch-outcome shape. It does not: bulk refresh already ships one, and this feature generalizes it. As written a plan author was licensed to invent a second shape, which is the outcome FR-018 exists to prevent. The three deltas are now named (machine-readable reason code, generic item key, one spelling of the counters), and Q1 records that this reverses the dependency direction #37166 states, so that issue needs amending. Nothing covered the uploaded bytes between the accepted submission and the run that consumes them. A new Staged file content group requires the content to outlive the wait, expiry to be a named per-file failure rather than a silent loss, reclaim on any terminal state, cluster readability, and a stated position on total staged bytes per author. FR-006 required creating each file "through the same path the existing single-file upload uses". That path is entered over REST and a background run cannot re-enter it, so the literal reading was unavailable. It now requires observable equivalence, with the reuse point chosen in the plan. SC-003 measured the batch against today's behaviour, which waits for each file to become searchable - the pathology FR-008 removes - making the criterion true by construction. It now asks for an absolute floor. Also records the domain-endpoint decision and the one-call client contract, and adds a Planning Obligations section for the work these findings uncover. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
(comment removed - was posted in error during automated tooling check) |
…ize rule (#37166) Sets FR-010's maximum files per batch to 50, configurable. Corrects the size and type requirements. The spec assumed no per-file size rule existed on the single-file path and that FR-011 was therefore a new control. It is not: ESContentletAPIImpl.validateBinary enforces both a size ceiling (maxFileLength) and an allowed-types rule (accept), declared per content type on the binary field, during the contentlet validation the creation path runs. Reusing them is not merely cheaper, it is what FR-006 requires: a batch that rejected a file the single-file upload accepts would not be observably equivalent. FR-011 and FR-012 now require surfacing that existing rejection as a per-file failure rather than introducing a second limit. The consequence, accepted knowingly, is that where an operator has configured nothing there is no ceiling. Also aligns the Context section with the FR-018 correction from the previous commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Thanks @fabrizzio-dotCMS this was a useful review. Findings 1–5 are all fixed in 1. The outcome contract already exists — agreed, and fixedYou're right, and this was my error: I read SC-006 is retargeted at the shipped type: it now requires the existing consumer to still read its On the dependency direction — I've kept the reversal for the reason you confirmed, and 2. The bytes between the 202 and the processing — the best catch hereThis was a real hole and I've added a Staged file content group (FR-031 … FR-035): content Two things I verified while writing them, since they constrain the fix:
3. Single-file entry point — resolved rather than deferredDecided, not left to the plan: a domain endpoint owning its own multipart handling, recorded Also added Q5: the client sends one call carrying the content and never handles a staging 4 and 5FR-006 now requires observable equivalence — same resolved content type, same permissions, SC-003 now asks for an absolute per-file throughput floor set in the plan, with the reason the One correction your review surfaced indirectlyChasing finding 2 I checked whether a per-file size rule already existed, and it does — I had it That makes FR-011 not a new control at all. More importantly, inventing a bulk-specific ceiling Also set FR-010's batch cap to 50 files, configurable. Still open, and both are mine to close
Finding 8's test-layer statement is in Planning Obligations, carrying the coverage #37166 already |
…37166) Addresses the review findings from @ihoffmann-dot. The two blockers raised turned out to be one defect. AbandonedJobDetector re-queues a run whose worker stopped reporting, so "what happens on a restart" was already answered - it is retried. Retrying a batch that created 30 of 50 files, without resuming, creates 30 duplicate assets. Bulk refresh avoided this with a no-retry policy because reindexing twice is harmless; creating a file twice is not. FR-036 through FR-040 therefore require resumable execution: completed files recorded durably as the run goes, one notification per batch across attempts, counts covering the whole batch, and a defined answer for a client that lost the connection mid-submission. FR-041 through FR-043 cover the concurrency gap. The spec described collision against an already-existing file, which the product validates today. It said nothing about two runs racing for the same name, where the existing check-then-create can pass for both. Exactly one must win. On size, the secondary finding got worse rather than better: the previous commit made the per-file ceiling the content type's own, which is unset by default, so count x per-file was no longer a bound at all. There is now a configurable batch total refused at submission (FR-013b), and per file the content type's ceiling falls back to a configurable value where none is declared. That fallback makes a batch stricter than a single upload, which is a knowing exception to FR-006 and is recorded as FR-011a rather than left to be found later. FR-016a settles the raw-message question: the reason code is what the client presents, the message is diagnostic and log-only, matching the frontend spec's FR-030. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@ihoffmann-dot thanks — both blockers are answered, and chasing them turned up that they are the The two blockers are one problemYour second question has an answer already in the framework: Which is exactly your first blocker. Retrying a batch that already created 30 of 50 files, from Decision: resumable execution (FR-036 … FR-039). The run records completed files durably as it The alternative — no-retry, so an interrupted batch just fails — was rejected: at 50 files a Idempotency on resubmission is FR-040: a client that lost the connection and resubmits must The collision race — you were right, and the spec only covered half of itThe spec described collision against an already-existing file. That case is real and grounded: But that is a check followed by a create. Two concurrent batches each containing Batch size — it was worse than "bounded indirectly", and now it is boundedWorth flagging that your finding got worse between your review and this fix, through a change That rule is unset by default. So Now bounded in two places:
The fallback means that for a content type with no ceiling, a large file is rejected in a batch and Raw vs resolved copyAgreed, and specified: FR-016a. The reason code is what the client maps to product copy; the Also added SC-009 … SC-011 so the three new behaviours are measurable, and the corresponding edge |
…ile-selection-uploads-only-the-first-file
…ze ceiling (#37166) Takes up three additions to the backend spec's client contract. **C-002b, failure reasons.** The server sends a reason code plus a diagnostic message, and the message is never shown - which the frontend spec already required in FR-030, so the two halves were already aligned on the principle but nothing said the client owed copy for each reason. FR-036 names all six (over size, disallowed type, name collision, per-file permission, staged content gone, unclassified) and states that a reason without copy is a hole the author sees. Staged-content-unavailable is new since the first draft. **C-002a, retry after a lost connection.** Resubmitting the same batch cannot duplicate the author's files. FR-037 makes the client treat an uncertain submission as retryable rather than as a failure the author has to reason about, and forbids asking them to go and check the folder first. **C-002, total size ceiling.** Now a sixth submission refusal alongside the file count. The edge case distinguishes it from "too many files": different problem, different fix, so they cannot share one message. Adds SC-012, and syncs the Contract Consumed section with the backend spec's current C-002 / C-002a / C-002b. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fabrizzio-dotCMS
left a comment
fabrizzio-dotCMS
left a comment
There was a problem hiding this comment.
Spec review — round 2, after the revisions
Reviewed: 2026-09-01 · head 8c3009b4 · grounded against origin/main (edbb1a0f) in ~/Coding/core
Since round 1: 9 commits, two long author replies, ihoffmann-dot APPROVED at 19:25Z.
Files: backend 673 lines (was 406) · frontend 591 (was 527)
Verdict
Round 1's findings 1–5 and 9 are genuinely closed in the committed text, not merely answered in the
thread — I re-read both documents end to end rather than trusting the reply. The additions made for
ihoffmann-dot's two blockers (User Story 5, FR-036 … FR-043) are the strongest new material: they
found that the two blockers were the same defect, which was the right call.
The specs are now 1264 lines and carry decisions with their reasoning, which is unusual and good.
The cost is that the newest requirements were written fast, and three of them make claims about the
job framework that the framework does not currently support, or ask for work the schema already
does. That is what round 2 is for.
One high finding (R2-1), five medium, five low. R2-1 is the only one I would want answered
before the gate closes; it is a sizing question, not an objection.
High
R2-1. FR-037 asks the job framework for something it does not have
FR-036 … FR-038 are the load-bearing requirements of the new User Story 5, and FR-037 states the
mechanism plainly:
the run MUST record which files it has completed durably and as it goes, not only in the
final outcome.
The job framework persists exactly three things about a run, and none of them is that:
| What | Where | When |
|---|---|---|
parameters() — Map<String,Object> |
JobContract.java:49 |
written once at createJob, immutable thereafter |
progress() — a single float |
JobContract.java:60; JobQueue#updateJobProgress(String, float) :284 |
mid-run, but it is one number |
result() — JobResult |
JobContract.java:47, harvested from JobProcessor#getResultMetadata(Job) |
once, at the terminal state |
BulkRefreshContentletsProcessor says so about itself, in a comment on the very method the spec
generalizes: "this map is persisted with the job … when it is requested this is the only place the
records survive." Its counters are AtomicInteger fields and itemResults is an in-memory list.
And the resume path carries nothing forward. PostgresJobQueue#putJobBackInQueue (:640-659)
writes id, queueName, PENDING, priority 0, now() — no result, no progress, no marker that
this is attempt two. The processor is @Dependent, so the resumed run gets a fresh instance with
every counter at zero.
So FR-037 is not "record the outcome carefully"; it is either a new job-framework capability
(mid-run durable per-item state) or a feature-owned durable store, and FR-038's "counts across
all attempts" needs the same thing. That is the largest piece of hidden work in either document and
it is not in §Planning Obligations — where staged-content lifetime, which is a smaller problem,
correctly is.
I am not asking the spec to choose. I am asking it to say that the choice exists and is expensive,
so /speckit-plan sizes User Story 5 honestly rather than discovering this on day one. The same
paragraph should note the consequence for FR-018: a shape designed only for getResultMetadata at
terminal state will not serve a run that must read its own prior progress back.
Worth recording in the spec's favour, since it makes the premise solid: AbandonedJobDetector
does re-queue unconditionally — processAbandonedJob → putJobBackInQueue (:141-142) never
consults the retry policy. So the premise "it is retried" holds even for a @NoRetryPolicy
processor, which is what makes Q6's rejection of the no-retry option correct rather than merely
preferred. Note the closest precedent went the other way: ImportContentletsProcessor is
@NoRetryPolicy (:81) and creates contentlets from a staged upload — the same shape as this
feature — so it is already exposed to the duplicate risk Q6 identifies. That is a defect in import,
not in this spec, but it is why the spec should not lean on import as a precedent.
Medium
R2-2. FR-042's window is already closed — by the schema, not by anything the plan must build
FR-042 says the product's collision rule is a check followed by a create and "the plan MUST close
that window rather than rely on the check alone", and FR-043 then constrains whatever closes it to
be scoped so unrelated uploads do not serialize.
Both are written as if an application-level lock were required. It is not. The identifier table
carries a unique constraint on the exact tuple:
create table identifier (
...
unique (parent_path, asset_name, host_inode)
);dotCMS/src/main/resources/postgres.sql:1033, added by
Task00785DataModelChanges.java:1380.
So "exactly one MUST succeed" (FR-041) is guaranteed by the database on every existing install. The
plan's work is to catch the constraint violation and map it to the collision reason, alongside
the pre-check that already throws FileAssetValidationException with
message.contentlet.fileasset.filename.already.exists (ESContentletAPIImpl.java:8718-8728) — and
FR-043 comes free, because a unique index contends only on the contended key.
This is a finding in the spec's favour and I would still fix the wording, because "the plan MUST
close that window" licenses a plan author to build a distributed lock for a race the schema already
loses on their behalf. One caveat to state while rewriting: the constraint is on the raw columns,
while the path trigger indexes lower(parent_path||asset_name||'/')
(postgres.sql:2236). Say whether "the same file name" in FR-041 means case-sensitively the same,
because the guarantee differs between the two readings.
R2-3. FR-013b, FR-013 and Q5 cannot all three hold as written
Three requirements, pairwise reasonable, jointly impossible:
- Q5 — one call, carrying the file content; the client never handles a staging identifier.
- FR-013b — the total-size ceiling is "refused at submission (FR-003) before any content is
staged." - FR-013 — "The system MUST NOT rely on a size limit supplied by the caller."
If the bytes arrive in the same request, the only signal available before reading them is what the
caller declares — Content-Length, or the per-part sizes. That is caller-supplied, so FR-013
forbids trusting it; and if you do not trust it, you learn the true total only by reading, at which
point content has been staged and FR-013b's "before" is false.
The product already resolves this the second way: TempFileAPI writes through a
BoundedOutputStream(maxFileSize(request), …) (TempFileAPI.java:241) and aborts when the bound is
crossed — bytes are staged, then reclaimed. That is a fine answer; it is just not "before any
content is staged."
Pick one and say it: refuse on the declared total as a fail-fast convenience while enforcing the
real ceiling during the stream (and then FR-013 is about enforcement, not about the fast refusal),
or accept that partial staging happens and require it to be reclaimed — which FR-033 already covers
for terminal states and would need to cover for a refused submission too. As written the three
requirements will send a plan author in a circle.
R2-4. FR-018 still defers a decision that has already been made twice
FR-018's third bullet:
The shipped counter is
failedCount; #37166 and the surrounding discussion sayfailCount. The
plan MUST pick one and both halves MUST use it.
It has been picked. Issue #37166's 2026-09-01 amendment, §1: "failedCount wins, matching what
already ships." And the author's own PR comment: "On the last one I've picked failedCount."
The spec is the last artifact still calling it open, which is precisely the state FR-018 exists to
prevent — the two halves are built by different developers and the spec is what they both read.
Pin failedCount in the text. (The bot raised this too; it is a one-word fix.)
R2-5. FR-035 is an obligation on the authors, not a requirement on the system
FR-035: … The system MUST additionally state a position on the total bytes one author may
hold staged across concurrent batches … "No limit across batches, deliberately" is an acceptable
position; leaving it unstated is not.
A system cannot state a position. This is a requirement that a decision be made later, written in
the register of a requirement about behaviour — it has no acceptance scenario, nothing in §Success
Criteria measures it, and no test can fail it. It reads as satisfying round 1's finding 9 while
actually deferring it.
Either decide it here (one line in §Decisions: "no cross-batch cap, deliberately — the job queue's
own concurrency governs it") or move it verbatim into §Planning Obligations, which is where the
document already puts exactly this kind of item and where it would read honestly. FR-013b and
FR-010 are real requirements and should not sit next to one that is not.
R2-6. Constitution V: the frontend spec still has no test statement
Round 1 finding 8 asked for one line per spec naming the layers covered and skipped. The backend
closed it properly, in §Planning Obligations, carrying the coverage #37166 enumerates.
The frontend spec did not. The words unit, Jest, integration, e2e and Karate appear
nowhere in its 591 lines; the only occurrences of "test" are the per-story Independent Test lines
and the section heading. It has no §Planning Obligations section to put it in. Issue #37166
enumerates the Jest coverage it expects — multi-file success, partial failure, cancellation,
single-file unchanged, no in-flight toast, grid stays rendered, two concurrent runs — so the
material exists and needs only to be carried across.
Principle V is non-negotiable and its wording is that a skipped test type must be named and
justified. Silence is the one answer it does not allow. This is half of round 1's finding 8 still
open, and it is the cheapest thing on this list to close.
R2-7. FR-040's second branch contradicts the frontend's FR-037
FR-040 offers the plan two ways to make resubmission safe:
either the resubmission is recognised as the same batch and returns the original handle, or
the already-created files are rejected by the collision rule.
The frontend consumes this as C-002a and states what it buys (FR-037):
The client therefore treats an uncertain submission as retryable, not as a failure the author
has to reason about, and never asks them to check the folder first.
The two branches do not deliver the same thing. Under the collision branch a resubmission
re-uploads every byte, creates a second batch with a second handle, runs to completion, reports
50 of 50 failed — file already exists, and fires a second notification. The author's data is
safe, which is what FR-040 promises; but what they are shown is a total failure of a batch that
actually succeeded, which is exactly "a failure the author has to reason about." The frontend also
has no way to tell that outcome apart from a genuine all-collided batch.
Both branches satisfy the letter of FR-040 and only one satisfies FR-037. Either require the handle
branch, or require the collision branch to be distinguishable at the contract (a reason, or a flag
on the outcome, saying "this batch was a duplicate submission") so FR-037 can keep its promise.
Low
R2-8. "Disallowed extension" is a MIME-type check, and it is skipped when the type is unknown
FR-016 lists "disallowed extension" among the reasons; User Story 2 says "a blocked extension";
frontend FR-036 promises copy for "the file type is not allowed."
What the code does (ESContentletAPIImpl#validateBinary, origin/main, from :8423): it resolves
APILocator.getFileAssetAPI().getMimeType(binary) and matches it against the allowedFileTypes
field variable as a MimeType. The extension is never consulted. And the rule is guarded by
if (UtilMethods.isSet(allowedFileTypes) && UtilMethods.isSet(binaryMimeType)
&& !FileAsset.UNKNOWN_MIME_TYPE.equals(binaryMimeType)) {so a file whose MIME type does not resolve skips the check entirely and is accepted. Two
consequences worth a line each: call it a file-type rule rather than an extension rule, and name the
unresolvable-type case, because the client copy currently promises a check the server sometimes
does not perform.
R2-9. The six machine-readable reasons come out of a layer that distinguishes them by message text
FR-016 requires reasons distinguishing at least six cases. Underneath:
- over size →
DotContentletValidationException.builder(LanguageUtil.get("message.contentlet.binary.file.exceeds.size", …)) - disallowed type →
DotContentletValidationException.builder(LanguageUtil.get("message.contentlet.binary.type.notallowed")) - name collision →
FileAssetValidationExceptionviafileAssetBuilder(LanguageUtil.get("message.contentlet.fileasset.filename.already.exists"))
The first two are the same exception type, differing only in a localized string, and all three
arrive through the same validate call. Deriving a stable machine-readable reason from that is real
work — pre-checking before the create, or extending the validation layer to carry a code — and it
is the mechanical foundation of FR-016, FR-016a, C-002b and frontend FR-036 all at once. A line in
§Planning Obligations, next to the FR-018 generalization it will probably travel with.
R2-10. FR-011a is the one knowing divergence and it has no acceptance scenario
User Story 2's scenario 5 covers only the content-type-declared ceiling — "and the same file
uploaded alone is rejected too, for the same reason" — which is the equivalent case. The
FR-011a case is the opposite: a file the batch rejects and a single upload accepts. It appears in
FR-011a and in one Edge Case bullet, and nowhere in a scenario or a success criterion.
Given it is described in the document itself as a deliberate exception to FR-006, it is the case
most likely to be reported as a bug by whoever meets it first, and the one that most needs a test
to exist. One scenario under User Story 2.
R2-11. SC-003 is honest now but still not evaluable at this gate
A batch of 50 files sustains a stated per-file throughput floor, set in the plan …
Round 1's objection is fixed — it is no longer true by construction, and the reasoning is written
inline so it cannot regress. But an SC whose number arrives in the next artifact cannot be checked
at the spec gate. Not worth blocking on, given the reason is recorded; noting it so nobody reads
SC-003 as measurable when it is currently a promise to become measurable.
R2-12. #33331 is a real home for C-004, but not a dated one
The deferral of frontend User Story 5 is well argued and #37166's amendment §6 records it properly
("Resolved: #33331 covers it explicitly, so no new ticket"), which closes round 1's finding 7 as
far as process goes. For the record: #33331 is OPEN and was last updated 2025-12-22 — over eight
months ago, with no linked work. The argument for deferring stands on its own merits; it just should
not be read as "arriving soon," and backend FR-025 … FR-028 should be understood as capability built
ahead of a consumer with no date.
R2-13. Two specs in one PR — still unfixed in the committed text
Round 1 finding 6; the author agreed to keep them together and fix the framing; the bot flagged it
again. It is still there, on both sides:
- frontend
:22-27— "The server side is delivered separately by another developer and is
specified inspecs/37166-bulk-file-upload/spec.md(PR #37300)" — pointing at this PR, for a
file in this PR's diff. - backend
:22-25— "The browser-side work of #37166 … is delivered separately by another
developer", with no mention that the frontend spec is in the same PR.
The implementations land separately; the specifications are here, together, on purpose. Say
that in both documents and drop the "(PR #37300)" pointer.
Also worth a line in §Assumptions (not findings)
- The abandonment clock is reset by progress.
AbandonedJobDetectortreats a run as abandoned
afterJOB_ABANDONMENT_THRESHOLD_MINUTES(default 30, scanned every 5) without an update
(AbandonedJobDetectorConfigProducer.java:14-15), andupdateJobProgresswrites
progress = ?, updated_at = ?(PostgresJobQueue.java:193-194) against the detector's
WHERE j.updated_at < ?(:94). So FR-024's per-file progress reporting is what keeps a long but
healthy batch from being declared abandoned and re-queued while it is still running — two live
executions of the same batch, which is a worse state than the restart FR-036 is written for. That
makes FR-024 (currently P2, "progress and cancellation") a dependency of FR-036 (P1). Cheap to
state, and it changes how the two are prioritized. - The a11y requirement is well aimed. The indicator is already
role="status" aria-live="polite"
with<span [innerHTML]="$actionExecutionLabel()">
(dot-content-drive-toolbar.component.html:110-119). So frontend FR-035's warning about a chatty
live region is a real risk of the change rather than a hypothetical, and FR-011's
no-markup-injection requirement is squarely on an existing[innerHTML]binding. Both are
correctly targeted; worth citing the binding so the plan does not have to rediscover it.
Claims verified against origin/main (edbb1a0f) — round 2
Read, not executed.
| Claim (spec or author reply) | Verdict | Evidence |
|---|---|---|
| The framework re-queues a run whose worker stopped reporting | ✅ | AbandonedJobDetector.java:113-125 scan, :141-142 requeue, :157 jobQueue.putJobBackInQueue |
…and does so regardless of @NoRetryPolicy |
✅ | processAbandonedJob never consults the retry policy; RetryPolicyProcessor.java:47-48 is the failure path only |
Bulk refresh sidesteps duplication with @NoRetryPolicy |
✅ | BulkRefreshContentletsProcessor.java:58, :65 — "Reindexing is not retried" |
ImportContentletsProcessor — content creation from a staged upload — is also @NoRetryPolicy |
✅ | :81; so the closest precedent chose the option Q6 rejected |
| The framework can durably record per-item progress mid-run | ❌ | JobContract.java:47-62; JobQueue.java:284 is one float; getResultMetadata is terminal-only — R2-1 |
putJobBackInQueue carries prior state forward |
❌ | PostgresJobQueue.java:640-659 writes id / queue / PENDING / priority 0 / now — R2-1 |
| The per-file size rule is a content-type field variable, unset by default | ✅ | ESContentletAPIImpl#validateBinary; BinaryField.MAX_FILE_LENGTH = "maxFileLength" (:35); -1 != maxLength guard |
| The allowed-types rule is likewise per content type | ✅ | BinaryField.ALLOWED_FILE_TYPES; same method |
| …and it is an extension check | ❌ | it is a MimeType match, skipped on UNKNOWN_MIME_TYPE — R2-8 |
| Size and type rejections are distinguishable by exception type | ❌ | both DotContentletValidationException, differing only in the localized message — R2-9 |
| Collision is a check followed by a create | ✅ | ESContentletAPIImpl.java:8692-8694 fileNameExists → :8718-8728 throw |
| …so the plan must close the race window | the schema closes it: unique (parent_path, asset_name, host_inode), postgres.sql:1033 — R2-2 |
|
TEMP_RESOURCE_MAX_AGE_SECONDS is global, no per-call override |
✅ | TempFileAPI.java:315, read from Config inside the private getTempFile(String) both overloads funnel into (:362, :381); default 1800 |
TEMP_RESOURCE_MAX_FILE_SIZE is narrowable per call, downward only, by the caller |
✅ | maxFileSize(request) :141-146, reads the caller's maxFileLength parameter; system default -1 |
| A bounded stream is the existing way a size ceiling is enforced mid-upload | ✅ | BoundedOutputStream(maxFileSize(request), …) :241 — relevant to R2-3 |
The shipped outcome shape is total / processed / successCount / failedCount / skippedCount / results |
✅ | getResultMetadata, BulkRefreshContentletsProcessor — and results is only emitted when asked for |
#37166 amended: dependency reversed, failedCount wins, cancellation deferral recorded |
✅ | issue body, Amendment — 2026-09-01, §§1 and 6 |
…and the spec pins failedCount |
❌ | FR-018 still says the plan must pick — R2-4 |
| #37322 filed for per-item in-flight marking | ✅ | OPEN, "[TASK] Content Drive: show which rows an operation is running on" |
| #33331 owns the stop control | ✅ ( |
OPEN, "[EPIC] Task manager UI to manage large uploads and moves", last updated 2025-12-22 — R2-12 |
The toolbar indicator is a polite live region carrying [innerHTML] |
✅ | dot-content-drive-toolbar.component.html:110-119 |
| Progress updates reset the abandonment clock | ✅ | PostgresJobQueue.java:193-194 vs :94 |
What round 2 confirms is closed
Re-read in the committed text, not taken from the reply:
- R1-1 — FR-018 now says generalize, names all three deltas, and SC-006 verifies it against
the shipped consumer. Q1 carries the correction and the "#37166 must be amended" note, and the
amendment exists. (Counter spelling excepted — R2-4.) - R1-2 — the staged-bytes hole is closed by FR-031 … FR-035 plus a §Planning Obligations entry
that states the real constraint (global lifetime, no per-call override) and the three options,
rather than pretending the spec can settle it. This was the largest round-1 finding and it is
the best-handled one. - R1-3 — Q4 records the single-file
JobParamsconstraint and adds the better argument the
round-1 finding missed: the generic entry point has no notion of a target folder, so it cannot
satisfy FR-003/FR-004's submission-time refusal at all. - R1-4 — FR-006 is now observable equivalence with the reuse point deferred, and the
filename-containment argument is recorded in §Legacy Considerations where it belongs. - R1-5 — SC-003 no longer measures against the unfixed baseline (see R2-11 for what is left).
- R1-9 — folded into FR-013b and FR-035 (see R2-5 for the form).
ihoffmann-dot's blockers — both, and the observation that they are one defect is the best
reasoning added in this round.- Corrections the author found on their own, which I want to record because they went past the
finding that prompted them: FR-011 no longer invents a bulk-specific ceiling (it would have
contradicted FR-006), and the consequence — no ceiling out of the box — is recorded as knowingly
accepted rather than left implicit. That is the right way to handle a decision you dislike.
Suggested resolution — round 2
- R2-1: add a §Planning Obligations entry sizing durable mid-run state. Nothing in the
framework provides it today. - R2-3: reconcile FR-013b / FR-013 / Q5 — decide whether staging-then-reclaiming counts as
"before any content is staged." - R2-2: rewrite FR-042/FR-043 around the existing unique constraint; say whether collision is
case-sensitive. - R2-6: one line of test strategy in the frontend spec. Principle V is non-negotiable.
- R2-7: make FR-040's two branches distinguishable, or require the handle branch.
- R2-4: pin
failedCount. - R2-5: move FR-035 to §Planning Obligations, or decide it.
- R2-13: fix the cross-reference framing in both documents.
- R2-8 … R2-12: line edits and one acceptance scenario; all can ride into
/speckit-plan.
R2-1 and R2-3 are the two I would want answered in the thread. Everything else is a line edit or a
plan obligation, and none of it changes the approach — which, two rounds in, has held up well.
|
@dario-daza @ihoffmann-dot — heads-up on sequencing: my round-2 review landed at 20:10Z, about 45 minutes after the approval at 19:25Z, both against the same head ( Most of it is fine to absorb during planning. One item I'd like carried into FR-037 requires the run to record which files it has completed "durably and as it goes, not only in the final outcome." The job framework doesn't persist that today. It stores exactly three things about a run:
And the resume path carries nothing forward: So FR-037 isn't "record the outcome carefully" — it's either a new job-framework capability (mid-run durable per-item state) or a feature-owned durable store, and FR-038's "counts across all attempts" needs the same thing. That's the largest piece of hidden work in either document, and it isn't in §Planning Obligations. Not asking to reopen the gate — just that the plan sizes this rather than discovering it. If it turns out to need a job-framework change, that's a separate issue and better found now than in PR 2. The five medium and five low findings in the review above are ordinary plan-phase material; no action needed here. |
4 participants
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:
specs/37166-bulk-file-upload/spec.mdspecs/37166-bulk-file-upload-frontend/spec.mdThey 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
multipleattribute 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.Per-row in-flight marking and bulk reindex's reporting are documented as out of scope, with reasons.
Proposed Changes
specs/37166-bulk-file-upload/spec.md, the backend spec: the batch-upload contract, limits, outcome shape, and notification behavior.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.#37062/#37063ticket references between the two specs.Checklist
/speckit-plan) once these specs are approved.Additional Info
This is PR 1 of the Spec-Kit flow (spec only). Approval here unblocks
/speckit-plan→/speckit-tasks→/speckit-implementfor 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/, sinceplan.mdis 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