fix(memory): async document export for hindsight-api 0.9.x (#2155)

[thinmintdev]

What changed

• GET /api/memory/banks/{bank_id}/document-transfer (src/hal0/api/routes/memory_admin.py:803-821) still tries the 0.8.x synchronous engine export first. If the engine answers 410, it now runs the engine's async export instead (_async_export_archive, memory_admin.py:752):

  1. POST /v1/default/banks/{id}/document-transfer/export?include_observations=…
  2. poll GET /v1/default/banks/{id}/operations/{operation_id} until the operation settles (_EXPORT_POLL_INTERVAL_S / _EXPORT_POLL_TIMEOUT_S, memory_admin.py:678-679, which use the CLI's 2s / 900s values)
  3. download the ZIP named by result_metadata.download_url

  The route still returns the ZIP to its caller. The CLI and any other API caller are both fixed here, and import (POST …/document-transfer) is untouched.

• Download safety (_export_download_target, memory_admin.py:710): the archive is only ever fetched from the configured engine origin. A root-relative download_url is used as given. An absolute one is used only if its scheme, host and port all match the engine client's base_url. A URL on any other host (for example a pre-signed URL from an S3, GCS or Azure file store, or a scheme-relative //host) is never followed. In that case the archive is fetched through the engine's own /v1/default/files/download/{storage_key}, and only when the key looks like banks/<this bank>/… with no ./.. segments. If neither works, the route returns 502.

• Errors: a failed, cancelled or not_found operation returns 502 memory.engine_error with operation_id, status and error_message. A poll timeout returns 504. Upstream 4xx/5xx and transport errors map the same way as before (_raise_transfer_error).

• CLI (src/hal0/cli/memory_migrate_commands.py:276): the export GET now has a read timeout of _POLL_TIMEOUT_S + 60. The old default was 60s, which the server-side poll can outlast. Stale docstrings that said "unchanged in 0.9.2" are corrected in both files.

Why

hindsight-api 0.9.2 replaced the sync export with a 410 tombstone (api_export_documents_removed in hindsight_api/api/http.py of the hindsight-api-slim==0.9.2 wheel). It still sets features.document_export_api: true on /version, so the --apply feature gate (memory_migrate_commands.py:245-253) passes and the failure only shows up at transfer time. That is why the flow is chosen by the 410 itself and not by the flag, as the issue suggests. In the same wheel, the default PostgreSQL file store returns a root-relative download_url (engine/storage/postgresql.py get_download_url), while the object-store backends return pre-signed URLs on other hosts. That difference is the reason for the storage_key fallback.

Verification

• New tests were written first. On unpatched main, all 9 new tests in tests/api/test_memory_admin_document_transfer.py failed with assert 410 == 200/502/504/404, which reproduces the issue. They cover: the 410 fallback happy path, an absolute same-origin URL, a foreign-host URL that uses storage_key, three refused foreign origins (another host, another port, scheme-relative), an operation that fails, a poll timeout, and a submit error passing through. One existing test now also asserts that 0.8.4's sync 200 needs exactly one request, and a new test checks that a non-410 error never starts the async flow. One CLI test was added asserting the export read timeout is longer than the server poll budget.
• uv run pytest tests/api/test_memory_admin_document_transfer.py tests/cli/test_memory_migrate_unify.py -q → 33 passed
• uv run ruff format --check src tests → 1304 files already formatted
• uv run ruff check src tests → All checks passed!
• uv run pytest tests/ -q -m "not integration" → 13196 passed, 26 skipped, 1 xfailed, plus 8 failures that are environmental and in files this PR does not touch. The sandbox runs as uid 0, so these root-ownership/unwritable-path tests fail: installer, updater and system/test_seam_check. cli/test_doctor.py::test_preflight_ports_soft_mode_downgrades_to_warning fails because ss is not installed.

Closes #2155

🤖 Generated with Claude Code

https://claude.ai/code/session_017bDysVfpfD2VTJfPcQwCcM

---

Generated by Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[thinmintdev]

Independent review: APPROVE (posted as COMMENT because GitHub blocks self-approval from this account)

Reviewed head f64d0514. I checked the engine's behaviour against the hindsight-api-slim==0.9.2 wheel source.

1. Does it fix the reported path?

• The CLI (memory_migrate_commands.py:271-277) calls api_get_bytes("/api/memory/banks/{src}/document-transfer"). That request goes to bank_document_transfer_export (memory_admin.py:801-821). When the engine answers 410, the route now calls _async_export_archive (:752), which submits the POST .../document-transfer/export, polls .../operations/{id}, and then downloads the archive.
• 0.9.2 wheel: the sync GET is the api_export_documents_removed 410 tombstone (api/http.py:7372-7378). The async POST returns 202 {operation_id} (:7387-7432). result_metadata carries download_url and storage_key, where storage_key = banks/{bank_id}/exports/{uuid}/transfer.zip (engine/memory_engine.py:2452-2463). The PostgreSQL store returns /v1/default/files/download/{key} (engine/storage/postgresql.py:145), served by files/download/{key:path} (http.py:7500). The submit, poll and download paths the PR uses all match the wheel.
• Result: the quoted HTTP 410: memory engine returned an error can no longer come from this route against 0.9.x. A 410 always takes the async path, and the async path's own errors are 502 or 504 with details.

2. Root cause and seam

• The fix sits in the hal0-api passthrough, which both the CLI and any API or dashboard caller go through. A grep shows no other caller of the engine export (grep -rn document-transfer src/).
• The flow is selected by the 410 itself, not by features.document_export_api. 0.8.4 keeps its single sync request (asserted in test_export_streams_zip_bytes). Other upstream errors never start the async flow (test_export_non_410_error_does_not_try_async). The selection logic is sound.

3. Security and safety

• _export_download_target (memory_admin.py:710) accepts a root-relative URL, or an absolute URL whose (scheme, host, port) equals the engine base_url. Anything else falls back to a storage_key that must look like banks/<bank>/... with no ., .. or empty segments; otherwise the route returns 502.
• I probed it with a scratch script: //evil, http:evil.com/x, http://127.0.0.1:9177@evil.com, http://127.0.0.1:9177.evil.com, a leading space and \\evil are all refused. /\evil.com, ///evil.com, /%2F%2Fevil.com and http://evil.com@127.0.0.1:9177 all resolve to the engine host. Relative URLs are merged onto base_url by httpx.
• The engine AsyncClient (hindsight_client.py:106) does not follow redirects (httpx default), so a 3xx cannot bounce the request off-host.
• Polling is bounded by _EXPORT_POLL_TIMEOUT_S (900s) and returns 504. Operations that are failed, cancelled or not_found return 502 with operation_id, status and error_message. A missing or unusable operation_id or download_url returns 502.

4. Tests fail without the fix

I ran git checkout origin/main -- src/ and then the two touched modules: 10 failed, 23 passed. All 9 new API tests and the new CLI timeout test failed. After restoring src/: 33 passed.

5. Regressions

• _shared.py is unchanged. The CLI only passes the existing timeout= kwarg to api_get_bytes. Import POST is unchanged.
• uv run pytest tests/api tests/cli -q -m "not integration" --deselect tests/cli/test_doctor.py::test_preflight_ports_soft_mode_downgrades_to_warning gives 3001 passed, 1 skipped, 1 deselected.

6 and 7. Scope and forbidden areas

The diff touches only memory_admin.py, memory_migrate_commands.py and their two test modules. There is no CHANGELOG, .github/, credentials, infrastructure or migration change. No host names or LAN addresses appear: only 127.0.0.1 and the 169.254.169.254 SSRF fixture, both in tests. The docs (docs/guides/enable-memory.mdx:231-245) stay accurate.

8. Lint and DCO

• ruff format --check: 1304 files already formatted. ruff check: All checks passed!
• The commit has Signed-off-by: Claude <noreply@anthropic.com>, which matches the author. DCO is documented but not gated (CONTRIBUTING.md:122).

Non-blocking nits (follow-up is fine)

• Duplicated timeout: the 900s budget is now stored twice, in memory_admin.py:679 and memory_migrate_commands.py:76 (anti-scar rule 1). The CLI test guards the relationship between them, but a single owner would be cleaner.
• Connect timeout: api_get_bytes (_shared.py:230-245) passes a plain float to httpx.Client, so the new 960s budget also applies to connect. _client_timeout() (_shared.py:26) already exists for this; using it would keep connect short.
• Archive cleanup: the downloaded export archive is left in engine storage. The engine prunes it with the operation row (memory_engine.py:2480-2540), so this is acceptable.

---

Generated by Claude Code