diff --git a/PERF.md b/PERF.md index 183ec93b1a..40195268f4 100644 --- a/PERF.md +++ b/PERF.md @@ -785,3 +785,49 @@ The macOS 60 s steady profile fell from 3.03% to 2.10% of a core. What remains is the factory's lease reads on runner requests (17%) and the chat presence beat (13%). The rig does not follow a live AgentField run, which the live round's idle did; the AgentField write above is that path's share. + +## Resident memory of the desktop engine + +`codeaf app` holds a conversation in memory only while something needs it. The +method and measurements are in [docs/perf/memory-2026-10-24.md](docs/perf/memory-2026-10-24.md). + +- **Members at rest are read, not held.** `tasks.list`, `asks.list` and + `processes.list` on a member of an open crew answer from its files when its + presence shows no working turn, question, task or process, no service is kept + and no stage is lent. `tasks.list` builds the owner-backed list by holding the + conversation for the length of one read (at most `appRestReads` = 2 at once), + then keeps it under a stamp of the files it was built from: in memory up to + `appRestMemoBytes` = 24 MiB (least recently read first; a list larger than + that is not kept), and as `/cache/app-tasks/.json` for the + next launch, valid for the same build and the same stamp only. Disk storage + keeps at most `appRestDiskTotalBytes` = 96 MiB and `appRestDiskEntries` = 256 + files, evicting oldest writes first. The inventory is scanned on first use and + every `appRestDiskScanEvery` = 1 minute by the idle sweep, which also prunes + missing or deleted agents; writes update the inventory without directory scans. + Agent deletion removes both entries immediately. Byte-identical settled task + checkpoints preserve their identity on open, so snapshot validation can admit + conversations with saved tasks. Owner-overlay failures propagate and are + never cached; reader completion is deferred and one-shot. Record, rest and + project-index admission share the two-second mtime/ctime settling guard. + `TestAppReadingAnIdleCrewMemberDoesNotKeepItsConversation` and + `TestAppReadsARestingMembersTasksFromTheCacheAfterARelaunch` pin the opens. +- **A crew member with nothing in flight is let go after `appAgentIdle`** like any + other conversation; the crew opens it again for traffic. +- **Journal records** (`RecordCache`) keep at most `recordCacheEntries` = 8 + records and drop one not read for `recordCacheIdle` = 30 s. The 256 MiB byte + budget and the 16 MiB per-file limit are unchanged. +- **The project task index** keeps the decoded rows of the last + `taskIndexKeptPaths` = 4 indexes of at most `taskIndexKeptBytes` = 4 MiB each. +- **Plan reads** (`plandb.Store`) reload the plan only when SQLite's + `data_version` or the database/log stamp moved. + `TestRepeatedReadsOfAnUnchangedPlanLoadItOnce` allows 3 MiB for 200 reads. + `TestAppTasksOpensThePlanOnceForAllTheRunRows` allows 16 MiB to list 150 rows. +- **The collector percent follows the live heap** (`memtrim.go`): the app starts + at Go's default percent 100. The governor selects 400 for a small heap, and a + larger one is given the percent that + lets about `heapHeadroom` = 32 MiB of garbage build up, never below + `heapGCMin` = 10. An explicit `GOGC` decides instead. When more than 48 MiB + and half of the live heap is held and unused, `debug.FreeOSMemory` runs, at most + once a minute. macOS keeps freed pages in a process's resident size until it + needs them (`MADV_FREE_REUSABLE`), so `ps` RSS shows the peak of a burst while + the footprint (`vmmap --summary`, Activity Monitor) shows what is held. diff --git a/cmd/codeaf/app.go b/cmd/codeaf/app.go index b15a68d2c4..de76b633b4 100644 --- a/cmd/codeaf/app.go +++ b/cmd/codeaf/app.go @@ -156,6 +156,7 @@ func runApp(args []string) error { defer leaving() defer leave.On(leaving, nil)() guard.Go("app/parent", func() { watchAppParent(ctx, os.Getppid, appParentPoll, leaving) }) + startHeapTrimmer(ctx) // THE APP'S KEYS LIVE IN THIS PROCESS'S MEMORY, installed before anything // can read a key, and no profile write may put one on disk while they are. defer config.UseAppCredentials(config.NewAppCredentials())() @@ -606,6 +607,14 @@ type appBackend struct { devices *appDevices stopSyncedSettings func() + // restMemo is the task lists of members at rest, so reading them does not + // hold their conversations (apprestmemo.go). + restMemo appRestMemo + restSlots chan struct{} + // waitReaders joins an explicit close's readers; nil waits on their signal. + // Tests replace the wait to observe the unlocked admission gate. + waitReaders func(<-chan struct{}) + // organizeProposer supplies a naming door when the backend is embedded without held conversations. organizeProposer session.TeamProposer @@ -842,8 +851,13 @@ type appAgent struct { // listening is when this light record began watching crew traffic. listening time.Time - // gate serializes opening and letting go of this one agent, so two sends - // racing for a closed conversation open it once. + // readers keeps the held conversation alive until the last disk/owner read + // returns. These fields are guarded by b.mu; pinning also holds gate. + readers int + readersDone chan struct{} + restRead bool + releasing bool + // gate serializes opening, work admission and release for this agent. gate sync.Mutex nameGate sync.Mutex // serializes a rename RPC and its view update view appwire.Agent @@ -1110,6 +1124,7 @@ func newAppBackend(emit appwire.Emitter, open appOpener) *appBackend { return func() { t.Stop() } }, agents: map[string]*appAgent{}, + restSlots: make(chan struct{}, appRestReads), closedDone: make(chan struct{}), sweepStop: make(chan struct{}), sweepDone: make(chan struct{}), @@ -3388,10 +3403,11 @@ func (b *appBackend) workEvent(id string, live *appLive, event session.Event) { // started, or is interrupted the moment it is ([appBackend.bootSettle]). No // stop waits on the gate an open holds. func (b *appBackend) Stop(id string) error { - a, err := b.findCrewOwner(id) + a, endRead, err := b.findCrewOwner(id) if err != nil { return err } + defer endRead() kept := false b.locked(func() { if a.starting { @@ -3434,6 +3450,7 @@ func (b *appBackend) CloseAgent(id string) error { func (b *appBackend) closeAgent(a *appAgent) (view appwire.Agent) { a.gate.Lock() defer a.gate.Unlock() + b.lockRelease(a) return b.closeAgentLocked(a) } @@ -3454,7 +3471,7 @@ func (b *appBackend) release(a *appAgent) { var id string b.locked(func() { id = a.view.ID - live, a.live, a.viewOpen = a.live, nil, false + live, a.live, a.viewOpen, a.restRead, a.releasing = a.live, nil, false, false, false if live != nil { a.lastLive = b.now() // Its conversation's open questions go with it, unanswered and @@ -3469,6 +3486,26 @@ func (b *appBackend) release(a *appAgent) { b.forgetAsksOf(id, live) } +func (b *appBackend) releaseIdle(a *appAgent) bool { + var live *appLive + var id string + b.locked(func() { + if a.readers > 0 || a.viewOpen || a.live == nil || a.live.running || a.live.compacting { + return + } + id = a.view.ID + live, a.live, a.viewOpen, a.restRead, a.crewHeld, a.releasing = a.live, nil, false, false, false, false + a.lastLive = b.now() + b.dropAsksLocked(a.view.ID, live.asks) + }) + if live == nil { + return false + } + b.finish(live) + b.forgetAsksOf(id, live) + return true +} + // finish gives a conversation that is no longer held back, and shuts its // outbox once the turn and the lanes have said their last. func (b *appBackend) finish(live *appLive) { @@ -3515,10 +3552,13 @@ func (b *appBackend) finish(live *appLive) { // ListAsks answers the agent's open asks. An agent with no conversation held // has none this process can answer. func (b *appBackend) ListAsks(id string) ([]appwire.Ask, error) { - a, err := b.findCrewOwner(id) + // A member at rest has no question a held conversation would add: one + // waiting on the person says so in its presence and is not at rest. + a, endRead, err := b.findCrewReader(id) if err != nil { return nil, err } + defer endRead() var asks []appwire.Ask b.locked(func() { if a.live != nil { @@ -3560,10 +3600,11 @@ func (b *appBackend) answerAskThrough(id, askID, option, text, scope string, sec if handled, err := b.answerCrewAsk(id, askID, option, text, ""); handled { return err } - a, err := b.findCrewOwner(id) + a, endRead, err := b.findCrewOwner(id) if err != nil { return err } + defer endRead() if handled, err := b.answerMoney(a, askID, option, text); handled { return err } @@ -4532,10 +4573,25 @@ func (b *appBackend) watchSpendRows() (stop func()) { // checkpoint does not name — and lays over them what a held conversation has // said since. func (b *appBackend) ListTasks(id string) ([]appwire.Task, error) { - a, err := b.findCrewOwner(id) + if a, err := b.find(id); err == nil && b.crewKeeps(id) && !b.isClosing() && !b.tidied(id) && b.memberRests(a) { + return b.restingTasks(a, id) + } + return b.listTasksOf(id) +} + +// listTasksOf is ListTasks for a conversation the crew keeps held when it is +// busy, and the files' answer otherwise. +func (b *appBackend) listTasksOf(id string) ([]appwire.Task, error) { + a, endRead, err := b.findCrewOwner(id) if err != nil { return nil, err } + defer endRead() + return b.listTasksHeld(a, id) +} + +// listTasksHeld reads disk and overlays the owner while the caller pins a. +func (b *appBackend) listTasksHeld(a *appAgent, id string) ([]appwire.Task, error) { var transcript string var fallback time.Time // A quick task is listed by AppQuickTasks, never as a task here (#1880). @@ -4627,7 +4683,11 @@ func (b *appBackend) ListTasks(id string) ([]appwire.Task, error) { add(appwire.Task{Result: result, ID: appIndexTaskID(id, row.ID), Agent: id, Parent: parent, Title: title, State: state, UpdatedAt: at, YourCall: state == appwire.TaskUnverified}) } } - for taskID, task := range b.heldTaskViews(a) { + owner, err := b.heldTaskViews(a) + if err != nil { + return nil, err + } + for taskID, task := range owner { if quickIDs[taskID] { continue } @@ -4854,6 +4914,7 @@ func (b *appBackend) Close() error { func (b *appBackend) letGo(a *appAgent) { a.gate.Lock() defer a.gate.Unlock() + b.lockRelease(a) b.release(a) } @@ -4863,6 +4924,7 @@ func (b *appBackend) letGoTimed(a *appAgent, w io.Writer) { began := time.Now() a.gate.Lock() defer a.gate.Unlock() + b.lockRelease(a) gated := time.Now() var id string b.locked(func() { id = a.view.ID }) @@ -4921,10 +4983,11 @@ func (b *appBackend) sweepOnce() { _, _ = b.SyncStatus() } now := b.now() + appRestDisk.maintain(now) var idle []*appAgent b.locked(func() { for _, a := range b.agents { - if live := a.live; live != nil && !a.viewOpen && !live.running && now.Sub(live.lastUse) >= appAgentIdle { + if live := a.live; live != nil && !a.viewOpen && !live.running && a.readers == 0 && now.Sub(live.lastUse) >= appAgentIdle { idle = append(idle, a) } } @@ -4945,13 +5008,15 @@ func (b *appBackend) sweepAgent(a *appAgent, now time.Time) { keep := true b.locked(func() { live = a.live - keep = live == nil || a.viewOpen || live.running || now.Sub(live.lastUse) < appAgentIdle + keep = live == nil || a.viewOpen || live.running || a.readers > 0 || now.Sub(live.lastUse) < appAgentIdle }) - // A member of an open crew stays held while the crew is open. - if !keep && !b.liveNeedsYou(live) && !b.crewKeeps(a.view.ID) && !b.serviceKeeps(a.view.ID) { + // A member of an open crew stays held while it has work in flight or a + // reason the crew would open it again at once; one with nothing to do goes + // like any other, and the crew opens it when traffic comes (applazy.go). + if !keep && appLiveWorkSettled(live) && !b.liveNeedsYou(live) && !b.serviceKeeps(a.view.ID) && (!b.crewKeeps(a.view.ID) || b.memberQuiet(a)) { // The state the app reads does not move: the conversation is as // idle on disk as it was in memory. - b.release(a) + b.releaseIdle(a) } } diff --git a/cmd/codeaf/appattention.go b/cmd/codeaf/appattention.go index 6f0069fe1a..24f41d328d 100644 --- a/cmd/codeaf/appattention.go +++ b/cmd/codeaf/appattention.go @@ -50,10 +50,11 @@ func (b *appBackend) ShowAsk(agent, id string) (*appwire.Ask, error) { } func (b *appBackend) clockAsk(agent, id string, start bool) (*appwire.Ask, error) { - a, err := b.findCrewOwner(agent) + a, endRead, err := b.findCrewOwner(agent) if err != nil { return nil, err } + defer endRead() live := b.liveOf(a) if live == nil { return nil, appwire.Errorf(appwire.CodeNotFound, "the ask %s is not open", id) @@ -160,10 +161,11 @@ func (b *appBackend) TaskSince(id, after string) (appwire.Since, error) { if !strings.HasPrefix(after, prefix) { return appwire.Since{}, nil } - a, live, err := b.heldLive(agent) + a, live, endLive, err := b.borrowLive(agent) if err != nil { return appwire.Since{}, err } + defer endLive() journal, ok := live.conn.Conv.(appTaskJournal) if !ok { return appwire.Since{}, nil diff --git a/cmd/codeaf/appbudget.go b/cmd/codeaf/appbudget.go index 8ea1717a19..2117cd228b 100644 --- a/cmd/codeaf/appbudget.go +++ b/cmd/codeaf/appbudget.go @@ -118,10 +118,11 @@ func (b *appBackend) stampAgentBudget(a *appAgent) appwire.Agent { // AgentBudget answers agents.budget.get. func (b *appBackend) AgentBudget(id string) (appwire.AgentBudget, error) { - a, err := b.findCrewOwner(id) + a, endRead, err := b.findCrewOwner(id) if err != nil { return appwire.AgentBudget{}, err } + defer endRead() return b.budgetOf(id, b.liveOf(a)) } @@ -132,10 +133,11 @@ func (b *appBackend) SetAgentBudget(set appwire.AgentBudgetSet) (appwire.AgentBu if set.USD != nil && (*set.USD <= 0 || math.IsNaN(*set.USD) || math.IsInf(*set.USD, 0)) { return appwire.AgentBudget{}, appwire.Errorf(appwire.CodeBadParams, "usd must be a positive amount, or null for none") } - a, err := b.findCrewOwner(set.ID) + a, endRead, err := b.findCrewOwner(set.ID) if err != nil { return appwire.AgentBudget{}, err } + defer endRead() if b.isClosing() { return appwire.AgentBudget{}, appwire.Errorf(appwire.CodeClosed, "codeaf is shutting down") } diff --git a/cmd/codeaf/appclose.go b/cmd/codeaf/appclose.go index d07cbacf2b..3bcd87d4eb 100644 --- a/cmd/codeaf/appclose.go +++ b/cmd/codeaf/appclose.go @@ -844,7 +844,7 @@ func appTaskWasInterrupted(p session.SessionPresence, fresh bool, id string, sta // heldTaskViews reads after the disk so a start arriving during the listing // wins. Locally owned tasks come from the engine, even ahead of its event lane. -func (b *appBackend) heldTaskViews(a *appAgent) map[string]appwire.Task { +func (b *appBackend) heldTaskViews(a *appAgent) (map[string]appwire.Task, error) { heard := map[string]appwire.Task{} var live *appLive var id string @@ -859,15 +859,17 @@ func (b *appBackend) heldTaskViews(a *appAgent) map[string]appwire.Task { // A lent stage lives in this process too, and may have parked a card before the // hold began, when no event could carry it here. if live != nil && (live.conn.Assembled || live.conn.Lent) { - if tasks, err := live.conn.Conv.AppTasks(); err == nil { - for _, view := range tasks { - task := appTaskOfView(id, view) - task.UpdatedAt = heard[task.ID].UpdatedAt - heard[task.ID] = task - } + tasks, err := live.conn.Conv.AppTasks() + if err != nil { + return nil, err + } + for _, view := range tasks { + task := appTaskOfView(id, view) + task.UpdatedAt = heard[task.ID].UpdatedAt + heard[task.ID] = task } } - return heard + return heard, nil } // Deleted journals keep their directory for deliverables. Only their lifetime diff --git a/cmd/codeaf/appcompaction.go b/cmd/codeaf/appcompaction.go index 09d659e932..7720167065 100644 --- a/cmd/codeaf/appcompaction.go +++ b/cmd/codeaf/appcompaction.go @@ -217,10 +217,11 @@ func appReadPasses(stop <-chan struct{}, passes <-chan session.Event, take func( // compaction.ended; the pass's own events reach the page through the // connection's pass lane ([appBackend.watchPasses]), ahead of that answer. func (b *appBackend) CompactAgent(id string) error { - a, live, err := b.heldLive(id) + a, live, endLive, err := b.borrowLive(id) if err != nil { return err } + defer endLive() conv, ok := live.conn.Conv.(appCompactConv) if !ok { return appwire.Errorf(appwire.CodeUnknownMethod, "this conversation cannot be compacted from here") @@ -240,7 +241,9 @@ func (b *appBackend) CompactAgent(id string) error { b.locked(func() { live.compacting = false }) return appwire.Errorf(appwire.CodeBusy, "too many compactions are running; ask again in a moment") } + endWorker := b.pinReaderGated(a) guard.Go("app/compact", func() { + defer endWorker() defer func() { <-appCompactSlots b.locked(func() { live.compacting = false }) diff --git a/cmd/codeaf/appconversation.go b/cmd/codeaf/appconversation.go index b748140908..b1cfc015eb 100644 --- a/cmd/codeaf/appconversation.go +++ b/cmd/codeaf/appconversation.go @@ -16,10 +16,11 @@ type appRewindConversation interface { } func (b *appBackend) ConversationPoints(id string) ([]appwire.ConversationPoint, error) { - _, live, err := b.heldLive(id) + _, live, endLive, err := b.borrowLive(id) if err != nil { return nil, err } + defer endLive() c, ok := live.conn.Conv.(appRewindConversation) if !ok { return nil, appwire.Errorf(appwire.CodeUnknownMethod, "this engine cannot rewind conversations") @@ -50,10 +51,11 @@ func (b *appBackend) ConversationPoints(id string) ([]appwire.ConversationPoint, } func (b *appBackend) RewindConversation(id string, point int) (string, error) { - a, live, err := b.heldLive(id) + a, live, endLive, err := b.borrowLive(id) if err != nil { return "", err } + defer endLive() a.gate.Lock() defer a.gate.Unlock() c, ok := live.conn.Conv.(appRewindConversation) @@ -68,10 +70,11 @@ func (b *appBackend) RewindConversation(id string, point int) (string, error) { } func (b *appBackend) UndoConversationRewind(id, token string) error { - a, live, err := b.heldLive(id) + a, live, endLive, err := b.borrowLive(id) if err != nil { return err } + defer endLive() a.gate.Lock() defer a.gate.Unlock() c, ok := live.conn.Conv.(appRewindConversation) @@ -97,10 +100,11 @@ func (b *appBackend) resetConversationCount(a *appAgent, entries []session.Displ } func (b *appBackend) DebugAgent(id string) (appwire.ConversationDebug, error) { - _, live, err := b.heldLive(id) + _, live, endLive, err := b.borrowLive(id) if err != nil { return appwire.ConversationDebug{}, err } + defer endLive() c, ok := live.conn.Conv.(interface { DebugConversation() (string, string, error) }) @@ -117,12 +121,18 @@ func (b *appBackend) ManualAgent(id, question, clientID string) (uint64, error) } func (b *appBackend) DeleteAgent(id string) error { - a, live, err := b.heldLive(id) + a, err := b.find(id) if err != nil { return err } a.gate.Lock() defer a.gate.Unlock() + b.lockRelease(a) + defer b.locked(func() { a.releasing = false }) + _, live, err := b.heldLiveLocked(a) + if err != nil { + return err + } c, ok := live.conn.Conv.(interface{ DeleteOwnConversation() error }) if !ok { return appwire.Errorf(appwire.CodeUnknownMethod, "this engine does not offer permanent deletion") @@ -138,5 +148,7 @@ func (b *appBackend) DeleteAgent(id string) error { } b.release(a) b.locked(func() { delete(b.agents, id) }) + b.restMemo.forget(id) + appRestDisk.forget(id) return nil } diff --git a/cmd/codeaf/appconvo.go b/cmd/codeaf/appconvo.go index efa388749b..4e37588c62 100644 --- a/cmd/codeaf/appconvo.go +++ b/cmd/codeaf/appconvo.go @@ -155,10 +155,11 @@ func (b *appBackend) steer(agent, text, clientID string, shown *appSending) erro return appEngineError(err) } b.resetCloseResumes(agent) - a, err := b.findCrewOwner(agent) + a, endRead, err := b.findCrewOwner(agent) if err != nil { return err } + defer endRead() if handled, err := b.agentFieldSteer(a, text); handled { return err } @@ -342,10 +343,11 @@ func (b *appBackend) queue(agent, text, clientID string, shown *appSending) ([]a if err := (session.Submission{ClientID: clientID}).Check(); err != nil { return nil, appEngineError(err) } - a, err := b.findCrewOwner(agent) + a, endRead, err := b.findCrewOwner(agent) if err != nil { return nil, err } + defer endRead() // An agent still starting has its first turn on its way: a message // queued now waits for that turn's end, like any other. It is queued // before the gate, which the open holds for as long as it takes; the @@ -471,11 +473,12 @@ func (b *appBackend) EditQueuedMessage(agent, id string, message appwire.AgentMe if err != nil { return nil, err } - a, err := b.find(agent) + _, live, endRead, err := b.borrowExisting(agent) if err != nil { return nil, err } - if live := b.liveOf(a); live != nil && !appKeepsQueuedMarks(live.conn.Conv) { + defer endRead() + if live != nil && !appKeepsQueuedMarks(live.conn.Conv) { return nil, appwire.Errorf(appwire.CodeUnknownMethod, "this conversation cannot keep a queued message's said or reads") } return b.changeQueue(agent, id, func(a *appAgent, at int) { @@ -697,10 +700,11 @@ func (b *appBackend) AnswerAskWith(agent, id string, answer appwire.AskAnswer) e if handled, err := b.answerCrewAsk(agent, id, answer.Option, answer.Text, answer.Why); handled { return err } - a, err := b.findCrewOwner(agent) + a, endRead, err := b.findCrewOwner(agent) if err != nil { return err } + defer endRead() live := b.liveOf(a) if live == nil { return b.AnswerAsk(agent, id, answer.Option, answer.Text) @@ -798,10 +802,11 @@ const appDecideMark = "decide:" // HoldAsk stops an ask's clock through its owning lane. Proposed tasks keep // their task action; the model's questions use the engine's question hold. func (b *appBackend) HoldAsk(agent, id string) error { - a, err := b.findCrewOwner(agent) + a, endRead, err := b.findCrewOwner(agent) if err != nil { return err } + defer endRead() live := b.liveOf(a) if live == nil { return appwire.Errorf(appwire.CodeNotFound, "the ask %s is not open", id) @@ -1849,10 +1854,11 @@ func (b *appBackend) TaskEntries(id, before string, limit int) ([]appwire.Entry, if !ok { return nil, false, appwire.Errorf(appwire.CodeBadParams, "%s is not a task id", id) } - a, live, err := b.heldLive(agent) + a, live, endLive, err := b.borrowLive(agent) if err != nil { return nil, false, err } + defer endLive() n, err := strconv.ParseUint(local, 10, 64) if err != nil { return nil, false, appwire.Errorf(appwire.CodeBadParams, "%s is not a task id", id) diff --git a/cmd/codeaf/appcrewmembers.go b/cmd/codeaf/appcrewmembers.go index 711b9d75e2..211eef281a 100644 --- a/cmd/codeaf/appcrewmembers.go +++ b/cmd/codeaf/appcrewmembers.go @@ -171,8 +171,10 @@ func (b *appBackend) memberStart(t teamstore.Team, ask appwire.MemberAsk) (strin if err != nil { return "", err } - if _, _, err := b.heldLive(agent.ID); err != nil { + if _, _, endLive, err := b.borrowLive(agent.ID); err != nil { return "", err + } else { + defer endLive() } m, err := b.agentMember(agent.ID) if err != nil { @@ -549,7 +551,9 @@ func (b *appBackend) CrewPropose(agent string) error { if !first { return nil } + endWorker := b.pinReader(a) guard.Go("app/crew-propose", func() { + defer endWorker() // The press is free again the moment the model has answered, before // the card is drawn, so a press made after the card is a new ask; the // deferred call is for a road that never reaches that moment. diff --git a/cmd/codeaf/appcreworganize.go b/cmd/codeaf/appcreworganize.go index a6ec3a5e41..07fd2770e0 100644 --- a/cmd/codeaf/appcreworganize.go +++ b/cmd/codeaf/appcreworganize.go @@ -42,10 +42,12 @@ func (b *appBackend) CrewsOrganize(ctx context.Context, ask appwire.OrganizeAsk) if len(loose) == 0 { return out, ctx.Err() } - proposer, err = b.organizeNamingDoor(loose[0]) + var endRead func() + proposer, endRead, err = b.organizeNamingDoor(loose[0]) if err != nil { return nil, appOrganizeError(err) } + defer endRead() } rows, res, err := session.OrganizeModel(ctx, proposer, convs, teams) if err != nil { @@ -117,46 +119,37 @@ func (b *appBackend) organizeFolders(ctx context.Context, out *appwire.OrganizeR // organizeNamingDoor discovers ownership before interpreting the local profile. // Joining an existing engine never boots a conversation merely to name teams. -func (b *appBackend) organizeNamingDoor(c session.OrganizeConversation) (session.TeamProposer, error) { - a, err := b.find(c.Key) +func (b *appBackend) organizeNamingDoor(c session.OrganizeConversation) (session.TeamProposer, func(), error) { + a, live, endRead, err := b.borrowExisting(c.Key) if err != nil { - return nil, err + return nil, nil, err } - if live := b.liveOf(a); live != nil { + if live != nil { proposer, _ := live.conn.Conv.(session.TeamProposer) - return proposer, nil + return proposer, endRead, nil } + endRead() conn, err := appJoinExisting(appOpenAsk{Workspace: c.Folder, Transcript: a.transcript}, nil) if err != nil { - return nil, err + return nil, nil, err } if conn != nil { - return &appOrganizeJoined{conn: conn}, nil + proposer, _ := conn.Conv.(session.TeamProposer) + return proposer, conn.Release, nil } source, err := v3RolesSource(c.Folder, b.crewProfile()) if err != nil { - return nil, err + return nil, nil, err } resolved, err := roles.Resolve(source, roles.RoleTitle, "") if err != nil { - return nil, session.ErrNoNamingModel + return nil, nil, session.ErrNoNamingModel } proc, err := openAppEngineProcess() if err != nil { - return nil, err + return nil, nil, err } settings := proc.Settings - return session.NewTeamProposer(session.TeamProposerConfig{Model: resolved, APIKey: settings.APIKey, BaseURL: settings.BaseURL, Sources: settings.Sources, ProfileDir: settings.ProfileDir, RolesSource: source}) -} - -// appOrganizeJoined releases the temporary view when its single ask completes. -type appOrganizeJoined struct{ conn *appConn } - -func (p *appOrganizeJoined) ProposeTeams(ctx context.Context, in session.TeamProposalInput) (session.TeamProposal, error) { - defer p.conn.Release() - proposer, ok := p.conn.Conv.(session.TeamProposer) - if !ok { - return session.TeamProposal{}, session.ErrNoNamingModel - } - return proposer.ProposeTeams(ctx, in) + proposer, err := session.NewTeamProposer(session.TeamProposerConfig{Model: resolved, APIKey: settings.APIKey, BaseURL: settings.BaseURL, Sources: settings.Sources, ProfileDir: settings.ProfileDir, RolesSource: source}) + return proposer, func() {}, err } diff --git a/cmd/codeaf/appcrewprocesspage.go b/cmd/codeaf/appcrewprocesspage.go index c1ead30630..5b0c8b5fde 100644 --- a/cmd/codeaf/appcrewprocesspage.go +++ b/cmd/codeaf/appcrewprocesspage.go @@ -889,9 +889,11 @@ func (b *appBackend) crewOwnerPage(ctx context.Context, o crewOwner, n int) (*ap }) // A member the person closed is read as saved: opening it would take it // out of Closed. - if a != nil && b.crewKeeps(o.id) { - if _, _, err := b.keptLive(o.id); err != nil && !errors.Is(err, errAppKeptClosed) { + if a != nil { + if _, endLive, err := b.findCrewOwner(o.id); err != nil { return nil, err + } else { + defer endLive() } } var live *appLive diff --git a/cmd/codeaf/appcrews.go b/cmd/codeaf/appcrews.go index f95de0c0bf..be0bf0a179 100644 --- a/cmd/codeaf/appcrews.go +++ b/cmd/codeaf/appcrews.go @@ -893,19 +893,21 @@ func (b *appBackend) isHeld(agent string) bool { // crewMemberOpen holds a started member's conversation and gives it the // approval posture its start named, or else its manager's. func (b *appBackend) crewMemberOpen(manager teamstore.Member, handle, agent, posture string) { - a, live, err := b.heldLive(agent) + a, live, endLive, err := b.borrowLive(agent) if err != nil { if !b.isClosing() { b.crewNote(manager, "could not start @"+handle+": "+err.Error()) } return } + defer endLive() // A manager the person closed since is not opened to read its posture: // its saved word is read instead. if posture == "" { - _, mlive, err := b.keptLive(appMemberAgent(manager)) + _, mlive, endManager, err := b.borrowKeptLive(appMemberAgent(manager)) switch { case err == nil: + defer endManager() if door, ok := mlive.conn.Conv.(appPostureDoor); ok { posture = door.ResolvedApprovalPosture() } diff --git a/cmd/codeaf/appinbox.go b/cmd/codeaf/appinbox.go index c6bff1f302..7b123a7a2b 100644 --- a/cmd/codeaf/appinbox.go +++ b/cmd/codeaf/appinbox.go @@ -423,10 +423,11 @@ func (b *appBackend) AnswerInbox(id, option, text, why, scope string) (time.Time if err != nil { return time.Time{}, err } - a, err := b.findCrewOwner(agent) + a, endRead, err := b.findCrewOwner(agent) if err != nil { return time.Time{}, err } + defer endRead() var refused error b.locked(func() { if a.live == nil { @@ -552,10 +553,11 @@ func (b *appBackend) LaterInbox(id string, later bool) error { b.emit.Emit(appwire.EventAskOpened, appwire.AskOpened{Ask: ask}) return nil } - a, err := b.findCrewOwner(agent) + a, endRead, err := b.findCrewOwner(agent) if err != nil { return err } + defer endRead() live := b.liveOf(a) if live == nil { return appwire.Errorf(appwire.CodeNotFound, "the Inbox item %s is no longer waiting", id) diff --git a/cmd/codeaf/applazy.go b/cmd/codeaf/applazy.go index e8b48b3455..b5f06d2f7d 100644 --- a/cmd/codeaf/applazy.go +++ b/cmd/codeaf/applazy.go @@ -1,11 +1,10 @@ package main import ( - "errors" + "fmt" "os" "path/filepath" "sort" - "strconv" "strings" "time" @@ -112,7 +111,7 @@ func (b *appBackend) needsLazyOpen(run *appCrewRun, read appCrewRead, row sessio if recovered && !(presence.Fresh(b.now()) && session.InUse(row.Transcript)) { return false } - busy := presence.State == session.PresenceWorking || presence.NeedsPerson() || len(presence.RunningTasks) > 0 || len(presence.Jobs) > 0 + busy := memberBusy(presence) return busy || b.serviceKeeps(row.ID) || session.NeedsTaskRecovery(row.Transcript) || session.NeedsProcessRecovery(row.Transcript) } @@ -233,7 +232,18 @@ func lazyStamp(path string) string { if err != nil { return "-" } - return path + ":" + strconv.FormatInt(info.Size(), 10) + ":" + strconv.FormatInt(info.ModTime().UnixNano(), 10) + if stamp := appInfoStamp(path, info); stamp != "" { + return stamp + } + return fmt.Sprintf("%s:%d:%d;", path, info.Size(), info.ModTime().UnixNano()) +} + +func appInfoStamp(path string, info os.FileInfo) string { + dev, ino, ctime, ok := appFileIdentity(info) + if !ok { + return "" + } + return fmt.Sprintf("%s:%d:%d:%d:%d:%d;", path, dev, ino, info.Size(), info.ModTime().UnixNano(), ctime) } func crewTrafficEvent(event fsnotify.Event) bool { @@ -272,22 +282,43 @@ func (b *appBackend) rememberLazyMember(id string) { // findCrewOwner restores the owner-backed answer an open crew had before its // idle members became light records. Other closed conversations stay closed. -func (b *appBackend) findCrewOwner(id string) (*appAgent, error) { - a, err := b.find(id) - if err != nil { - return nil, err +func (b *appBackend) findCrewOwner(id string) (*appAgent, func(), error) { + return b.readAgent(id, true) +} + +// findCrewReader pins an existing owner and opens only a busy crew member. +func (b *appBackend) findCrewReader(id string) (*appAgent, func(), error) { + return b.readAgent(id, false) +} + +// memberRests is true for a member whose conversation is not held and has +// nothing a held conversation would answer differently from its files: no +// working turn, question, task or process in its presence, no kept service, no +// lent stage. The crew beat opens it for new traffic; a person's view or send +// opens it through [appBackend.ensure]. +func (b *appBackend) memberRests(a *appAgent) bool { + var held bool + b.locked(func() { held = a.live != nil || a.opening }) + return !held && b.memberQuiet(a) +} + +// memberQuiet is true for a member, held or not, whose files and services show +// nothing in flight, so letting its conversation go loses nothing the crew +// would not open it again for. +func (b *appBackend) memberQuiet(a *appAgent) bool { + var id, transcript string + b.locked(func() { id, transcript = a.view.ID, a.transcript }) + if transcript == "" { + return false } - // A conversation already held, or being opened, is not opened again: the - // open holds the agent's gate for as long as it takes, and a caller that - // only reads or stops answers from what is there instead of queueing. A - // stop-all alone waits, within its bound ([appBackend.liveOnceOpened]). - // At quit nothing is opened again either, and the gates are the wrap-up's. - // A member the person closed is answered from what is there too: opening - // it would take it out of Closed. - if b.crewKeeps(id) && !b.held(id) && !b.isClosing() { - if _, _, err = b.keptLive(id); errors.Is(err, errAppKeptClosed) { - err = nil - } + if _, staged := liveStageFor(transcript); staged || b.serviceKeeps(id) { + return false } - return a, err + presence, _ := session.ReadSessionPresenceAnyAge(session.DirOf(transcript)) + return !memberBusy(presence) +} + +// memberBusy is what a presence record says about work in flight. +func memberBusy(presence session.SessionPresence) bool { + return presence.State == session.PresenceWorking || presence.NeedsPerson() || len(presence.RunningTasks) > 0 || len(presence.Jobs) > 0 } diff --git a/cmd/codeaf/applazy_read_test.go b/cmd/codeaf/applazy_read_test.go new file mode 100644 index 0000000000..df7d4af953 --- /dev/null +++ b/cmd/codeaf/applazy_read_test.go @@ -0,0 +1,201 @@ +package main + +import ( + "github.com/Agent-Field/codeaf/internal/home" + "os" + "path/filepath" + "sync" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/appwire" +) + +// crewHeldIdleMember is an agent whose conversation was let go while an open +// crew still lists it, which is what every idle member is after a launch. +func crewHeldIdleMember(t *testing.T) (*appBackend, *appAgent, *int) { + t.Helper() + // The remembered lists go under the cache root, which is the test's own. + t.Setenv(home.EnvVar, t.TempDir()) + conv := &fakeConversation{} + var mu sync.Mutex + var opens, releases int + open := fakeOpener(conv, &opens, &releases, &mu) + backend := newFakeBackend(newAppRecorder(), func(ask appOpenAsk) (*appConn, error) { + conn, err := open(ask) + if conn != nil && ask.Transcript != "" { + conn.Transcript = ask.Transcript + } + return conn, err + }) + backend.now = func() time.Time { return time.Now().Add(time.Hour) } + t.Cleanup(func() { + backend.locked(func() { backend.crewRun = nil }) + backend.Close() + }) + agent, err := mustCreate(t, backend, appwire.CreateAsk{Workspace: t.TempDir()}) + if err != nil { + t.Fatal(err) + } + a, err := backend.find(agent.ID) + if err != nil { + t.Fatal(err) + } + a.gate.Lock() + backend.release(a) + a.gate.Unlock() + backend.locked(func() { + backend.crewRun = &appCrewRun{held: map[string][]string{agent.ID: {"crew"}}} + }) + return backend, a, &opens +} + +// READING AN IDLE CREW MEMBER DOES NOT KEEP ITS CONVERSATION. Listing tasks +// and asks answers for a member at rest without leaving its journal replayed in +// memory, which for a crew of hundreds of members was the bulk of the engine's +// resident set. The task list is the owner-backed one, built once and +// remembered until a file it was built from changes. +func TestAppReadingAnIdleCrewMemberDoesNotKeepItsConversation(t *testing.T) { + backend, a, opens := crewHeldIdleMember(t) + dir := t.TempDir() + var id string + // Every open keeps the requested transcript, as a real conversation does. + place := func() { + backend.locked(func() { a.transcript, id = filepath.Join(dir, "transcript.jsonl"), a.view.ID }) + } + place() + before := *opens + for i := 0; i < 3; i++ { + place() + if _, err := backend.ListTasks(id); err != nil { + t.Fatal(err) + } + if _, err := backend.ListAsks(id); err != nil { + t.Fatal(err) + } + if backend.liveOf(a) != nil { + t.Fatal("the read left a conversation held") + } + } + if got := *opens - before; got != 1 { + t.Fatalf("three reads opened the conversation %d times, want the one that built the list", got) + } + if err := os.WriteFile(filepath.Join(dir, "tasks.json"), []byte(`{}`), 0o600); err != nil { + t.Fatal(err) + } + place() + if _, err := backend.ListTasks(id); err != nil { + t.Fatal(err) + } + if got := *opens - before; got != 2 { + t.Fatalf("a changed task file was read with %d opens in all, want the list rebuilt once more", got) + } +} + +// A MEMBER WITH WORK IN FLIGHT IS STILL OPENED BY A READ: its presence says a +// turn is running, so the held conversation is the only complete answer. +func TestAppReadingABusyCrewMemberOpensItsConversation(t *testing.T) { + backend, a, opens := crewHeldIdleMember(t) + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "presence.json"), []byte(`{"schema":1,"state":"working"}`), 0o600); err != nil { + t.Fatal(err) + } + var id string + backend.locked(func() { a.transcript, id = filepath.Join(dir, "transcript.jsonl"), a.view.ID }) + before := *opens + if !backend.crewKeeps(id) || backend.memberRests(a) { + t.Fatal("a member with a working presence was taken for resting") + } + _, _ = backend.ListTasks(id) + if *opens-before != 1 { + t.Fatalf("a busy member was opened %d times by a read, want once", *opens-before) + } +} + +// THE LIST A MEMBER AT REST WAS BUILT WITH SURVIVES A RELAUNCH. Building it +// replays the member's whole journal, which for a long conversation is the +// larger part of the engine's memory for the moment it takes; the answer is +// kept beside the other caches under the same stamp, so a launch that finds the +// files unchanged reads a small file instead. +func TestAppReadsARestingMembersTasksFromTheCacheAfterARelaunch(t *testing.T) { + backend, a, opens := crewHeldIdleMember(t) + dir := t.TempDir() + var id string + place := func() { + backend.locked(func() { a.transcript, id = filepath.Join(dir, "transcript.jsonl"), a.view.ID }) + } + place() + before := *opens + if _, err := backend.ListTasks(id); err != nil { + t.Fatal(err) + } + if got := *opens - before; got != 1 { + t.Fatalf("the first list opened %d times, want 1", got) + } + + // A relaunch keeps the files and loses the engine's memory. + backend.restMemo = appRestMemo{} + place() + if _, err := backend.ListTasks(id); err != nil { + t.Fatal(err) + } + if got := *opens - before; got != 1 { + t.Fatalf("after a relaunch the list opened the conversation again (%d opens in all)", got) + } + + // A file the list was built from changing makes it stale on disk too. + if err := os.WriteFile(filepath.Join(dir, "tasks.json"), []byte(`{}`), 0o600); err != nil { + t.Fatal(err) + } + backend.restMemo = appRestMemo{} + place() + if _, err := backend.ListTasks(id); err != nil { + t.Fatal(err) + } + if got := *opens - before; got != 2 { + t.Fatalf("a changed task file was answered from the cache (%d opens in all)", got) + } +} + +// A CREW MEMBER WITH NOTHING TO DO IS LET GO LIKE ANY OTHER once it has idled. +// It used to stay held for as long as its crew was open, so a person who had +// looked at a hundred members in a day kept a hundred journals in memory until +// the app quit. The crew opens a member again when traffic comes for it. +func TestAppLetsAnIdleCrewMemberGoAfterTheIdleTime(t *testing.T) { + t.Setenv(home.EnvVar, t.TempDir()) + conv := &fakeConversation{} + var mu sync.Mutex + var opens, releases int + backend := newFakeBackend(newAppRecorder(), fakeOpener(conv, &opens, &releases, &mu)) + t.Cleanup(func() { + backend.locked(func() { backend.crewRun = nil }) + backend.Close() + }) + clock := time.Now() + backend.now = func() time.Time { return clock } + agent, err := mustCreate(t, backend, appwire.CreateAsk{Workspace: t.TempDir()}) + if err != nil { + t.Fatal(err) + } + backend.locked(func() { + backend.crewRun = &appCrewRun{held: map[string][]string{agent.ID: {"crew"}}} + backend.agents[agent.ID].crewHeld = true + }) + released := func() int { mu.Lock(); defer mu.Unlock(); return releases } + + clock = clock.Add(appAgentIdle - time.Second) + backend.sweepOnce() + if released() != 0 { + t.Fatal("a member was let go before the idle time") + } + clock = clock.Add(2 * time.Second) + backend.sweepOnce() + if released() != 1 { + t.Fatalf("an idle crew member was released %d times, want once", released()) + } + var crewHeld bool + backend.locked(func() { crewHeld = backend.agents[agent.ID].crewHeld }) + if crewHeld { + t.Fatal("the crew still counted the member as held, so it would be opened again at once") + } +} diff --git a/cmd/codeaf/appmachinepolicy.go b/cmd/codeaf/appmachinepolicy.go index 041e7564c2..76e85aea17 100644 --- a/cmd/codeaf/appmachinepolicy.go +++ b/cmd/codeaf/appmachinepolicy.go @@ -32,16 +32,17 @@ type appAutonomyDoor interface { // autonomyDoor resolves the conversation's own handle so host edits reach the // same project and safety floors as typing the terminal command there. -func (b *appBackend) autonomyDoor(agent string) (appAutonomyDoor, error) { - _, live, err := b.heldLive(agent) +func (b *appBackend) autonomyDoor(agent string) (appAutonomyDoor, func(), error) { + _, live, endLive, err := b.borrowLive(agent) if err != nil { - return nil, err + return nil, nil, err } door, ok := live.conn.Conv.(appAutonomyDoor) if !ok { - return nil, appwire.Errorf(appwire.CodeUnknownMethod, "this conversation keeps no question rules") + endLive() + return nil, nil, appwire.Errorf(appwire.CodeUnknownMethod, "this conversation keeps no question rules") } - return door, nil + return door, endLive, nil } func appAutonomyRows(agent string, door appAutonomyDoor) (appwire.AutonomyResult, error) { rules := door.Autonomy() @@ -65,10 +66,11 @@ func appAutonomyRows(agent string, door appAutonomyDoor) (appwire.AutonomyResult return out, nil } func (b *appBackend) GetAutonomy(agent string) (appwire.AutonomyResult, error) { - door, err := b.autonomyDoor(agent) + door, endLive, err := b.autonomyDoor(agent) if err != nil { return appwire.AutonomyResult{}, err } + defer endLive() return appAutonomyRows(agent, door) } func (b *appBackend) SetAutonomy(p appwire.AutonomySetParams) (appwire.AutonomyResult, error) { @@ -76,10 +78,11 @@ func (b *appBackend) SetAutonomy(p appwire.AutonomySetParams) (appwire.AutonomyR if refused != "" { return appwire.AutonomyResult{}, appwire.Errorf(appwire.CodeBadParams, "%s", refused) } - door, err := b.autonomyDoor(p.Agent) + door, endLive, err := b.autonomyDoor(p.Agent) if err != nil { return appwire.AutonomyResult{}, err } + defer endLive() if err := door.SetAutonomy(kind, policy); err != nil { return appwire.AutonomyResult{}, appwire.Errorf(appwire.CodeBadParams, "%s", err) } diff --git a/cmd/codeaf/appmemoryapplies.go b/cmd/codeaf/appmemoryapplies.go index 97bb470de6..9cffbe041d 100644 --- a/cmd/codeaf/appmemoryapplies.go +++ b/cmd/codeaf/appmemoryapplies.go @@ -12,10 +12,11 @@ func (b *appBackend) ApplicableMemory(ctx context.Context, id string) (session.M if err := ctx.Err(); err != nil { return session.MemoryScopeSnapshot{}, err } - a, err := b.findCrewOwner(id) + a, endRead, err := b.findCrewOwner(id) if err != nil { return session.MemoryScopeSnapshot{}, err } + defer endRead() live := b.liveOf(a) if live == nil { return session.MemoryScopeSnapshot{}, appwire.Errorf(appwire.CodeEngine, "live memory scope is unknown until this conversation is open") diff --git a/cmd/codeaf/appmoney.go b/cmd/codeaf/appmoney.go index 2fa86e5f30..88652b2206 100644 --- a/cmd/codeaf/appmoney.go +++ b/cmd/codeaf/appmoney.go @@ -403,10 +403,11 @@ func (b *appBackend) RaiseCap(raise appwire.CapRaise) error { now := b.now() switch raise.Scope { case appwire.CapAgent: - a, err := b.findCrewOwner(raise.ID) + a, endRead, err := b.findCrewOwner(raise.ID) if err != nil { return err } + defer endRead() if err := b.lentFence(a, "budget"); err != nil { return err } diff --git a/cmd/codeaf/appprocessactions.go b/cmd/codeaf/appprocessactions.go index 2289301e26..c19cfbb99f 100644 --- a/cmd/codeaf/appprocessactions.go +++ b/cmd/codeaf/appprocessactions.go @@ -87,10 +87,11 @@ func (b *appBackend) ProcessActions(ctx context.Context, id string) (*appwire.Pr if err != nil { return nil, err } - a, err := b.findCrewOwner(agent) + a, endRead, err := b.findCrewOwner(agent) if err != nil { return nil, err } + defer endRead() res := &appwire.ProcessActionsResult{ID: id, Kind: appwire.ProcessKindJob} var record session.JobNotice unknown := "" diff --git a/cmd/codeaf/appprocesspage.go b/cmd/codeaf/appprocesspage.go index b083988f41..5aabe23fd0 100644 --- a/cmd/codeaf/appprocesspage.go +++ b/cmd/codeaf/appprocesspage.go @@ -66,10 +66,11 @@ func (b *appBackend) PageProcesses(ctx context.Context, ask appwire.ProcessPageA if err != nil { return nil, err } - a, err := b.findCrewOwner(ask.Agent) + a, endRead, err := b.findCrewOwner(ask.Agent) if err != nil { return nil, err } + defer endRead() var transcript string b.locked(func() { transcript = a.transcript }) convDir := "" @@ -463,10 +464,11 @@ func (b *appBackend) processRecord(ctx context.Context, id string, legacyList bo if !ok { return session.JobNotice{}, "", appwire.Errorf(appwire.CodeBadParams, "%s is not a process id", id) } - a, err := b.findCrewOwner(agent) + a, endRead, err := b.findCrewOwner(agent) if err != nil { return session.JobNotice{}, "", err } + defer endRead() var transcript string b.locked(func() { transcript = a.transcript }) convDir := "" diff --git a/cmd/codeaf/appprogramreport.go b/cmd/codeaf/appprogramreport.go index 527ee71381..5f08357d5a 100644 --- a/cmd/codeaf/appprogramreport.go +++ b/cmd/codeaf/appprogramreport.go @@ -43,10 +43,11 @@ func (b *appBackend) ProgramReport(id, launch, name string) (appwire.ProgramRepo return appwire.ProgramReportResult{}, err } defer release() - _, live, err := b.heldLive(agent) + _, live, endLive, err := b.borrowLive(agent) if err != nil { return appwire.ProgramReportResult{}, err } + defer endLive() reader, ok := live.conn.Conv.(appProgramReportReader) if !ok { return appwire.ProgramReportResult{}, appwire.Errorf(appwire.CodeUnknownMethod, "this engine does not serve tasks.program_report") diff --git a/cmd/codeaf/appquit.go b/cmd/codeaf/appquit.go index 1de3386396..7250712c09 100644 --- a/cmd/codeaf/appquit.go +++ b/cmd/codeaf/appquit.go @@ -141,7 +141,16 @@ func (b *appBackend) resumeOneWith(id string, reply bool, tasks []string, kind, if b.isClosing() { return } - a, live, err := b.keptLive(id) + a, err := b.find(id) + if err != nil { + return + } + a.gate.Lock() + defer a.gate.Unlock() + if b.tidied(id) { + return + } + _, live, err := b.heldLiveLocked(a) if err != nil { return } diff --git a/cmd/codeaf/appread.go b/cmd/codeaf/appread.go new file mode 100644 index 0000000000..6c8eb2a4f4 --- /dev/null +++ b/cmd/codeaf/appread.go @@ -0,0 +1,184 @@ +package main + +import "sync" + +// readAgent pins a conversation for the whole caller's read, including its +// disk reads and the final owner overlay. An already-held conversation is +// pinned under b.mu without waiting on the owner gate; opening and pinning a +// busy crew member share admission's gate so release cannot fit between them. +func (b *appBackend) readAgent(id string, owner bool) (*appAgent, func(), error) { + a, err := b.find(id) + if err != nil { + return nil, nil, err + } + var opening bool + var endRead func() + b.locked(func() { + opening = a.opening + if a.live != nil && !a.releasing { + endRead = b.pinReaderLocked(a) + } + }) + if endRead != nil { + return a, endRead, nil + } + if opening || b.isClosing() || !b.crewKeeps(id) || b.tidied(id) || (!owner && b.memberRests(a)) { + return a, func() {}, nil + } + a.gate.Lock() + defer a.gate.Unlock() + if b.tidied(id) || b.isClosing() { + return a, func() {}, nil + } + if b.crewKeeps(id) && (owner || !b.memberRests(a)) { + if _, _, err := b.heldLiveLocked(a); err != nil { + return nil, nil, err + } + } + return a, b.pinReader(a), nil +} + +// pinReader is called under the member gate. The count and its completion +// signal are under b.mu so returning a reader never needs the admission gate. +func (b *appBackend) pinReader(a *appAgent) func() { + var endRead func() + b.locked(func() { + endRead = b.pinReaderLocked(a) + }) + return endRead +} + +func (b *appBackend) pinReaderLocked(a *appAgent) func() { + if a.readers == 0 { + a.readersDone = make(chan struct{}) + } + a.readers++ + var once sync.Once + return func() { + once.Do(func() { + rest := false + b.locked(func() { + a.readers-- + if a.readers == 0 { + close(a.readersDone) + rest = a.restRead + } + }) + if rest { + b.letRestingGo(a) + } + }) + } +} + +// borrowLive keeps the conversation alive through a reader's last use. +func (b *appBackend) borrowLive(id string) (*appAgent, *appLive, func(), error) { + a, err := b.find(id) + if err != nil { + return nil, nil, nil, err + } + a.gate.Lock() + defer a.gate.Unlock() + _, live, err := b.heldLiveLocked(a) + if err != nil { + return nil, nil, nil, err + } + return a, live, b.pinReader(a), nil +} + +// borrowKeptLive is borrowLive for background paths that must not reopen a +// conversation the person has closed. +func (b *appBackend) borrowKeptLive(id string) (*appAgent, *appLive, func(), error) { + a, err := b.find(id) + if err != nil { + return nil, nil, nil, err + } + a.gate.Lock() + defer a.gate.Unlock() + if b.tidied(id) { + return nil, nil, nil, errAppKeptClosed + } + _, live, err := b.heldLiveLocked(a) + if err != nil { + return nil, nil, nil, err + } + return a, live, b.pinReader(a), nil +} + +// pinReaderGated pins a reader under the member gate. +func (b *appBackend) pinReaderGated(a *appAgent) func() { + a.gate.Lock() + defer a.gate.Unlock() + return b.pinReader(a) +} + +// lockRelease waits for readers outside the admission gate, then checks again +// under it. A reader may itself need that gate before returning its reference. +// Explicit release and quit remain bounded by appReleaseWait so a wedged reader +// cannot stall closing. Callers hold a.gate. +func (b *appBackend) lockRelease(a *appAgent) { + var deadline chan struct{} + var stop func() + defer func() { + if stop != nil { + stop() + } + }() + for { + var done <-chan struct{} + b.locked(func() { + if a.readers > 0 { + done = a.readersDone + } else { + a.releasing = true + } + }) + if done == nil { + return + } + if b.waitReaders == nil && deadline == nil { + deadline = make(chan struct{}) + stop = b.after(appReleaseWait, func() { close(deadline) }) + } + if b.waitReaderTurnUnlocked(a, done, deadline) { + b.locked(func() { a.releasing = true }) + return + } + } +} + +func (b *appBackend) waitReaderTurnUnlocked(a *appAgent, done, deadline <-chan struct{}) bool { + a.gate.Unlock() + defer a.gate.Lock() + if b.waitReaders != nil { + b.waitReaders(done) + return false + } + select { + case <-done: + return false + case <-deadline: + return true + } +} + +// borrowExisting pins only an already-held conversation; suggestions and naming +// reads must never open an idle owner as a side effect. +func (b *appBackend) borrowExisting(id string) (*appAgent, *appLive, func(), error) { + a, err := b.find(id) + if err != nil { + return nil, nil, nil, err + } + var live *appLive + var endRead func() + b.locked(func() { + if a.live != nil && !a.releasing { + live = a.live + endRead = b.pinReaderLocked(a) + } + }) + if endRead == nil { + endRead = func() {} + } + return a, live, endRead, nil +} diff --git a/cmd/codeaf/apprender.go b/cmd/codeaf/apprender.go index 3d46ae2da3..5aa082ebb5 100644 --- a/cmd/codeaf/apprender.go +++ b/cmd/codeaf/apprender.go @@ -66,10 +66,11 @@ func (b *appBackend) renderDir(a *appAgent) string { // ListRenders pages the conversation's whole render history, newest first. func (b *appBackend) ListRenders(agent, cursor string, limit int) (appwire.RenderPage, error) { - a, err := b.findCrewOwner(agent) + a, endRead, err := b.findCrewOwner(agent) if err != nil { return appwire.RenderPage{}, err } + defer endRead() var page session.RenderPage engine, unsupported := b.renderEngine(a) switch { @@ -92,10 +93,11 @@ func (b *appBackend) ListRenders(agent, cursor string, limit int) (appwire.Rende // GetRender answers one render by its stable id from the whole history. func (b *appBackend) GetRender(agent, id string) (appwire.Render, error) { - a, err := b.findCrewOwner(agent) + a, endRead, err := b.findCrewOwner(agent) if err != nil { return appwire.Render{}, err } + defer endRead() var record session.RenderRecord engine, unsupported := b.renderEngine(a) switch { @@ -115,10 +117,11 @@ func (b *appBackend) GetRender(agent, id string) (appwire.Render, error) { // CancelRender stops a running render. It needs the conversation held: a render // can only be running in a process that holds it. func (b *appBackend) CancelRender(agent, id string) (appwire.RenderCancel, error) { - a, err := b.findCrewOwner(agent) + a, endRead, err := b.findCrewOwner(agent) if err != nil { return appwire.RenderCancel{}, err } + defer endRead() engine, unsupported := b.renderEngine(a) switch { case unsupported: diff --git a/cmd/codeaf/appreply_suggestion.go b/cmd/codeaf/appreply_suggestion.go index a4f4548df3..898724b958 100644 --- a/cmd/codeaf/appreply_suggestion.go +++ b/cmd/codeaf/appreply_suggestion.go @@ -9,14 +9,14 @@ import ( // Suggestions never open an unheld conversation, send a turn or invoke a tool. // Crew, factory and bare chats all use their actual agent's metered model door. func (b *appBackend) SuggestReply(ctx context.Context, p appwire.ReplySuggestionParams) (appwire.ReplySuggestion, error) { - a, err := b.find(p.Agent) + _, live, endRead, err := b.borrowExisting(p.Agent) if err != nil { return appwire.ReplySuggestion{}, err } - live := b.liveOf(a) if live == nil { return appwire.ReplySuggestion{}, nil } + defer endRead() door, ok := live.conn.Conv.(interface { SuggestReply(context.Context, string) (string, error) }) diff --git a/cmd/codeaf/apprestdisk.go b/cmd/codeaf/apprestdisk.go new file mode 100644 index 0000000000..8167dfe17a --- /dev/null +++ b/cmd/codeaf/apprestdisk.go @@ -0,0 +1,150 @@ +package main + +import ( + "cmp" + "os" + "path/filepath" + "slices" + "strings" + "sync" + "time" + + "github.com/Agent-Field/codeaf/internal/session" +) + +const ( + appRestDiskTotalBytes = 96 << 20 + appRestDiskEntries = 256 + appRestDiskScanEvery = time.Minute +) + +// The inventory is rebuilt at first use and once a minute by the idle sweep. +// Writes update it directly, so enforcing the bounds never scans per request. +var appRestDisk appRestDiskCache + +type appRestDiskEntry struct { + name string + size int64 + written time.Time +} + +type appRestDiskCache struct { + mu sync.Mutex + root string + nextScan time.Time + entries map[string]appRestDiskEntry + bytes int64 +} + +func (c *appRestDiskCache) maintain(now time.Time) { + c.mu.Lock() + defer c.mu.Unlock() + c.loadLocked(now) + c.trimLocked() +} + +func (c *appRestDiskCache) loadLocked(now time.Time) { + root := filepath.Dir(appRestPath("entry")) + if c.root == root && now.Before(c.nextScan) { + return + } + files, err := os.ReadDir(root) + if err != nil && !os.IsNotExist(err) { + return + } + c.root, c.nextScan = root, now.Add(appRestDiskScanEvery) + c.entries, c.bytes = map[string]appRestDiskEntry{}, 0 + agents, known := appRestAgentFolders() + for _, file := range files { + info, err := file.Info() + if err != nil { + continue + } + agent := strings.TrimSuffix(file.Name(), ".json") + if !info.Mode().IsRegular() || strings.HasSuffix(file.Name(), ".json") && known && !agents[agent] { + _ = os.Remove(filepath.Join(root, file.Name())) + continue + } + c.rememberLocked(file.Name(), info) + } +} + +// Only folder names and deletion markers are needed, never journal or task +// contents. A failed listing cannot establish that an agent was deleted. +func appRestAgentFolders() (map[string]bool, bool) { + projects, err := os.ReadDir(session.PlacesRoot()) + if os.IsNotExist(err) { + return nil, true + } + if err != nil { + return nil, false + } + agents := map[string]bool{} + for _, project := range projects { + if !project.IsDir() { + continue + } + root := filepath.Join(session.PlacesRoot(), project.Name()) + folders, err := os.ReadDir(root) + if err != nil { + return nil, false + } + for _, folder := range folders { + if folder.IsDir() && appCloseConversationExists(filepath.Join(root, folder.Name())) { + agents[folder.Name()] = true + } + } + } + return agents, true +} + +func (c *appRestDiskCache) rememberLocked(name string, info os.FileInfo) { + c.bytes -= c.entries[name].size + c.entries[name] = appRestDiskEntry{name: name, size: info.Size(), written: info.ModTime()} + c.bytes += info.Size() +} + +func (c *appRestDiskCache) trimLocked() { + if c.bytes <= appRestDiskTotalBytes && len(c.entries) <= appRestDiskEntries { + return + } + entries := make([]appRestDiskEntry, 0, len(c.entries)) + for _, entry := range c.entries { + entries = append(entries, entry) + } + slices.SortFunc(entries, func(a, b appRestDiskEntry) int { + if order := a.written.Compare(b.written); order != 0 { + return order + } + return cmp.Compare(a.name, b.name) + }) + for _, entry := range entries { + if c.bytes <= appRestDiskTotalBytes && len(c.entries) <= appRestDiskEntries { + break + } + c.dropLocked(entry.name) + } +} + +func (c *appRestDiskCache) dropLocked(name string) { + if err := os.Remove(filepath.Join(c.root, name)); err != nil && !os.IsNotExist(err) { + return + } + c.bytes -= c.entries[name].size + delete(c.entries, name) +} + +func (c *appRestDiskCache) forget(agent string) { + path := appRestPath(agent) + if path == "" { + return + } + c.mu.Lock() + defer c.mu.Unlock() + c.loadLocked(time.Now()) + if c.root != filepath.Dir(path) { + _ = os.Remove(path) + return + } + c.dropLocked(filepath.Base(path)) +} diff --git a/cmd/codeaf/apprestdisk_test.go b/cmd/codeaf/apprestdisk_test.go new file mode 100644 index 0000000000..c50e8406ac --- /dev/null +++ b/cmd/codeaf/apprestdisk_test.go @@ -0,0 +1,133 @@ +package main + +import ( + "fmt" + "os" + "path/filepath" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/appwire" + "github.com/Agent-Field/codeaf/internal/home" + "github.com/Agent-Field/codeaf/internal/session" + "github.com/Agent-Field/codeaf/internal/teams" +) + +func TestAppRestDiskBoundsEntryCountOldestFirst(t *testing.T) { + t.Setenv(home.EnvVar, t.TempDir()) + for i := range 257 { + appRestWrite(fmt.Sprintf("agent-%03d", i), "stamp", []appwire.Task{{ID: "1"}}) + } + files, err := os.ReadDir(filepath.Dir(appRestPath("agent-000"))) + if err != nil { + t.Fatal(err) + } + if len(files) > 256 { + t.Errorf("disk cache kept %d entries, want at most 256", len(files)) + } + if _, err := os.Stat(appRestPath("agent-000")); !os.IsNotExist(err) { + t.Error("oldest entry was not evicted") + } + if _, ok := appRestRead("agent-256", "stamp"); !ok { + t.Error("newest entry was evicted") + } +} + +func restDiskFixture(t *testing.T, agent string, size int64, at time.Time) { + t.Helper() + dir := filepath.Join(session.PlacesRoot(), "project", agent) + if err := os.MkdirAll(dir, 0700); err != nil { + t.Fatal(err) + } + path := appRestPath(agent) + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + t.Fatal(err) + } + file, err := os.Create(path) + if err != nil { + t.Fatal(err) + } + if err := file.Truncate(size); err != nil { + t.Fatal(err) + } + if err := file.Close(); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(path, at, at); err != nil { + t.Fatal(err) + } +} + +func TestAppRestDiskBoundsAggregateBytesOldestFirst(t *testing.T) { + t.Setenv(home.EnvVar, t.TempDir()) + old := time.Unix(1700000000, 0) + for i, agent := range []string{"oldest", "middle", "newest"} { + // Sparse legacy files exercise the aggregate bound without large allocations. + restDiskFixture(t, agent, 24<<20, old.Add(time.Duration(i)*time.Second)) + } + restDiskFixture(t, "overflow", 24<<20, old.Add(3*time.Second)) + appRestWrite("current", "stamp", nil) + files, err := os.ReadDir(filepath.Dir(appRestPath("current"))) + if err != nil { + t.Fatal(err) + } + var total int64 + for _, file := range files { + info, err := file.Info() + if err != nil { + t.Fatal(err) + } + total += info.Size() + } + if total > 96<<20 { + t.Errorf("disk cache retained %d bytes, want at most 96 MiB", total) + } + if _, err := os.Stat(appRestPath("oldest")); !os.IsNotExist(err) { + t.Error("oldest file was not evicted for byte bound") + } + if _, err := os.Stat(appRestPath("newest")); err != nil { + t.Error("newer file was evicted before oldest") + } +} + +func TestAppRestDiskSweepPrunesDeletedAgents(t *testing.T) { + b, _ := restBackend(t, &restConversation{}) + now := time.Now().Add(time.Hour) + b.now = func() time.Time { return now } + for _, agent := range []string{"live", "gone", "deleted"} { + restDiskFixture(t, agent, 1, now) + } + gone := filepath.Join(session.PlacesRoot(), "project", "gone") + if err := os.Remove(gone); err != nil { + t.Fatal(err) + } + marker := filepath.Join(session.PlacesRoot(), "project", "deleted", teams.ConversationDeletedFile) + if err := os.WriteFile(marker, nil, 0600); err != nil { + t.Fatal(err) + } + b.sweepOnce() + for _, agent := range []string{"gone", "deleted"} { + if _, err := os.Stat(appRestPath(agent)); !os.IsNotExist(err) { + t.Errorf("deleted agent %s kept disk cache", agent) + } + } + if _, err := os.Stat(appRestPath("live")); err != nil { + t.Fatal("live agent cache was pruned") + } + // New external entries are checked on a later sweep, without another request. + restDiskFixture(t, "later", 1, now) + if err := os.Remove(filepath.Join(session.PlacesRoot(), "project", "later")); err != nil { + t.Fatal(err) + } + // An ordinary write and a sweep inside the interval must reuse the inventory. + appRestWrite("live", "stamp", nil) + b.sweepOnce() + if _, err := os.Stat(appRestPath("later")); err != nil { + t.Fatal("ordinary request rescanned the cache directory") + } + now = now.Add(time.Minute) + b.sweepOnce() + if _, err := os.Stat(appRestPath("later")); !os.IsNotExist(err) { + t.Fatal("periodic sweep did not prune external entry") + } +} diff --git a/cmd/codeaf/apprestmemo.go b/cmd/codeaf/apprestmemo.go new file mode 100644 index 0000000000..c0b1f104d4 --- /dev/null +++ b/cmd/codeaf/apprestmemo.go @@ -0,0 +1,527 @@ +package main + +import ( + "container/list" + "encoding/json" + "io" + "io/fs" + "os" + "path/filepath" + "reflect" + "strconv" + "strings" + "sync" + "time" + "unsafe" + + "github.com/Agent-Field/codeaf/internal/appwire" + "github.com/Agent-Field/codeaf/internal/buildinfo" + "github.com/Agent-Field/codeaf/internal/cachedir" + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/filememo" + "github.com/Agent-Field/codeaf/internal/fsreplace" + "github.com/Agent-Field/codeaf/internal/session" +) + +// appRestMemoBytes bounds what the engine keeps of answers about members at +// rest. Past it the least recently read answer goes first and is rebuilt from +// the member's files on the next read. +const appRestMemoBytes = 24 << 20 + +// appRestReads is how many members at rest are held at once to be read. Each +// is a whole journal replayed into memory for the length of one read, so the +// peak of a view of every member is this many journals and not all of them. +const appRestReads = 2 + +// appRestMemo remembers an answer that only a held conversation could give, +// for as long as the files that answer was built from have not changed. It is +// what lets a member at rest stay closed: opening one replays its whole +// journal into memory, and a view of every member of every open crew asks +// each of them for the same task list over and over. +type appRestMemo struct { + mu sync.Mutex + bytes int + order *list.List // front is the most recently read + entries map[string]*list.Element +} + +type appRestEntry struct { + agent string + stamp string + tasks []appwire.Task + size int +} + +// get is the remembered tasks of agent when they were built from files that +// still read as stamp. The slice is the caller's own; the rows it holds are +// shared and never written after they are remembered. +func (m *appRestMemo) get(agent, stamp string) ([]appwire.Task, bool) { + if stamp == "" { + return nil, false + } + m.mu.Lock() + defer m.mu.Unlock() + el, ok := m.entries[agent] + if !ok { + return nil, false + } + entry := el.Value.(*appRestEntry) + if entry.stamp != stamp { + m.dropLocked(el) + return nil, false + } + m.order.MoveToFront(el) + return append([]appwire.Task{}, entry.tasks...), true +} + +// put remembers tasks under stamp. An answer larger than the whole bound is +// not kept, so one huge member cannot push every other out and be rebuilt. +func (m *appRestMemo) put(agent, stamp string, tasks []appwire.Task) bool { + size := appRestEntrySize(agent, stamp, tasks) + m.mu.Lock() + defer m.mu.Unlock() + if m.entries == nil { + m.entries, m.order = map[string]*list.Element{}, list.New() + } + if el, ok := m.entries[agent]; ok { + m.dropLocked(el) + } + if stamp == "" || size > appRestMemoBytes { + return false + } + m.entries[agent] = m.order.PushFront(&appRestEntry{agent: agent, stamp: stamp, tasks: append([]appwire.Task{}, tasks...), size: size}) + m.bytes += size + for m.bytes > appRestMemoBytes { + m.dropLocked(m.order.Back()) + } + return true +} + +func (m *appRestMemo) dropLocked(el *list.Element) { + entry := m.order.Remove(el).(*appRestEntry) + delete(m.entries, entry.agent) + m.bytes -= entry.size +} + +// forget drops what is remembered about agent. +func (m *appRestMemo) forget(agent string) { + m.mu.Lock() + defer m.mu.Unlock() + if el, ok := m.entries[agent]; ok { + m.dropLocked(el) + } +} + +// appTasksSize estimates retained Go memory, not its JSON encoding. It counts +// struct and slice storage (including spare capacity), pointed-to values and +// string bytes recursively. Aliases are counted twice, conservatively; allocator +// rounding and map buckets are covered by the entry allowance below. +func appTasksSize(tasks []appwire.Task) int { + return int(unsafe.Sizeof(tasks)) + appReferencedSize(reflect.ValueOf(tasks)) +} + +func appReferencedSize(v reflect.Value) int { + if v.Type() == reflect.TypeFor[time.Time]() { + return 0 + } + switch v.Kind() { + case reflect.String: + return v.Len() + case reflect.Pointer: + if !v.IsNil() { + return int(v.Type().Elem().Size()) + appReferencedSize(v.Elem()) + } + case reflect.Slice: + size := v.Cap() * int(v.Type().Elem().Size()) + for i := 0; i < v.Len(); i++ { + size += appReferencedSize(v.Index(i)) + } + return size + case reflect.Struct: + size := 0 + for i := 0; i < v.NumField(); i++ { + size += appReferencedSize(v.Field(i)) + } + return size + } + return 0 +} + +func appRestEntrySize(agent, stamp string, tasks []appwire.Task) int { + // Entry, LRU element, map key/value and conservative bucket/allocator space. + return appTasksSize(tasks) + len(agent) + len(stamp) + int(unsafe.Sizeof(appRestEntry{})) + int(unsafe.Sizeof(list.Element{})) + 128 +} + +// Two seconds covers coarse filesystem timestamp ticks. A recent or future +// mtime is racy-clean even if both stat calls return the same stamp. +const appRestSettle = filememo.TimestampSettle + +// appRestFiles is the task view's durable input, including owner overlays: +// checkpoints and deletion filters, presence, plans, and child task journals. +// Metadata dials (meta/session), working beliefs (state), reading watermarks +// (told/team), and background jobs (processes) do not define task rows. +var appRestFiles = []string{ + "presence.json", "tasks.json", ".deleted-tasks.json", ".conversation-deleted", + "plandb.db", "plandb.db-wal", "tasks", +} + +// restStamp changes whenever a file a member's task list is built from does: +// the folder's own files, the project's task index and the spend cap. +func (b *appBackend) restStamp(a *appAgent) string { + var transcript string + b.locked(func() { transcript = a.transcript }) + root, folder := session.FolderOf(transcript) + // Flat legacy journals can reference checkpoints and node journals outside + // their own folder. Keep their reads bounded without certifying partial inputs. + if !folder { + return "" + } + dirs := []string{filepath.Dir(transcript)} + if root != dirs[0] { + dirs = append(dirs, root) + } + paths := []string{transcript, config.BudgetConfigPath(b.crewProfile()), session.TaskIndexPath(transcript)} + for _, dir := range dirs { + files, ok := appRestPaths(dir) + if !ok { + return "" + } + paths = append(paths, files...) + } + var stamp strings.Builder + now := b.now() + for _, path := range paths { + if !appRestStampPath(&stamp, path, now) { + return "" + } + } + return stamp.String() +} + +// appRestPaths includes both a cell's truth folder and its session root. +// Plans and presence stay at the root while task records travel with .cell. +func appRestPaths(dir string) ([]string, bool) { + paths := make([]string, 0, len(appRestFiles)+2) + for _, name := range appRestFiles { + paths = append(paths, filepath.Join(dir, name)) + } + // Archived plans stop at the first missing suffix, as their reader does. + for suffix := 1; ; suffix++ { + path := filepath.Join(dir, "plandb.db."+strconv.Itoa(suffix)) + paths = append(paths, path, path+"-wal") + if _, err := os.Stat(path); err != nil { + return paths, os.IsNotExist(err) + } + } +} + +// appRestStampPath uses one stat per entry for identity and racy-clean checks. +func appRestStampPath(stamp *strings.Builder, path string, now time.Time) bool { + err := filepath.WalkDir(path, func(path string, entry fs.DirEntry, walkErr error) error { + if os.IsNotExist(walkErr) { + stamp.WriteString(path + ":-;") + return nil + } + if walkErr != nil { + return walkErr + } + info, err := entry.Info() + if err != nil { + return err + } + if !info.Mode().IsRegular() && !info.IsDir() { + return os.ErrInvalid + } + part := appInfoStamp(path, info) + _, _, ctime, _ := appFileIdentity(info) + if part == "" || !filememo.TimestampsSettled(now, info.ModTime(), time.Unix(0, ctime), appRestSettle) { + return os.ErrInvalid + } + stamp.WriteString(part) + return nil + }) + return err == nil +} + +// restingTasks answers a member at rest from its remembered task list, and +// builds the list the one time its files have changed by holding the +// conversation just long enough to read it. The answer is the owner-backed +// one either way; the conversation's journal is not kept in memory for it. +func (b *appBackend) restingTasks(a *appAgent, id string) ([]appwire.Task, error) { + if b.restSlots != nil { + b.restSlots <- struct{}{} + defer func() { <-b.restSlots }() + } + tasks, cached, tidied, stamp, wasLive, endRead, err := b.restingTasksAdmit(a, id) + if err != nil { + return nil, err + } + if cached { + return tasks, nil + } + if tidied { + return b.listTasksHeld(a, id) + } + defer endRead() + tasks, err = b.listTasksHeld(a, id) + // Returning the last reference may close the conversation. Its writes must + // also match the pre-open snapshot; no post-release stamp can certify rows. + endRead() + if err != nil { + return nil, err + } + if !wasLive && stamp != "" && b.restStamp(a) == stamp && b.memberRests(a) && b.restMemo.put(id, stamp, tasks) { + appRestWrite(id, stamp, tasks) + } + return tasks, nil +} + +func (b *appBackend) restingTasksAdmit(a *appAgent, id string) (tasks []appwire.Task, cached, tidied bool, stamp string, wasLive bool, endRead func(), err error) { + a.gate.Lock() + defer a.gate.Unlock() + if b.tidied(id) || b.isClosing() { + return nil, false, true, "", false, nil, nil + } + stamp = b.restStamp(a) + live := b.liveOf(a) + if live == nil && b.memberQuiet(a) { + if tasks, ok := b.restMemo.get(id, stamp); ok { + return tasks, true, false, "", false, nil, nil + } + if tasks, ok := appRestRead(id, stamp); ok && b.restStamp(a) == stamp && b.restMemo.put(id, stamp, tasks) { + return tasks, true, false, "", false, nil, nil + } + } + if _, _, err := b.heldLiveLocked(a); err != nil { + return nil, false, false, "", false, nil, err + } + endRead = b.pinReader(a) + if live == nil { + b.locked(func() { a.restRead = true }) + } + return nil, false, false, stamp, live != nil, endRead, nil +} + +// appRestFile is a remembered task list on disk: under the cache root, so +// deleting the cache only costs the next launch a rebuild. It is only read by +// the build that wrote it, because what a list says is that build's reading of +// the files, and only while the files still read as its stamp. +type appRestFile struct { + Build string `json:"build"` + Stamp string `json:"stamp"` + Tasks []appwire.Task `json:"tasks"` +} + +func appRestPath(agent string) string { + if agent == "" || filepath.Base(agent) != agent { + return "" + } + return filepath.Join(cachedir.Root(), "app-tasks", agent+".json") +} + +// appRestDiskBytes also bounds a corrupt cache before decoding. The decoder +// streams from a limited reader, so early syntax errors do not read the tail. +const appRestDiskBytes = appRestMemoBytes + +func appRestRead(agent, stamp string) ([]appwire.Task, bool) { + path := appRestPath(agent) + if path == "" || stamp == "" { + return nil, false + } + info, err := os.Lstat(path) + if err != nil { + return nil, false + } + if !info.Mode().IsRegular() || info.Size() > appRestDiskBytes { + _ = os.Remove(path) + return nil, false + } + file, err := openAppRestFile(path) + if err != nil { + return nil, false + } + defer file.Close() + info, err = file.Stat() + if err != nil || !info.Mode().IsRegular() || info.Size() > appRestDiskBytes { + _ = os.Remove(path) + return nil, false + } + decoder := json.NewDecoder(io.LimitReader(file, appRestDiskBytes+1)) + tasks, ok := appRestDecode(decoder, agent, stamp) + if !ok { + _ = os.Remove(path) + } + return tasks, ok +} + +// appRestDecode accepts only this build's envelope. Rows are admitted one at +// a time so a compact array of empty objects cannot expand past the Go budget. +func appRestDecode(decoder *json.Decoder, agent, stamp string) ([]appwire.Task, bool) { + if token, err := decoder.Token(); err != nil || token != json.Delim('{') { + return nil, false + } + var file appRestFile + seen := map[string]bool{} + for decoder.More() { + token, err := decoder.Token() + key, ok := token.(string) + if err != nil || !ok || seen[key] { + return nil, false + } + seen[key] = true + if !file.readField(decoder, key, agent, stamp) { + return nil, false + } + } + if token, err := decoder.Token(); err != nil || token != json.Delim('}') { + return nil, false + } + if _, err := decoder.Token(); err != io.EOF { + return nil, false + } + return file.Tasks, seen["tasks"] && file.Build == buildinfo.Identity() && file.Stamp == stamp +} + +// readField keeps the envelope's accepted fields and their admission together. +func (file *appRestFile) readField(decoder *json.Decoder, key, agent, stamp string) bool { + switch key { + case "build": + return decoder.Decode(&file.Build) == nil && file.Build == buildinfo.Identity() + case "stamp": + return decoder.Decode(&file.Stamp) == nil && file.Stamp == stamp + case "tasks": + var ok bool + file.Tasks, ok = appRestDecodeTasks(decoder, agent, stamp) + return ok + default: + return false + } +} + +func appRestDecodeTasks(decoder *json.Decoder, agent, stamp string) ([]appwire.Task, bool) { + token, err := decoder.Token() + if err != nil { + return nil, false + } + if token == nil { + return nil, true + } + if token != json.Delim('[') { + return nil, false + } + var tasks []appwire.Task + references := appRestEntrySize(agent, stamp, nil) + rowBytes := int(unsafe.Sizeof(appwire.Task{})) + for decoder.More() { + var task appwire.Task + if decoder.Decode(&task) != nil { + return nil, false + } + references += appReferencedSize(reflect.ValueOf(task)) + if references+(len(tasks)+1)*rowBytes > appRestMemoBytes { + return nil, false + } + if len(tasks) == cap(tasks) { + capacity := min(max(16, cap(tasks)*2), (appRestMemoBytes-references)/rowBytes) + rows := make([]appwire.Task, len(tasks), capacity) + copy(rows, tasks) + tasks = rows + } + if references+cap(tasks)*rowBytes > appRestMemoBytes { + return nil, false + } + tasks = append(tasks, task) + } + if token, err := decoder.Token(); err != nil || token != json.Delim(']') { + return nil, false + } + return tasks, true +} + +// appRestWrite keeps a list for the next launch. A failure is silence: the +// list was answered and the cache is only a saving. +func appRestWrite(agent, stamp string, tasks []appwire.Task) { + path := appRestPath(agent) + if path == "" || stamp == "" || appRestEntrySize(agent, stamp, tasks) > appRestMemoBytes { + return + } + raw, err := json.Marshal(appRestFile{Build: buildinfo.Identity(), Stamp: stamp, Tasks: tasks}) + if err != nil || len(raw) > appRestDiskBytes || os.MkdirAll(filepath.Dir(path), 0o700) != nil { + return + } + appRestDisk.mu.Lock() + defer appRestDisk.mu.Unlock() + appRestDisk.loadLocked(time.Now()) + if appRestDisk.root != filepath.Dir(path) || appRestDisk.entries == nil { + return + } + temp, err := os.CreateTemp(filepath.Dir(path), agent+".*.tmp") + if err != nil { + return + } + _, werr := temp.Write(raw) + if cerr := temp.Close(); werr != nil || cerr != nil || fsreplace.Rename(temp.Name(), path) != nil { + _ = os.Remove(temp.Name()) + return + } + if info, err := os.Stat(path); err == nil { + appRestDisk.rememberLocked(filepath.Base(path), info) + appRestDisk.trimLocked() + } +} + +// letRestingGo returns an idle owner only after its last reader and a fresh +// live-work check, under the same gate as every admission. +func (b *appBackend) letRestingGo(a *appAgent) bool { + if !a.gate.TryLock() { + return false + } + defer a.gate.Unlock() + var live *appLive + var id string + running := false + b.locked(func() { + live, id = a.live, a.view.ID + running = a.readers > 0 || live != nil && (live.running || live.compacting) + }) + if live == nil { + return true + } + if running || b.liveNeedsYou(live) || b.serviceKeeps(id) || !appLiveWorkSettled(live) { + return false + } + return b.releaseIdle(a) +} + +// appLiveWorkSettled re-reads work under the admission gate. Foreground state and +// an earlier list say nothing about a just-admitted background task or job. +func appLiveWorkSettled(live *appLive) bool { + tasks, err := live.conn.Conv.AppTasks() + if err != nil { + return false + } + for _, task := range tasks { + if !appTaskSettled(task.State) && task.State != "your_call" { + return false + } + } + quicks, err := live.conn.Conv.AppQuickTasks() + if err != nil { + return false + } + for _, quick := range quicks { + if quick.State == "running" { + return false + } + } + jobs, err := live.conn.Conv.JobNotices() + if err != nil { + return false + } + for _, job := range jobs { + if job.State == session.JobRunning { + return false + } + } + return true +} diff --git a/cmd/codeaf/apprestmemo_production_test.go b/cmd/codeaf/apprestmemo_production_test.go new file mode 100644 index 0000000000..c7d65a04de --- /dev/null +++ b/cmd/codeaf/apprestmemo_production_test.go @@ -0,0 +1,186 @@ +package main + +import ( + "os" + "path/filepath" + "sync/atomic" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/session" +) + +// The production opener restores both dials and recovers a nonempty checkpoint. +// A warm open canonicalizes the fixture; all later opens must preserve identity. +func productionRestBackend(t *testing.T, cell bool) (*appBackend, *appAgent, *atomic.Int32) { + t.Helper() + _, workspace := wakeEnvironment(t, resolvedTempDir(t)) + bucket, err := v3ProjectDir(workspace) + if err != nil { + t.Fatal(err) + } + id := "00000000000000e3" + dir := writeV3Session(t, bucket, id, "Saved settled work", time.Now()) + if cell { + if err := os.Mkdir(filepath.Join(dir, ".cell"), 0700); err != nil { + t.Fatal(err) + } + if err := os.Rename(filepath.Join(dir, "transcript.jsonl"), filepath.Join(dir, ".cell", "transcript.jsonl")); err != nil { + t.Fatal(err) + } + } + if err := session.SaveMeta(dir, session.Meta{ID: id, Workspace: workspace, Created: time.Now(), Effort: "high", Approval: session.PostureAsk}); err != nil { + t.Fatal(err) + } + place := session.Place{Dir: dir, Workspace: workspace} + checkpoint := `{"type":"tasks","version":1,"seq":1,"nodes":[{"id":1,"title":"Saved settled task","brief":"Already finished","acceptance":"Task finished","state":"done","report":"Finished","noted":true}]}` + if err := os.WriteFile(place.Tasks(), []byte(checkpoint), 0600); err != nil { + t.Fatal(err) + } + conn, err := appOpenInProcess(appOpenAsk{Workspace: workspace, Transcript: place.Transcript()}) + if err != nil { + t.Fatal(err) + } + conn.Release() + opens := &atomic.Int32{} + b := newFakeBackend(newAppRecorder(), func(ask appOpenAsk) (*appConn, error) { + opens.Add(1) + conn, err := appOpenInProcess(ask) + if err == nil { + agent, ok := session.LiveAgentFor(conn.Transcript) + if !ok || agent.ConversationEffort() != "high" || agent.ApprovalPosture() != session.PostureAsk { + t.Error("production opener did not restore both live dials") + } + } + return conn, err + }) + b.now = func() time.Time { return time.Now().Add(time.Hour) } + // First-run daily-limit migration is a real configuration change, outside + // this unchanged-session regression. Settle it before taking any snapshot. + if err := b.settleDayLimit(); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { b.locked(func() { b.crewRun = nil }); b.Close() }) + a, err := b.find(id) + if err != nil { + t.Fatal(err) + } + b.locked(func() { b.crewRun = &appCrewRun{held: map[string][]string{id: {"crew"}}} }) + return b, a, opens +} + +func TestAppRestProductionSavedSettingsFillBothCaches(t *testing.T) { + for _, cell := range []bool{false, true} { + name := "legacy" + if cell { + name = "cell" + } + t.Run(name, func(t *testing.T) { + b, a, opens := productionRestBackend(t, cell) + // Every persistent writer on this settled open: recoverTasks writes the + // checkpoint; restored approval and effort each call SaveMeta, which writes + // meta.json and, for cells, session.json. Check each destination independently. + paths := []string{filepath.Join(filepath.Dir(a.transcript), "tasks.json"), filepath.Join(session.DirOf(a.transcript), "meta.json")} + if cell { + paths = append(paths, filepath.Join(filepath.Dir(a.transcript), "session.json")) + } + before := map[string]os.FileInfo{} + for _, path := range paths { + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + before[path] = info + } + for pass := range 3 { + tasks, err := b.ListTasks(a.view.ID) + if err != nil || len(tasks) != 1 || tasks[0].Title != "Saved settled task" { + t.Fatalf("listing %d: %+v, %v", pass+1, tasks, err) + } + if b.liveOf(a) != nil { + t.Fatal("idle conversation stayed held") + } + for _, path := range paths { + after, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if !os.SameFile(before[path], after) || !before[path].ModTime().Equal(after.ModTime()) { + t.Errorf("listing %d replaced unchanged %s", pass+1, path) + } + } + if got := opens.Load(); got != 1 { + t.Errorf("listing %d: %d opens, want 1", pass+1, got) + } + if pass == 1 { + b.restMemo = appRestMemo{} + } + } + meta, err := session.LoadMeta(session.DirOf(a.transcript)) + if err != nil || meta.Effort != "high" || meta.Approval != session.PostureAsk { + t.Fatalf("saved settings changed: %+v, %v", meta, err) + } + }) + } +} + +func TestAppRestTombstoneCommitInvalidatesBothCaches(t *testing.T) { + for _, cache := range []string{"memory", "disk"} { + t.Run(cache, func(t *testing.T) { + b, a, _ := productionRestBackend(t, false) + index := session.TaskIndexPath(a.transcript) + row := `{"id":"2","sessionId":"` + a.view.ID + `","title":"Index-only task","status":"done"}` + "\n" + if err := os.WriteFile(index, []byte(row), 0600); err != nil { + t.Fatal(err) + } + tasks, err := b.ListTasks(a.view.ID) + if err != nil || len(tasks) != 2 { + t.Fatalf("prime cache: %+v, %v", tasks, err) + } + stamp := b.restStamp(a) + if _, ok := b.restMemo.get(a.view.ID, stamp); !ok { + t.Fatal("memory cache not primed") + } + if _, ok := appRestRead(a.view.ID, stamp); !ok { + t.Fatal("disk cache not primed") + } + paths := []string{filepath.Join(filepath.Dir(a.transcript), "tasks.json"), index} + before := map[string]os.FileInfo{} + for _, path := range paths { + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + before[path] = info + } + // This is the durable state after commitTaskDeletions, with the process + // lost before finishTaskCleanup can touch the checkpoint or index. + tombstone := filepath.Join(filepath.Dir(a.transcript), ".deleted-tasks.json") + if err := os.WriteFile(tombstone, []byte(`{"1":true,"2":true}`), 0600); err != nil { + t.Fatal(err) + } + for _, path := range paths { + after, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if !os.SameFile(before[path], after) || !before[path].ModTime().Equal(after.ModTime()) { + t.Fatalf("crash fixture changed %s", path) + } + } + if got := session.ReadSessionTasks(a.transcript); len(got) != 0 { + t.Fatalf("uncached checkpoint did not filter deletion: %+v", got) + } + if got := session.ReadTaskIndex(index); len(got) != 0 { + t.Fatalf("uncached index did not filter deletion: %+v", got) + } + if cache == "disk" { + b.restMemo = appRestMemo{} + } + tasks, err = b.ListTasks(a.view.ID) + if err != nil || len(tasks) != 0 { + t.Fatalf("%s cache resurrected deleted tasks: %+v, %v", cache, tasks, err) + } + }) + } +} diff --git a/cmd/codeaf/apprestmemo_regression_test.go b/cmd/codeaf/apprestmemo_regression_test.go new file mode 100644 index 0000000000..8e2ae685ae --- /dev/null +++ b/cmd/codeaf/apprestmemo_regression_test.go @@ -0,0 +1,610 @@ +package main + +import ( + "encoding/json" + "errors" + "os" + "path/filepath" + "runtime" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + "unsafe" + + "github.com/Agent-Field/codeaf/internal/appwire" + "github.com/Agent-Field/codeaf/internal/buildinfo" + "github.com/Agent-Field/codeaf/internal/home" + "github.com/Agent-Field/codeaf/internal/session" +) + +// These hooks pause the engine's real conversation door, without timing races. +type restConversation struct { + fakeConversation + readTasks func() ([]session.AppTaskView, error) + retry func(uint64) error +} + +func (c *restConversation) AppTasks() ([]session.AppTaskView, error) { + if c.readTasks != nil { + return c.readTasks() + } + return c.fakeConversation.AppTasks() +} +func (c *restConversation) RetryTask(id uint64) error { + if c.retry != nil { + return c.retry(id) + } + return c.fakeConversation.RetryTask(id) +} + +func restBackend(t *testing.T, c *restConversation) (*appBackend, *appAgent) { + t.Helper() + t.Setenv(home.EnvVar, t.TempDir()) + transcript := filepath.Join(t.TempDir(), "transcript.jsonl") + b := newFakeBackend(newAppRecorder(), func(ask appOpenAsk) (*appConn, error) { + return &appConn{Conv: c, Workspace: ask.Workspace, Transcript: transcript, Assembled: true, Release: func() {}}, nil + }) + b.now = func() time.Time { return time.Now().Add(time.Hour) } + t.Cleanup(func() { b.locked(func() { b.crewRun = nil }); b.Close() }) + view, err := mustCreate(t, b, appwire.CreateAsk{Workspace: t.TempDir()}) + if err != nil { + t.Fatal(err) + } + a, err := b.find(view.ID) + if err != nil { + t.Fatal(err) + } + b.letGo(a) + b.locked(func() { b.crewRun = &appCrewRun{held: map[string][]string{view.ID: {"crew"}}} }) + return b, a +} + +func TestAppRestReleaseCoordinatesWithRetry(t *testing.T) { + c := &restConversation{} + b, a := restBackend(t, c) + var running atomic.Bool + c.retry = func(uint64) error { + if a.gate.TryLock() { + a.gate.Unlock() + t.Error("retry admitted outside the release gate") + } + running.Store(true) + return nil + } + var once sync.Once + c.readTasks = func() ([]session.AppTaskView, error) { + old := !running.Load() + once.Do(func() { + if err := b.RetryTask(a.view.ID + ":1"); err != nil { + t.Error(err) + } + }) + state := session.TaskRunning + if old { + state = session.TaskFailed + } + return []session.AppTaskView{{ID: "1", State: string(state)}}, nil + } + if _, err := b.ListTasks(a.view.ID); err != nil { + t.Fatal(err) + } + if b.liveOf(a) == nil { + t.Fatal("listing cancelled newly retried background work") + } +} + +func TestAppRestConcurrentReadersKeepOwnerOverlay(t *testing.T) { + c := &restConversation{} + b, a := restBackend(t, c) + first, second, finish := make(chan struct{}), make(chan struct{}), make(chan struct{}) + var calls atomic.Int32 + c.readTasks = func() ([]session.AppTaskView, error) { + switch calls.Add(1) { + case 1: + close(first) + <-second + case 2: + close(second) + <-finish + } + return []session.AppTaskView{{ID: "1", State: string(session.TaskDone), Step: 7}}, nil + } + type answer struct { + tasks []appwire.Task + err error + } + results := make(chan answer, 2) + go func() { tasks, err := b.ListTasks(a.view.ID); results <- answer{tasks, err} }() + <-first + go func() { tasks, err := b.ListTasks(a.view.ID); results <- answer{tasks, err} }() + <-second + one := <-results + if b.liveOf(a) == nil { + t.Error("first reader released the second reader's conversation") + } + close(finish) + two := <-results + for _, got := range []answer{one, two} { + if got.err != nil || len(got.tasks) != 1 || got.tasks[0].Steps == nil || *got.tasks[0].Steps != 7 { + t.Errorf("owner overlay lost: %+v, %v", got.tasks, got.err) + } + } + if b.liveOf(a) != nil { + t.Error("last reader did not release the idle conversation") + } +} + +func TestAppRestDoesNotCertifyWritesDuringRead(t *testing.T) { + c := &restConversation{} + b, a := restBackend(t, c) + path := filepath.Join(filepath.Dir(a.transcript), "tasks.json") + var calls atomic.Int32 + c.readTasks = func() ([]session.AppTaskView, error) { + n := calls.Add(1) + if n == 1 { + if err := os.WriteFile(path, []byte(`{}`), 0600); err != nil { + t.Fatal(err) + } + } + title := "new" + if n == 1 { + title = "old" + } + return []session.AppTaskView{{ID: "1", Title: title, State: string(session.TaskDone)}}, nil + } + if _, err := b.ListTasks(a.view.ID); err != nil { + t.Fatal(err) + } + b.restMemo = appRestMemo{} // Relaunch must not revive the stale answer either. + got, err := b.ListTasks(a.view.ID) + if err != nil || len(got) != 1 || got[0].Title != "new" { + t.Fatalf("write during read certified stale rows: %+v %v", got, err) + } +} + +func TestAppRestStampDetectsSameSizeReplacement(t *testing.T) { + b, a := restBackend(t, &restConversation{}) + path := filepath.Join(filepath.Dir(a.transcript), "tasks.json") + old := time.Now().Add(-time.Hour) + write := func(path, text string) { + t.Helper() + if err := os.WriteFile(path, []byte(text), 0600); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(path, old, old); err != nil { + t.Fatal(err) + } + } + write(path, "old") + before := b.restStamp(a) + write(path+".new", "new") + if err := os.Rename(path+".new", path); err != nil { + t.Fatal(err) + } + if after := b.restStamp(a); after == before { + t.Fatal("same-size replacement with restored mtime left cache valid") + } + before = b.restStamp(a) + write(path, "now") + if after := b.restStamp(a); after == before { + t.Fatal("in-place write with restored mtime left cache valid") + } +} + +func TestAppRestDoesNotCacheRecentFiles(t *testing.T) { + c := &restConversation{} + b, a := restBackend(t, c) + path := filepath.Join(filepath.Dir(a.transcript), "tasks.json") + if err := os.WriteFile(path, []byte(`{}`), 0600); err != nil { + t.Fatal(err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + now := info.ModTime().Add(time.Second) + b.now = func() time.Time { return now } + var calls atomic.Int32 + c.readTasks = func() ([]session.AppTaskView, error) { calls.Add(1); return nil, nil } + for range 2 { + if _, err := b.ListTasks(a.view.ID); err != nil { + t.Fatal(err) + } + } + if calls.Load() < 2 { + t.Fatal("file inside timestamp resolution was cached") + } +} + +func TestAppRestBudgetCountsRetainedRows(t *testing.T) { + tasks := make([]appwire.Task, appRestMemoBytes/int(unsafe.Sizeof(appwire.Task{}))+1) + var m appRestMemo + m.put("agent", "stamp", tasks) + if _, ok := m.get("agent", "stamp"); ok { + t.Fatal("memory cache admitted more than 24 MiB of task structs") + } +} + +func TestAppRestRejectedMemoryEntryIsNeverWritten(t *testing.T) { + t.Setenv(home.EnvVar, t.TempDir()) + tasks := make([]appwire.Task, appRestMemoBytes/int(unsafe.Sizeof(appwire.Task{}))+1) + appRestWrite("agent", "stamp", tasks) + if _, err := os.Stat(appRestPath("agent")); !os.IsNotExist(err) { + t.Fatalf("memory-rejected list written to disk: %v", err) + } +} + +func TestAppRestDropsOversizedAndCorruptDiskFiles(t *testing.T) { + t.Setenv(home.EnvVar, t.TempDir()) + path := appRestPath("agent") + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + t.Fatal(err) + } + for _, kind := range []string{"oversized", "corrupt"} { + t.Run(kind, func(t *testing.T) { + f, err := os.Create(path) + if err != nil { + t.Fatal(err) + } + if kind == "oversized" { + err = f.Truncate(64 << 20) + } else { + _, err = f.WriteString("!" + strings.Repeat(" ", 8<<20)) + } + if err != nil { + t.Fatal(err) + } + f.Close() + var before, after runtime.MemStats + runtime.ReadMemStats(&before) + _, ok := appRestRead("agent", "stamp") + runtime.ReadMemStats(&after) + if ok { + t.Fatal("invalid disk file admitted") + } + if n := after.TotalAlloc - before.TotalAlloc; n > 1<<20 { + t.Errorf("invalid disk file allocated %d bytes before rejection", n) + } + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Error("invalid disk file was not dropped") + } + }) + } +} + +func TestAppRestDiskReadWaitsForAdmission(t *testing.T) { + b, a := restBackend(t, &restConversation{}) + b.restSlots = make(chan struct{}) + appRestWrite(a.view.ID, b.restStamp(a), []appwire.Task{{ID: "cached", Title: "cached"}}) + done := make(chan struct{}) + go func() { _, _ = b.ListTasks(a.view.ID); close(done) }() + select { + case <-done: + t.Fatal("disk decoding bypassed read admission") + case <-b.restSlots: // The reader has reached admission, before decoding. + } + b.restSlots <- struct{}{} // Return the reservation on the reader's exit. + <-done +} + +func TestAppRestRealIdleOpenKeepsTheMemoryWin(t *testing.T) { + testAppRestRealIdleOpen(t, false) +} + +func TestAppRestRealSettledTasksFillBothCaches(t *testing.T) { + testAppRestRealIdleOpen(t, true) +} + +func testAppRestRealIdleOpen(t *testing.T, savedTasks bool) { + t.Helper() + t.Setenv(home.EnvVar, t.TempDir()) + path := filepath.Join(t.TempDir(), "transcript.jsonl") + if savedTasks { + checkpoint := `{"type":"tasks","version":1,"seq":1,"nodes":[{"id":1,"title":"Saved settled task","brief":"Already finished","acceptance":"Task finished","state":"done","report":"Finished","noted":true}]}` + if err := os.WriteFile(filepath.Join(filepath.Dir(path), "tasks.json"), []byte(checkpoint), 0600); err != nil { + t.Fatal(err) + } + } + var opens atomic.Int32 + b := newFakeBackend(newAppRecorder(), func(ask appOpenAsk) (*appConn, error) { + engine, err := session.New(session.Config{Workspace: ask.Workspace, SessionFile: path, Model: "test/model", APIKey: "test-key", BaseURL: "http://127.0.0.1:1"}) + if err != nil { + return nil, err + } + opens.Add(1) + c := &restConversation{readTasks: engine.AppTasks} + return &appConn{Conv: c, Workspace: ask.Workspace, Transcript: path, Assembled: true, Release: func() { _ = engine.Close() }}, nil + }) + b.mint = func(string) (string, error) { return path, nil } + b.now = func() time.Time { return time.Now().Add(time.Hour) } + t.Cleanup(func() { b.locked(func() { b.crewRun = nil }); b.Close() }) + view, err := mustCreate(t, b, appwire.CreateAsk{Workspace: t.TempDir()}) + if err != nil { + t.Fatal(err) + } + a, err := b.find(view.ID) + if err != nil { + t.Fatal(err) + } + b.letGo(a) + b.locked(func() { b.crewRun = &appCrewRun{held: map[string][]string{view.ID: {"crew"}}} }) + before := opens.Load() + stamp := b.restStamp(a) + for pass := range 3 { + got, err := b.ListTasks(view.ID) + if err != nil { + t.Fatal(err) + } + if savedTasks && (len(got) != 1 || got[0].Title != "Saved settled task" || got[0].State != appwire.TaskDone) { + t.Fatalf("saved task missing: %+v", got) + } + if b.liveOf(a) != nil { + t.Fatal("real idle session remained held") + } + if n := opens.Load() - before; n != 1 { + t.Fatalf("pass %d opened real session %d times, want 1; snapshot stable=%v", pass+1, n, stamp == b.restStamp(a)) + } + if pass == 1 { + b.restMemo = appRestMemo{} + } + } + +} + +func TestAppRestDiskDecodeBoundsCompactRows(t *testing.T) { + t.Setenv(home.EnvVar, t.TempDir()) + path := appRestPath("agent") + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + t.Fatal(err) + } + header, err := json.Marshal(appRestFile{Build: buildinfo.Identity(), Stamp: "stamp"}) + if err != nil { + t.Fatal(err) + } + raw := strings.Replace(string(header), `"tasks":null`, `"tasks":[`+strings.Repeat(`{},`, 200000)+`{}]`, 1) + if err := os.WriteFile(path, []byte(raw), 0600); err != nil { + t.Fatal(err) + } + var before, after runtime.MemStats + runtime.ReadMemStats(&before) + _, ok := appRestRead("agent", "stamp") + runtime.ReadMemStats(&after) + if ok { + t.Fatal("compact disk rows bypassed the retained-memory budget") + } + if n := after.TotalAlloc - before.TotalAlloc; n > 4*appRestMemoBytes { + t.Fatalf("compact rows allocated %d bytes, want under %d", n, 4*appRestMemoBytes) + } +} + +func TestAppRestExplicitCloseDrainsReadersOutsideAdmission(t *testing.T) { + b, a := restBackend(t, &restConversation{}) + _, _, endRead, err := b.borrowLive(a.view.ID) + if err != nil { + t.Fatal(err) + } + waiting := make(chan struct{}) + b.waitReaders = func(done <-chan struct{}) { close(waiting); <-done } + closed := make(chan struct{}) + go func() { b.letGo(a); close(closed) }() + <-waiting + if a.gate.TryLock() { + a.gate.Unlock() + } else { + t.Error("close held admission while waiting for a reader that may need it") + } + if b.liveOf(a) == nil { + t.Error("close released an active reader's conversation") + } + endRead() + <-closed + if b.liveOf(a) != nil { + t.Fatal("close failed to release after the last reader returned") + } +} + +func TestAppRestOwnerReadFailureKeepsConversation(t *testing.T) { + c := &restConversation{} + b, a := restBackend(t, c) + var reads atomic.Int32 + c.readTasks = func() ([]session.AppTaskView, error) { + if reads.Add(1) > 1 { + return nil, os.ErrPermission + } + return []session.AppTaskView{{ID: "1", State: "done"}}, nil + } + if _, err := b.ListTasks(a.view.ID); err != nil { + t.Fatal(err) + } + if b.liveOf(a) == nil { + t.Fatal("failed live-work revalidation was treated as idle") + } + if _, ok := b.restMemo.get(a.view.ID, b.restStamp(a)); ok { + t.Error("failed revalidation entered memory cache") + } + if _, err := os.Stat(appRestPath(a.view.ID)); !os.IsNotExist(err) { + t.Error("failed revalidation entered disk cache") + } + +} + +func TestAppRestCellStampIncludesRootPlanAndChildRecords(t *testing.T) { + b, a := restBackend(t, &restConversation{}) + root := t.TempDir() + truth := filepath.Join(root, ".cell") + if err := os.MkdirAll(filepath.Join(truth, "tasks"), 0700); err != nil { + t.Fatal(err) + } + b.locked(func() { a.transcript = filepath.Join(truth, "transcript.jsonl") }) + for _, path := range []string{filepath.Join(root, "plandb.db"), filepath.Join(root, "presence.json"), filepath.Join(truth, ".deleted-tasks.json"), filepath.Join(truth, "tasks", "node.jsonl")} { + if err := os.WriteFile(path, []byte("old"), 0600); err != nil { + t.Fatal(err) + } + before := b.restStamp(a) + if before == "" { + t.Fatal("settled cell fixture was not stampable") + } + if err := os.WriteFile(path+".new", []byte("new"), 0600); err != nil { + t.Fatal(err) + } + if err := os.Rename(path+".new", path); err != nil { + t.Fatal(err) + } + if b.restStamp(a) == before { + t.Errorf("cell cache missed change to %s", path) + } + } +} + +func TestAppRestCellReadsPresenceFromSessionRoot(t *testing.T) { + b, a := restBackend(t, &restConversation{}) + root := t.TempDir() + if err := os.Mkdir(filepath.Join(root, ".cell"), 0700); err != nil { + t.Fatal(err) + } + b.locked(func() { a.transcript = filepath.Join(root, ".cell", "transcript.jsonl") }) + if err := os.WriteFile(filepath.Join(root, "presence.json"), []byte(`{"schema":1,"state":"working"}`), 0600); err != nil { + t.Fatal(err) + } + if b.memberRests(a) { + t.Fatal("cell member with working root presence was admitted as resting") + } +} + +func TestAppRestPanickingReaderReturnsReference(t *testing.T) { + c := &restConversation{} + b, a := restBackend(t, c) + var calls int + c.readTasks = func() ([]session.AppTaskView, error) { + calls++ + if calls == 1 { + panic("reader failed") + } + return nil, nil + } + func() { + defer func() { + if recover() != "reader failed" { + t.Error("listing did not propagate reader panic") + } + }() + _, _ = b.ListTasks(a.view.ID) + }() + var readers int + b.locked(func() { + readers = a.readers + // Repair a leaked reference so a red test can still close its fixture. + if readers > 0 { + a.readers = 0 + close(a.readersDone) + } + }) + if readers != 0 { + t.Fatalf("panicking listing leaked %d reader references", readers) + } + b.letGo(a) + if b.liveOf(a) != nil { + t.Fatal("explicit close did not release the conversation") + } +} + +func TestAppReaderCompletionIsOneShot(t *testing.T) { + b, a := restBackend(t, &restConversation{}) + _, _, endRead, err := b.borrowLive(a.view.ID) + if err != nil { + t.Fatal(err) + } + defer endRead() + endRead() + endRead() + b.locked(func() { + if a.readers != 0 { + t.Errorf("completion returned one reference more than once: readers=%d", a.readers) + } + }) +} + +func TestAppRestOverlayFailureIsNeverCached(t *testing.T) { + c := &restConversation{} + b, a := restBackend(t, c) + checkpoint := `{"type":"tasks","version":1,"seq":1,"nodes":[{"id":1,"title":"Saved settled task","brief":"Already finished","acceptance":"Task finished","state":"done","noted":true}]}` + if err := os.WriteFile(filepath.Join(filepath.Dir(a.transcript), "tasks.json"), []byte(checkpoint), 0600); err != nil { + t.Fatal(err) + } + + var reads int + c.readTasks = func() ([]session.AppTaskView, error) { + reads++ + if reads == 1 { + return nil, os.ErrPermission + } + return []session.AppTaskView{{ID: "1", State: "done", Step: 7, Cap: 12}}, nil + } + if _, err := b.ListTasks(a.view.ID); !errors.Is(err, os.ErrPermission) { + t.Errorf("owner overlay failure was hidden: %v", err) + } + if _, ok := b.restMemo.get(a.view.ID, b.restStamp(a)); ok { + t.Error("failed owner overlay entered memory cache") + } + if _, err := os.Stat(appRestPath(a.view.ID)); !os.IsNotExist(err) { + t.Error("failed owner overlay entered disk cache") + } + for range 2 { + got, err := b.ListTasks(a.view.ID) + if err != nil || len(got) != 1 || got[0].Steps == nil || *got[0].Steps != 7 || got[0].BudgetUSD == nil || *got[0].BudgetUSD != 12 { + t.Fatalf("retry/relaunch lost owner fields: %+v %v", got, err) + } + b.restMemo = appRestMemo{} + } +} + +func TestAppRestStampIgnoresNonTaskPersistence(t *testing.T) { + b, a := restBackend(t, &restConversation{}) + dir := filepath.Dir(a.transcript) + for _, name := range []string{"meta.json", "session.json", "state.json", "told.json", "team.json", "processes/head.json"} { + t.Run(name, func(t *testing.T) { + before := b.restStamp(a) + if before == "" { + t.Fatal("fixture was not stampable") + } + path := filepath.Join(dir, name) + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("changed"), 0600); err != nil { + t.Fatal(err) + } + if got := b.restStamp(a); got != before { + t.Fatalf("non-task persistence %s invalidated snapshot", name) + } + }) + } +} + +func TestAppRestCellStampIncludesDeletionFilters(t *testing.T) { + b, a := restBackend(t, &restConversation{}) + root := t.TempDir() + truth := filepath.Join(root, ".cell") + if err := os.Mkdir(truth, 0700); err != nil { + t.Fatal(err) + } + b.locked(func() { a.transcript = filepath.Join(truth, "transcript.jsonl") }) + // Checkpoint deletion filters live beside the transcript. The project index + // also consults the canonical owner's root for task and conversation deletion. + for _, dir := range []string{root, truth} { + for _, name := range []string{".deleted-tasks.json", ".conversation-deleted"} { + before := b.restStamp(a) + if before == "" { + t.Fatal("fixture was not stampable") + } + if err := os.WriteFile(filepath.Join(dir, name), []byte(`{"1":true}`), 0600); err != nil { + t.Fatal(err) + } + if got := b.restStamp(a); got == before { + t.Fatalf("deletion filter %s/%s did not invalidate snapshot", dir, name) + } + } + } +} diff --git a/cmd/codeaf/appsecret.go b/cmd/codeaf/appsecret.go index fc9b8d2b70..bddc17deae 100644 --- a/cmd/codeaf/appsecret.go +++ b/cmd/codeaf/appsecret.go @@ -10,10 +10,11 @@ import "github.com/Agent-Field/codeaf/internal/appwire" // and this refuses an ask that never asked for a secret, because the words of // an ordinary question go to the model. func (b *appBackend) AnswerAskSecret(agent, id, secret string) error { - a, err := b.findCrewOwner(agent) + a, endRead, err := b.findCrewOwner(agent) if err != nil { return err } + defer endRead() open, takesSecret := false, false b.locked(func() { if a.live == nil { diff --git a/cmd/codeaf/appsessionactions.go b/cmd/codeaf/appsessionactions.go index 9253d99107..e71ea86a8b 100644 --- a/cmd/codeaf/appsessionactions.go +++ b/cmd/codeaf/appsessionactions.go @@ -301,10 +301,11 @@ func (b *appBackend) SessionActions(agent string) (appwire.ActionCapabilities, e return appwire.ActionCapabilities{}, appActionReadsBusy() } defer release() - _, live, err := b.heldLive(agent) + _, live, endLive, err := b.borrowLive(agent) if err != nil { return appwire.ActionCapabilities{}, err } + defer endLive() return appActionDoorsOf(live.conn.Conv).capabilities(agent), nil } diff --git a/cmd/codeaf/appskills.go b/cmd/codeaf/appskills.go index 34b53ed3ac..932235a984 100644 --- a/cmd/codeaf/appskills.go +++ b/cmd/codeaf/appskills.go @@ -19,19 +19,20 @@ type appSkillDoor interface { AttachedSkills() []string } -func (b *appBackend) skillConversation(agent string) (*appLive, appSkillDoor, error) { - _, live, err := b.heldLive(agent) +func (b *appBackend) skillConversation(agent string) (*appLive, appSkillDoor, func(), error) { + _, live, endLive, err := b.borrowLive(agent) if err != nil { - return nil, nil, err + return nil, nil, nil, err } door, ok := live.conn.Conv.(appSkillDoor) if far, asks := live.conn.Conv.(interface{ SkillsSupported() bool }); asks && !far.SkillsSupported() { ok = false } if !ok { - return nil, nil, appwire.Errorf(appwire.CodeUnknownMethod, "this conversation cannot carry attached skills") + endLive() + return nil, nil, nil, appwire.Errorf(appwire.CodeUnknownMethod, "this conversation cannot carry attached skills") } - return live, door, nil + return live, door, endLive, nil } // ListSkills uses the terminal's merge and order. The shelf is always read @@ -42,10 +43,11 @@ func (b *appBackend) ListSkills(agent string) ([]skillpick.Row, error) { return nil, appActionReadsBusy() } defer release() - live, door, err := b.skillConversation(agent) + live, door, endLive, err := b.skillConversation(agent) if err != nil { return nil, err } + defer endLive() facts, readErr := door.SkillFacts(store.FactActive, skillpick.ListLimit) if readErr != nil { return nil, appEngineError(readErr) @@ -76,10 +78,11 @@ func (b *appBackend) ListSkills(agent string) ([]skillpick.Row, error) { // SetSkill calls the terminal's attachment doors and checks the engine's // answer, since an older remote handle may return its previous replica. func (b *appBackend) SetSkill(p appwire.SkillSetParams) (appwire.SkillState, error) { - _, door, err := b.skillConversation(p.Agent) + _, door, endLive, err := b.skillConversation(p.Agent) if err != nil { return appwire.SkillState{}, err } + defer endLive() if p.On { door.AttachSkills(p.Name) } else { diff --git a/cmd/codeaf/appstamp_darwin.go b/cmd/codeaf/appstamp_darwin.go new file mode 100644 index 0000000000..117f42e2f8 --- /dev/null +++ b/cmd/codeaf/appstamp_darwin.go @@ -0,0 +1,22 @@ +//go:build darwin + +package main + +import ( + "os" + "syscall" +) + +// appFileIdentity includes change time because a writer can restore mtime. +func appFileIdentity(info os.FileInfo) (uint64, uint64, int64, bool) { + st, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return 0, 0, 0, false + } + return uint64(st.Dev), uint64(st.Ino), st.Ctimespec.Nano(), true +} + +// openAppRestFile cannot block on a pipe swapped in after the size check. +func openAppRestFile(path string) (*os.File, error) { + return os.OpenFile(path, os.O_RDONLY|syscall.O_NONBLOCK, 0) +} diff --git a/cmd/codeaf/appstamp_linux.go b/cmd/codeaf/appstamp_linux.go new file mode 100644 index 0000000000..ff5a569629 --- /dev/null +++ b/cmd/codeaf/appstamp_linux.go @@ -0,0 +1,22 @@ +//go:build linux + +package main + +import ( + "os" + "syscall" +) + +// appFileIdentity includes change time because a writer can restore mtime. +func appFileIdentity(info os.FileInfo) (uint64, uint64, int64, bool) { + st, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return 0, 0, 0, false + } + return uint64(st.Dev), uint64(st.Ino), st.Ctim.Nano(), true +} + +// openAppRestFile cannot block on a pipe swapped in after the size check. +func openAppRestFile(path string) (*os.File, error) { + return os.OpenFile(path, os.O_RDONLY|syscall.O_NONBLOCK, 0) +} diff --git a/cmd/codeaf/appstamp_other.go b/cmd/codeaf/appstamp_other.go new file mode 100644 index 0000000000..fb9d12c9c8 --- /dev/null +++ b/cmd/codeaf/appstamp_other.go @@ -0,0 +1,10 @@ +//go:build !linux && !darwin + +package main + +import "os" + +// Unknown identity or change time cannot certify a cached reading. +func appFileIdentity(os.FileInfo) (uint64, uint64, int64, bool) { return 0, 0, 0, false } + +func openAppRestFile(path string) (*os.File, error) { return os.Open(path) } diff --git a/cmd/codeaf/apptaskrouting.go b/cmd/codeaf/apptaskrouting.go index 6b05bac867..ab9dfc4047 100644 --- a/cmd/codeaf/apptaskrouting.go +++ b/cmd/codeaf/apptaskrouting.go @@ -22,10 +22,11 @@ func (b *appBackend) TaskRouting(id string, changes int) (appwire.TaskRouting, e if !ok { return appwire.TaskRouting{}, appwire.Errorf(appwire.CodeBadParams, "%s is not a task id", id) } - _, live, err := b.heldLive(agent) + _, live, endLive, err := b.borrowLive(agent) if err != nil { return appwire.TaskRouting{}, err } + defer endLive() view, err := live.conn.Conv.TaskRouting(local, changes) if err != nil { return appwire.TaskRouting{}, appRoutingError(err) @@ -40,7 +41,13 @@ func (b *appBackend) ActOnTaskRouting(p appwire.TaskRoutingActParams) (appwire.T if !ok { return appwire.TaskRoutingActResult{}, appwire.Errorf(appwire.CodeBadParams, "%s is not a task id", p.ID) } - _, live, err := b.heldLive(agent) + a, err := b.find(agent) + if err != nil { + return appwire.TaskRoutingActResult{}, err + } + a.gate.Lock() + defer a.gate.Unlock() + _, live, err := b.heldLiveLocked(a) if err != nil { return appwire.TaskRoutingActResult{}, err } @@ -294,10 +301,11 @@ func (b *appBackend) TaskRoutingOptions(id string) (appwire.TaskRoutingOptionsVi if !ok { return appwire.TaskRoutingOptionsView{}, appwire.Errorf(appwire.CodeBadParams, "%s is not a task id", id) } - _, live, err := b.heldLive(agent) + _, live, endLive, err := b.borrowLive(agent) if err != nil { return appwire.TaskRoutingOptionsView{}, err } + defer endLive() got, err := live.conn.Conv.TaskRoutingOptions(local) if err != nil { return appwire.TaskRoutingOptionsView{}, appRoutingError(err) diff --git a/cmd/codeaf/apptidy.go b/cmd/codeaf/apptidy.go index b0705bfdc2..416986e59a 100644 --- a/cmd/codeaf/apptidy.go +++ b/cmd/codeaf/apptidy.go @@ -256,10 +256,11 @@ func (b *appBackend) unmarkTidy(id string) { // Rename gives an agent the name the person typed, in its conversation. func (b *appBackend) Rename(id, title string) (appwire.Agent, error) { - a, live, err := b.heldLive(id) + a, live, endLive, err := b.borrowLive(id) if err != nil { return appwire.Agent{}, err } + defer endLive() a.nameGate.Lock() defer a.nameGate.Unlock() door, ok := live.conn.Conv.(appRenamer) diff --git a/cmd/codeaf/appwork.go b/cmd/codeaf/appwork.go index 03c4c3d69c..b7de996983 100644 --- a/cmd/codeaf/appwork.go +++ b/cmd/codeaf/appwork.go @@ -128,7 +128,7 @@ func (b *appBackend) processRowFromList(id string) (string, session.JobNotice, e if !ok { return "", session.JobNotice{}, appwire.Errorf(appwire.CodeBadParams, "%s is not a process id", id) } - rows, _, err := b.processRows(agent) + rows, err := b.processRows(agent) if err != nil { return "", session.JobNotice{}, err } @@ -695,10 +695,11 @@ func (b *appBackend) GetTask(id string) (appwire.TaskPage, error) { if !ok { return appwire.TaskPage{}, appwire.Errorf(appwire.CodeBadParams, "%s is not a task id", id) } - _, live, err := b.heldLive(agent) + _, live, endLive, err := b.borrowLive(agent) if err != nil { return appwire.TaskPage{}, err } + defer endLive() view, found, err := live.conn.Conv.AppTask(local) if err != nil { return appwire.TaskPage{}, appEngineError(err) @@ -1026,7 +1027,13 @@ func (b *appBackend) SetTaskModel(id, model, state string, version *uint64) erro if !ok { return appwire.Errorf(appwire.CodeBadParams, "%s is not a task id", id) } - _, live, err := b.heldLive(agent) + a, err := b.find(agent) + if err != nil { + return err + } + a.gate.Lock() + defer a.gate.Unlock() + _, live, err := b.heldLiveLocked(a) if err != nil { return err } @@ -1144,10 +1151,11 @@ func (b *appBackend) UndoQuick(id string) error { if !ok { return appwire.Errorf(appwire.CodeBadParams, "%s is not a quick task id", id) } - _, live, err := b.heldLive(agent) + _, live, endLive, err := b.borrowLive(agent) if err != nil { return err } + defer endLive() _, changed, err := live.conn.Conv.AppQuickTaskAction(session.AppTaskIntent{ID: local, Action: "undo"}) if err != nil { return appTaskError(err) @@ -1325,11 +1333,12 @@ func (b *appBackend) processMoved(agent string, live *appLive, job *session.JobN // processRows is an agent's processes: the live registry's while its // conversation is held, its record on disk otherwise. -func (b *appBackend) processRows(agent string) ([]session.JobNotice, *appLive, error) { - a, err := b.findCrewOwner(agent) +func (b *appBackend) processRows(agent string) ([]session.JobNotice, error) { + a, endRead, err := b.findCrewReader(agent) if err != nil { - return nil, nil, err + return nil, err } + defer endRead() var transcript string live := b.liveOf(a) b.locked(func() { transcript = a.transcript }) @@ -1337,12 +1346,17 @@ func (b *appBackend) processRows(agent string) ([]session.JobNotice, *appLive, e rows, answered, err := appLiveJobNotices(live.conn.Conv, appProcessListWait) if answered { if err != nil { - return nil, nil, appListError(err) + return nil, appListError(err) } for i := range rows { rows[i] = appRecoverJob(rows[i], live.replayJobs) } - return rows, live, nil + b.locked(func() { + for _, row := range rows { + live.logs[appProcessOf(agent, row).ID] = row.LogPath + } + }) + return rows, nil } // THE ENGINE HOLDING IT IS BUSY (a long tool call, a seal of a large // workspace, a quit's wrap-up): the record on disk answers instead, @@ -1351,9 +1365,9 @@ func (b *appBackend) processRows(agent string) ([]session.JobNotice, *appLive, e } rows, err := session.ReadProcessNotices(transcript) if err != nil { - return nil, nil, appListError(err) + return nil, appListError(err) } - return rows, nil, nil + return rows, nil } // appListError says a history the whole list cannot carry as too_large, @@ -1368,7 +1382,7 @@ func appListError(err error) error { // ListProcesses answers an agent's processes, running and ended. func (b *appBackend) ListProcesses(agent string) ([]appwire.Process, error) { - rows, live, err := b.processRows(agent) + rows, err := b.processRows(agent) if err != nil { return nil, err } @@ -1378,9 +1392,6 @@ func (b *appBackend) ListProcesses(agent string) ([]appwire.Process, error) { continue } process := appProcessOf(agent, row) - if live != nil { - b.locked(func() { live.logs[process.ID] = row.LogPath }) - } out = append(out, process) } // One list: the agent's kept-running processes are its processes too. @@ -1499,10 +1510,11 @@ func (b *appBackend) StopProcess(id string) error { return err } job := row.ID - _, live, err := b.heldLive(agent) + _, live, endLive, err := b.borrowLive(agent) if err != nil { return err } + defer endLive() // An older engine cannot prove the run's identity or whether a committed // render refused its stop. Never turn that unknown into a successful Cancel. if served, err := b.stampedStop(live, id, job); served { @@ -1521,7 +1533,13 @@ func (b *appBackend) RerunProcess(id string) (appwire.Process, error) { return appwire.Process{}, err } job := row.ID - _, live, err := b.heldLive(agent) + a, err := b.find(agent) + if err != nil { + return appwire.Process{}, err + } + a.gate.Lock() + defer a.gate.Unlock() + _, live, err := b.heldLiveLocked(a) if err != nil { return appwire.Process{}, err } @@ -1557,10 +1575,11 @@ func (b *appBackend) watchLong(id string, live *appLive, turn uint64, started ti // so, and the task arrives on its own as task.changed with the move's notice. // A reply that ends before it could move leaves a notice saying so. func (b *appBackend) MoveToTask(agent string, turn uint64) (appwire.MoveResult, error) { - a, err := b.findCrewOwner(agent) + a, endRead, err := b.findCrewOwner(agent) if err != nil { return appwire.MoveResult{}, err } + defer endRead() result, proposed, err := b.moveToTaskGated(a, agent, turn) if proposed == nil { return result, err @@ -1702,12 +1721,16 @@ func (b *appBackend) Sleep() error { } }) for _, a := range agents { - live := b.liveOf(a) + var id string + b.locked(func() { id = a.view.ID }) + _, live, endRead, err := b.borrowExisting(id) + if err != nil { + continue + } if live == nil { + endRead() continue } - var id string - b.locked(func() { id = a.view.ID }) rows, err := b.runningJobs(live) if err == nil { for _, row := range rows { @@ -1723,6 +1746,7 @@ func (b *appBackend) Sleep() error { view = a.view }) live.box.Event(appwire.EventAgentChanged, appwire.AgentChanged{Agent: view}) + endRead() } return powerErr } diff --git a/cmd/codeaf/appworkprogram.go b/cmd/codeaf/appworkprogram.go index b90f45c08b..94670ebf76 100644 --- a/cmd/codeaf/appworkprogram.go +++ b/cmd/codeaf/appworkprogram.go @@ -108,10 +108,11 @@ func (b *appBackend) ProgramActions(id, before string) (appwire.ProgramActionsRe if !ok { return appwire.ProgramActionsResult{}, appwire.Errorf(appwire.CodeBadParams, "%s is not a task id", id) } - _, live, err := b.heldLive(agent) + _, live, endLive, err := b.borrowLive(agent) if err != nil { return appwire.ProgramActionsResult{}, err } + defer endLive() pager, ok := live.conn.Conv.(appProgramPager) if !ok { return appwire.ProgramActionsResult{}, appwire.Errorf(appwire.CodeUnknownMethod, "this engine does not serve tasks.program_actions") diff --git a/cmd/codeaf/memtrim.go b/cmd/codeaf/memtrim.go new file mode 100644 index 0000000000..9bf6ef28aa --- /dev/null +++ b/cmd/codeaf/memtrim.go @@ -0,0 +1,173 @@ +package main + +import ( + "context" + "os" + "runtime" + "runtime/debug" + "runtime/metrics" + "sync" + "time" + + "github.com/Agent-Field/codeaf/internal/guard" +) + +// heapTrim* decide when a surface hands idle heap back to the system. +// +// Terminal surfaces begin with a collector target of five times the live heap +// (tuneForTheSurface). The app begins at Go's default and its governor below +// selects a target from the live heap. After a burst of reads the collector runs +// once, and the runtime then returns the freed pages only a little at a time. +// The process sits at several times its live heap for as long as nothing +// allocates. The trimmer closes that gap with one forced collection and a full +// return, and only when what is held and not live is a large share. +const ( + // heapTrimFloor is the smallest excess worth a collection. + heapTrimFloor = 48 << 20 + // heapTrimEvery is how often the heap is looked at, and heapTrimCooldown + // the least time between two trims, so a heap that fills again at once is + // not collected over and over. + heapTrimEvery = 5 * time.Second + heapTrimCooldown = time.Minute + + // heapHeadroom is the garbage the collector may let build up, and + // heapGCMin and heapGCMax bound the percent it is turned into. The + // maximum is the surface's own generous setting, kept for a heap small + // enough that a multiple of it is little memory. + heapHeadroom = 32 << 20 + heapGCMin = 10 + heapGCMax = 400 +) + +// heapGCPercent is the collector percent that lets a heap of live bytes build +// about heapHeadroom of garbage before it is collected. +func heapGCPercent(live uint64) int { + if live == 0 { + return heapGCMax + } + return int(min(max(uint64(heapHeadroom)*100/live, heapGCMin), heapGCMax)) +} + +// heapReading is what the runtime says about its heap: Live is what the last +// collection found reachable, Held is the memory the runtime has mapped for +// itself and not returned. +type heapReading struct { + Live, Held uint64 +} + +// excess is what is held beyond what is live. +func (r heapReading) excess() uint64 { + if r.Held <= r.Live { + return 0 + } + return r.Held - r.Live +} + +// worthTrimming is true when the excess is over the floor and over half of the +// live heap, so a large heap is not collected for a share it needs. +func (r heapReading) worthTrimming() bool { + excess := r.excess() + return excess > heapTrimFloor && excess > r.Live/2 +} + +type heapTrimmer struct { + read func() heapReading + free func() + now func() time.Time + last time.Time + // percent, when set, is given the collector percent the live heap calls + // for each time it moves a fifth from the last one given. + percent func(int) + mu sync.Mutex + given int +} + +// tune gives the collector the percent the live heap calls for. +func (t *heapTrimmer) tune(reading heapReading) { + if t.percent == nil { + return + } + t.mu.Lock() + defer t.mu.Unlock() + want := heapGCPercent(reading.Live) + if t.given == 0 || want*5 < t.given*4 || want*4 > t.given*5 { + t.given = want + t.percent(want) + } +} + +// step is one look at the heap. +func (t *heapTrimmer) step() { + reading := t.read() + t.tune(reading) + now := t.now() + if !t.last.IsZero() && now.Sub(t.last) < heapTrimCooldown { + return + } + if !reading.worthTrimming() { + return + } + t.last = now + t.free() +} + +// readHeap is the runtime's own account, read without stopping the world. +func readHeap() heapReading { + samples := []metrics.Sample{ + {Name: "/gc/heap/live:bytes"}, + {Name: "/memory/classes/total:bytes"}, + {Name: "/memory/classes/heap/released:bytes"}, + } + metrics.Read(samples) + for _, sample := range samples { + if sample.Value.Kind() != metrics.KindUint64 { + return heapReading{} + } + } + live, total, released := samples[0].Value.Uint64(), samples[1].Value.Uint64(), samples[2].Value.Uint64() + if total < released { + return heapReading{} + } + return heapReading{Live: live, Held: total - released} +} + +// startHeapTrimmer looks at the heap until ctx ends. +func startHeapTrimmer(ctx context.Context) { + t := &heapTrimmer{read: readHeap, free: debug.FreeOSMemory, now: time.Now} + // An explicit GOGC decides, as it does in tuneForTheSurface. + if os.Getenv("GOGC") == "" { + t.percent = func(p int) { debug.SetGCPercent(p) } + } + if t.percent != nil { + watchCollections(func() { t.tune(t.read()) }) + } + guard.Go("heap/trim", func() { + t.step() + ticker := time.NewTicker(heapTrimEvery) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + t.step() + } + } + }) +} + +// watchCollections calls after once for every completed collection, so the +// percent follows the heap through a burst instead of lagging a tick behind it. +// A finalizer on an unreachable sentinel runs after the cycle that found it +// unreachable; it plants the next sentinel before it returns. +func watchCollections(after func()) { + type sentinel struct{ _ [16]byte } + var plant func() + plant = func() { + runtime.SetFinalizer(&sentinel{}, func(*sentinel) { + plant() + after() + }) + } + plant() +} diff --git a/cmd/codeaf/memtrim_test.go b/cmd/codeaf/memtrim_test.go new file mode 100644 index 0000000000..5fbaaa3743 --- /dev/null +++ b/cmd/codeaf/memtrim_test.go @@ -0,0 +1,95 @@ +package main + +import ( + "testing" + "time" +) + +// A BURST OF WORK LEAVES THE HEAP WITH MEMORY IT WILL NOT NEED AGAIN FOR HOURS: +// the collector is set to wait for five times the live heap, and the runtime +// hands pages back to the system only slowly. The trimmer gives them back once +// the held-but-unused share is large, and no more often than its cooldown. +func TestHeapTrimmerReturnsOnlyAnExcessAndOnlyOncePerCooldown(t *testing.T) { + const mib = 1 << 20 + clock := time.Unix(1000, 0) + var reading heapReading + trims := 0 + tr := &heapTrimmer{ + read: func() heapReading { return reading }, + free: func() { trims++ }, + now: func() time.Time { return clock }, + } + step := func(live, held uint64) { + reading = heapReading{Live: live * mib, Held: held * mib} + tr.step() + } + + step(60, 100) // 40 MiB over what is live: not worth a collection + if trims != 0 { + t.Fatalf("trimmed %d times for a small excess", trims) + } + step(60, 400) // 340 MiB over + if trims != 1 { + t.Fatalf("trimmed %d times for a large excess, want 1", trims) + } + step(60, 400) + if trims != 1 { + t.Fatalf("trimmed again inside the cooldown (%d)", trims) + } + clock = clock.Add(heapTrimCooldown) + step(60, 400) + if trims != 2 { + t.Fatalf("did not trim after the cooldown (%d)", trims) + } + clock = clock.Add(heapTrimCooldown) + step(400, 600) // a big live heap carries proportionally more + if trims != 2 { + t.Fatalf("trimmed a heap whose excess is under half its live size (%d)", trims) + } +} + +// THE COLLECTOR WAITS FOR A FIXED NUMBER OF BYTES OF GARBAGE, NOT FOR A MULTIPLE +// OF THE LIVE HEAP. A multiple is generous for a small heap and ruinous for a +// large one: five times 120 MiB is 600 MiB of garbage the process holds, and on +// a system that takes freed pages back lazily the resident set shows all of it. +// The percent follows the live heap so the headroom stays about the same. +func TestHeapGovernorKeepsTheHeadroomNearlyConstant(t *testing.T) { + const mib = 1 << 20 + for _, tc := range []struct { + live uint64 + want int + }{ + {0, heapGCMax}, + {5, heapGCMax}, // a small heap keeps the surface's generous multiple + {80, 40}, + {200, 16}, + {4000, heapGCMin}, + } { + if got := heapGCPercent(tc.live * mib); got != tc.want { + t.Errorf("heapGCPercent(%d MiB) = %d, want %d", tc.live, got, tc.want) + } + } + + var set []int + tr := &heapTrimmer{ + read: func() heapReading { return heapReading{Live: 80 * mib} }, + free: func() {}, + now: time.Now, + percent: func(p int) { set = append(set, p) }, + } + tr.step() + tr.step() // the same live heap does not set it again + if len(set) != 1 || set[0] != 40 { + t.Fatalf("percent set %v, want [40] once", set) + } + tr.read = func() heapReading { return heapReading{Live: 84 * mib} } // 38: within a fifth of 40 + tr.step() + if len(set) != 1 { + t.Fatalf("a small move of the live heap set the percent again: %v", set) + } + tr.read = func() heapReading { return heapReading{Live: 200 * mib} } + tr.step() + if len(set) != 2 || set[1] != 16 { + t.Fatalf("percent set %v, want a second setting of 16", set) + } +} diff --git a/docs/changes/unreleased/1933-app-engine-memory.md b/docs/changes/unreleased/1933-app-engine-memory.md new file mode 100644 index 0000000000..0d5b716f18 --- /dev/null +++ b/docs/changes/unreleased/1933-app-engine-memory.md @@ -0,0 +1,50 @@ +--- +kind: changed +title: The desktop engine no longer holds every crew member in memory +pr: 1933 +surface: [engine] +invalidates: + - "A member of an open crew stayed held in memory for as long as its crew was open, and listing tasks, asks or processes opened every member. Now a member with nothing in flight is read from its files, held only for one read to build its task list, and let go after the idle time like any other conversation." + - "A settled checkpoint used to be replaced on every open, preventing task-list cache admission for conversations with saved tasks. Unchanged session persistence now shares byte comparison before atomic replacement, including checkpoints and metadata; historical memory measurements did not record cache coverage and need a fresh run." + - "The journal record cache kept up to 32 records until the app quit. It now keeps at most 8 and drops one that nobody has read for 30 seconds." +--- + +`tasks.list`, `asks.list` and `processes.list` answer for a crew member at rest +without keeping its conversation. The task list is the same owner-backed list as +before; it is remembered while the member's files are unchanged, in memory and +under `/cache/app-tasks` for the next launch (same build only). Plan reads, +the project task index and the run-row listing no longer repeat their reads for +each row, and the collector percent follows the live heap. No protocol field +changed. See docs/perf/memory-2026-10-24.md. + +Idle release now shares task admission's gate and waits for every active reader. +It checks current background work rather than trusting a prior listing. Cache +entries require matching file identities and timestamps before and after their +read; files inside a two-second timestamp tick are not retained. Both cache +paths share read admission, disk decoding is size-bounded, and the 24 MiB memory +account includes retained Go structures and their referenced data. Index eviction +clears discarded references, and record expiration targets the earliest deadline. +The performance note now distinguishes the app's actual GC defaults from the +terminal's settings and identifies the missing historical environment record. + +Reader completion is one-shot and deferred, including when a read panics. Owner +read errors propagate instead of certifying a disk-only task list. Opening a +settled task checkpoint preserves its file identity when its encoded bytes are +unchanged, so real conversations with saved tasks can populate both caches. +Record, rest and project-index caches share the same modification/change-time +settling guard. Disk task-list storage is capped at 96 MiB and 256 files, with +the oldest files evicted first. A first-use scan and minute-spaced idle sweeps +prune entries for missing or deleted agents; ordinary writes reuse the inventory. +Deleting an agent in the app also drops its memory and disk task-list entries. + +Unchanged persistence shares one size-then-content comparison before the existing +private temporary-file and rename write. Checkpoints use that same writer instead +of their own skip. Restoring effort or approval patches only its own metadata +field, preserving the other dial and model identity. The production opener is +covered with saved dials and nonempty settled checkpoints in both folder layouts: +three listings perform one open, including a disk-cache hit. + +Task-list stamps include task and conversation deletion markers, so a crash after +tombstone commit cannot revive deleted tasks from either cache. Stamps exclude +unrelated metadata, beliefs, reading watermarks and background-process records; +docs/perf/memory-2026-10-24.md records the inputs and exclusions. diff --git a/docs/perf/memory-2026-10-24.md b/docs/perf/memory-2026-10-24.md new file mode 100644 index 0000000000..4264c8b78a --- /dev/null +++ b/docs/perf/memory-2026-10-24.md @@ -0,0 +1,179 @@ +# Desktop engine resident memory, 2026-10-24 + +`codeaf app` is the desktop app's engine. On a large home it sat near 1 GB +resident at idle. This records how that was measured, what held the memory, and +what changed. + +## Workload + +A private copy of a real desktop home (cloned with `cp -c`, never the original): +2.5 GB on disk, 20 open crews, 211 agents, 250 journals (the largest 98 MB, ten +over 10 MB, 460 MB in all), 2,638 tasks (one agent has 909, with an 1.1 MB plan +store). Machine: macOS, darwin/arm64, 28 cores. + +## Method + +1. Build `GOOS=darwin GOARCH=arm64 CGO_ENABLED=0 go build ./cmd/codeaf` and run + `env -u GOGC -u GOMEMLIMIT codeaf app --home ` as the desktop app does, + with stdin held open. Both collector environment overrides must be absent. + `CODEAF_DIAGNOSTIC_PROFILE_DIR` and `CODEAF_DIAGNOSTIC_PROFILE_HEAP=1` write a + heap profile at 60 s (`internal/diagprofile`). +2. With no client attached the engine idles at about 180 MB. The rest comes from + what the desktop asks for, so a small driver speaks the JSON-lines protocol the + way the all-crews task view does: `crews.list`, `agents.list`, then `tasks.list` + and `processes.list` for every agent, then `agents.entries` for one chat, then a + second `tasks.list` pass. +3. Sample at 1, 3 and 10 minutes: `ps` RSS, `vmmap --summary` physical footprint + and CPU time. `go tool pprof -sample_index=inuse_space` and `alloc_space` on + the heap profile. +4. Check equivalence: the ids and states of all 2,638 tasks and 2,094 processes + were identical before and after. + +macOS keeps pages the Go runtime frees (`MADV_FREE_REUSABLE`) in a process's +resident size until it needs them, so `ps` RSS reports the peak of a burst. The +physical footprint (Activity Monitor's Memory column) reports what is held. Both +are given. + +## What held the memory (baseline, heap at 60 s: 383 MB in use) + +| Retainer | In use | +| --- | --- | +| Journal replay of held conversations (`newAgent` → `readJournal`) | 275 MB | +| Journal records for appended jobs/steps (`RecordCache`, up to 256 MiB, 32 records, never expired) | 50 MB | +| everything else | about 60 MB | + +The replay came from `findCrewOwner`: every read of a member of an open crew +(`tasks.list`, `asks.list`, `processes.list`) opened its conversation and kept it +held for as long as the crew was open. With `GOGC` unset, the baseline app uses Go’s default collector percent, +100, which targets about twice the live heap. `codeaf app` does not call +`tuneForTheSurface`; the terminal’s initial setting of 400 does not apply. The wave-1 lazy opening (`4e6b49999`) had measured +230 MB; the read paths had since drifted back to opening everything. + +Allocation, not retention, drove the peaks: 17.8 GB were allocated in 60 s, 38% +by `plandb` reloading the whole plan on every read (once per run row), 15% by +decoding the project task index once per conversation, and a quadratic copy of +run rows. + +## Changes + +| Change | Gate | +| --- | --- | +| `plandb.Store` reloads only when SQLite `data_version` or the db/log stamp moved | 200 reads: 153 MB → under 3 MB | +| `AppTasks` opens the plan once for all run rows; `markPlanInterrupted` stops copying rows | 150 rows: 235 MB → under 16 MB | +| Project task index decoded once per file stamp, shared | 40 reads: 180 MB → under 64 MB | +| `RecordCache` drops records unread for 30 s, keeps 8 | `TestAKeptRecordNobodyAsksForIsDroppedWhenItIdles` | +| Members at rest are read, not held; their task list is built in one short hold and remembered by file stamp, in memory (24 MiB LRU) and under `/cache/app-tasks` | `TestAppReadingAnIdleCrewMemberDoesNotKeepItsConversation`, `TestAppReadsARestingMembersTasksFromTheCacheAfterARelaunch` | +| Idle crew members are let go after the 10-minute idle time | `TestAppLetsAnIdleCrewMemberGoAfterTheIdleTime` | +| Collector percent follows the live heap (32 MiB of headroom); unused held heap is returned | `TestHeapGovernorKeepsTheHeadroomNearlyConstant`, `TestHeapTrimmerReturnsOnlyAnExcessAndOnlyOncePerCooldown` | + +## Result + +Engine process, same copy and driver. RSS is `ps`, footprint is `vmmap`. + +| | 1 min | 3 min | 10 min | footprint at 10 min | peak footprint | CPU at 10 min | +| --- | --- | --- | --- | --- | --- | --- | +| Before | 986 MB | 987 MB | 989 MB | 843 MB | 941 MB | 75 s | +| After, first launch (no cache) | 413 MB | 415 MB | 417 MB | 206 MB | 359 MB | 39 s | +| After, relaunch (cache present) | 242 MB | 243 MB | 245 MB | 170 MB | 195 MB | 20 s | + +Heap in use at 60 s: 383 MB before, 89 MB after (relaunch). No growth from 1 to +10 minutes in any run (before +3 MB, after +4 MB and +3 MB). + +For the reproduction command above, `GOGC` and `GOMEMLIMIT` are unset. The +runtime starts at collector percent 100 and the default unlimited soft memory +limit (`math.MaxInt64` bytes). The app installs no half-RAM limit. After this +change, `startHeapTrimmer` adjusts the collector percent to +`clamp(32 MiB * 100 / live heap, 10, 400)` on its first step, after collections +and on its five-second checks, with a 20% change threshold. An explicit +`GOGC=400` disables that adjustment and is a different experiment. The trimmer +also returns unused heap when its excess exceeds both 48 MiB and half the live +heap, at most once per minute. + +The table records the original measurements, not a rerun of the review fixes; +the original per-run environment and runtime settings were not captured. +Review 2 also reproduced a cache-admission failure with a nonempty settled task +checkpoint: opening the owner unconditionally replaced `tasks.json`, changing +the pre-open stamp and preventing either cache from filling. Byte-identical +checkpoint and metadata writes now share one persistence writer. With review 1's snapshot validation, agents +with such checkpoints could not cache their lists. The original measurements +predate that validation and used post-release stamps, so finding 3 alone does +not prove their caches failed to fill for most agents. The driver recorded no +per-agent cache-hit or checkpoint coverage; the table cannot establish that the +current validated cache achieves the same results. Rerun with saved nonempty +checkpoints and record opens, cache admissions and hits before drawing that +conclusion. The deterministic regression now proves one open across three +listings (build, memory hit, disk hit) through `appOpenInProcess`, with saved +effort and approval and a real settled task, in legacy folders and cell folders. +Reproduce with the explicit unset environment above before attributing the +numbers to a particular collector configuration. The 245 MB figure meets the +250 MB target on a relaunch. The first launch +does not: building each member's task list once replays its journal, and the +largest journals (98 MB, 31 MB) drive a transient peak of 359 MB footprint. + +## Cache bounds and validity + +The 24 MiB task-list budget estimates retained Go memory: task structs, slice +capacity, pointed-to values, string bytes, stamps, entry and LRU storage, plus +128 bytes per entry for map/allocator overhead. Shared references are counted +again conservatively; process-owned time-zone data is excluded. This is an +estimate of retained cache data, not a process RSS ceiling. + +Both memory and disk task-list reads use the same two read slots. Disk files +are stat'ed before decoding and capped at 24 MiB; an entry the memory budget +rejects is never written. Aggregate storage is capped at 96 MiB and 256 files, +oldest writes evicted first. The initial inventory scan and minute-spaced idle +sweeps prune cache files whose agent folder is missing or marked deleted; normal +writes update the inventory without rescanning. App deletion drops both cache +entries immediately. Invalid disk entries are removed. Files must match +replacement-sensitive stamps from before opening through the completed read +and release. Files modified or changed within two seconds are read afresh. Task +child files and both a cell’s truth folder and session root are stamped too. +Flat legacy journals with external dependencies are read without caching. An unstable read is returned but never cached. +Owner-overlay errors propagate and are never cached. Byte-identical persistence +writes preserve the file identity needed to validate settled conversations. The +shared writer checks regular-file size before reading and comparing bytes; +changed bytes still use a private temporary file and atomic rename. Each dial +stamp patches its own field, so restoring one cannot clear the other or replace +the model identity with a new opening epoch. +Reader references keep the owner alive, with deferred one-shot completion even +when listing panics; idle release rechecks current tasks, +quick tasks and jobs under the same gate that admits work. + +### Task-list stamp inputs + +The stamp follows `listTasksHeld` and the owner-backed `AppTasks` overlay: + +| Input | Why it affects the listing | +| --- | --- | +| `tasks.json` and the project task index | Persisted task rows and their state. | +| `.deleted-tasks.json` beside the journal and at the canonical index owner | Both checkpoint and index readers filter tombstoned tasks before cleanup finishes. | +| `.conversation-deleted` | The index reader also filters deleted owners. | +| `transcript.jsonl`, `tasks/` recursively | Recovered task/run information, work logs, steps, program records and plan trajectories. | +| `plandb.db`, its WAL and contiguous archived stores/WALs | Owner plan rows, holds, progress and notes. | +| Root `presence.json` | Interruption and quiet-member checks. | +| Profile budget config | The per-task cap carried on owner task views. | + +The following files are excluded individually: + +| Excluded input | Why it does not define task rows | +| --- | --- | +| `meta.json` | Conversation identity and summary, including legacy effort/approval; task identity and state come from the checkpoint/index. | +| `session.json` | Cell conversation dials and working context; task controls are checkpoint/plan records. | +| `state.json` | Conversation beliefs used in prompts, rather than task-list data. | +| `told.json` | What the conversation has already been told about other work. | +| `team.json` | Team-traffic reading cursors. | +| `processes/` | Background jobs shown through the separate process listing. Its first-read head creation must not invalidate a task snapshot. | + +Deletion regression fixtures commit only the tombstone, leaving checkpoint and +index identities intact. Both memory and disk cache reads then agree with the +uncached readers and omit the deleted checkpoint and index-only tasks. + +## Left + +- First launch peak: a read-only way to build the task list without replaying the + journal (a task-only open) would remove it; `Open` also writes recovery state, + so it needs care. +- A held conversation costs about its journal size. Journals above a compaction + keep every display row in memory. +- No protocol field changed and none needs paging by the UI. `tasks.list` of one + agent can be 3 MB; paging it would shrink the largest transient further. diff --git a/internal/manual/chat/commands.md b/internal/manual/chat/commands.md index c473855e8b..866d579c6e 100644 --- a/internal/manual/chat/commands.md +++ b/internal/manual/chat/commands.md @@ -1047,6 +1047,13 @@ using the same working root. Older admitted cache locations remain in use; `/cache` includes them in its size and lists them before an explicit clean. Temporary files and saved work are outside the cache cleanup. +The desktop engine also keeps rebuildable task-list answers in +`~/.codeaf/cache/app-tasks`, capped at 96 MiB total and 256 files. Oldest files +are evicted first. The engine prunes entries for missing or deleted conversations +on its first inventory scan and once per minute while running; deleting a +conversation in the app drops its entry immediately. Cleaning this cache makes +the next task listing rebuild its answer. + With nothing in it, `/cache` answers `the cache is empty · ~/.codeaf/cache`. **The cache is not your conversations.** Conversation history, tasks, settings and diff --git a/internal/session/approvalposture.go b/internal/session/approvalposture.go index 7cfa0f6794..7d51974518 100644 --- a/internal/session/approvalposture.go +++ b/internal/session/approvalposture.go @@ -260,7 +260,14 @@ func (a *Agent) guardianOn() bool { // stampApproval writes the posture down the moment it changes, for // [Agent.stampEffort]'s reason: a person who opens the gate and closes the // terminal before saying anything else must find it open when they come back. -func (a *Agent) stampApproval() { a.stampMeta() } +func (a *Agent) stampApproval() { + if metaStampSettling != nil { + metaStampSettling() + } + a.metaStamp().settle() + dir, snapshot := a.metaSnapshotAt() + a.updateMeta(dir, snapshot, func(meta *Meta) { meta.Approval = snapshot.Approval }) +} func knownPosture(word string) bool { for _, known := range ApprovalPostures { diff --git a/internal/session/persistence_identity_test.go b/internal/session/persistence_identity_test.go new file mode 100644 index 0000000000..cb77120ec7 --- /dev/null +++ b/internal/session/persistence_identity_test.go @@ -0,0 +1,181 @@ +package session + +import ( + "os" + "path/filepath" + "testing" + "time" +) + +// These are the persistence destinations of a settled opener: task recovery, +// metadata summary and cell truth. The production opener test also exercises +// the approval/effort setters that reach SaveMeta. +func TestSessionPersistenceWritersPreserveUnchangedFiles(t *testing.T) { + for _, writer := range []string{"checkpoint", "metadata", "cell metadata"} { + t.Run(writer, func(t *testing.T) { + dir := t.TempDir() + place := Place{Dir: dir, Workspace: dir} + if writer == "cell metadata" { + if err := os.Mkdir(filepath.Join(dir, ".cell"), 0700); err != nil { + t.Fatal(err) + } + } + meta := Meta{ID: "saved", Workspace: dir, Created: time.Unix(100, 0), Title: "high", Effort: "high", Approval: PostureAsk} + document := taskDocument{Type: taskDocumentType, Version: taskFileVersion, Seq: 1, Nodes: []taskRecord{{ID: 1, Title: "high", Brief: "finished", Acceptance: "finished", State: TaskDone, Noted: true}}} + paths := []string{place.MetaPath()} + write := func() error { return SaveMeta(dir, meta) } + if writer == "checkpoint" { + paths = []string{place.Tasks()} + store := newTaskStore(place.Tasks()) + write = func() error { return store.writeLocked(document) } + } + if writer == "cell metadata" { + paths = append(paths, filepath.Join(dir, ".cell", "session.json")) + } + if err := write(); err != nil { + t.Fatal(err) + } + // A fixed old timestamp makes even an in-place rewrite detectable without + // sleeps or relying on the filesystem timestamp resolution. + old := time.Unix(200, 0) + before := map[string]os.FileInfo{} + for _, path := range paths { + if err := os.Chtimes(path, old, old); err != nil { + t.Fatal(err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + before[path] = info + } + for range 2 { + if err := write(); err != nil { + t.Fatal(err) + } + for _, path := range paths { + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if !os.SameFile(before[path], info) || !info.ModTime().Equal(old) { + t.Errorf("unchanged writer replaced %s", path) + } + } + } + // Both same-size content changes and size changes must still be persisted. + for _, title := range []string{"auto", "a longer title"} { + meta.Title, meta.Effort, document.Nodes[0].Title = title, title, title + if err := write(); err != nil { + t.Fatal(err) + } + if writer == "checkpoint" { + got, ok := loadTaskCheckpoint(place.Tasks()) + if !ok || len(got.Nodes) != 1 || got.Nodes[0].Title != title { + t.Fatalf("changed checkpoint missing: %+v", got) + } + } else { + got, err := LoadMeta(dir) + if err != nil || got.Title != title || got.Effort != title { + t.Fatalf("changed metadata missing: %+v, %v", got, err) + } + } + for _, path := range paths { + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if os.SameFile(before[path], info) { + t.Errorf("changed writer did not replace %s", path) + } + if info.Mode().Perm() != 0600 { + t.Errorf("%s permissions = %o, want 600", path, info.Mode().Perm()) + } + before[path] = info + } + } + }) + } +} + +// List the settled opener's session writers, rather than relying on one +// checkpoint regression to stand for every write performed during restoration. +// Task recovery writes the recovered graph's snapshot. Pending-run/index/answer +// reconciliation writes only for a state transition; list those too. Presence +// has a changing heartbeat and is removed on close, rather than persisted state. +func TestSessionOpenerWritersAreIdempotent(t *testing.T) { + for _, layout := range []string{"legacy", "cell"} { + t.Run(layout, func(t *testing.T) { + dir := t.TempDir() + if layout == "cell" { + if err := os.Mkdir(filepath.Join(dir, ".cell"), 0700); err != nil { + t.Fatal(err) + } + } + place := Place{Dir: dir, Workspace: dir} + agent, _ := newTestAgent(t, &scriptedCompleter{}, func(cfg *Config) { + cfg.SessionFile, cfg.Place = place.Transcript(), place + cfg.ApprovalGate = &fakeApprovalGate{standing: PostureAsk} + }) + agent.SetConversationEffort("high") + if err := agent.SetApprovalPosture(PostureAsk); err != nil { + t.Fatal(err) + } + agent.writeSpend(dir, 1, 10) + agent.SettleWrites() + document := taskDocument{Type: taskDocumentType, Version: taskFileVersion, Seq: 1, Nodes: []taskRecord{{ID: 1, Title: "Saved settled task", Brief: "finished", Acceptance: "finished", State: TaskDone, Noted: true}}} + if err := newTaskStore(place.Tasks()).writeLocked(document); err != nil { + t.Fatal(err) + } + agent.recoverTasks() + writers := []struct { + name string + write func() + }{ + {"recoverTasks/graph.checkpoint", func() { agent.graph().checkpoint() }}, + {"recoverBeltRun", agent.recoverBeltRun}, + {"endInterruptedProgramRun", agent.endInterruptedProgramRun}, + {"closeInflightTaskIndexRows", agent.closeInflightTaskIndexRows}, + {"drainAnswers", agent.drainAnswers}, + {"restore approval/SaveMeta", func() { + if err := agent.SetApprovalPosture(PostureAsk); err != nil { + t.Fatal(err) + } + }}, + {"restore effort/SaveMeta", func() { + if !agent.SetConversationEffort("high") { + t.Fatal("effort rejected") + } + }}, + {"restore spend/SaveMeta", func() { agent.stampRestoredSpend(Usage{CostUSD: 1, Input: 10}) }}, + } + paths := []string{place.Tasks(), place.MetaPath()} + if layout == "cell" { + paths = append(paths, filepath.Join(dir, ".cell", "session.json")) + } + for _, writer := range writers { + t.Run(writer.name, func(t *testing.T) { + before := map[string]os.FileInfo{} + for _, path := range paths { + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + before[path] = info + } + writer.write() + agent.SettleWrites() + for _, path := range paths { + after, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if !os.SameFile(before[path], after) || !before[path].ModTime().Equal(after.ModTime()) { + t.Errorf("%s replaced unchanged %s", writer.name, path) + } + } + }) + } + }) + } +} diff --git a/internal/session/place.go b/internal/session/place.go index 2e1187b628..33184fdb04 100644 --- a/internal/session/place.go +++ b/internal/session/place.go @@ -16,6 +16,7 @@ package session import ( + "bytes" "encoding/json" "errors" "fmt" @@ -545,7 +546,9 @@ func saveMetaLocked(dir string, meta Meta) error { return nil } -// writeJSONAtomic replaces path with v's JSON, temp-and-rename beside it. +// writeJSONAtomic preserves file identity for identical JSON. It checks size +// before comparing bytes; changed values still land whole through a private +// temporary file and atomic rename. func writeJSONAtomic(path string, v any) error { raw, err := json.MarshalIndent(v, "", " ") if err != nil { @@ -561,12 +564,16 @@ func writeJSONAtomic(path string, v any) error { raw = value.AgentIdentity.PreserveJSON(raw) } } + raw = append(raw, '\n') + if sameFileBytes(path, raw) { + return nil + } tmp, err := os.CreateTemp(filepath.Dir(path), ".meta-*.json") if err != nil { return err } name := tmp.Name() - if _, err := tmp.Write(append(raw, '\n')); err != nil { + if _, err := tmp.Write(raw); err != nil { tmp.Close() os.Remove(name) return err @@ -582,6 +589,17 @@ func writeJSONAtomic(path string, v any) error { return nil } +// sameFileBytes rejects nonregular files before reading; only equal sizes need +// a content comparison. Read failures leave replacement to the atomic writer. +func sameFileBytes(path string, raw []byte) bool { + info, err := os.Lstat(path) + if err != nil || !info.Mode().IsRegular() || info.Size() != int64(len(raw)) { + return false + } + previous, err := os.ReadFile(path) + return err == nil && bytes.Equal(previous, raw) +} + // MovedCopyWord is how every surface marks a chat that works in its own copy // of a project that lives on another machine: "proj-a (copy from another // machine)". It is one const because the footer (internal/tui3's [movedWord]) diff --git a/internal/session/placemeta.go b/internal/session/placemeta.go index 116ea30398..a7842b6d5d 100644 --- a/internal/session/placemeta.go +++ b/internal/session/placemeta.go @@ -377,7 +377,14 @@ func (a *Agent) fillMetaLocked(meta Meta) Meta { // // EVERY FAILURE IS SILENCE, for this file's stated reason: the rung is a // convenience and the session is the record. -func (a *Agent) stampEffort() { a.stampMeta() } +func (a *Agent) stampEffort() { + if metaStampSettling != nil { + metaStampSettling() + } + a.metaStamp().settle() + dir, snapshot := a.metaSnapshotAt() + a.updateMeta(dir, snapshot, func(meta *Meta) { meta.Effort = snapshot.Effort }) +} // stampPlaces writes the folders this conversation is about down the moment the // set changes, for [Agent.stampEffort]'s reason: a person who names a place and @@ -399,9 +406,12 @@ func (a *Agent) stampPlaces() { a.stampMeta() } // bytes, and it happens once per file first written rather than once per write. func (a *Agent) stampTrees() { a.stampMeta() } -// stampMeta updates the configuration and working context those three stamps -// share. The transaction reads the latest title and spending from disk and -// preserves them; this snapshot owns only model, effort, approval, places and trees. +// stampMeta updates the configuration and working context [Agent.stampPlaces] +// and [Agent.stampTrees] share. The transaction reads the latest title and +// spending from disk and preserves them; this snapshot owns only model, effort, +// approval, places and trees. [Agent.stampEffort] and [Agent.stampApproval] +// settle the same way but patch only their own field, so restoring a dial when a +// conversation opens leaves the file untouched when nothing moved (#1933). // EVERY FAILURE IS SILENCE, for this file's stated reason. // // AN EARLIER DEFERRED STAMP LANDS FIRST. [Agent.stampUserLocked] captures the diff --git a/internal/session/task_store.go b/internal/session/task_store.go index 4f96e7f3e9..b4114e520f 100644 --- a/internal/session/task_store.go +++ b/internal/session/task_store.go @@ -1011,6 +1011,10 @@ func (s *taskStore) writeLocked(document taskDocument) error { // replaceTaskCheckpoint makes stale temporary files private before writing // and installs the complete checkpoint only after the file closes successfully. func replaceTaskCheckpoint(path string, encoded []byte) error { + raw := append(encoded, '\n') + if sameFileBytes(path, raw) { + return nil + } if directory := filepath.Dir(path); directory != "" && directory != "." { if err := os.MkdirAll(directory, 0o755); err != nil { return err @@ -1031,7 +1035,7 @@ func replaceTaskCheckpoint(path string, encoded []byte) error { file.Close() return err } - _, writeErr := file.Write(append(encoded, '\n')) + _, writeErr := file.Write(raw) closeErr := file.Close() if err := errors.Join(writeErr, closeErr); err != nil { return err @@ -1934,6 +1938,7 @@ func (a *Agent) recoverTasks() { graph := a.graph() recovery := graph.rehydrate(document, a.config.Workspace, a.settlePolicy()) + graph.prepareNodeTreeLimits() // The consume-once receipt reaches the disk BEFORE anything else happens: a // second crash between here and the first turn must not hand the same // interrupt to a second recovery.