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: renderspec.notification.lambdaFunctionConfigurations[].filter.key.filterRulesfrom.Values.bucket.notificationFilterPrefix(omit when empty), andspec.lifecycle.rulesfrom.Values.bucket.lifecycleRules(omit when empty).src/services/s3-notifier/.chart/values.yaml:bucket.notificationFilterPrefix: "",bucket.lifecycleRules: []defaults.- Tests:
helm templategolden-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 toaws_iam_role.production_lambda_rolethe samelifecycle { 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 planshows 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 existingIncomplete Multipart Cleanuprule (verbatim from the live Bucket CR spec) and the newexpire-bulk-stagingrule (filter.prefix: "Projects/_bulk-staging/",expiration.days: 7, plusnoncurrentVersionExpiration.noncurrentDays: 1andexpiration.expiredObjectDeleteMarkercleanup — 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:
lifecycleRuleswith the bulk-staging rule (their buckets are dedicated; notification filter prefix stays empty/bucket-wide as today). Preview caveat: preview object keys are prefixedpreview/pr-{n}/byGetS3PathPrefix(), so the preview lifecycle rule's filter must be rendered per-PR aspreview/pr-{n}/Projects/_bulk-staging/(the pr-preview workflow already writes per-PR values files) — a bareProjects/_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 (nameOrIDfix), 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-productionSynced/Healthy; (3)aws s3api get-bucket-notification-configurationandget-bucket-lifecycle-configurationshow 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,BulkPdfFileOutcomesrc/libs/kernel/SyRF.SharedKernel/PdfPathNormalization/—PdfPathNormalizer.cs,normalization-vectors.jsonsrc/libs/kernel/SyRF.SharedKernel/Enums/UploadKind.cs— addBulkPdfUploadsrc/services/api/SyRF.API.Endpoint/Controllers/— newBulkPdfUploadController(mirrorsSearchController.getSignatureSigV4 pattern)src/services/project-management/SyRF.ProjectManagement.Endpoint/Consumers/— the two consumers + endpoint registrationsrc/libs/project-management/SyRF.ProjectManagement.Mongo.Data/Repositories/ProjectRepository.cs— class mapssrc/charts/syrf-common/env-mapping.yaml+pnpm run generate:flagsoutputappsettings—PdfBaseUrlenv-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¶
- Wave 0 merged + production sync verified no-op (gates in PR-0c).
- 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
Sendis 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. - 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
BulkPdfUploadJobvia script (same domain method the signature endpoint uses), then sendsIProcessBulkPdfUploadCommanddirectly 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. - Wave 4 merged; full browser-driven staging E2E (folder pick → preview → zip → upload →
live progress → CSV) now that the frontend exists; then enable
bulkPdfUploadin production values (this is the launch); announce; monitor first real uploads (agent logs via docker on arrnc-api; job outcomes in Mongo; Lambda metrics). - 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-pullenv-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) |