Skip to content

Bulk PDF Upload v2 — Implementation Plan

Design: 2026-08-11-bulk-pdf-upload-v2-design.md. This plan is executable PR-by-PR; each PR section lists repo, scope, key files, tests, and acceptance criteria. Waves order the PRs by dependency; PRs within a wave are parallel.

Wave map

Wave PR Repo Summary
0 PR-0a camaradesuk/syrf s3-notifier chart: template notification filter + lifecycle rules
0 PR-0b camaradesuk/camarades-infrastructure prod role ignore_changes handover block
0 PR-0c camaradesuk/cluster-gitops per-env filter/lifecycle values + Function CR adoption fix + verified no-op prod sync
1 PR-1 camaradesuk/syrf Domain + API: job entity, endpoints, flag, policy, PM consumers, normalization contract
1 PR-5 camaradesuk/server-config clamd role, syrf-pdf-net, CIFS mount, syrf runner, agent slot
2 PR-2 camaradesuk/syrf s3-notifier code: dispatch switch, BulkPdfUpload kind, presigned GET
3 PR-3 camaradesuk/syrf Agent service + chart + image CI + arrnc-api deploy workflow
3 PR-6 camaradesuk/cluster-gitops agent service wiring for staging + previews (after PR-3)
4 PR-4 camaradesuk/syrf Frontend: folder upload, preview, zip worker, results UX, user guide

