feat(scratch): make a workspace a session, with both retention timers #857

Merged

coilysiren merged 1 commit from aos/claude/bk79-session-workspace into main

2026-08-16 04:03:25 +00:00

coilyco-ops commented

2026-08-16 03:10:00 +00:00

Member

Refs #156. The proposal Kai approved, built whole: session identity, nested partitioning, both timers, the per-session cap, oldest-first eviction, and a sweeper that actually runs.

The shape

<session>/<requester>/notes.txt

Nested rather than renamed, which is the part worth reviewing. The 4 MiB quota is measured over the partition directory (partitionBytes() walks s.root), so a flat rename moves that bound rather than adding one: it silently stops meaning per-requester and starts meaning per-session. A member with five live threads would then hold five partitions against a 128Mi emptyDir whose overflow evicts the pod rather than refusing a write.

Nesting gives two things to measure, so the per-requester ceiling survives and the per-session cap is genuinely additional, exactly as the retention decision required.

Behaviour

A thread is one workspace for everyone in it. Outside a thread the channel-and-user pairing is private.
Writes land in the caller's own subtree; reads span the session. A bare path is still your own file, another member's is reached by the path a listing shows, and listing the root lists the session.
Session identity is hashed before it reaches a path, because the model reads these and no member identifier belongs there. A t-/c- prefix carries the retention rule, so the sweeper needs no separate state.

Retention, implemented rather than specified

Threads collect after 7 days quiet, channel pairings after 1 hour idle. The sweeper runs on a timer and once at startup, so a pod restarting after an outage does not hold expired files for a full interval.

A session's age is its newest file. Directories are skipped deliberately - removing a file moves its directory's timestamp, so counting them would let eviction keep a dead session alive forever. An empty session falls back to its own directory time so it still expires. A directory this scheme did not write is left alone.

The cap evicts oldest-first inside the overflowing session only and names what it removed in the tool result, so a vanished file is explainable. The per-requester ceiling still refuses, because no single file is the right one to evict when one member is simply using too much.

The number

SIRENS_ECHO_SESSION_BYTES = 1 MiB, derived: it must sit under the 4 MiB per-requester ceiling or it never binds first, and 4÷1 gives a member four concurrent full sessions. Nesting keeps the deploy's 32-requester arithmetic true. All four values are knobs.

A latent bug this uncovered

An unmounted scratchpad wrote into the working directory. filepath.EvalSymlinks("") returns "." with no error, so path confinement passed against an empty root. It was inert only because measuring the empty root errored first - and my accounting change stopped erroring there, which surfaced it as a real internal/community/tool-output/ directory appearing in the repo during a test run.

Now refused explicitly, and pinned by TestAnUnmountedScratchpadWritesNothing. Worth flagging because the property was accidental rather than designed, and nothing tested it.

Tests

Every decision is proved rather than described: a thread shared across two members, a channel pairing staying private, own-path precedence, eviction taking the oldest, eviction not reaching a neighbouring session, each timer firing on its own schedule, a foreign directory surviving, the kind surviving the disk round trip, and no identifier reaching a path.

Two of those caught real bugs while I wrote them - listing the root showed only your own files, and directory timestamps made every session look fresh.

Verification

just gate PASS on all six steps. go test -race ./internal/community/ clean.

Not in this PR

The existing per-requester ceiling test moved to exercising across sessions, because inside one session the smaller cap now evicts before 4 MiB is reached. That is the decided behaviour rather than a regression, but it is a behavioural change to a shipped bound and worth a look.

