rossoctl / rossoctl/context-guru
fix(offload): kept-verbatim marks are global, so one session's expand exempts that content for every session
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 54
- Forks
- 22
- Avg merge
- 20h 42m
- Merged PRs (30d)
- 89
Description
What happens
keptKey carries no session, so a kept-verbatim mark is global to the store:
// components/offload/state.go
func keptKey(ck string) string { return "cg:keep:" + ck }
func MarkKeptVerbatim(st store.Store, original string) {
st.Put(keptKey(contentKey(original)), []byte{1})
}
func isKeptVerbatim(c *components.Ctx, ck string) bool {
_, ok := c.Store.Get(keptKey(ck)) // no c.Session
return ok
}
The mark's purpose is narrow and per-session: session A expanded this content, so re-compacting it there would just make A expand it again — a per-turn bounce loop. But the key has no session in it, so the exemption applies to every session sharing the store.
Why it costs money rather than correctness
skipReduce consults isKeptVerbatim for every offloader, so a mark written by one session makes that content permanently uncompactable for all of them. Nothing breaks: requests are correct, just larger.
The distribution is what makes it worth fixing rather than noting. Content that appears in many sessions is exactly the content most worth compacting, and it is also the content most likely to have been expanded by someone. So the leak lands preferentially on the highest-value candidates, it accumulates monotonically (marks are never cleared), and it is invisible: the offloader records kept_verbatim_after_expand, which is indistinguishable from the legitimate same-session case.
A shared-store deployment converges toward "the union of everything anyone ever expanded is exempt for everyone".
The fix, and why it is not a one-liner
Put the session in the key — cg:keep:<session>:<contentkey> — and thread it through MarkKeptVerbatim (which the proxy's expand loop calls with the session id already in hand) and isKeptVerbatim (which has c.Session). An empty session must be a no-op write, not a fallback to a global key, or the leak returns through the default path.
Two things make it more than a rename:
- Marks already on disk are under the old key shape and would all be ignored at once. A session mid-expand-loop when the new binary rolls would lose its exemption and start bouncing. So the read side wants to accept both shapes for a deprecation window, with only the new shape written.
KeptVerbatim(st, original)is also read by the expand repair to charge a restored original to the dashboard exactly once. Whether that reader wants session scoping is a separate question — a global "this was already counted" may well be what it wants, since the double-count it prevents is per-store. Scoping both without asking would be assuming they are the same question.
Provenance
Found while re-cutting PR #80 onto main. That branch had already made these marks session-scoped, and its two tests for the property fail on main because the leak is real there:
TestKeptVerbatimDoesNotLeakAcrossSessions— session B must not inherit session A's exemption, and A must keep its own.TestMarkKeptVerbatimIgnoresAnEmptySession— an empty session must write nothing.
Both are on the re-cut branch as t.Skip with their bodies preserved commented, pointing here. They were not adapted to the two-argument signature deliberately: an adapted body would compile and read as a real test while asserting nothing, because both sessions would share one global mark. Un-skip and restore them verbatim when the scoped key lands.
Filed separately rather than fixed inside PR #80 because it is a store key-format change touching every offloader, plus a migration decision — none of which is about co-reference.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in components/offload/state.go at keptKey, MarkKeptVerbatim, isKeptVerbatim, and skipReduce, then trace the proxy expand loop's call site. Restore and unskip TestKeptVerbatimDoesNotLeakAcrossSessions and TestMarkKeptVerbatimIgnoresAnEmptySession from the recut branch, and run the offload tests. Done means session isolation, no write for an empty session, old-key read compatibility, and only new keys being written.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100