Merge order = wave order. PR-5 production apply (manual dispatch) happens with Wave 3. Exception to "PRs within a wave are parallel": PR-6 targets the chart/image PR-3 publishes (the ApplicationSet would otherwise generate an Application for a chart revision that doesn't exist yet), so within Wave 3, PR-3 must merge and its image/chart tag land in GHCR/cluster-gitops before PR-6 merges.

Wave-3 prerequisite (runner-boundary review): before PR-3 merges, the self-hosted deploy runner on arrnc-api must be approved and CLAUDE.md's runner-boundary section updated in PR-3 to define the deploy-runner category (server-config-managed host, gatekeeper-restricted sudo, no build/test workloads, repo-scoped) — OR PR-3 switches to the fallback: a GitHub-hosted job calling container-web-deploy over SSH with a restricted key. Settle this before PR-3 is written, not during its review.


Wave 0 — ADR-010 minimal unblock (prerequisite)

Why bundled: the notifier change (PR-2) cannot reach production through the chart while the Phase-5 cutover is stuck, and the chart's first successful prod sync as-written would drop the Projects/ notification filter and the live multipart-cleanup lifecycle rule (design §7).

PR-0a — chart templating (syrf)

  • src/services/s3-notifier/.chart/templates/bucket.yaml: render spec.notification.lambdaFunctionConfigurations[].filter.key.filterRules from .Values.bucket.notificationFilterPrefix (omit when empty), and spec.lifecycle.rules from .Values.bucket.lifecycleRules (omit when empty).
  • src/services/s3-notifier/.chart/values.yaml: bucket.notificationFilterPrefix: "", bucket.lifecycleRules: [] defaults.
  • Tests: helm template golden-file assertions for empty/filtered/lifecycle variants.
  • Acceptance: chart renders byte-identical manifests for existing envs when new values unset.

PR-0b — Terraform role parity (camarades-infrastructure)

  • terraform/lambda/main.tf: add to aws_iam_role.production_lambda_role the same lifecycle { ignore_changes = [tags, tags_all, description, assume_role_policy] } block the staging role gained in the ADR-010 Phase-4 handover (main.tf:189-204).
  • Acceptance: terraform plan shows no proposed changes to the production role.

PR-0c — values + adoption fix + verification (cluster-gitops)

  • syrf/environments/production/s3-notifier/values.yaml: bucket.notificationFilterPrefix: "Projects/"; bucket.lifecycleRules: carrying both the existing Incomplete Multipart Cleanup rule (verbatim from the live Bucket CR spec) and the new expire-bulk-staging rule (filter.prefix: "Projects/_bulk-staging/", expiration.days: 7, plus noncurrentVersionExpiration.noncurrentDays: 1 and expiration.expiredObjectDeleteMarker cleanup — the chart enables bucket versioning, so expiry without a noncurrent-version rule would leave every "deleted" ZIP retained indefinitely as a noncurrent version behind a delete marker).
  • Staging + preview values: lifecycleRules with the bulk-staging rule (their buckets are dedicated; notification filter prefix stays empty/bucket-wide as today). Preview caveat: preview object keys are prefixed preview/pr-{n}/ by GetS3PathPrefix(), so the preview lifecycle rule's filter must be rendered per-PR as preview/pr-{n}/Projects/_bulk-staging/ (the pr-preview workflow already writes per-PR values files) — a bare Projects/_bulk-staging/ filter would match nothing in previews.
  • Resolve the terminal Function CR: delete the conflicting AdoptedResource/Function pair and re-adopt in order on chart v1.17.1 (nameOrID fix), per the chart's own forward-path comment (adopted-resource.yaml:40-45).
  • Gates (in order, STOP on failure): (1) re-authenticate AWS CLI and independently confirm live bucket notification + lifecycle + Lambda config before touching anything; (2) after sync, s3-notifier-production Synced/Healthy; (3) aws s3api get-bucket-notification-configuration and get-bucket-lifecycle-configuration show the prefix filter and both rules — i.e. the cutover is a no-op for runtime behaviour; (4) a staging search-import smoke test passes (the P-3 verification ADR-010 never ran).
  • Acceptance: production app Synced/Healthy; notification + lifecycle verified unchanged plus the new rule; chartTag pinned ≥ the PR-0a chart version.

Wave 1

PR-1 — Domain + API (syrf)

Scope (design §4.1–§4.3): BulkPdfUploadJob embedded entity + guards + counts + lazy Expired transition + bounded history (last 20/search, pruned on new-job creation); Study.BulkPdfDeliveredAt and BulkPdfDeliveredPath; centralized bulk-aware PDF-URL builder routed through ALL five existing call sites (Study.GetLinkToPdf, StudyROBDto, StudyListItemDto, StatsWithIncompleteDto/StudyBaseDto, StudyDto, PdfConverterService) with per-segment percent-encoding; BulkPdfUploadJobDto + ProjectDetailsDto.BulkPdfUploadJobs + AutoMapper mapping (SignalR wiring); search/project deletion guard while a job is non-terminal; BulkPdfUpload policy; feature flag; signature/history/report/pdfPaths endpoints; PM receive endpoint with progress + finalize consumers (single writer, concurrency 1, optimistic-concurrency retry policy, delivered-outcome-qualified study marking); CSV report generation (RFC 4180 + formula neutralization); message contracts (path ≤ 512 B / detail ≤ 256 B caps); normalization helper + vector file + portability/collision/root-relative contracts; BsonClassMap registrations.

Key files

  • src/libs/project-management/SyRF.ProjectManagement.Core/Model/ProjectAggregate/BulkPdfUploadJob.cs (new)
  • .../Model/StudyAggregate/Study.cs (BulkPdfDeliveredAt, link generation)
  • src/libs/project-management/SyRF.ProjectManagement.Messages/Commands/IProcessBulkPdfUploadCommand, IReportBulkPdfUploadProgressCommand, IFinalizeBulkPdfUploadCommand, BulkPdfFileOutcome
  • src/libs/kernel/SyRF.SharedKernel/PdfPathNormalization/PdfPathNormalizer.cs, normalization-vectors.json
  • src/libs/kernel/SyRF.SharedKernel/Enums/UploadKind.cs — add BulkPdfUpload
  • src/services/api/SyRF.API.Endpoint/Controllers/ — new BulkPdfUploadController (mirrors SearchController.getSignature SigV4 pattern)
  • src/services/project-management/SyRF.ProjectManagement.Endpoint/Consumers/ — the two consumers + endpoint registration
  • src/libs/project-management/SyRF.ProjectManagement.Mongo.Data/Repositories/ProjectRepository.cs — class maps
  • src/charts/syrf-common/env-mapping.yaml + pnpm run generate:flags output
  • appsettingsPdfBaseUrl env-supplied (no hardcoded default)

Tests: domain guard/idempotency/monotonicity suite (port the cases from #2373's PdfUploadJobTests/PdfUploadJobIdempotencyTests, fresh code); normalizer vs vector file; finalize consumer matching/dedup/Missing-Unmatched taxonomy (port UpdateStudyPdfStatusConsumerTests cases); controller authz + flag gating; CSV generation.

Acceptance: all endpoints 404 behind the flag; no behaviour change flag-off — the only production-visible code path PR-1 touches is link generation, which branches on Study.BulkPdfDeliveredAt, a field no existing document has (null → legacy URL) and which only the finalize consumer ever sets, so flag-off behaviour is byte-identical and no MongoDB migration is needed; swagger + generated TS client regenerated.

PR-5 — Hosting (server-config)

Scope (design §6): docker_networks + syrf-pdf-net; roles/clamav (Ansible-managed, restart_policy: unless-stopped, host volume for signature DB, no published ports, and a templated clamd.conf drop-in raising StreamMaxLength/MaxScanSize/MaxFileSize to cover the 500 MB entry cap) + structural ansible_managed: true projects entry; cifs_mounts item for the ecrf1 share → /srv/data/syrf-pdf/production/output (mount_user = agent uid, smb_version per share); new cifs/syrf-pdfs age scope; github_runner block for camaradesuk/syrf (os_user: runner-syrf); gatekeeper project entry syrf-pdf-agent-production (host_port: 8089 nominal, volume_path: /srv/data/syrf-pdf/production, container_volume_path: /app/data, skip_web_acl: true) + mirror entry in deployments/arrnc-api.yml (managed: false, gated: true); non-secret env in portal-env-overrides.yaml; sentinel-file creation task (output/.syrf-storage-ok) guarded on the mount being active.

Tests/acceptance: repo CI (deployment-manifest exhaustiveness, secrets validation, lifecycle vocabulary) green; rehearsal-host apply clean; production apply deferred to Wave 3 (manual dispatch). Needs the share name + credentials (design §10) before the production apply, not before merge.


Wave 2

PR-2 — Notifier code (syrf)

Scope (design §4.4): dispatch Enum.TryParse + switch with logged no-op default; BulkPdfUpload case (metadata validation, 12 h presigned GET, Send to agent queue on the metadata vhost); update the stale sections of docs/architecture/systematic-search-upload-flow.md this touches (uploadkind/virtualhost, flattened contract).

Key files: src/services/s3-notifier/SyRF.S3FileSavedNotifier.Endpoint/S3FileReceivedFunction.cs; tests in the endpoint test project (dispatch table: ReferenceUpload / ReferenceUpdate / BulkPdfUpload / missing-metadata / unknown-kind ⇒ no-throw).

Acceptance: unknown-kind and missing-metadata events log and return without throwing (kills the current single-study-PDF error/retry noise). Note for the PR-2 description: this is a deliberate semantics change — today an unrecognised object falls into the search-import branch and throws, triggering S3 retries; after this PR unknown kinds are a logged no-op with no retry. Reviewers should see that stated, not discover it. LocalStack integration test covers the new kind end-to-end to a RabbitMQ assertion — including asserting the presigned GET actually dereferences (an HTTP GET against LocalStack inside the test env, not just non-null; note the URL host will be localhost:4566, so the Wave-3 agent fixtures must use the same LocalStack endpoint to stay compatible). Ships as s3-notifier-v{next} through the standard promotion (staging auto, production manual — now unblocked by Wave 0).


Wave 3

PR-3 — Agent service (syrf)

Scope (design §4.5): src/services/pdf-agent/SyRF.PdfAgent worker + consumer, clamd INSTREAM client (limits sized per design §6.2), download byte-cap enforcement, extraction guards + portability/collision contracts + .pdf-extension enforcement, conflict policies incl. SHA-256, sentinel check, crash-safe outcome journal (scratch/{uploadId}/journal.jsonl), lock/age-guarded scratch sweep, progress/finalize reporting; Dockerfile (dedicated non-root uid); pdf-agent.slnf; GitVersion; syrf.sln entries; .chart/ (deployment with Recreate strategy + PVC + sentinel init container + clamd sidecar with raised limits + optional static file server for staging/preview); ci-cd.yml build/push/tag jobs; full PR-level CI wiring — the agent must be added to pr-tests.yml change predicates, detect-service-changes.sh, and pr-preview.yml's detection/version/image matrix + preview deployment values, or agent PRs get no tests and previews can't run the PR's own agent image; .github/workflows/deploy-pdf-agent.yml (workflow_dispatch, runs-on: [self-hosted, linux, live], container-web-deploy with GHCR GITHUB_TOKEN auth and -e secrets — respecting the shared-runner boundary rules in CLAUDE.md); docker-compose.dev.yml gains a clamd container; LocalStack init extended.

Tests: consumer unit tests with fixture ZIPs (clean / infected via EICAR / zip-slip / zip-bomb / macOS junk / non-PDF / duplicate paths / unicode names); conflict-policy matrix incl. hash-compare; sentinel fail-closed; integration test against dockerised clamd + LocalStack.

Acceptance: image builds + pushes in CI with pdf-agent-v{semver} tags; staging E2E (below) passes; production deploy dispatch works after the PR-5 production apply.

PR-6 — Agent env wiring (cluster-gitops)

Sequencing: merges after PR-3 (not parallel — see wave-map note above); targets the chart/image tag PR-3 publishes.

Scope: syrf/services/pdf-agent/{config,values}.yaml; staging + preview environment values (PVC size, PdfBaseUrl, static server enabled, RabbitMQ vhost); ApplicationSet picks the service up like any other. Preview flag remains off by default; enabled per-PR via #preview-config when testing.

Acceptance: staging agent Synced/Healthy consuming the staging vhost; a preview PR with the flag enabled gets a working in-cluster pipeline.

Wave-3 operational step (not a PR): run the PR-5 production apply manual dispatch, verify mount + sentinel + clamd health on arrnc-api, then run the deploy workflow and a production smoke test with a 2-file fixture folder against a scratch project before any flag change. The smoke test's direct-command path (flag stays off) needs a real BulkPdfUploadJob to finalize against: the runbook first creates one via an operator script that calls the same domain method the signature endpoint uses (Project.CreateBulkPdfUploadJob, PR-1) — bypassing only the HTTP flag gate, not the domain invariants — then sends IProcessBulkPdfUploadCommand referencing that job's uploadId.


Wave 4

PR-4 — Frontend (syrf)

Scope (design §4.7): folder selection (drag-drop + picker), client validation, pre-upload match preview (shared normalization, GET .../pdfPaths), streaming ZIP web worker (STORE), S3 PUT with progress, SignalR-driven status, result summary panel, history table, CSV download; TS normalization mirror + vector-file test; user-guide page (help.syrf.org.uk) for the new workflow.

Tests: store/component specs for every state (validating, preview, uploading, scanning, copying, complete, failed, infected); worker zip unit test; normalization parity spec loading normalization-vectors.json; systematic-searches integration spec for flag gating.

Acceptance: flag-off = zero UI change; preview-env E2E: drop folder → confirm preview → watch progress → verify served PDF link + CSV report.


Rollout & verification sequence

  1. Wave 0 merged + production sync verified no-op (gates in PR-0c).
  2. Waves 1–2 merged; staging flag on; routing verification only (the agent doesn't exist yet, so neither does its queue): signature endpoint issues, ZIP lands in staging S3, and the notifier's Send is verified via Lambda logs plus a temporary operator-declared diagnostic queue bound to the command exchange in the staging vhost (RabbitMQ management UI) — deleted after the check. Without a bound queue the message would be unroutable, which is expected and harmless at this stage.
  3. Wave 3 merged (PR-3 image, then PR-6 wiring — sequential, not parallel; PR-6 targets a chart revision PR-3 publishes): staging pipeline rehearsal, API-driven (the browser flow is Wave 4): request a signature via the API (flag is on in staging since Wave 1), PUT a fixture ZIP with a script using the returned signature headers, then verify the full path S3 → notifier → in-cluster agent → PVC → SignalR → CSV, including an EICAR fixture to prove the Infected path. Then server-config production apply → agent deployed on arrnc-api → production infra smoke with the flag still off: an operator first creates a BulkPdfUploadJob via script (same domain method the signature endpoint uses), then sends IProcessBulkPdfUploadCommand directly to the production queue referencing it, for a pre-staged 2-file fixture ZIP against a scratch project (documented runbook step — exercises agent, clamd, CIFS write, PM finalize, without needing the flag-gated API), verify files on the share + job terminal state.
  4. Wave 4 merged; full browser-driven staging E2E (folder pick → preview → zip → upload → live progress → CSV) now that the frontend exists; then enable bulkPdfUpload in production values (this is the launch); announce; monitor first real uploads (agent logs via docker on arrnc-api; job outcomes in Mongo; Lambda metrics).
  5. Close #2373 as superseded (comment linking this plan); remove its worktree; delete stale branch after a grace period. Update epic #2223 and close the subsumed April issues (#2516–#2546 set) with pointers to the v2 PRs.

Rollback levers

  • Feature flag off (stops new uploads instantly; in-flight jobs finish or fail safely).
  • Agent: stop/redeploy previous image via the gatekeeper (--skip-pull env-only redeploys supported); messages queue durably while it's down.
  • Notifier: standard zip-version rollback via GitHub Release + config pin — with a floor: once any bulk upload has occurred, PR-2's version is the minimum safe notifier (pre-PR-2 code throws + S3-retries on any non-ReferenceUpdate kind, so an in-flight BulkPdfUpload event would loop). Rolling back below it requires the flag off and bulk events drained first.
  • Wave 0 is deliberately verified as behaviour-neutral before anything rides on it.

Risks

Risk Mitigation
ADR-010 Function CR fix goes sideways PR-0c hard gates + AWS-side pre-verification; production runtime stays on the TF Lambda until Synced/Healthy is proven
CIFS mount flaps under the container Sentinel fail-closed; _netdev,nofail fstab opts; runbook: restart container after remount
Normalization drift between TS and C# Single vector file consumed by both test suites; adding a vector on one side breaks the other until implemented
1 GB browser zipping on low-end machines STORE mode + streaming worker; documented guidance; cap enforced client- and agent-side
RabbitMQ plaintext exposure (existing posture) Separate AMQPS hardening PR; agent flips URI when it lands
clamd signature staleness freshclam auto-updates; health check surfaces staleness in logs; scan failure ⇒ job Failed (never silently unscanned)