kayg/calternal
Watch
1
0
Fork
You've already forked calternal
 0
Code Issues 190 Pull requests 61 Projects Releases Packages Wiki Activity Actions 26

Plan: attachment previews (#865) #871

Open
kayg wants to merge 1 commit from plan/previews-865 into dev
pull from: plan/previews-865
merge into: kayg:dev
Conversation 4 Commits 1 Files changed 1 +230
kayg commented 2026-11-02 16:49:12 +00:00
Owner
Copy link

Attachment previews currently have no isolated generation pipeline. This plan defines P1–P6 from #865: sandboxed decoders, asynchronous resumable jobs, two WebP sizes, batch metadata, derived storage, device caching and native-library packaging.

Design: docs/plans/0865-previews.md. Backend implementation follows BM8 in one PR; cards follow in the pictures port (#846). No implementation code in this PR.

Decisions: rootless Bubblewrap namespaces, Landlock, seccomp and process rlimits, without procfs or per-job cgroups. Default-off generation requires a final-production-image probe and both-architecture package gates before enablement. Bounded header fill and resumable backfill cover existing/outside-written attachments; verified content hashes plus generation checks prevent stale reuse. Retain attachments.meta and add list/peek/open descriptors. FFmpeg uses a minimal LGPL library build; HEIC stays independently off pending codec/patent clearance.

Preview work never gates uploads or note writes. Keep the approved type icon until ready, without moving text. Initial latency targets are unvalidated; implementation gates and measurement conditions are specified in the plan.

Open risks: procfs-free native decoder paths and outer-container limits still require final-image verification; minimal native closures require both-architecture checks. Coordinate the additive metadata contract with #846. Artifacts and run results belong in issue/PR comments only.

Refs #865. Never merge this PR automatically.

Attachment previews currently have no isolated generation pipeline. This plan defines P1–P6 from #865: sandboxed decoders, asynchronous resumable jobs, two WebP sizes, batch metadata, derived storage, device caching and native-library packaging. Design: docs/plans/0865-previews.md. Backend implementation follows BM8 in one PR; cards follow in the pictures port (#846). No implementation code in this PR. Decisions: rootless Bubblewrap namespaces, Landlock, seccomp and process rlimits, without procfs or per-job cgroups. Default-off generation requires a final-production-image probe and both-architecture package gates before enablement. Bounded header fill and resumable backfill cover existing/outside-written attachments; verified content hashes plus generation checks prevent stale reuse. Retain attachments.meta and add list/peek/open descriptors. FFmpeg uses a minimal LGPL library build; HEIC stays independently off pending codec/patent clearance. Preview work never gates uploads or note writes. Keep the approved type icon until ready, without moving text. Initial latency targets are unvalidated; implementation gates and measurement conditions are specified in the plan. Open risks: procfs-free native decoder paths and outer-container limits still require final-image verification; minimal native closures require both-architecture checks. Coordinate the additive metadata contract with #846. Artifacts and run results belong in issue/PR comments only. Refs #865. Never merge this PR automatically.
docs: plan sandboxed attachment previews (#865)
Some checks failed
CI / server (pull_request) Has been cancelled
Details
CI / rust (pull_request) Has been cancelled
Details
CI / web (pull_request) Has been cancelled
Details
Markdown extraction contract / markdown-native-contract (pull_request) Has been cancelled
Details
Markdown extraction contract / markdown-editor-contract (pull_request) Has been cancelled
Details
6c40b72ab5
kayg commented 2026-11-02 16:49:29 +00:00
Author
Owner
Copy link

Validation at head 6c40b72ab5c6c9e8cc93b9bf98998d3e1d8cdae8: git diff --check origin/dev...HEAD passed; git status --short is empty. Only docs/plans/0865-previews.md changed. No builds or tests run for this plan-only PR, per the control-host rules. Performance numbers in the plan are proposed gates, not run results. No merge performed.

Validation at head `6c40b72ab5c6c9e8cc93b9bf98998d3e1d8cdae8`: `git diff --check origin/dev...HEAD` passed; `git status --short` is empty. Only `docs/plans/0865-previews.md` changed. No builds or tests run for this plan-only PR, per the control-host rules. Performance numbers in the plan are proposed gates, not run results. No merge performed.
kayg commented 2026-11-02 17:10:03 +00:00
Author
Owner
Copy link

verdict: changes needed

must-fix

  1. Make the sandbox and resource-limit contract match rootless production. On dev-01, Podman is 5.8.9, rootless, cgroup v2 with the systemd manager. The production image was not present there, so I used Debian bookworm-slim with the same keep-id:uid=10001,gid=10001 mapping and Bubblewrap 0.12.0 from the pinned Nixpkgs. bwrap --unshare-all with a private tmpfs root succeeded. Its --proc /proc mount failed with EPERM. Landlock ABI 6 successfully denied a file read, a seccomp filter successfully denied socket(), and lowering RLIMIT_CPU, RLIMIT_AS, RLIMIT_FSIZE and RLIMIT_NOFILE worked. Creating a child under /sys/fs/cgroup failed; the cgroup was not delegated into the container. Please specify the exact Quadlet/systemd delegation and outer CPU/memory limits, then test child-cgroup creation in the final image. If that cannot be delegated, replace the per-job cgroup requirement with the process limits and outer service cap that are actually available, update the stated 512 MiB/1 GiB guarantees, and keep previews disabled until the chosen gate passes. Test the worker root without /proc or show a permitted way to mount it. I did not test nsjail; the planned Bubblewrap path is viable with the noted procfs limitation.

  2. Define metadata backfill and source invalidation. #846 currently gets dimensions through attachments.meta by reading each referenced file, up to 200 paths, before the first picture draw. This plan changes that action to cached descriptors. It does not say how the cache gets dimensions for existing attachments or files written outside the upload action. Add an initial bounded backfill or a fast header-only fill in the notes.list/peek batch so oriented dimensions are present before #846 mounts boxes; do not wait for PDF decoding. Also make the path-to-content identity rule explicit. crates/attachments/src/place.rs::existing currently treats a matching indexed hash plus matching length as a duplicate, so a same-size replacement can reuse stale metadata or return the wrong “Already uploaded” path. Persist a file-generation fingerprint, invalidate and rehash when it changes, and check the source identity before and after decoding and before serving. Do not rely on watcher events alone.

  3. Pin decoder builds that implement the stated loader, protocol and licensing policy. I evaluated the exact flake nixpkgs pin on x86_64-linux and aarch64-linux. The pin already has pdfium 8066 for both architectures, and the libvips package is named vips (8.18.7), not libvips. The default vips derivation brings in optional loaders including libarchive, ImageMagick, libraw, librsvg, OpenSlide and Poppler, beyond the plan's allowlist. The default ffmpeg 9.0.1 enables network support and GPL components including x264/x265, so it does not meet “local file/FD protocols only” as packaged. libheif has the libde265 decoder on both architectures, but its derivation also includes x265; the LGPL package metadata does not settle HEVC patent coverage. Please name the pdfium package already in the pin, define a restricted vips loader set and an FFmpeg build with network protocols and unneeded GPL encoders disabled, and assert the final loader/protocol/decoder lists on both architectures. Keep HEIC independently unavailable with the icon until its codec and patent decision is cleared.

should-fix

  1. Make the production enablement gate explicit. The first backend PR is broad: several decoder stacks, queue persistence and reconciliation, metadata APIs, cache eviction, two Linux closures, the rootless image profile and a new performance harness. P6 does call for one backend PR, so keep that boundary if it is intentional, but add an operator-controlled, default-off preview switch and state that production stays off until the final-image sandbox and package gates pass. Document which formats are enabled when an optional codec is absent.

  2. Treat the latency numbers as initial targets, not validated gates. The 2s/3s/5s no-backlog targets look plausible for a 320 output, but 20 samples make p95 effectively the second-slowest sample. Increase repetitions before using p95 as a release gate, or label it diagnostic until a larger run. Include worker cold start and the production source-storage path in the measurement; originals are on the data root while preview state is on SSD per docs/storage.md. The plan correctly keeps functional write/typing checks barrier-based and puts timing in the performance harness.

fine

Keep the P2 same-slot icon and no processing text, independent 320/1280 readiness, per-user content/recipe storage keys, immediate access revocation with later GC, and the async journal-to-queue path. The batched additive descriptors for list/peek/open are the right integration for #846, and keeping cards in the pictures port follows P6. Keep the write-stall barriers, queue fairness cases and storage-backup exclusion as gates.

## verdict: changes needed ### must-fix 1. **Make the sandbox and resource-limit contract match rootless production.** On dev-01, Podman is 5.8.9, rootless, cgroup v2 with the systemd manager. The production image was not present there, so I used Debian bookworm-slim with the same `keep-id:uid=10001,gid=10001` mapping and Bubblewrap 0.12.0 from the pinned Nixpkgs. `bwrap --unshare-all` with a private tmpfs root succeeded. Its `--proc /proc` mount failed with `EPERM`. Landlock ABI 6 successfully denied a file read, a seccomp filter successfully denied `socket()`, and lowering `RLIMIT_CPU`, `RLIMIT_AS`, `RLIMIT_FSIZE` and `RLIMIT_NOFILE` worked. Creating a child under `/sys/fs/cgroup` failed; the cgroup was not delegated into the container. Please specify the exact Quadlet/systemd delegation and outer CPU/memory limits, then test child-cgroup creation in the final image. If that cannot be delegated, replace the per-job cgroup requirement with the process limits and outer service cap that are actually available, update the stated 512 MiB/1 GiB guarantees, and keep previews disabled until the chosen gate passes. Test the worker root without `/proc` or show a permitted way to mount it. I did not test nsjail; the planned Bubblewrap path is viable with the noted procfs limitation. 2. **Define metadata backfill and source invalidation.** #846 currently gets dimensions through `attachments.meta` by reading each referenced file, up to 200 paths, before the first picture draw. This plan changes that action to cached descriptors. It does not say how the cache gets dimensions for existing attachments or files written outside the upload action. Add an initial bounded backfill or a fast header-only fill in the `notes.list`/`peek` batch so oriented dimensions are present before #846 mounts boxes; do not wait for PDF decoding. Also make the path-to-content identity rule explicit. `crates/attachments/src/place.rs::existing` currently treats a matching indexed hash plus matching length as a duplicate, so a same-size replacement can reuse stale metadata or return the wrong “Already uploaded” path. Persist a file-generation fingerprint, invalidate and rehash when it changes, and check the source identity before and after decoding and before serving. Do not rely on watcher events alone. 3. **Pin decoder builds that implement the stated loader, protocol and licensing policy.** I evaluated the exact flake nixpkgs pin on x86_64-linux and aarch64-linux. The pin already has `pdfium` 8066 for both architectures, and the libvips package is named `vips` (8.18.7), not `libvips`. The default `vips` derivation brings in optional loaders including libarchive, ImageMagick, libraw, librsvg, OpenSlide and Poppler, beyond the plan's allowlist. The default `ffmpeg` 9.0.1 enables network support and GPL components including x264/x265, so it does not meet “local file/FD protocols only” as packaged. `libheif` has the `libde265` decoder on both architectures, but its derivation also includes x265; the LGPL package metadata does not settle HEVC patent coverage. Please name the `pdfium` package already in the pin, define a restricted `vips` loader set and an FFmpeg build with network protocols and unneeded GPL encoders disabled, and assert the final loader/protocol/decoder lists on both architectures. Keep HEIC independently unavailable with the icon until its codec and patent decision is cleared. ### should-fix 4. **Make the production enablement gate explicit.** The first backend PR is broad: several decoder stacks, queue persistence and reconciliation, metadata APIs, cache eviction, two Linux closures, the rootless image profile and a new performance harness. P6 does call for one backend PR, so keep that boundary if it is intentional, but add an operator-controlled, default-off preview switch and state that production stays off until the final-image sandbox and package gates pass. Document which formats are enabled when an optional codec is absent. 5. **Treat the latency numbers as initial targets, not validated gates.** The 2s/3s/5s no-backlog targets look plausible for a 320 output, but 20 samples make p95 effectively the second-slowest sample. Increase repetitions before using p95 as a release gate, or label it diagnostic until a larger run. Include worker cold start and the production source-storage path in the measurement; originals are on the data root while preview state is on SSD per `docs/storage.md`. The plan correctly keeps functional write/typing checks barrier-based and puts timing in the performance harness. ### fine Keep the P2 same-slot icon and no processing text, independent 320/1280 readiness, per-user content/recipe storage keys, immediate access revocation with later GC, and the async journal-to-queue path. The batched additive descriptors for list/peek/open are the right integration for #846, and keeping cards in the pictures port follows P6. Keep the write-stall barriers, queue fairness cases and storage-backup exclusion as gates.
kayg commented 2026-11-02 17:17:48 +00:00
Author
Owner
Copy link

Revised in 5d3a4bbe254a44927ad94d100de597c8995f1440, addressing every finding in the review, in this one design revision:

  1. Rootless sandbox/resource contract: removed child cgroups and all procfs mounts. Use Bubblewrap namespaces, mandatory Landlock/seccomp, seekable inherited-FD adapters, no helper scratch files, RLIMIT_AS 512 MiB, CPU/FD/core/file limits, parent deadlines and enforced pipe-byte caps. Two slots imply at most 1 GiB decoder address space, not a hard pool RSS promise. Outer Quadlet Podman limits are explicitly 3 GiB/no additional swap/4 CPUs. The enablement gate must run the probe and real fixtures inside the final production image, with the production UID mapping/security profile and no /proc, checking both allowed work and denied operations. This gate is planned, not claimed to have passed.

  2. Backfill and invalidation: added resumable pages of existing indexed files and bounded Photos/Documents discovery for outside writes, using the existing queue window. Fresh list/peek/open/meta batches can fill missing oriented dimension hints from at most 64 KiB per file, without PDF/native decoding. Persist blake3 and device/inode/size/nanosecond mtime/ctime fingerprints; the fingerprint validates a previously hashed generation, while blake3 remains content identity. Changed or unverifiable generations invalidate access and trigger background rehash. Check before/after decode, publication and serving, including 304s. Repair place::existing and attachments.check so matching length cannot return a false “Already uploaded”; stale candidates are misses without a write-path rehash wait. Added restored-mtime, replacement and missed-watcher cases.

  3. Minimal packages/licensing: name the pinned pkgs.pdfium 8066 and pkgs.vips 8.18.7, with explicit loader exclusions and V8/XFA disabled. FFmpeg 9.0.1 is a restricted LGPL library-only build: disable-everything/autodetect/network/GPL/nonfree/programs, then enable only listed demuxers/parsers/video decoders and required libraries. Custom seekable FD AVIO removes CLI/protocol/procfs dependencies. No encoders, muxers, filters, network protocols, x264 or x265; GPL FFmpeg is unnecessary for posters despite Calternal being AGPLv3. Assert actual component/closure lists on both architectures. Optional HEIC uses decoder-only libheif/libde265, no x265, and stays separately off until codec/patent clearance and its fixture gate. FFmpeg HEVC remains disabled.

  4. Enablement/scope: retain P6's single backend PR intentionally. CALTERNAL_PREVIEWS defaults off; explicit operator enablement follows final-image and package gates. Missing optional HEIC/HEVC keeps the icon while the listed supported image/PDF/video/text formats remain eligible. Isolation failures disable generation rather than weakening it. Kept same-slot icons, independent sizes, async queue, fairness/write-stall tests and backup exclusions.

  5. Latency targets: explicitly unvalidated. Twenty samples are diagnostic only; at least 200 per type/cache cohort precede promotion to a release gate. Include worker cold start, source validation, data-root/HDD originals and SSD state. Report peak RSS separately from configured address-space limits. Functional checks remain barrier-based.

Validation: git diff --check origin/dev...HEAD passed; git status --short is empty. This revision changes only the plan. No builds, decoder probes, performance runs or implementation tests were run, and no run results were added to the document. No merge performed. Remaining verification belongs to the first implementation PR's final-image/package gates.

Revised in `5d3a4bbe254a44927ad94d100de597c8995f1440`, addressing every finding in [the review](https://git.kayg.org/kayg/calternal/pulls/871#issuecomment-75418), in this one design revision: 1. **Rootless sandbox/resource contract:** removed child cgroups and all procfs mounts. Use Bubblewrap namespaces, mandatory Landlock/seccomp, seekable inherited-FD adapters, no helper scratch files, RLIMIT_AS 512 MiB, CPU/FD/core/file limits, parent deadlines and enforced pipe-byte caps. Two slots imply at most 1 GiB decoder address space, not a hard pool RSS promise. Outer Quadlet Podman limits are explicitly 3 GiB/no additional swap/4 CPUs. The enablement gate must run the probe and real fixtures inside the **final production image**, with the production UID mapping/security profile and no /proc, checking both allowed work and denied operations. This gate is planned, not claimed to have passed. 2. **Backfill and invalidation:** added resumable pages of existing indexed files and bounded Photos/Documents discovery for outside writes, using the existing queue window. Fresh list/peek/open/meta batches can fill missing oriented dimension hints from at most 64 KiB per file, without PDF/native decoding. Persist blake3 and device/inode/size/nanosecond mtime/ctime fingerprints; the fingerprint validates a previously hashed generation, while blake3 remains content identity. Changed or unverifiable generations invalidate access and trigger background rehash. Check before/after decode, publication and serving, including 304s. Repair place::existing and attachments.check so matching length cannot return a false “Already uploaded”; stale candidates are misses without a write-path rehash wait. Added restored-mtime, replacement and missed-watcher cases. 3. **Minimal packages/licensing:** name the pinned pkgs.pdfium 8066 and pkgs.vips 8.18.7, with explicit loader exclusions and V8/XFA disabled. FFmpeg 9.0.1 is a restricted **LGPL library-only build**: disable-everything/autodetect/network/GPL/nonfree/programs, then enable only listed demuxers/parsers/video decoders and required libraries. Custom seekable FD AVIO removes CLI/protocol/procfs dependencies. No encoders, muxers, filters, network protocols, x264 or x265; GPL FFmpeg is unnecessary for posters despite Calternal being AGPLv3. Assert actual component/closure lists on both architectures. Optional HEIC uses decoder-only libheif/libde265, no x265, and stays separately off until codec/patent clearance and its fixture gate. FFmpeg HEVC remains disabled. 4. **Enablement/scope:** retain P6's single backend PR intentionally. CALTERNAL_PREVIEWS defaults off; explicit operator enablement follows final-image and package gates. Missing optional HEIC/HEVC keeps the icon while the listed supported image/PDF/video/text formats remain eligible. Isolation failures disable generation rather than weakening it. Kept same-slot icons, independent sizes, async queue, fairness/write-stall tests and backup exclusions. 5. **Latency targets:** explicitly unvalidated. Twenty samples are diagnostic only; at least 200 per type/cache cohort precede promotion to a release gate. Include worker cold start, source validation, data-root/HDD originals and SSD state. Report peak RSS separately from configured address-space limits. Functional checks remain barrier-based. Validation: `git diff --check origin/dev...HEAD` passed; `git status --short` is empty. This revision changes only the plan. No builds, decoder probes, performance runs or implementation tests were run, and no run results were added to the document. No merge performed. Remaining verification belongs to the first implementation PR's final-image/package gates.
kayg commented 2026-11-07 19:33:44 +00:00
Author
Owner
Copy link

housekeeping #1009: rebased+green, left open for review.

rebased preview plan; proposal retained.

head: 7496354b17a3f75bebe55e8f90d9a7847babadbd. rebased onto dev cd7f84432febfc63b788fbb9b9f9950452982d74 and pushed with the original exact force-with-lease.

validation on the Mac VM, through Nix and the shared Mac lock/slot: root bun install --frozen-lockfile; in apps/web, bun run check, bun run lint, bun run build, bun run test; PERF_DIST=<worktree>/apps/web/dist bash perf/web.sh --bundle-only --gate. all passed. this documentation-only branch has no targeted browser specs or Rust implementation changes. exact commands, logs, and dev bundle control.

original-head missing-commit/patch inventory, file pr-871.md.

housekeeping #1009: rebased+green, left open for review. rebased preview plan; proposal retained. head: `7496354b17a3f75bebe55e8f90d9a7847babadbd`. rebased onto dev `cd7f84432febfc63b788fbb9b9f9950452982d74` and pushed with the original exact force-with-lease. validation on the Mac VM, through Nix and the shared Mac lock/slot: root `bun install --frozen-lockfile`; in `apps/web`, `bun run check`, `bun run lint`, `bun run build`, `bun run test`; `PERF_DIST=<worktree>/apps/web/dist bash perf/web.sh --bundle-only --gate`. all passed. this documentation-only branch has no targeted browser specs or Rust implementation changes. [exact commands, logs, and dev bundle control](https://git.kayg.org/attachments/64244177-fd81-4992-860e-4d5d22ca01cd). [original-head missing-commit/patch inventory](https://git.kayg.org/attachments/fe3c09dc-dea3-47a0-92e2-b44a777083e5), file `pr-871.md`.
Some checks failed
CI / server (pull_request) Has been cancelled
Details
CI / rust (pull_request) Has been cancelled
Details
CI / web (pull_request) Has been cancelled
Details
Markdown extraction contract / markdown-native-contract (pull_request) Has been cancelled
Details
Markdown extraction contract / markdown-editor-contract (pull_request) Has been cancelled
Details
Merge conflict checking is in progress. Try again in few moments.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin plan/previews-865:plan/previews-865
git switch plan/previews-865

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch dev
git merge --no-ff plan/previews-865
git switch plan/previews-865
git rebase dev
git switch dev
git merge --ff-only plan/previews-865
git switch plan/previews-865
git rebase dev
git switch dev
git merge --no-ff plan/previews-865
git switch dev
git merge --squash plan/previews-865
git switch dev
git merge --ff-only plan/previews-865
git switch dev
git merge plan/previews-865
git push origin dev
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
backend
Rust server

  
design
Mockups, UI and TASTE

  
docs
ADRs, CONTEXT, TASTE

  
idea
Parked idea, never worked until promoted

  
infra
Deploy, CI, VMs

  
perf
Performance and budgets

  
reliability
Data integrity, corruption traps

  
web
Web client

No labels
backend
design
docs
idea
infra
perf
reliability
web
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kayg/calternal!871
No description provided.