fix: make indexing review paths replay-safe

This commit is contained in:
Lucas Winther committed 2026-09-27 19:12:56 +02:00
1 parent 77fa48c7a2
commit 653c32db43
24 files changed
+759 -86

No files matched your search

+6 -3
View File
@@ -83,7 +83,9 @@ Only `engine/` may import Recoll or invoke `recollindex`. Membership jobs first
state in SQLite, atomically export the helper's `RCM1` flat map, then ask the engine to force-index
affected canonical paths. Map records use length-prefixed absolute UTF-8 paths, so filenames can
contain newlines. The C helper reads this private map without starting Python. Engine retries are
therefore idempotent. Persistent directory rules reconcile at startup and on an explicit reindex,
therefore idempotent. Jobs save paths needed for replay before changing rule membership, and collection
deletion commits the cleanup job in the same SQLite transaction. Persistent directory rules reconcile
at startup and on an explicit reindex, preserving prior membership for unreadable files or subtrees,
not through live filesystem monitoring.
### Storage model
@@ -114,8 +116,9 @@ derives effective membership from SQLite and includes direct and rule provenance
Search passes all requested collection IDs into one engine query, then verifies hit paths and effective
membership in SQLite before returning them. Full-text and original-file routes resolve a live canonical
path under the configured root. Original responses use range-capable `FileResponse`, with inline display
limited to safe media types.
path under the configured root. Original downloads open a contained regular file through no-follow
directory descriptors and stream from the pinned descriptor, including single byte ranges; inline
display is limited to safe media types.
The image installs Recoll and its Python binding from Debian trixie, and syncs the application into a
system-site-packages venv so the binding remains importable. A separate build stage compiles the C
+16
View File
@@ -3,6 +3,22 @@
Append-only, newest first. Never edit or delete an entry — supersede it with a new one that links back.
One entry per decision a future maintainer would otherwise have to reverse-engineer.
## 2026-09-27 — Persist replay paths and pin original downloads
**Status:** accepted
**Context:** Review found that a job could mutate rule or collection state before its indexing step
failed, leaving a retry without the old paths. `FileResponse` also reopened a path after containment
checking, allowing a concurrent host-side symlink swap.
**Decision:** persist affected paths in a running job before rule mutations; enqueue collection cleanup
atomically with collection deletion. Open original files beneath the document root with no-follow
directory descriptors and stream from that checked descriptor, including byte-range responses.
**Alternatives:** recompute old paths on retry (the database may already have lost them); retain
`FileResponse` after a path-only check (it reopens the path).
**Consequences:** interrupted jobs retain their required reindex paths. File responses are tied to an
opened regular file even if directory entries change later. The descriptor traversal relies on Linux
`openat` semantics supplied by the current Docker deployment; desktop packaging must revisit it.
---
## 2026-09-18 — Cross-platform desktop packaging is spec 005
**Status:** accepted
**Supersedes:** "The product is four specs, built in order" (below, same date), only by appending a fifth
+13 -7
View File
@@ -5,7 +5,7 @@ Small, specific, independently actionable. Remove only when actually done.
## Resume here
Last touched 2026-09-26. Spec 001 implementation is in progress.
Last touched 2026-09-27. Spec 001 implementation is in review closure.
Where the workflow stands (`AGENTS.md` -> Workflow: brainstorm -> spec -> plan -> TDD -> review):
@@ -19,9 +19,19 @@ Where the workflow stands (`AGENTS.md` -> Workflow: brainstorm -> spec -> plan -
identity, additive membership, durable jobs, the atomic membership map, the engine boundary,
session authentication, the HTTP shell, membership/rule/job routes, scoped search and document
retrieval, Docker packaging and acceptance coverage. Their checkboxes are current in the plan.
Final code review and branch handoff remain.
The first code review found replay, reconciliation, and download races. A fix checkpoint is on
`feat/001-document-discovery`; final verification and branch handoff remain.
The next action is Task 14 Step 6, "Request review before integration." Do not repeat Tasks 1–14.
The next action is Task 14 Step 6: run the full verification matrix, request a follow-up review of the
fixes, then hand off the branch. Do not repeat Tasks 1–14. The review fixes include idempotent rule
creation, persisted replay paths, atomic collection cleanup enqueue, unreadable-file preservation,
departed-path reindexing, directory-rule pagination, recovery request validation, and descriptor-pinned
downloads. The generated real-container lifecycle test covers these flows through Compose. Its 9-test
contract/integration suite passed with `RECOLL_MODERN_REQUIRE_DOCKER=1` on 2026-09-27; the host suite
passed with 157 passed and 4 skipped, and lint/type checking passed. The complete final matrix and
follow-up review still need to run before completion. Docker Compose may be running on
port 9124 with `RECOLL_MODERN_DOCUMENTS=/tmp/recoll-modern-corpus`; preserve its named volumes and
document data. Use `docker compose down` without `-v` when done.
Things a resuming agent will otherwise get wrong:
@@ -70,10 +80,6 @@ Things a resuming agent will otherwise get wrong:
paths exist, and fail if `AGENTS.md` exceeds 150 lines.
## Known issues
- Original file download checks containment before returning `FileResponse`; harden against a local
symlink swap between that check and the response's file open before shipping to untrusted local
writers. The Compose document mount is read-only to the service, but host writers may still replace
entries concurrently.
- The API test suite reports deprecation warnings from the installed FastAPI/Starlette TestClient
dependency. Replace its client integration when the dependency stack supports the successor API.
- Directory sources currently require a named child of the document root; decide whether the root