kami/Maven
1
0
Fork 0
Code Issues 1 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Vision: store and describe images through a shared media intake (#252) #72

Closed
claude wants to merge 1 commits from overnight/senses-media-vision into overnight/mcp-tools
pull from: overnight/senses-media-vision
merge into: kami:overnight/mcp-tools
Conversation 2 Commits 1 Files Changed 19
+2444 -26
claude commented 2026-08-01 02:53:31 +02:00
Contributor
Copy Link
Copy Source

What changed

internal/media — the intake all three senses share. A content-addressed blob store on
disk (sha256 key, sidecar JSON metadata, 0700/0600), plus image decode / alpha-flatten /
downscale / JPEG re-encode with no new dependencies. Prune() enforces retention.

internal/vision — Provider seam with one method, a Disabled floor so no call site
needs a nil check, and LocalProvider speaking the OpenAI multimodal shape at a llama-server.
Intake stores the blob before describing it, and Rerun(id, question) describes a stored
blob later.

Config — new top-level media and vision blocks, both absent by default.

IPC — describe_image, AuthRead, hook-gated: nil hook ⇒ ErrUnknownMethod, the same
"off unless configured" shape ingest_mail uses.

cmd/mavend/vision.go — wiring, plus the hourly retention prune loop.

Why it looks like this

Vision needs a model this box does not have. Checked /mnt/hdd1/llms: sixteen ggufs, all
text-only, no mmproj anywhere, and the resident Qwen3-1.7B is text-only by construction. So
the describing half is BLOCKED on a model download and what ships is the intake, the
storage, the config seam and the provider — tested against a fake server. Intake.Accept
stores first and describes second precisely so that today's state degrades to "it's kept, I
can't read it yet, here is the id" instead of losing the image.

The plan's RemoteProvider step is refused. It called for "an OpenAI-compatible vision API
endpoint". CLAUDE.md's surviving constraint is no cloud model, inference stays on the box, and a
photo of his flat is the worst possible exception. vision.NewLocal validates the endpoint at
construction: loopback, private IP, or localhost. A bare hostname is refused too — it could
resolve anywhere.

Privacy invariants, enforced not just documented: blobs never leave the box, are never search
input, are never embedded, and are pruned on a loop. Only the derived description becomes a
note, and only when the caller passes save_note.

How it was verified

make build and make test both exit 0. New tests: 27 across internal/media (dedupe keeps
first-seen time so a re-sent photo cannot outlive retention; path-traversal ids refused;
permissions; prune), internal/vision (every non-private endpoint form refused; data-URI wire
shape; reasoning_content fallback; store-survives-describe-failure), and internal/config.

Not verified against a real vision model — there isn't one. That is the QA step on the task.

Vikunja #252

## What changed **`internal/media`** — the intake all three senses share. A content-addressed blob store on disk (sha256 key, sidecar JSON metadata, 0700/0600), plus image decode / alpha-flatten / downscale / JPEG re-encode with no new dependencies. `Prune()` enforces retention. **`internal/vision`** — `Provider` seam with one method, a `Disabled` floor so no call site needs a nil check, and `LocalProvider` speaking the OpenAI multimodal shape at a llama-server. `Intake` stores the blob *before* describing it, and `Rerun(id, question)` describes a stored blob later. **Config** — new top-level `media` and `vision` blocks, both absent by default. **IPC** — `describe_image`, `AuthRead`, hook-gated: nil hook ⇒ `ErrUnknownMethod`, the same "off unless configured" shape `ingest_mail` uses. **`cmd/mavend/vision.go`** — wiring, plus the hourly retention prune loop. ## Why it looks like this Vision needs a model this box does not have. Checked `/mnt/hdd1/llms`: sixteen ggufs, all text-only, no `mmproj` anywhere, and the resident Qwen3-1.7B is text-only by construction. So the **describing half is BLOCKED on a model download** and what ships is the intake, the storage, the config seam and the provider — tested against a fake server. `Intake.Accept` stores first and describes second precisely so that today's state degrades to "it's kept, I can't read it yet, here is the id" instead of losing the image. **The plan's `RemoteProvider` step is refused.** It called for "an OpenAI-compatible vision API endpoint". CLAUDE.md's surviving constraint is no cloud model, inference stays on the box, and a photo of his flat is the worst possible exception. `vision.NewLocal` validates the endpoint at construction: loopback, private IP, or `localhost`. A bare hostname is refused too — it could resolve anywhere. Privacy invariants, enforced not just documented: blobs never leave the box, are never search input, are never embedded, and are pruned on a loop. Only the derived description becomes a note, and only when the caller passes `save_note`. ## How it was verified `make build` and `make test` both exit 0. New tests: 27 across `internal/media` (dedupe keeps first-seen time so a re-sent photo cannot outlive retention; path-traversal ids refused; permissions; prune), `internal/vision` (every non-private endpoint form refused; data-URI wire shape; `reasoning_content` fallback; store-survives-describe-failure), and `internal/config`. Not verified against a real vision model — there isn't one. That is the QA step on the task. Vikunja #252
claude added 1 commit 2026-08-01 02:53:32 +02:00
Store and describe images through a shared media intake (#252) d92349ca6e 
Vision needs a second model this box does not have, so the shipped half is
the part that works without one: an image arrives, is sniffed, is stored
content-addressed, and is prepared for inference. The describing half is
written and tested against a fake server, and refuses any endpoint that is
not on this box.

internal/media is the intake all three senses share — hearing and speaker
recognition store their audio in the same place under the same retention.
Blobs stay out of the sqlite store; only the derived text becomes a note,
and only when the caller asks. Retention is enforced by an hourly prune
loop rather than by a comment.

The plan's RemoteProvider step is refused: no cloud model, inference stays
on the box, and vision.NewLocal validates that at construction.
claude reviewed 2026-08-01 11:35:38 +02:00
claude left a comment
Author
Contributor
Copy Link
Copy Source

The shape of internal/media is the good part. Sidecars instead of another
sqlite table means the store reads with ls when something goes wrong. Blobs
stay out of the encrypted database. validID guards every path built from a
caller-supplied id. Put keeps the first-seen Created, so re-sending the same
photo hourly cannot hold it past retention. The plan asked for a cloud vision
call. Refusing outright, and writing the refusal into the package comment, is
the right answer.

The findings below are all about one gap. The invariants are stated as prose in
package comments, and three of them are not what the code does.

1. One PNG OOMs mavend, and it only needs AuthRead

PrepareImage sniffs, then decodes the whole image, then flattenAndScale
allocates image.NewRGBA(image.Rect(0, 0, sw, sh)) at the source dimensions
before it scales anything. Nothing anywhere checks pixel count. The only cap is
media.DefaultMaxBytes, 64 MiB, and that is on the compressed input.

Walked through. A 20000x20000 PNG of flat colour compresses to a few hundred
kilobytes, well under the cap. png.Decode produces an NRGBA of 400 million
pixels, about 1.6 GB. flattenAndScale then allocates a second RGBA of the same
dimensions, another 1.6 GB, and composites into it. That is 3.2 GB of live heap from one request. On a laptop. In the process that
owns the database and the socket. MaxDim: 896 never gets a chance to help, because the downscale target
is only allocated after the full-size one.

image.DecodeConfig reads only the header and gives Width and Height for
all three formats. Reject on w*h over a few tens of megapixels before
decode, and make flattenAndScale walk the source through src.At rather
than materialising a full-size RGBA first. The flatten-onto-white step does not
need its own full-size buffer.

2. The method exists whenever media is configured, not when vision is on

Three comments say otherwise, including the wire contract.

From the cmd/mavend/vision.go header:

no vision block with enabled + a local endpoint ⇒ the store is wired but
the describing half refuses, and the method still does not exist.

From ipc.DescribeImageReq:

The method exists only when core has both a media store and an enabled
vision block; otherwise it answers ErrUnknownMethod.

From Server.DescribeImageFn:

Set by the daemon only when a media store is configured AND vision is
enabled with a local endpoint.

newVisionIntake returns a non-nil intake with vision.Disabled{} whenever
keeper != nil, and its own doc comment argues for exactly that. So one comment
in this diff contradicts the other three, and the code follows the minority.

The consequence is not cosmetic. The package comment says this box has media
set and no vision model. In that state MethodDescribeImage is live at AuthRead
and does nothing but write attacker-chosen bytes to disk. No description is
produced. Every enrolled module can call it.

Pick one. Storing without describing may well be useful. If so, say it in
ipc.DescribeImageReq and Server.DescribeImageFn too. Those two are what
another surface reads before deciding whether to send. Otherwise gate on
cfg.Vision.LooksAtImages().

3. SaveNote writes to memory at AuthRead, under a source no module owns

The policy comment argues the rung by what the method cannot do. "It cannot
write a fact, set a reminder, or touch the tool allowlist." It can write a note.
writeNote embeds the description with EmbedPassage and stores it under
media:image:<id-prefix>. An embedded note is recall corpus. It comes back in a
later turn as something she knows.

That is the property AuthWrite exists to protect. The comment on AuthWrite
states it plainly: "a module only writes sources it owns", so a compromised
poller cannot forge a source. media:image:* is owned by no enrollment, and
DescribeImage lets any AuthRead caller write it. MethodIngestMail sits on
the same rung and its justification holds, because its output lands on a review
page. A note does not land on a review page.

The narrow fix is to require AuthWrite when SaveNote is set. Can already
re-parses params for WriteFact, so the machinery is there. Leaving the
description-only call at AuthRead is defensible on its own.

Separate but adjacent: the note is a small VLM's guess, stored as plain note
text with no marker. The file header says a 1.7B-class guess "is not a fact worth carrying around".
Then SaveNote carries it around, in the same shape as something he told her.

4. Retention does not cover a blob whose sidecar is missing

Prune iterates List(""). List walks for .json files and drops anything
readMeta rejects. So a blob whose sidecar is corrupt, or absent, is invisible
to Prune and stays on disk forever.

Put produces exactly that state. It calls writeFile(blobPath, data) first
and writeMeta(metaPath, b) second. A full disk, a permission change, a crash
between the two, and the bytes are on disk with no sidecar. Put returns an
error, the caller reports failure, and an image nobody knows about is now
permanent.

The package comment calls this the point of the whole package:

an unpruned store is a bug: audio of people accumulating forever on disk is
the failure mode this capability has to avoid.

Write the sidecar first. Better, have Prune also sweep blob files that have no
readable sidecar and are older than retention. That also collects whatever a
previous version leaked.

Smaller notes

  • LocalProvider uses a bare http.Client with the default redirect policy.
    checkPrivate validates the configured literal at construction and nothing
    checks a hop. A 302 from the local llama-server sends the image, as a data URI
    in a POST body, to whatever the redirect names. The package comment says "No provider in this
    repo may upload one". The private check exists because "a photo of his flat is
    the single worst thing to make an exception for". One line,
    CheckRedirect: func(...) error { return http.ErrUseLastResponse }, makes
    the claim true. While there, cap the response body. The decoder reads whatever
    the endpoint sends.
  • The per-blob cap does not bound the store. ErrTooLarge's comment says the cap stops "a
    runaway capture" filling "the disk that mavend's database lives on". Nothing
    limits blob count. Content-addressed storage dedupes identical bytes, and one
    flipped pixel defeats that. 64 MiB per call times
    unlimited calls inside a 7-day window fills the disk. A total-bytes budget,
    checked in Put against a cheap running total, is what the comment describes.
  • MediaConfig and VisionConfig get no validate() entry, unlike mcp. A
    dir that cannot be created is caught at openMediaStore and logged, so the
    capability silently stays off. A typo in endpoint is the same. Both are the
    kind of thing that should fail at startup.
  • Nothing enforces "exactly one of Data or ID". When both are set, describe
    takes the ID branch and drops the bytes without a word. The doc on
    DescribeImageReq states the rule. The code should too.
  • extFor maps image/webp to .webp, but SniffImage refuses webp before
    anything reaches Put, so that arm is unreachable for images. Harmless, but
    it reads as though webp works.
  • runPrune is started with a bare go and is not in the daemon's wg, unlike
    the other loops in run. Shutdown does not wait for a prune in flight.
  • Untested: a decode bomb, a sidecar-less blob surviving Prune, Put failing
    between blob and sidecar, and SaveNote when the embedder is nil.
The shape of `internal/media` is the good part. Sidecars instead of another sqlite table means the store reads with `ls` when something goes wrong. Blobs stay out of the encrypted database. `validID` guards every path built from a caller-supplied id. `Put` keeps the first-seen `Created`, so re-sending the same photo hourly cannot hold it past retention. The plan asked for a cloud vision call. Refusing outright, and writing the refusal into the package comment, is the right answer. The findings below are all about one gap. The invariants are stated as prose in package comments, and three of them are not what the code does. ## 1. One PNG OOMs mavend, and it only needs AuthRead `PrepareImage` sniffs, then decodes the whole image, then `flattenAndScale` allocates `image.NewRGBA(image.Rect(0, 0, sw, sh))` at the *source* dimensions before it scales anything. Nothing anywhere checks pixel count. The only cap is `media.DefaultMaxBytes`, 64 MiB, and that is on the compressed input. Walked through. A 20000x20000 PNG of flat colour compresses to a few hundred kilobytes, well under the cap. `png.Decode` produces an NRGBA of 400 million pixels, about 1.6 GB. `flattenAndScale` then allocates a second RGBA of the same dimensions, another 1.6 GB, and composites into it. That is 3.2 GB of live heap from one request. On a laptop. In the process that owns the database and the socket. `MaxDim: 896` never gets a chance to help, because the downscale target is only allocated after the full-size one. `image.DecodeConfig` reads only the header and gives `Width` and `Height` for all three formats. Reject on `w*h` over a few tens of megapixels before `decode`, and make `flattenAndScale` walk the source through `src.At` rather than materialising a full-size RGBA first. The flatten-onto-white step does not need its own full-size buffer. ## 2. The method exists whenever `media` is configured, not when vision is on Three comments say otherwise, including the wire contract. From the `cmd/mavend/vision.go` header: > no `vision` block with enabled + a local endpoint ⇒ the store is wired but > the describing half refuses, and the method still does not exist. From `ipc.DescribeImageReq`: > The method exists only when core has both a media store and an enabled > vision block; otherwise it answers ErrUnknownMethod. From `Server.DescribeImageFn`: > Set by the daemon only when a media store is configured AND vision is > enabled with a local endpoint. `newVisionIntake` returns a non-nil intake with `vision.Disabled{}` whenever `keeper != nil`, and its own doc comment argues for exactly that. So one comment in this diff contradicts the other three, and the code follows the minority. The consequence is not cosmetic. The package comment says this box has `media` set and no vision model. In that state `MethodDescribeImage` is live at AuthRead and does nothing but write attacker-chosen bytes to disk. No description is produced. Every enrolled module can call it. Pick one. Storing without describing may well be useful. If so, say it in `ipc.DescribeImageReq` and `Server.DescribeImageFn` too. Those two are what another surface reads before deciding whether to send. Otherwise gate on `cfg.Vision.LooksAtImages()`. ## 3. `SaveNote` writes to memory at AuthRead, under a source no module owns The policy comment argues the rung by what the method cannot do. "It cannot write a fact, set a reminder, or touch the tool allowlist." It can write a note. `writeNote` embeds the description with `EmbedPassage` and stores it under `media:image:<id-prefix>`. An embedded note is recall corpus. It comes back in a later turn as something she knows. That is the property `AuthWrite` exists to protect. The comment on `AuthWrite` states it plainly: "a module only writes sources it owns", so a compromised poller cannot forge a source. `media:image:*` is owned by no enrollment, and `DescribeImage` lets any AuthRead caller write it. `MethodIngestMail` sits on the same rung and its justification holds, because its output lands on a review page. A note does not land on a review page. The narrow fix is to require AuthWrite when `SaveNote` is set. `Can` already re-parses params for `WriteFact`, so the machinery is there. Leaving the description-only call at AuthRead is defensible on its own. Separate but adjacent: the note is a small VLM's guess, stored as plain note text with no marker. The file header says a 1.7B-class guess "is not a fact worth carrying around". Then `SaveNote` carries it around, in the same shape as something he told her. ## 4. Retention does not cover a blob whose sidecar is missing `Prune` iterates `List("")`. `List` walks for `.json` files and drops anything `readMeta` rejects. So a blob whose sidecar is corrupt, or absent, is invisible to `Prune` and stays on disk forever. `Put` produces exactly that state. It calls `writeFile(blobPath, data)` first and `writeMeta(metaPath, b)` second. A full disk, a permission change, a crash between the two, and the bytes are on disk with no sidecar. `Put` returns an error, the caller reports failure, and an image nobody knows about is now permanent. The package comment calls this the point of the whole package: > an unpruned store is a bug: audio of people accumulating forever on disk is > the failure mode this capability has to avoid. Write the sidecar first. Better, have `Prune` also sweep blob files that have no readable sidecar and are older than retention. That also collects whatever a previous version leaked. ## Smaller notes - `LocalProvider` uses a bare `http.Client` with the default redirect policy. `checkPrivate` validates the configured literal at construction and nothing checks a hop. A 302 from the local llama-server sends the image, as a data URI in a POST body, to whatever the redirect names. The package comment says "No provider in this repo may upload one". The private check exists because "a photo of his flat is the single worst thing to make an exception for". One line, `CheckRedirect: func(...) error { return http.ErrUseLastResponse }`, makes the claim true. While there, cap the response body. The decoder reads whatever the endpoint sends. - The per-blob cap does not bound the store. `ErrTooLarge`'s comment says the cap stops "a runaway capture" filling "the disk that mavend's database lives on". Nothing limits blob *count*. Content-addressed storage dedupes identical bytes, and one flipped pixel defeats that. 64 MiB per call times unlimited calls inside a 7-day window fills the disk. A total-bytes budget, checked in `Put` against a cheap running total, is what the comment describes. - `MediaConfig` and `VisionConfig` get no `validate()` entry, unlike `mcp`. A `dir` that cannot be created is caught at `openMediaStore` and logged, so the capability silently stays off. A typo in `endpoint` is the same. Both are the kind of thing that should fail at startup. - Nothing enforces "exactly one of Data or ID". When both are set, `describe` takes the `ID` branch and drops the bytes without a word. The doc on `DescribeImageReq` states the rule. The code should too. - `extFor` maps `image/webp` to `.webp`, but `SniffImage` refuses webp before anything reaches `Put`, so that arm is unreachable for images. Harmless, but it reads as though webp works. - `runPrune` is started with a bare `go` and is not in the daemon's `wg`, unlike the other loops in `run`. Shutdown does not wait for a prune in flight. - Untested: a decode bomb, a sidecar-less blob surviving `Prune`, `Put` failing between blob and sidecar, and `SaveNote` when the embedder is nil.
kami closed this pull request 2026-08-01 14:51:57 +02:00
kami commented 2026-08-01 14:52:51 +02:00
Owner
Copy Link
Copy Source

Landed on master. The stack was one linear chain, so #84 carried every commit from #50 up, and master now contains this branch in full. Merging this PR on its own is an empty diff, so it is closed rather than merged. The review findings for it were fixed in the 2026-08-01 pass and are on master as commits on the stack tip, not on this branch.

Landed on master. The stack was one linear chain, so #84 carried every commit from #50 up, and master now contains this branch in full. Merging this PR on its own is an empty diff, so it is closed rather than merged. The review findings for it were fixed in the 2026-08-01 pass and are on master as commits on the stack tip, not on this branch.

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
claude
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
kami
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kami/Maven#72
Delete Branch "overnight/senses-media-vision"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?