Skip to content

A file-backed cache entry written by another process becomes permanently invisible after a local Store #3927

Description

@dwin-gharibi

Description

The pkg/cache package doc promises cross-process visibility:

Two processes simultaneously caching different keys both see their writes preserved …
Cache.Lookup reloads the in-memory map when the file's mtime has advanced since its last
load, so cross-process writes become visible without a restart.

The first half holds — the file is correct. The second half is defeated by Store.

persistToDisk (pkg/cache/cache.go:176-199) reads the on-disk map into a fresh local map,
merges only its own key, writes it back, and then advances c.mtime:

entries := make(map[string]string)
if err := loadFromFile(c.path, entries); err != nil {   // sibling entries land HERE
    return err
}
...
entries[key] = response
if err := writeJSON(c.path, entries); err != nil {      // and are preserved on disk
    return err
}
c.mtime = mtimeOf(c.path)                               // we now look up to date

Store then adds only its own key to the in-memory map (:166). The sibling's entries were
read, written back to disk, and discarded from memory — while c.mtime was advanced to match
the very file that contains them.

maybeReload (:206-242) reloads only when the file's mtime differs from c.mtime:

info, err = os.Stat(c.path)
if err != nil || info.ModTime().Equal(c.mtime) {
    return   // "already up to date"
}

Together these leave a permanent blind spot: the instance has marked itself current against a
file it never fully loaded. Nothing reloads those entries until some third write moves the
mtime again.

The redundant-write shortcut at :188-191 has the same shape — it refreshes c.mtime after
reading the on-disk map without adopting it.

Expected Behavior

After Store, entries written by another process are visible to Lookup — either adopted at
store time or picked up by the next reload.

Actual Behavior

They are silently missing, and stay missing.

Steps to Reproduce

Two *Cache instances on one path (the configuration the package doc describes; in production
these are two agent processes sharing a cache file):

cfg := cache.Config{Enabled: true, Path: path}
a, _ := cache.New(cfg)
b, _ := cache.New(cfg)

b.Store("question-1", "answer-1")   // "process" B
a.Store("question-2", "answer-2")   // "process" A
file on disk = {
  "question-1": "answer-1",     <- written by B, correctly preserved
  "question-2": "answer-2"      <- written by A
}

a.Lookup("question-2") = "answer-2", found=true    own entry fine
a.Lookup("question-1") = "",         found=false   on disk, invisible to A

Remove A's intervening Store and the documented mtime reload works exactly as advertised:

a.Lookup("question-1") = "answer-1", found=true

That control is what isolates Store/persistToDisk as the cause — the reload machinery itself
is sound.

The shortcut branch reproduces the same way: have B store two keys, then have A store a pair the
file already contains (taking the existing == response path) and look up B's other key.

Docker Agent version

No response

OS & terminal

No response

Model used

No response

Error output

Screenshots

No response

Additional context

Impact

A cache miss where a hit was available: the agent re-invokes the model for a question another
process already answered — paying tokens and latency, and defeating the point of a shared
file-backed cache.

It is not transient. The blind spot survives until an unrelated write bumps the mtime, so two
agents sharing a cache file and alternating writes can keep each other blind indefinitely.

On-disk consistency is unaffected — no data is lost from the file. This is purely a stale
in-memory view.

Additional context

  • TestFileCache_crossProcessConcurrentStoresPreserveAllEntries (cache_test.go:290) covers
    this scenario but asserts only the on-disk result, which is exactly the half that works.
    The in-memory half was never asserted.
  • Any fix must merge rather than replace c.entries: TestFileCache_persistenceFailureKeepsInMemory
    (cache_test.go:186) pins that a failed Store deliberately keeps its entry in memory, and
    wholesale adoption of the on-disk view would discard it on the next successful Store.
  • Cache has no Delete, so entries are never removed from the file — merging cannot resurrect
    a deleted key.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/coreCore agent runtime, session management

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions