feat(hub): see what each agent session read, not just what it changed (BEA-98) - #135
Open
ssowonny wants to merge 1 commit into
Open
feat(hub): see what each agent session read, not just what it changed (BEA-98)#135ssowonny wants to merge 1 commit into
ssowonny wants to merge 1 commit into
Conversation
… (BEA-98) History showed what an agent run CHANGED. What it read lived in a daily aggregate with no session dimension, so the two could not be joined and nobody could answer "when my agent answered, what did it look at — and was it the fresh version or archive/retired-spec.md?". The join is one string carried through four places: hook -> spool -> hub -> run card. A run card now marks each change the run also read, lists the files it read and never touched, and says on screen why a read can be missing. The three landmines the issue asks be named here: 1. Op.Note is USER-SETTABLE (`bdrive sync --note`), so joining reads to writes on the note string would let any member with write access forge a note that collides with a teammate's run card and hang their reads off it. Fixed by adding journal.Op.Session — set only by `bdrive sync --hook`, never by --note — and joining on that. The note stays settable and stays untrusted; the join simply never reads it. Op.Session is additive JSONL and, like Mtime, is never an input to Less or Replay, so replay determinism is untouched and older ops carry "". The read half has the same hole one step further on: POST /reads takes the session id from the CLIENT, so a member could report reads under a teammate's session and paint files onto their card. Every session row is therefore pinned to the ownsDevice-validated device, and the query requires ?session= AND ?device= together — a forged row can only be found under the forger's own device, which MayActAs guarantees is never somebody else's. 2. BUCKET CARDINALITY. Putting the session in the read_stats key would take a 2k-file project from ~2k to ~100k rows/day, into a table ReadLedger loads whole at boot and full-scans on every heat request, hub-wide — so it would slow the Dashboard for projects that never ran an agent. This is the escape hatch the spec itself names, taken up front: session rows live in their own read_sessions repo, outside ReadLedger.byKey. No read_stats PK migration, no change to the resident-row count, ?by=device byte-identical. They get their own retention (session_retention_days, default 30) which DELETES rather than folds — no heat total was ever derived from them. 3. READS ARE RECORDED ONLY FOR PATHS IN THE CURRENT REPLAY, so a session that read a file it then deleted shows a change with no read. That is by design, and the run card says so in its footer rather than leaving it to read as a bug. Privacy ruling, written into internal/webapp/reads.go before anything serves it: a session id appears only in History responses on the op that carries it, and as a ?session= filter INPUT. It is never enumerated — no listing endpoint, no session column in /heat output, nothing new in ?by=device. Also: PendingReads now dedupes on (path, session), not path alone. Two agent sessions on one device between syncs used to collapse into one event carrying whichever session flushed last — one session's reads silently credited to another. Tests: journal round-trip + Less-ignores-Session; the forge test (`sync --note "claude-code session <someone-else's>"` leaves Session empty); a multi-device syncer test carrying the session through convergence; spool per-session dedup; hub round-trip, cross-device forge, query contract and non-enumeration; db_conformance on file, sqlite AND postgres; runs.ts grouping incl. legacy fallback; a Playwright spec on the seeded run card.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
Op.Sessionthat only the sync hook can write.--noteis user-settable, so joining on the note string would let anyone hang their reads off a teammate's card.read_sessionsrepo, not in theread_statskey — that table is loaded whole at boot and full-scanned on every heat request, so session cardinality there would slow the Dashboard for projects that never ran an agent. No PK migration, no change to resident row count.sec13fe.spec.tse2e tests fail — they also fail onorigin/main(verified in a clean worktree at27cc558). Not from this branch.Before / after
Same seeded run. The header goes from
2 filestoread 3 · changed 2; the file the run read and rewrote gets aREADmarker on its own row; the two it only read get their own list; the footer states the deleted-file rule.Mobile (375px) — no horizontal overflow
Why the note could not be the join key
flowchart LR Hook["bdrive sync --hook<br/>(the agent hook)"] Note["Op.Note<br/><i>any member can set it</i>"] Sess["Op.Session<br/><i>hook-only, no CLI flag reaches it</i>"] Card["History run card"] Hook --> Note Hook --> Sess Note -- "displayed" --> Card Sess -- "joined on" --> Card classDef bad fill:#ef444422,stroke:#ef4444,stroke-width:2px classDef good fill:#22c55e22,stroke:#22c55e,stroke-width:2px class Note bad class Sess goodbdrive sync --note "claude-code session <someone-else's-id>"is a thing any member with write access can run. Grouping on that string merged their ops into the victim's card; joining reads on it would have attached their reads too.Op.Sessionis the un-forgeable half — additive JSONL, and likeMtimenever an input toLessorReplay, so replay determinism is untouched and pre-existing ops carry""and still group by(note, device)exactly as before.The read half has the same hole one step on:
POST /readstakes the session id from the client. Every session row is pinned to theownsDevice-validated device, andGET /heatrequires?session=and?device=together — so a forged report can only ever be found under the forger's own device, whichMayActAsguarantees is never someone else's.The carry
hook → spool → hub → card, one string:
read-logparsessession_idfrom the same stdin payloadhooksyncalready reads, once per invocationstore.ReadEvent.Session;PendingReadsdedupes on(path, session), notpath— two sessions on one device between syncs used to collapse into one event carrying whichever flushed lastremote.ReadEvent.Session, copied into the outgoing batch by the syncerhandleReadReportrecords aSessionRead{Project, Session, Device, Path, Last}through the new repo, device pinned;journal.SafePath,ownsDeviceand the snapshot-existence check all still applyGET /heat?session=&device=→{"paths": [...]};RunGroupmarks rows and lists the read-only onesStorage shape — the one deliberate departure from the plan's spec
The specification put the session in the
read_statsprimary key. The Implementation Plan overrode that with a separateread_sessionsrepo, and this PR follows the plan. The reason isReadLedger.Heat: it iterates the entire in-memory bucket map on every heat request, for every project on the hub, under one mutex. So:read_statsand its PK are untouched — no SQLite table rebuild, no Postgres migration, no zero-row-loss proof needed because there is nothing to migrate.byKey.?by=deviceoutput is byte-identical (AgentHeatreadsbyKey).reads.session_retention_days, default 30 — and past it they are deleted, not folded. No heat total was ever derived from them, so nothing is lost from any count.The plan's own note applies: this reverses cleanly. Nothing about
Op.Session, the wire format, or the API shape depends on which table the rows land in.Privacy ruling
Written into
internal/webapp/reads.gobefore anything serves it, as both the spec and the plan ask:Sound because the id is already visible to every project member inside
Op.Notetoday, so serving it as its own field discloses nothing new — while refusing to enumerate keeps/heatidentity-free exactly as the package comment documents.TestSessionIdsAreNeverEnumeratedpins it.Architecture changes
architecture/cli-sync.mdjournal.OpgainsSession(hook-set, display/join-only, never an input toLess/Replay— the same standingMtimeholds);syncer.SessiongainsSessionID, the field the hook sets and--notecannot reach;store.LogReadtakes the session andPendingReadsdedupes on(path, session)instead ofpath; bothstore.ReadEventandremote.ReadEventcarry it.flowchart TB Sess["<div style='text-align:left'><b>syncer.Session</b><br/>+Folder, MountID, Store, Device, Account<br/>+Backend remote.Backend<br/>+Note string<br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ +SessionID string</span><br/>+Prune bool<br/>+Cycle(ctx) Result</div>"] Store["<div style='text-align:left'><b>store.Store</b><br/>+PutBlob / OpenBlob / HasBlob<br/>+AppendOps / DeviceOps / AllOps<br/>+SaveNote / LoadNote<br/><span style='background:#ef444455;padding:0 4px;border-radius:3px'>❌ <s>+LogRead(rel)</s></span><br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ +LogRead(rel, session)</span><br/><span style='background:#ef444455;padding:0 4px;border-radius:3px'>❌ <s>+PendingReads dedup on path</s></span><br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ +PendingReads dedup on path+session</span></div>"] Op["<div style='text-align:left'><b>journal.Op</b><br/>+Seq +Lamport +Time +Device<br/>+Author +User +UserName<br/>+Kind +Path +Blob +Size +Mode<br/>+Note (user-settable)<br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ +Session (hook-set only)</span><br/>+Mtime</div>"] RE["<div style='text-align:left'><b>remote.ReadEvent</b><br/>+Path<br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ +Session</span><br/>+Time</div>"] SE["<div style='text-align:left'><b>store.ReadEvent</b><br/>+Path<br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ +Session</span><br/>+Time</div>"] NoteOp["Session is display/join-only:<br/>never an input to Less or Replay,<br/>so replay stays deterministic and<br/>older ops simply carry the empty string.<br/>Note is forgeable, a session id is not."] Sess -- "scan stamps every op it commits" --> Op Sess --> Store Store -- "spools" --> SE Sess -- "<span style='background:#22c55e55;padding:0 5px;border-radius:3px'>✅ copies Session into the batch</span>" --> RE Op -.- NoteOp classDef added fill:#22c55e22,stroke:#22c55e,stroke-width:2px classDef removed fill:#ef444422,stroke:#ef4444,stroke-width:2px,stroke-dasharray:4 3 classDef noteBox fill:#88888822,stroke:#888888,stroke-dasharray:2 2 class NoteOp noteBox linkStyle 3 stroke:#22c55e,stroke-width:2pxarchitecture/webapp-server.mdMetaStoregains a seventh repo,SessionReadRepo(read_sessionsin SQL,sessions.jsonin the file backend), andReadLedgergains the session half —RecordSession/SessionPaths/WithSessionsover a newSessionReadrow, plus its own shorter retention.ReadStatandReadRepoare unchanged.flowchart TB MS["<div style='text-align:left'><b>MetaStore</b><br/>+Accounts() +Projects() +Orgs()<br/>+Shares() +Devices()<br/>+Reads() ReadRepo<br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ +SessionReads() SessionReadRepo</span><br/>+Close()</div>"] RR["<div style='text-align:left'><b>ReadRepo</b><br/>+Load() +PutBatch +DeleteBatch</div>"] SRR["<div style='text-align:left'><b>SessionReadRepo</b><br/>+PutBatch(reads)<br/>+ListBySession(project, session, device)<br/>+PruneBefore(t)</div>"] RL["<div style='text-align:left'><b>ReadLedger</b><br/>-repo ReadRepo<br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ -sessions SessionReadRepo</span><br/><span style='background:#ef444455;padding:0 4px;border-radius:3px'>❌ <s>-retention</s></span><br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ -retention, sessionRetention</span><br/><span style='background:#ef444455;padding:0 4px;border-radius:3px'>❌ <s>-byKey, dirty, seen</s></span><br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ -byKey, dirty, seen, pendingSess</span><br/>+Record(...)<br/>+Heat(project, prefix, days)<br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ +RecordSession(project, session, device, path)</span><br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ +SessionPaths(project, session, device)</span><br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ +WithSessions(repo, days)</span></div>"] RS["<div style='text-align:left'><b>ReadStat</b><br/>+Project +Path +Day +Kind +Actor<br/>+Count +Last<br/><i>key and table unchanged</i></div>"] SR["<div style='text-align:left'><b>SessionRead</b><br/>+Project +Session +Device +Path +Last</div>"] Why["Why its own repo, not a read_stats column:<br/>byKey is loaded whole at boot and<br/>full-scanned by Heat on every request, hub-wide.<br/>Session cardinality there would slow the<br/>Dashboard for projects that never ran an agent.<br/>So: no PK migration, no change to resident rows,<br/>?by=device byte-identical."] Pin["Device is always the ownsDevice-validated id,<br/>never a client field — and the query needs<br/>?session= AND ?device= together.<br/>A forged report can only ever be found<br/>under the forger's own device."] MS --> RR MS -- "<span style='background:#22c55e55;padding:0 5px;border-radius:3px'>✅ new repo</span>" --> SRR RL --> RR RL -- "<span style='background:#22c55e55;padding:0 5px;border-radius:3px'>✅</span>" --> SRR RL -.-> RS RL -. "<span style='background:#22c55e55;padding:0 5px;border-radius:3px'>✅</span>" .-> SR SR -.- Why SR -.- Pin classDef added fill:#22c55e22,stroke:#22c55e,stroke-width:2px classDef removed fill:#ef444422,stroke:#ef4444,stroke-width:2px,stroke-dasharray:4 3 classDef noteBox fill:#88888822,stroke:#888888,stroke-dasharray:2 2 class SRR added class SR added class Why noteBox class Pin noteBox linkStyle 1 stroke:#22c55e,stroke-width:2px linkStyle 3 stroke:#22c55e,stroke-width:2px linkStyle 5 stroke:#22c55e,stroke-width:2pxWhat was run
go build ./...,go vet ./...go test ./...db_conformance_test.goBDRIVE_TEST_POSTGRES)npm test(frontend units)runs.tsgrouping casesnpm run e2eorigin/main, see belowfrontend/check-dist.shinternal/webapp/static is freshThe two e2e failures are
sec13fe.spec.ts › VolumeApp (single-volume mode). I checked outorigin/main(27cc558) into a separate worktree and ran that spec there: the same two fail, identically. Not from this branch, and not fixed here.New tests worth naming:
TestSyncNoteCannotForgeASession— landmine 1, explicitly:bdrive sync --note "claude-code session <someone-else's-id>"produces an op with an emptySession.TestSessionReadsCannotBePaintedOntoAnotherDevicesRun— its read-half twin, at the hub.TestSessionCarriesThroughTwoDevices— the multi-device syncer test CLAUDE.md requires for a sync change: session on the ops and on the reported reads, empty on the peer that has none, convergence intact.TestSessionIdsAreNeverEnumerated— the privacy ruling.internal/webapp/frontend/e2e/session-run.spec.ts— the card itself against the seeded hub.One thing still open
The plan set
session_retention_daysto 30 rather than the spec's 14, on the grounds that with the rows outsidebyKeythe cost of keeping them is disk, not resident memory. That reasoning holds, but the number itself still has no measurement behind it — this PR does not produce one, because there is now no resident-row bound to measure. If a busy hub'sread_sessionstable turns out to be the thing that grows, the knob is already there.Build session
(only works on the machine this branch was built on)