Refs #156. The proposal Kai approved, built whole: session identity, nested partitioning, both timers, the per-session cap, oldest-first eviction, and a sweeper that actually runs. ## The shape ``` <session>/<requester>/notes.txt ``` **Nested rather than renamed**, which is the part worth reviewing. The 4 MiB quota is measured over the partition directory (`partitionBytes()` walks `s.root`), so a flat rename **moves** that bound rather than adding one: it silently stops meaning per-requester and starts meaning per-session. A member with five live threads would then hold five partitions against a 128Mi `emptyDir` whose overflow **evicts the pod** rather than refusing a write. Nesting gives two things to measure, so the per-requester ceiling survives and the per-session cap is genuinely additional, exactly as the retention decision required. ## Behaviour * **A thread is one workspace for everyone in it.** Outside a thread the channel-and-user pairing is private. * **Writes** land in the caller's own subtree; **reads** span the session. A bare path is still your own file, another member's is reached by the path a listing shows, and listing the root lists the session. * **Session identity is hashed** before it reaches a path, because the model reads these and no member identifier belongs there. A `t-`/`c-` prefix carries the retention rule, so the sweeper needs no separate state. ## Retention, implemented rather than specified Threads collect after **7 days** quiet, channel pairings after **1 hour** idle. The sweeper runs on a timer and once at startup, so a pod restarting after an outage does not hold expired files for a full interval. A session's age is its **newest file**. Directories are skipped deliberately - removing a file moves its directory's timestamp, so counting them would let eviction keep a dead session alive forever. An empty session falls back to its own directory time so it still expires. A directory this scheme did not write is left alone. The cap **evicts oldest-first inside the overflowing session only** and names what it removed in the tool result, so a vanished file is explainable. The per-requester ceiling still **refuses**, because no single file is the right one to evict when one member is simply using too much. ## The number `SIRENS_ECHO_SESSION_BYTES` = 1 MiB, derived: it must sit under the 4 MiB per-requester ceiling or it never binds first, and 4÷1 gives a member four concurrent full sessions. Nesting keeps the deploy's 32-requester arithmetic true. All four values are knobs. ## A latent bug this uncovered **An unmounted scratchpad wrote into the working directory.** `filepath.EvalSymlinks("")` returns `"."` with no error, so path confinement passed against an empty root. It was inert only because *measuring* the empty root errored first - and my accounting change stopped erroring there, which surfaced it as a real `internal/community/tool-output/` directory appearing in the repo during a test run. Now refused explicitly, and pinned by `TestAnUnmountedScratchpadWritesNothing`. Worth flagging because the property was accidental rather than designed, and nothing tested it. ## Tests Every decision is proved rather than described: a thread shared across two members, a channel pairing staying private, own-path precedence, eviction taking the oldest, eviction not reaching a neighbouring session, each timer firing on its own schedule, a foreign directory surviving, the kind surviving the disk round trip, and no identifier reaching a path. Two of those caught real bugs while I wrote them - listing the root showed only your own files, and directory timestamps made every session look fresh. ## Verification `just gate` PASS on all six steps. `go test -race ./internal/community/` clean. ## Not in this PR The existing per-requester ceiling test moved to exercising **across** sessions, because inside one session the smaller cap now evicts before 4 MiB is reached. That is the decided behaviour rather than a regression, but it is a behavioural change to a shipped bound and worth a look.

coilyco-ops added 1 commit

2026-08-16 03:10:00 +00:00

feat(scratch): make a workspace a session, with both retention timers

ci / test (pull_request) Successful in 39s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / publish-observed (pull_request) Has been skipped

Details

ci / image-build (pull_request) Successful in 22s

Details

639973db7b

The scratchpad partitioned by requester, so a thread was several private
workspaces rather than the shared one Kai decided on. It now partitions
by session: the thread inside one, the channel-and-user pairing outside.

Nested rather than renamed, which is the part worth reviewing. The 4 MiB
quota is measured over the partition directory, so renaming the partition
would have moved that bound instead of adding one: it would silently stop
meaning per-requester and start meaning per-session, and a member with
five live threads would hold five partitions against a 128Mi emptyDir
whose overflow evicts the pod. Nesting gives two things to measure, so
the per-requester ceiling survives and the per-session cap is genuinely
additional.

Reads span the session and writes land in the caller's own subtree, so a
thread is shared while the quota stays attributable. A bare path is still
your own file; another member's is reached by the path a listing shows.

Retention is implemented rather than specified. Threads are collected
after 7 days quiet, channel pairings after 1 hour idle, and the sweeper
runs on a timer and once at startup. A session's age is its newest file:
counting directories would let eviction keep a dead session alive, since
removing a file moves its directory's timestamp.

The session cap evicts oldest-first inside the overflowing session only,
and names what it removed in the tool result. The per-requester ceiling
still refuses, because no single file is the right one to evict when a
member is simply using too much.

SIRENS_ECHO_SESSION_BYTES is 1 MiB, derived: it has to sit under the
4 MiB ceiling or it never binds first, and four full sessions per member
is plausible. All four values are knobs.

Found on the way: an unmounted scratchpad wrote into the working
directory, because EvalSymlinks("") resolves to "." and confinement
therefore passed. It was inert only because measuring the empty root
errored first, which my accounting change stopped doing. Now stated
rather than accidental, and pinned by a test that would have caught the
tool-output directory this wrote into the repo.

`just gate` PASS on all six steps. `go test -race` clean.

Refs #156

Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

coilyco-ops requested review from coilysiren

2026-08-16 03:10:00 +00:00