documents connector S3: IAM role support, S3 timeouts, non-PDF coverage, per-query re-scan

[abbccdda]

Follow-ups from verifying #171 (documents connector S3 / object-store support) against a real S3 bucket and a MinIO endpoint. The items fixed on that PR are listed at the bottom for context; everything in "Action items" is deliberately out of scope for it.

Action items

1. Support IAM roles, or stop advertising them

object_store's AmazonS3Builder::from_env() already resolves credentials from EC2 instance metadata, ECS task endpoints (AWS_CONTAINER_CREDENTIALS_RELATIVE_URI) and EKS web-identity tokens (AWS_ROLE_ARN + AWS_WEB_IDENTITY_TOKEN_FILE). skardi rejects all three before they are reached: the credential check requires AWS_ACCESS_KEY_ID or AWS_PROFILE, and under an instance profile / IRSA / task role neither is set.

• crates/server/src/remote_storage.rs:317 (require_s3_region_and_creds, used by the documents preflight)
• crates/server/src/remote_storage.rs:432 (setup_object_store, used by csv/parquet/lance)

Meanwhile reject_credential_options' own error text recommends "IAM roles/instance profiles on AWS infrastructure" as a remedy, so the guidance contradicts the behaviour. docs/S3_USAGE.md was corrected on #171 to describe the assume-role-and-export workaround, but the code gap remains.

Fix: accept "no static key, but a role-shaped environment" instead of demanding AWS_ACCESS_KEY_ID — e.g. treat the presence of AWS_ROLE_ARN/AWS_WEB_IDENTITY_TOKEN_FILE, AWS_CONTAINER_CREDENTIALS_*, or an available IMDS endpoint as satisfying the check, and let from_env() do the resolution. Worth closing before anyone deploys the documents connector (or an S3 csv/parquet source) to EKS or ECS, since today it fails at startup with MissingAwsConfig.

Also worth reconsidering whether AWS_PROFILE should satisfy the check at all: it currently passes validation and then fails at request signing, because object_store never reads ~/.aws/. Rejecting it with a pointer to aws configure export-credentials would turn a confusing auth failure into an actionable config error.

2. Apply the retry_with_timeout convention to the S3 path

retry_with_timeout is used 29× across the codebase (typically with a ~5s connect timeout) but appears nowhere in crates/skardi/src/sources/providers/documents/blob.rs or crates/server/src/remote_storage.rs. The S3 operations therefore rely solely on object_store's defaults: 30s per request, 5s connect, 10 retries, 180s retry ceiling.

Not unbounded, so this is an inconsistency rather than a hang — but an unresponsive or misconfigured endpoint stalls startup for up to ~3 minutes at the registration preflight, where other providers fail fast in ~5s. Multiply by per-file operations for a scan.

Fix: wrap the preflight list/put and the per-object get/put in the existing helper, or configure RetryConfig/ClientOptions on the builder to match the house timeouts.

3. Verify non-PDF formats over S3

.docx / .xlsx / image inputs were never confirmed end-to-end from S3 during verification, because the machine used lacked LibreOffice and ImageMagick. The S3 side worked for all of them — they were listed and fetched successfully — and only liteparse's conversion step failed, so this is a coverage gap rather than a known defect.

The new CI step (Execute documents S3 live tests against MinIO) runs PDF-only fixtures for the same reason. Fix: install libreoffice and imagemagick on the runner (or in a fixture image) and extend crates/skardi/tests/documents_s3_live.rs to cover a .docx, an .xlsx and an image fixture over the object store.

4. Caching / avoid re-scanning per query

There is no caching layer: every query re-lists the prefix, re-fetches and re-parses every matched file, and re-writes the image outputs, overwriting the previous scan's objects. Measured on a 4-PDF, 9-page corpus: ~7s and ~21 S3 requests per query, rewriting 12 objects each time. Files are also fetched and parsed sequentially, so wall-clock is linear in corpus size.

The cost model is now documented (docs/documents.md § "Performance and cost") with mitigations, which was the in-scope part. This item is the real fix, and it is a design change rather than a patch — options include content-addressed caching keyed on (etag, options), skipping image writes when the target object already exists, per-file concurrency, and pushing listing/prefix predicates down into the scan (overlaps #167).

Fixed on #171 (for context, no action needed)

• llm_extract could not read s3:// image_refs — they fell through to std::fs::read("s3://…"), silently breaking multimodal escalation whenever image_store was s3://. Now resolved through the connector's BlobStore, behind the existing LLM_EXTRACT_IMAGE_FETCH opt-in.
• Wholesale image_store write failure now errors instead of returning rows whose image refs are all silently empty; partial failures log an aggregate count.
• docs/documents.md and docs/S3_USAGE.md corrected — the llm_extract compatibility claim, the undocumented image-fetch opt-in, the GetObject-vs-PutObject permission asymmetry, and three of four advertised auth methods that do not work.
• Live object-store coverage added (documents_s3_live.rs, 5 tests) and wired into CI against MinIO.

Verified working against real S3 during review

List/get/put round trip; prefix scoping (a sibling corpus/docs-2/ never matched corpus/docs); recursive listing with /-separated relative keys; doc_id byte-identical between the local and S3 backends; Content-Type stamped on written objects; self-ingestion guard holding across two scans with image_store nested inside path; and all seven registration guards rejecting with actionable errors (same-bucket, credentials-in-config, image_store-as-ancestor, read preflight on a forbidden prefix, write probe without s3:PutObject, nonexistent bucket, wholesale parse failure).