chore: bound the caches and maps that never evict #445

Open
opened 2026-07-25 23:30:39 +02:00 by dries · 0 comments
Owner

Code-quality audit (P2). Seven in-memory caches/maps grow for the process lifetime with no eviction path. Individually small, collectively a slow leak in a long-running daemon; the git diff cache is the largest single offender.

Sites

  • internal/git/diff.go:124 — package-level diffCacheStorage map[string]diffEntry. lookupCached returns nil for a stale entry but leaves the row in place. Each value is a *Diff capped at totalDiffCap (2 MB), retained forever. Every worktree is a distinct key, so /wt usage multiplies the key count. Same shape (smaller values) at internal/git/info.go:68-119cache.entries is only pruned by the explicit invalidate(dir) after a checkout.
  • internal/platforms/opencode/portdiscovery.go:42sessionPortAffinity sync.Map. Written by rememberSessionPort on every port resolution (portdiscovery.go:542, operations.go:61), removed only by an explicit forgetSessionPort. Retains every session ID ever touched, including deleted ones.
  • internal/platforms/opencode/live_prompts.go:113-127ObservePromptResolved deletes the entries row and then writes a new changed[key]. changed is only pruned in ClearPromptsForPort (:209-212), and only for keys still present in entries — which this path just removed. Net effect: every permission/question the user answers adds a permanent entry.
  • internal/platforms/opencode/httpcache.go:123-137invalidatePort has zero production callers (only httpcache_test.go:150), yet its doc comment asserts the mechanism is wired to port disappearance. Nothing prunes by port and get never deletes expired entries. ocruntime.AllocateLoopbackPort hands each managed relaunch a fresh random port, so every project restart adds a key set that is never reclaimed.
  • internal/workflows/service.go:1866,1878ownedRuns / reconciledRuns. Repo-wide grep finds no delete( for either. One entry per workflow run, forever.
  • internal/autoapprove/statemachine.go:147autoApprove deliberately retains recorded verdicts (if st.verdict == "" { delete(...) }) so REST resurrection can replay them: one entry per (session, permission) forever. :366safeCommandCache keeps a per-session map of every safe command hash, no eviction.
  • internal/queuesvc/service.go:120lockFor stores a *sync.Mutex per session ID that ever enqueues/flushes/sweeps; nothing removes them.

Suggested fix

Per site, whichever is smallest:

  • Opportunistic delete on stale read (git/diff.go, git/info.go, httpcache.get).
  • Prune on the existing lifecycle edge that already exists — session deletion (sessionPortAffinity), run terminal state (ownedRuns / reconciledRuns), session idle+empty (queuesvc.locks).
  • live_prompts: also delete changed[key] in ClearPromptsForPort for keys whose entry is already gone, or age out changed by version.
  • httpcache: either wire invalidatePort into port-disappearance detection or delete it and fix the doc comment. Add a size cap.

Acceptance criteria

  • Each site above either evicts on an existing lifecycle edge or is bounded (size cap / TTL sweep).
  • invalidatePort's doc comment matches reality (wired or removed).
  • A test per site asserts the map shrinks after the eviction trigger.

Effort: S each.

Code-quality audit (P2). Seven in-memory caches/maps grow for the process lifetime with no eviction path. Individually small, collectively a slow leak in a long-running daemon; the git diff cache is the largest single offender. ## Sites - [ ] **`internal/git/diff.go:124`** — package-level `diffCacheStorage map[string]diffEntry`. `lookupCached` returns `nil` for a stale entry but leaves the row in place. Each value is a `*Diff` capped at `totalDiffCap` (2 MB), retained forever. Every worktree is a distinct key, so `/wt` usage multiplies the key count. Same shape (smaller values) at `internal/git/info.go:68-119` — `cache.entries` is only pruned by the explicit `invalidate(dir)` after a checkout. - [ ] **`internal/platforms/opencode/portdiscovery.go:42`** — `sessionPortAffinity sync.Map`. Written by `rememberSessionPort` on every port resolution (`portdiscovery.go:542`, `operations.go:61`), removed only by an explicit `forgetSessionPort`. Retains every session ID ever touched, including deleted ones. - [ ] **`internal/platforms/opencode/live_prompts.go:113-127`** — `ObservePromptResolved` deletes the `entries` row and then writes a new `changed[key]`. `changed` is only pruned in `ClearPromptsForPort` (`:209-212`), and only for keys still present in `entries` — which this path just removed. Net effect: every permission/question the user answers adds a permanent entry. - [ ] **`internal/platforms/opencode/httpcache.go:123-137`** — `invalidatePort` has zero production callers (only `httpcache_test.go:150`), yet its doc comment asserts the mechanism is wired to port disappearance. Nothing prunes by port and `get` never deletes expired entries. `ocruntime.AllocateLoopbackPort` hands each managed relaunch a fresh random port, so every project restart adds a key set that is never reclaimed. - [ ] **`internal/workflows/service.go:1866,1878`** — `ownedRuns` / `reconciledRuns`. Repo-wide grep finds no `delete(` for either. One entry per workflow run, forever. - [ ] **`internal/autoapprove/statemachine.go:147`** — `autoApprove` deliberately retains recorded verdicts (`if st.verdict == "" { delete(...) }`) so REST resurrection can replay them: one entry per (session, permission) forever. **`:366`** — `safeCommandCache` keeps a per-session map of every safe command hash, no eviction. - [ ] **`internal/queuesvc/service.go:120`** — `lockFor` stores a `*sync.Mutex` per session ID that ever enqueues/flushes/sweeps; nothing removes them. ## Suggested fix Per site, whichever is smallest: - Opportunistic delete on stale read (`git/diff.go`, `git/info.go`, `httpcache.get`). - Prune on the existing lifecycle edge that already exists — session deletion (`sessionPortAffinity`), run terminal state (`ownedRuns` / `reconciledRuns`), session idle+empty (`queuesvc.locks`). - `live_prompts`: also delete `changed[key]` in `ClearPromptsForPort` for keys whose entry is already gone, or age out `changed` by version. - `httpcache`: either wire `invalidatePort` into port-disappearance detection or delete it and fix the doc comment. Add a size cap. ## Acceptance criteria - [ ] Each site above either evicts on an existing lifecycle edge or is bounded (size cap / TTL sweep). - [ ] `invalidatePort`'s doc comment matches reality (wired or removed). - [ ] A test per site asserts the map shrinks after the eviction trigger. Effort: S each.
Sign in to join this conversation.
No milestone
No project
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
dries/ocman#445
No description provided.