diff --git a/AGENTS.md b/AGENTS.md index 877c5c67..26891ec2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -73,6 +73,7 @@ Do not push, publish releases, or modify remote issues/PRs unless the maintainer - See `docs/extensions/README.md` for the extension architecture and `docs/extensions/adding-a-tool.md` for the step-by-step guide; every tool or feature extension must include a `README.md`. - `extensions/tools//spec.yaml` (`kind: sandbox`) defines tool runtime, auth, network, settings, and provider behavior. - `extensions/features//spec.yaml` (`kind: mixin`) defines optional packages, runtime setup, auth, network, and published-port behavior; a feature may also ship a `skills/` directory that is composed into the tool's skills when the feature is enabled. +- Memory scope, disable arguments, and preserved runtime state are declared with `sandbox.memoryScope`, `sandbox.noMemoryArgs`, and `sandbox.statePaths`; see [Agent Memory](docs/runtime/stores.md#agent-memory) for lifecycle and cleanup behavior. - Tool settings templates live under `extensions/tools//templates/` and are baked into the image. - Full host config overrides use `~/.config/enclave/tools//` globally and `~/.config/enclave/projects///config/` per project. - JSON/TOML patches mirror native paths under `~/.config/enclave/patches//` globally and `~/.config/enclave/projects//patches//` per project. @@ -87,7 +88,7 @@ Enclave follows platform-standard roots: the XDG base directories on Linux and o - State: `~/.local/state/enclave/` (`$XDG_STATE_HOME`) - Cache: `~/.cache/enclave/` (`$XDG_CACHE_HOME`) - Embedded runtime assets: `~/.cache/enclave/assets//` -- Per-project agent memory: `~/.local/state/enclave/projects///memory/` (Claude only; agent-writable, never shared between projects or agents) +- Per-project agent memory: `~/.local/state/enclave/projects///memory/` (agent-writable; Claude uses this root, Codex uses `/` matching its config-store key; never shared between projects or agents) - User-defined subcommands: `~/.config/enclave/commands/{host,session}/` (executable files become `enclave ` verbs) Per-project config/state is keyed by project hash and kept outside the worktree. See `docs/configuration.md` and `docs/runtime/stores.md` for details. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 88303d74..06b3c839 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -171,7 +171,7 @@ The restricted network request flow has a separate ## Key Concepts -- **Profiles** (`extensions/tools//spec.yaml`, `kind: sandbox`): define tool command, session continuation args (`continueArgs`, `resumeArgs`), config location, optional settings/skills metadata (`settingsFile`, `settingsTarget`, `skillsDir`), optional host passthrough allow-list (`passthroughPaths`), optional QEMU bundle minimum memory (`qemuMinMemoryMiB`) and config-store cache hint (`qemuStoreCacheMmap`), declared credential sources (`credentials.sources`) including API-key metadata, YOLO flag, and per-provider auth configuration (`providers`: credentials, auth files, auth session checks, OAuth ports). +- **Profiles** (`extensions/tools//spec.yaml`, `kind: sandbox`): define tool command, session continuation args (`continueArgs`, `resumeArgs`), config location, optional settings/skills metadata (`settingsFile`, `settingsTarget`, `skillsDir`), optional agent-memory policy (`memoryDir`, `memoryScope`, `noMemoryArgs`) and pinned runtime state (`statePaths`), optional host passthrough allow-list (`passthroughPaths`), optional QEMU bundle minimum memory (`qemuMinMemoryMiB`) and config-store cache hint (`qemuStoreCacheMmap`), declared credential sources (`credentials.sources`) including API-key metadata, YOLO flag, and per-provider auth configuration (`providers`: credentials, auth files, auth session checks, OAuth ports). - **Runtime assets** (`runtime-assets/gateway-allowlists/`, `runtime-assets/build-scripts/`, `runtime-assets/auth-reconcile.sh`, `runtime-assets/net.sh`): DNS allowlists, Docker weaving scripts, and shared entrypoint helpers baked into the image. Tool templates live in `extensions/tools//templates/` and are aggregated during build. - **Asset discovery**: `ENCLAVE_HOME` has explicit precedence, followed by a valid app root above the resolved executable path. This keeps package-managed installs on `/usr/share/enclave` and in-tree builds on live checkout files. Other binaries extract their embedded assets into an append-only `assets//` directory under the platform cache root. The former unversioned data-root lookup is not used. - **Image selection**: images are per-tool. The default tag is `enclave-:latest` for the selected `--tool` (default `claude`); `--slim` uses `enclave-:slim`. When running from a git checkout on a non-default branch, the tag is prefixed with the branch name and hash (for example, `enclave-codex:branch---latest`) to avoid overwriting main images. `--base-image` or devcontainer mode derives a separate tag (e.g., `enclave-codex:base--latest`) unless `--image-name` is set. @@ -220,7 +220,7 @@ Other host-side data: - `yarn/` - Yarn cache - `bun/` - Bun cache - **History**: `~/.local/state/enclave/projects///history/` for shell history. -- **Agent memory**: `~/.local/state/enclave/projects///memory/` for per-project, agent-writable memory (Claude only). Bind-mounted into the harness's native memory path (writable), never shared between projects or agents, disabled with `--no-memory`, and skipped for ephemeral (`--ephemeral`) sessions. +- **Agent memory**: `~/.local/state/enclave/projects///memory/` for per-project, agent-writable memory, bind-mounted into the harness's native memory path (`sandbox.memoryDir`). Never shared between projects or agents, disabled with `--no-memory`, and skipped for ephemeral (`--ephemeral`) sessions. `sandbox.memoryScope: session` keys it by config-store key (`memory//`), and `sandbox.statePaths` protects tool runtime state during config overlays. See [Agent Memory](runtime/stores.md#agent-memory) for isolation and cleanup policy. - **Home config files**: `~/.local/state/enclave/projects///home-config/` for host-home files created in the container: - `npmrc` → `~/.npmrc` - `yarnrc` → `~/.yarnrc` diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 2aca34fb..80f87ec0 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -189,9 +189,15 @@ Mutation commands (`add-domain`, `remove-domain`, `set-mode`) apply the new poli | `enclave cleanup --all` | All projects and tools | | `enclave cleanup --ephemeral` | Remove stopped containers and ephemeral session stores | | `enclave cleanup --dry-run` | Preview what would be removed | -| `enclave cleanup --keep cache,history,auth,memory` | Preserve the listed stores (comma-separated or repeated `--keep`): `cache` (package caches), `history` (shell history), `auth` (auth stores, with `--all`), `memory` (per-project agent memory, no selective effect with `--all`) | +| `enclave cleanup --keep cache,history,auth,memory` | Preserve the listed stores (comma-separated or repeated `--keep`): `cache` (package caches), `history` (shell history and the config store, including conversation history), `auth` (auth stores, with `--all`), `memory` (per-project agent memory, no selective effect with `--all`) | | `enclave cleanup --build-cache` | Prune Docker build cache (requires confirmation) | +For a tool with session-scoped memory (Codex), `history` and `memory` name one +unit: `--keep memory` also preserves the config store and `--keep history` also +preserves memory. With `--ephemeral`, `--keep memory` preserves each session +store that holds memory; the other `--keep` kinds do not apply there. See +[Agent Memory](runtime/stores.md#agent-memory). + --- ## Flags @@ -266,7 +272,7 @@ Mutation commands (`add-domain`, `remove-domain`, `set-mode`) apply the new poli |------|-------------| | `--no-cache` | Disable package caches | | `--no-history` | Disable shell history | -| `--no-memory` | Disable per-project agent memory | +| `--no-memory` | Disable per-project agent memory; see [memory controls](runtime/stores.md#agent-memory) | --- diff --git a/docs/configuration.md b/docs/configuration.md index 1b7642aa..677722a4 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -63,7 +63,7 @@ supported under `tool_overrides.`. | `allow_domains` | Extra domains added to the gateway allowlist (bare DNS names; ignored when `allow_all_network=true`) | | `no_cache` | Disable package caches | | `no_history` | Disable shell history | -| `no_memory` | Disable per-project agent memory | +| `no_memory` | Disable per-project agent memory; see [memory controls](runtime/stores.md#agent-memory) | | `session_monitor` | Run agents under the managed tmux session (enables `status` snapshots) | | `session_tint` | Terminal background color marking a session-owned terminal, as `#rrggbb` (unset: no tint) | | `base_image` | Docker base image override | diff --git a/docs/extensions/README.md b/docs/extensions/README.md index b1735eb1..0f8a2eb7 100644 --- a/docs/extensions/README.md +++ b/docs/extensions/README.md @@ -304,12 +304,29 @@ providers: - { file: .credentials.json, type: file_exists } ``` -`sandbox.*` fields (`configDir`, `skillsDir`, `memoryDir`, -`settingsFile`, `settingsTarget`, `yoloFlag`, `yoloEnabled`, `continueArgs`, `resumeArgs`, -`passthroughPaths`, `qemuMinMemoryMiB`, `qemuStoreCacheMmap`, -`hostConfigDir`, `hostCredentialsFile`, and `hostOauthJson`) are enclave-native -tool metadata. `sandbox.entrypoint.run` is the shared sbx-style command to -launch the tool. +`sandbox.*` fields (`configDir`, `skillsDir`, `memoryDir`, `memoryScope`, +`noMemoryArgs`, `statePaths`, `settingsFile`, `settingsTarget`, `yoloFlag`, +`yoloEnabled`, `continueArgs`, `resumeArgs`, `passthroughPaths`, +`qemuMinMemoryMiB`, `qemuStoreCacheMmap`, `hostConfigDir`, `hostCredentialsFile`, +and `hostOauthJson`) are enclave-native tool metadata. `sandbox.entrypoint.run` +is the shared sbx-style command to launch the tool. + +Memory and runtime state policy is declared in the spec, including for installed +third-party tools: + +- `memoryDir`: home-relative native memory directory. +- `memoryScope`: `project` (default) shares memory within a project/tool; + `session` requires `memoryDir` and `configDir` and pairs memory with each config + store for writer isolation and cleanup. +- `noMemoryArgs`: argv inserted before user arguments for `--no-memory` and + `--ephemeral`. Use native controls to disable memory use and generation. + Entries are trimmed and blank ones dropped, as for `continueArgs`/`resumeArgs`. +- `statePaths`: config-relative runtime-state paths to preserve during overlays + and exclude from host config passthrough. Requires `configDir`. A trailing `/` + matches a directory tree; globs match paths or basenames. Absolute paths, + traversal, and selection directives are rejected. + +See [Agent Memory](../runtime/stores.md#agent-memory) for lifecycle semantics. When using `templates/`, set `sandbox.configDir`, `sandbox.settingsFile` (aggregated name like `-settings.json`: the `-` prefix followed by diff --git a/docs/extensions/adding-a-tool.md b/docs/extensions/adding-a-tool.md index eef66217..e1977233 100644 --- a/docs/extensions/adding-a-tool.md +++ b/docs/extensions/adding-a-tool.md @@ -66,6 +66,12 @@ the complete set and semantics): - `skillsDir`: (optional) path below `configDir` where shared and tool-specific managed skills are composed. It may be home-relative or absolute, matching `configDir`. +- `memoryDir` / `memoryScope` / `noMemoryArgs`: (optional) native memory path, + project or session scope, and native disable arguments. See + [Agent Memory](../runtime/stores.md#agent-memory) before enabling background + memory generation. +- `statePaths`: (optional) config-relative runtime state preserved across config + overlays and excluded from host config passthrough (including database sidecars). - `settingsFile` / `settingsTarget`: aggregated template filename under `/usr/local/share/enclave/templates/` and its target below `configDir`. - `yoloFlag` / `yoloEnabled`: flag to skip approvals, and whether yolo mode is on diff --git a/docs/extensions/installing.md b/docs/extensions/installing.md index 2ae85484..a1c25319 100644 --- a/docs/extensions/installing.md +++ b/docs/extensions/installing.md @@ -91,10 +91,13 @@ runs in a container on each automatic update check, widens the network allowlist or denies domains, publishes a container port on the host, declares credentials and where they're released as HTTP headers, whether it flips on the approval-bypass flag (`sandbox.yoloFlag`, active by default once declared) or -appends its own argv to the agent for `--continue`/`--resume` -(`sandbox.continueArgs`/`resumeArgs`, which can carry the same flag), launches -an IDE on your host after start (`postStart.openIDE`), ships skills, host -config/credential passthrough, or seeds files into your project directory. An +appends its own argv to the agent for `--continue`/`--resume`/`--no-memory` +(`sandbox.continueArgs`/`resumeArgs`/`noMemoryArgs`, which can carry the same +flag), keeps a writable agent-memory directory between sessions and at what +scope (`sandbox.memoryDir`/`memoryScope`), pins store paths the config overlay +may not replace (`sandbox.statePaths`), launches an IDE on your host after start +(`postStart.openIDE`), ships skills, host config/credential passthrough, or +seeds files into your project directory. An update shows the same information as a diff against what's currently installed, so a newly granted capability is visible before you accept it. diff --git a/docs/persistence.md b/docs/persistence.md index 2013da6f..d3897af7 100644 --- a/docs/persistence.md +++ b/docs/persistence.md @@ -45,7 +45,7 @@ Per-project data is stored on the host and reused across sessions: |------|----------| | Package caches | `~/.cache/enclave///` | | Shell history | `~/.local/state/enclave/projects///history/` | -| Agent memory | `~/.local/state/enclave/projects///memory/` (Claude) | +| Agent memory | `~/.local/state/enclave/projects///memory/` (Claude); `memory//` (Codex, matching the config-store key) | | Config/env/auth stores | Host directories under `~/.local/state/enclave/` (bind-mounted; no Docker volumes) | | Embedded runtime assets | `~/.cache/enclave/assets//` | @@ -58,8 +58,8 @@ the standard Apple locations, in a reverse-DNS application directory: config and state under `~/Library/Application Support/org.eclipse.enclave/` (`config/`, `state/`) and caches under `~/Library/Caches/org.eclipse.enclave/`. -Agent memory is skipped for `--ephemeral` sessions: memory written during them -is discarded with the session's config store. +See [Agent Memory](runtime/stores.md#agent-memory) for memory isolation, +`--no-memory`, ephemeral runs, and cleanup retention rules. Disable specific persistence: diff --git a/docs/runtime/stores.md b/docs/runtime/stores.md index 159817f2..cf4f5d14 100644 --- a/docs/runtime/stores.md +++ b/docs/runtime/stores.md @@ -31,7 +31,12 @@ the `config-store/default` directory. Additional concurrent sessions that would otherwise clobber the same writable config instead get a suffixed store keyed by session or worktree identity, while shared auth remains in the tool-global auth store. Tool state such as conversation history, settings, and cached tokens -persists between runs. +persists between runs. A tool's `sandbox.statePaths` declares config-relative +runtime state that must survive config-source overlays and stay out of host +config passthrough. Codex pins the database indexing its memories there, along +with its `state_*.sqlite*` and `thread_history_*.sqlite*` thread databases. The +thread databases are rebuildable projections of the preserved `sessions/` +rollouts, pinned to avoid re-deriving them on every overlaid run. **Ephemeral mode** (`--ephemeral`): A fresh store directory is created with a unique suffix key for each session and removed after the container exits. The @@ -39,6 +44,70 @@ unique suffix key for each session and removed after the container exits. The Source: [`internal/runtime/volume_manager.go`](../../internal/runtime/volume_manager.go) `BuildPrep` (intent), [`internal/backend/docker/prepare.go`](../../internal/backend/docker/prepare.go) `prepareConfigStore` (mechanics) +## Agent Memory + +Tools declare their native memory directory with `sandbox.memoryDir`. Host memory +lives under `~/.local/state/enclave/projects///memory/`: + +| Scope | Host layout | Bundled tool | +|-------|-------------|--------------| +| `project` (default) | `memory/` | Claude (`~/.claude/memory`) | +| `session` | `memory//` | Codex (`~/.codex/memories`) | + +Session scope keeps each memory repository with the config-store database that +coordinates its writers. Reusing a named session reuses its memory; changing the +name starts with a separate store. Worktrees and concurrent-session suffixes +also have separate stores. This deliberately favors writer isolation over +sharing learned context. An ad-hoc concurrent store may never be reused and +remains until cleanup. + +Because the key is the config-store key, memory follows the config store +wherever it goes. In a linked worktree that means the key is `default` until the +project has a config override or patch, and the worktree's own key afterwards: +adding the first override moves the worktree's memory (and its conversation +history) to a fresh store, so the agent starts over. Nothing is deleted. The +previous store stays until cleanup. + +Enabling `memoryScope` on a tool that already ran without one does not migrate +anything. Memory the tool previously wrote inside its config store stays there, +shadowed by the new mount at the same path; remove it by hand, or run +`enclave cleanup` for the project, if the split state is a problem. + +`--no-memory` omits the memory mount and applies the tool's `sandbox.noMemoryArgs` +at launch. Codex declares `-c features.memories=false` and Claude +`--settings '{"autoMemoryEnabled": false}'`, disabling memory use and generation +without changing saved settings or deleting existing memories. `--ephemeral` +applies the same launch arguments and discards the config store. Tools without +`noMemoryArgs`, or tools started manually from a shell, retain their native +behavior: without the mount their writes land in the config store, which +persists unless the session is ephemeral. + +For session-scoped memory, project cleanup treats memory and the entire config +store as a unit: `--keep memory` also retains the config store (including +conversation history), and `--keep history` also retains memory. Default cleanup +removes both. With `cleanup --all`, the existing whole-project-tree behavior +applies: `--keep memory` has no selective effect. + +`cleanup --ephemeral` removes memory alongside each selected non-default config +store, which includes the store of a named session even though its container is +left alone. The same pairing applies: `--keep memory` retains each session store +that holds memory, together with that memory. Stores with no memory to keep are +removed either way: every store of a project-scoped tool, and the throwaway +stores of `--ephemeral` runs, which never get a memory mount. The other +`--keep` kinds have no effect on an ephemeral cleanup. + +Selective cleanup reads the scope from the tool spec. A tool with no spec, +meaning state left behind by an extension that has since been removed, cleans up +under the default scope rather than failing. A spec that exists but does not load +aborts a project cleanup before anything is deleted, but only when `--keep memory` +or `--keep history` couples memory to the config store. The ephemeral sweep +handles such a tool by itself instead of aborting for every other tool: with +`--keep memory` the tool's stores are skipped, since the scope decides what would +be kept; otherwise they are removed with a warning and its session memory stays +behind until the spec loads again. Every other plan warns and proceeds under the +default scope, which is the plan a working spec would have produced anyway, so a +broken extension never blocks the removal of its own state. + ## Managed Skills Tools opt into managed skills with `sandbox.skillsDir` in diff --git a/extensions/tools/claude/README.md b/extensions/tools/claude/README.md index eff910f2..30bafbd6 100644 --- a/extensions/tools/claude/README.md +++ b/extensions/tools/claude/README.md @@ -42,9 +42,10 @@ Claude Code's auto memory is enabled by default (`autoMemoryEnabled`), pinned to `~/.claude/memory` inside the container. That directory is bind-mounted from the per-project host location `~/.local/state/enclave/projects//claude/memory/`, so memory is scoped per project and stored outside the working directory (never -committed). Pass `--no-memory` to disable the mount. Ephemeral -(`--ephemeral`) sessions skip the host mount, so memory written during them -is discarded with the session's config store. +committed). Pass `--no-memory` to skip the mount and disable memory generation +(`--settings '{"autoMemoryEnabled": false}'` is added to the launch arguments). +Ephemeral (`--ephemeral`) sessions do the same, so no memory is written or +carried over. ## Network Access diff --git a/extensions/tools/claude/spec.yaml b/extensions/tools/claude/spec.yaml index e816b648..0ac69874 100644 --- a/extensions/tools/claude/spec.yaml +++ b/extensions/tools/claude/spec.yaml @@ -17,6 +17,9 @@ sandbox: qemuMinMemoryMiB: 4096 skillsDir: .claude/skills memoryDir: .claude/memory + # Command-line settings take precedence over the baked settings template, + # which opts auto-memory in, so this disables generation without the mount. + noMemoryArgs: [--settings, '{"autoMemoryEnabled": false}'] settingsFile: claude-settings.json settingsTarget: .claude/settings.json yoloFlag: --dangerously-skip-permissions diff --git a/extensions/tools/codex/README.md b/extensions/tools/codex/README.md index 3bdd045c..ea579aa3 100644 --- a/extensions/tools/codex/README.md +++ b/extensions/tools/codex/README.md @@ -32,6 +32,7 @@ The default `config.toml` template configures: - Update checks disabled - Model personality prompt disabled - Fast mode (fast service tier with increased plan usage) opted out +- Memories enabled Model selection and reasoning effort are left at the Codex defaults; override them via config patches if needed. @@ -56,6 +57,38 @@ To show branch and usage limits in Codex's TUI status line, add a TOML patch: status_line = ["git-branch", "context-remaining", "five-hour-limit", "weekly-limit"] ``` +## Memory + +Enclave's default template enables [Codex memories](https://developers.openai.com/codex/memories) +with `features.memories = true`. This lets Codex use eligible conversation content +for memory generation and consumes additional model quota. A full override such +as `~/.config/enclave/tools/codex/config.toml` replaces the template, so those +users get the feature setting from their own config (Codex defaults to off). + +Enclave isolates memory by config-store key. See [Agent Memory](../../../docs/runtime/stores.md#agent-memory) +for session reuse, `--no-memory`, ephemeral runs, and cleanup semantics. + +To disable memories by default, set the `no_memory` +[config option](../../../docs/configuration.md) in the enclave project or global +config, the per-project equivalent of `--no-memory`, or add a Codex config patch: + +```toml +[features] +memories = false +``` + +For finer control while the feature is enabled, the +[configuration reference](https://developers.openai.com/codex/config-reference) +documents `[memories]` settings including `generate_memories` (allow new chats +as generation inputs), `use_memories` (inject existing memories), +`disable_on_external_context`, and `max_unused_days`. + +Codex consolidates eligible idle conversations in the background. Enclave +containers exit with the tool, terminating in-flight consolidation; memories +from one session typically appear during a later session using the same +store. Use `/memories` inside Codex to control memory use and generation for the +current conversation. + ## Files | File | Purpose | diff --git a/extensions/tools/codex/spec.yaml b/extensions/tools/codex/spec.yaml index c96f4bcf..cd8aee9d 100644 --- a/extensions/tools/codex/spec.yaml +++ b/extensions/tools/codex/spec.yaml @@ -17,6 +17,20 @@ sandbox: qemuMinMemoryMiB: 4096 qemuStoreCacheMmap: true skillsDir: .codex/skills + memoryDir: .codex/memories + memoryScope: session + noMemoryArgs: [-c, features.memories=false] + statePaths: + # Paths are config-relative (.codex/), unlike the home-relative memoryDir. + # Memory files, their SQLite index, and writer locks; codex reconciles + # stale locks itself, so surviving an overlay is safe. + - memories/ + - memories_*.sqlite* + - thread-writer-locks/ + # Thread/session databases: rebuildable from the preserved sessions/ + # rollouts, pinned to skip the re-backfill and keep non-rollout state. + - state_*.sqlite* + - thread_history_*.sqlite* settingsFile: codex-config.toml settingsTarget: .codex/config.toml yoloFlag: --dangerously-bypass-approvals-and-sandbox diff --git a/extensions/tools/codex/templates/config.toml b/extensions/tools/codex/templates/config.toml index ea5bb041..aa06106f 100644 --- a/extensions/tools/codex/templates/config.toml +++ b/extensions/tools/codex/templates/config.toml @@ -31,6 +31,12 @@ metrics_exporter = "none" log_user_prompt = false [features] +# Opted in, unlike fast_mode below. A container starts with no accumulated +# context, so without memories every session relearns the project from scratch, +# and no other setting recovers that. Background consolidation does use quota. +# Opt out per run with enclave --no-memory or permanently with a config patch; +# see this tool's README. +memories = true # Fast mode became default-on upstream (stable, default_enabled=true). With no # service_tier configured it silently adopts the model's default tier — "Fast" # on enterprise/business/team plans: fastest inference with increased plan diff --git a/internal/app/cleanup.go b/internal/app/cleanup.go index 79e50b8e..6aa8e0ce 100644 --- a/internal/app/cleanup.go +++ b/internal/app/cleanup.go @@ -9,9 +9,11 @@ package app import ( "context" + "errors" "fmt" "os" "path/filepath" + "slices" "sort" "strings" @@ -28,6 +30,68 @@ import ( // mirrors defaultStoreKey in internal/backend/docker. const persistentConfigStoreKey = "default" +// Cleanup dir kinds. Each maps to a --keep flag, except configStoreKind, which +// is a sub-kind of history: it is kept by --keep history like the rest, but +// session-scoped memory couples to it alone rather than to every history dir. +const ( + cacheKind = "cache" + historyKind = "history" + configStoreKind = "config-store" + memoryKind = "memory" + authKind = "auth" + ephemeralKind = "ephemeral" +) + +// memoryScopeResolver answers each tool's declared memory scope for one cleanup +// run, caching the result so a full sweep over every project does not re-read +// the same specs once per project. A path-resolution failure is carried into +// every lookup, so it surfaces exactly where a plan consults the scope. +type memoryScopeResolver struct { + paths model.Paths + pathsErr error + scopes map[string]string +} + +func newMemoryScopeResolver(paths model.Paths, pathsErr error) *memoryScopeResolver { + return &memoryScopeResolver{paths: paths, pathsErr: pathsErr, scopes: map[string]string{}} +} + +// scopeFor returns tool's memory scope, never the empty string: a spec that +// declares none resolves to the default exactly like a tool with no installed +// spec at all. A missing spec is not an error because the host state tree keeps +// a directory per tool that ever ran in a project, including tools since removed +// from the extension tree, and cleanup must still be able to delete those. A +// spec that exists but does not load is reported; whether that aborts the run is +// the caller's decision, since only some plans can act on the answer. +func (r *memoryScopeResolver) scopeFor(tool string) (string, error) { + if r.pathsErr != nil { + return "", fmt.Errorf("resolve extension paths: %w", r.pathsErr) + } + if scope, ok := r.scopes[tool]; ok { + return scope, nil + } + scope := model.MemoryScopeProject + profile, err := config.LoadProfile(r.paths, tool) + switch { + case err == nil: + // Load-time normalization only resolves the scope of tools that + // declare memory; for the rest the undeclared scope is the default. + scope = model.ResolveMemoryScope(profile.MemoryScope) + case !errors.Is(err, os.ErrNotExist): + return "", fmt.Errorf("load %s: %w", tool, err) + } + r.scopes[tool] = scope + return scope, nil +} + +// memoryScopeAffectsPlan reports whether a tool's memory scope can change what +// a non-ephemeral cleanup removes. Only the flag combinations that couple +// memory to the config store depend on it; a full-tree cleanup removes both +// regardless, and the ephemeral sweep handles scope errors per tool itself. +func memoryScopeAffectsPlan(cleanup model.CleanupOptions) bool { + return !cleanup.CleanupAll && (cleanup.CleanupKeepHist || cleanup.CleanupKeepMemory) +} + func runCleanup(run model.RunOptions, cleanup model.CleanupOptions) int { home, err := config.ResolveHostHome() if err != nil { @@ -48,6 +112,14 @@ func runCleanup(run model.RunOptions, cleanup model.CleanupOptions) int { } project = proj } + // Session-scoped memory is removed together with its config store, so the + // plan depends on each tool's declared scope. A path or spec problem is + // carried by the resolver and handled where a plan consults the scope: + // aborting before anything is deleted when the scope can change the plan, + // and proceeding under the default otherwise. Cleanup is what a user + // reaches for when the extension tree is broken or gone. + paths, pathsErr := config.ResolvePaths() + scopes := newMemoryScopeResolver(paths, pathsErr) if cleanup.CleanupEphemeral { containerNames, containersErr := resolveEphemeralContainers(run, cleanup, project) @@ -57,7 +129,7 @@ func runCleanup(run model.RunOptions, cleanup model.CleanupOptions) int { } // Ephemeral config stores are host directories keyed by a session or // worktree suffix. - storeDirs := resolveEphemeralStoreDirs(run, cleanup, home, project) + storeDirs := resolveEphemeralStoreDirs(run, cleanup, home, project, scopes) if cleanup.CleanupDryRun { printEphemeralCleanupPlan(containerNames, storeDirs) cleanupBuildCache(cleanup) @@ -70,7 +142,19 @@ func runCleanup(run model.RunOptions, cleanup model.CleanupOptions) int { return 0 } - dirPaths := cleanupDirsForRemoval(run, cleanup, home, project) + memoryScope, err := scopes.scopeFor(run.Tool) + if err != nil { + if memoryScopeAffectsPlan(cleanup) { + logx.Errorf("Failed to resolve memory cleanup policy: %v", err) + return 1 + } + // No --keep flag couples memory to the config store here, so the + // unreadable scope cannot change what is removed. Refusing would strand + // the state of exactly the broken extension the user is cleaning up. + logx.Warnf("Ignoring unreadable memory cleanup policy: %v", err) + memoryScope = model.MemoryScopeProject + } + dirPaths := cleanupDirsForRemoval(run, cleanup, home, project, memoryScope) if cleanup.CleanupDryRun { printCleanupPlan(dirPaths) @@ -156,8 +240,9 @@ func resolveEphemeralContainers(run model.RunOptions, cleanup model.CleanupOptio // resolveEphemeralStoreDirs enumerates the host directories backing ephemeral // config stores (every config-store key other than the persistent "default" -// key). -func resolveEphemeralStoreDirs(run model.RunOptions, cleanup model.CleanupOptions, home string, project model.Project) []cleanupDir { +// key). Tools whose memory scope cannot be resolved are warned about and +// handled conservatively instead of aborting the sweep. +func resolveEphemeralStoreDirs(run model.RunOptions, cleanup model.CleanupOptions, home string, project model.Project, scopes *memoryScopeResolver) []cleanupDir { var hashes []string if cleanup.CleanupAll { hashes = listSubdirs(config.HostProjectsDir(home)) @@ -178,11 +263,47 @@ func resolveEphemeralStoreDirs(run model.RunOptions, cleanup model.CleanupOption } for _, tool := range tools { storeRoot := config.HostStoreConfigRootDir(home, tool, hash) - for _, key := range listSubdirs(storeRoot) { + keys := listSubdirs(storeRoot) + if len(keys) == 0 { + continue + } + scope, err := scopes.scopeFor(tool) + if err != nil { + // One broken spec must not abort the sweep for every other + // tool and project. Under --keep memory the scope decides + // which stores are kept, so the broken tool's stores are + // skipped rather than removed against the user's intent. + // Otherwise the default scope only under-deletes: stores go, + // session memory dirs are left behind until the spec loads. + if cleanup.CleanupKeepMemory { + logx.Warnf("Skipping %s stores, --keep memory needs its memory scope: %v", tool, err) + continue + } + logx.Warnf("Ignoring unreadable memory cleanup policy for %s, leaving its session memory in place: %v", tool, err) + scope = model.MemoryScopeProject + } + for _, key := range keys { if key == persistentConfigStoreKey { continue } - dirs = append(dirs, cleanupDir{Kind: "ephemeral", Path: filepath.Join(storeRoot, key)}) + memoryDir := "" + if scope == model.MemoryScopeSession { + if dir := config.HostProjectMemorySessionDir(home, hash, tool, key); util.PathExists(dir) { + memoryDir = dir + } + } + // Session memory and its config store are one unit here too, so + // --keep memory retains the pair. Keys with nothing to keep are + // still removed: every store of a tool without session memory, + // and the timestamp-keyed stores of --ephemeral runs, which + // never get a memory mount. + if memoryDir != "" && cleanup.CleanupKeepMemory { + continue + } + dirs = append(dirs, cleanupDir{Kind: ephemeralKind, Path: filepath.Join(storeRoot, key)}) + if memoryDir != "" { + dirs = append(dirs, cleanupDir{Kind: memoryKind, Path: memoryDir}) + } } } } @@ -214,10 +335,10 @@ type cleanupDir struct { func resolveCleanupDirs(run model.RunOptions, cleanup model.CleanupOptions, home string, project model.Project) []cleanupDir { if cleanup.CleanupAll { dirs := []cleanupDir{ - {Kind: "cache", Path: config.HostCacheDir(home)}, + {Kind: cacheKind, Path: config.HostCacheDir(home)}, // The state projects tree holds every project's config/env stores // and history, so a full cleanup removes them all at once. - {Kind: "history", Path: config.HostProjectsDir(home)}, + {Kind: historyKind, Path: config.HostProjectsDir(home)}, // The image inbox is global (not project-scoped), so it is only // removed by a full cleanup. Held images are user-imported content. {Kind: "inbox", Path: config.HostImageInboxDir(home)}, @@ -229,14 +350,14 @@ func resolveCleanupDirs(run model.RunOptions, cleanup model.CleanupOptions, home projectDataDir := config.HostProjectToolDir(home, project.Hash, run.Tool) return []cleanupDir{ - {Kind: "cache", Path: config.HostCacheToolProjectDir(home, run.Tool, project.Hash)}, - {Kind: "history", Path: filepath.Join(projectDataDir, "history")}, - {Kind: "history", Path: config.HostProjectHomeConfigDir(home, project.Hash, run.Tool)}, - {Kind: "history", Path: config.HostProjectGeneratedConfigDir(home, project.Hash, run.Tool)}, - {Kind: "history", Path: filepath.Join(projectDataDir, model.GeneratedSkillsDirName)}, - {Kind: "history", Path: config.HostStoreConfigRootDir(home, run.Tool, project.Hash)}, - {Kind: "history", Path: config.HostStoreEnvDir(home, run.Tool, project.Hash)}, - {Kind: "memory", Path: config.HostProjectMemoryDir(home, project.Hash, run.Tool)}, + {Kind: cacheKind, Path: config.HostCacheToolProjectDir(home, run.Tool, project.Hash)}, + {Kind: historyKind, Path: filepath.Join(projectDataDir, "history")}, + {Kind: historyKind, Path: config.HostProjectHomeConfigDir(home, project.Hash, run.Tool)}, + {Kind: historyKind, Path: config.HostProjectGeneratedConfigDir(home, project.Hash, run.Tool)}, + {Kind: historyKind, Path: filepath.Join(projectDataDir, model.GeneratedSkillsDirName)}, + {Kind: configStoreKind, Path: config.HostStoreConfigRootDir(home, run.Tool, project.Hash)}, + {Kind: historyKind, Path: config.HostStoreEnvDir(home, run.Tool, project.Hash)}, + {Kind: memoryKind, Path: config.HostProjectMemoryDir(home, project.Hash, run.Tool)}, } } @@ -246,38 +367,49 @@ func resolveCleanupDirs(run model.RunOptions, cleanup model.CleanupOptions, home func authStoreCleanupDirs(home string) []cleanupDir { var dirs []cleanupDir for _, tool := range listSubdirs(config.HostStoreAuthRootDir(home)) { - dirs = append(dirs, cleanupDir{Kind: "auth", Path: config.HostStoreAuthTreeDir(home, tool)}) + dirs = append(dirs, cleanupDir{Kind: authKind, Path: config.HostStoreAuthTreeDir(home, tool)}) } for _, feature := range listSubdirs(config.HostStoreFeatureAuthRootDir(home)) { - dirs = append(dirs, cleanupDir{Kind: "auth", Path: config.HostStoreFeatureAuthDir(home, feature)}) + dirs = append(dirs, cleanupDir{Kind: authKind, Path: config.HostStoreFeatureAuthDir(home, feature)}) } return dirs } // cleanupDirsForRemoval resolves the host directories to remove for the given -// cleanup options, applying the keep-* gating. Each host-dir kind is preserved -// when its matching --keep-* flag is set. -func cleanupDirsForRemoval(run model.RunOptions, cleanup model.CleanupOptions, home string, project model.Project) []cleanupDir { +// cleanup options, applying keep-* flags and session memory/config coupling. +func cleanupDirsForRemoval(run model.RunOptions, cleanup model.CleanupOptions, home string, project model.Project, memoryScope string) []cleanupDir { dirs := resolveCleanupDirs(run, cleanup, home, project) if cleanup.CleanupKeepCache { - dirs = filterDirs(dirs, "cache") + dirs = filterDirs(dirs, cacheKind) } if cleanup.CleanupKeepHist { - dirs = filterDirs(dirs, "history") + dirs = filterDirs(dirs, historyKind, configStoreKind) } if cleanup.CleanupKeepMemory { - dirs = filterDirs(dirs, "memory") + dirs = filterDirs(dirs, memoryKind) } if cleanup.CleanupKeepAuth { - dirs = filterDirs(dirs, "auth") + dirs = filterDirs(dirs, authKind) + } + if !cleanup.CleanupAll && memoryScope == model.MemoryScopeSession { + // Session memory and the config store holding the database that indexes + // it form one cleanup unit: keeping either keeps both. The remaining + // history dirs (shell history, env store, generated config) are not + // coupled and stay subject to --keep history alone. + if cleanup.CleanupKeepHist { + dirs = filterDirs(dirs, memoryKind) + } + if cleanup.CleanupKeepMemory { + dirs = filterDirs(dirs, configStoreKind) + } } return dirs } -func filterDirs(dirs []cleanupDir, kind string) []cleanupDir { +func filterDirs(dirs []cleanupDir, kinds ...string) []cleanupDir { var filtered []cleanupDir for _, dir := range dirs { - if dir.Kind == kind { + if slices.Contains(kinds, dir.Kind) { continue } filtered = append(filtered, dir) diff --git a/internal/app/cleanup_test.go b/internal/app/cleanup_test.go index 0ae9616c..068e0d98 100644 --- a/internal/app/cleanup_test.go +++ b/internal/app/cleanup_test.go @@ -8,8 +8,10 @@ package app import ( + "fmt" "os" "path/filepath" + "reflect" "strings" "testing" @@ -69,8 +71,8 @@ func TestCleanupDirsNeverTargetConfigRoot(t *testing.T) { } } - check("per-project", cleanupDirsForRemoval(run, model.CleanupOptions{}, home, project)) - check("all", cleanupDirsForRemoval(run, model.CleanupOptions{CleanupAll: true}, home, project)) + check("per-project", cleanupDirsForRemoval(run, model.CleanupOptions{}, home, project, model.MemoryScopeProject)) + check("all", cleanupDirsForRemoval(run, model.CleanupOptions{CleanupAll: true}, home, project, model.MemoryScopeProject)) } func TestFilterDirsDropsMemory(t *testing.T) { @@ -82,13 +84,13 @@ func TestFilterDirsDropsMemory(t *testing.T) { dirs := resolveCleanupDirs(run, model.CleanupOptions{}, home, project) memoryPath := config.HostProjectMemoryDir(home, project.Hash, run.Tool) - filtered := filterDirs(dirs, "memory") + filtered := filterDirs(dirs, memoryKind) if len(filtered) != len(dirs)-1 { - t.Fatalf("filterDirs(dirs, \"memory\") returned %d entries, want %d", len(filtered), len(dirs)-1) + t.Fatalf("filterDirs(dirs, memoryKind) returned %d entries, want %d", len(filtered), len(dirs)-1) } for _, dir := range filtered { - if dir.Kind == "memory" { + if dir.Kind == memoryKind { t.Fatalf("filterDirs did not drop memory entry: %q", dir.Path) } if dir.Path == memoryPath { @@ -125,8 +127,8 @@ func TestCleanupDirGatingForMemory(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - dirs := cleanupDirsForRemoval(run, tt.cleanup, home, project) - if got := hasKind(dirs, "memory"); got != tt.wantMemory { + dirs := cleanupDirsForRemoval(run, tt.cleanup, home, project, model.MemoryScopeProject) + if got := hasKind(dirs, memoryKind); got != tt.wantMemory { t.Fatalf("memory present = %v, want %v", got, tt.wantMemory) } if tt.wantAny && len(dirs) == 0 { @@ -139,34 +141,288 @@ func TestCleanupDirGatingForMemory(t *testing.T) { } } -func TestResolveEphemeralStoreDirsSkipsDefaultKey(t *testing.T) { +func TestCleanupKeepsSessionMemoryAndConfigTogether(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + scope string + cleanup model.CleanupOptions + wantMemory bool + wantStore bool + }{ + {name: "project/default", scope: model.MemoryScopeProject}, + {name: "project/keep memory", scope: model.MemoryScopeProject, + cleanup: model.CleanupOptions{CleanupKeepMemory: true}, wantMemory: true}, + {name: "project/keep history", scope: model.MemoryScopeProject, + cleanup: model.CleanupOptions{CleanupKeepHist: true}, wantStore: true}, + {name: "project/keep both", scope: model.MemoryScopeProject, + cleanup: model.CleanupOptions{CleanupKeepHist: true, CleanupKeepMemory: true}, wantMemory: true, wantStore: true}, + {name: "project/all ignores keep memory", scope: model.MemoryScopeProject, + cleanup: model.CleanupOptions{CleanupAll: true, CleanupKeepMemory: true}}, + {name: "session/default", scope: model.MemoryScopeSession}, + {name: "session/keep memory keeps store", scope: model.MemoryScopeSession, + cleanup: model.CleanupOptions{CleanupKeepMemory: true}, wantMemory: true, wantStore: true}, + {name: "session/keep history keeps memory", scope: model.MemoryScopeSession, + cleanup: model.CleanupOptions{CleanupKeepHist: true}, wantMemory: true, wantStore: true}, + {name: "session/keep both", scope: model.MemoryScopeSession, + cleanup: model.CleanupOptions{CleanupKeepHist: true, CleanupKeepMemory: true}, wantMemory: true, wantStore: true}, + {name: "session/all ignores keep memory", scope: model.MemoryScopeSession, + cleanup: model.CleanupOptions{CleanupAll: true, CleanupKeepMemory: true}}, + } { + t.Run(tc.name, func(t *testing.T) { + home := t.TempDir() + project := model.Project{Hash: "project"} + run := model.RunOptions{Tool: "custom"} + memory := config.HostProjectMemoryDir(home, project.Hash, run.Tool) + store := config.HostStoreConfigRootDir(home, run.Tool, project.Hash) + for _, root := range []string{memory, store} { + if err := os.MkdirAll(filepath.Join(root, "default"), 0o700); err != nil { + t.Fatal(err) + } + } + dirs := cleanupDirsForRemoval(run, tc.cleanup, home, project, tc.scope) + cleanupDirs(dirs) + for root, want := range map[string]bool{memory: tc.wantMemory, store: tc.wantStore} { + _, err := os.Stat(root) + if (err == nil) != want { + t.Errorf("%s exists = %v, want %v", root, err == nil, want) + } + } + }) + } +} + +// writeScopedToolSpec writes a minimal sandbox spec declaring memoryScope into +// a temporary user-tools tree, so cleanup tests exercise the scope branches +// without depending on what the bundled extensions happen to declare. +func writeScopedToolSpec(t *testing.T, tool string, scope string) model.Paths { + t.Helper() + + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, tool), 0o700); err != nil { + t.Fatal(err) + } + spec := fmt.Sprintf(`{"schemaVersion":"1","kind":"sandbox","name":%q,"sandbox":{ + "configDir":".tool", "memoryDir":".tool/memories", "memoryScope":%q}}`, tool, scope) + if err := os.WriteFile(filepath.Join(root, tool, config.SpecFilenameJSON), []byte(spec), 0o600); err != nil { + t.Fatal(err) + } + return model.Paths{UserToolsDir: root} +} + +// TestResolveEphemeralStoreDirs covers the ephemeral plan for a session-scoped +// tool: the persistent key is never touched, a key with session memory is +// planned as a store/memory pair, and a key without one (an --ephemeral run's +// throwaway store, which never gets a memory mount) is planned alone. +func TestResolveEphemeralStoreDirs(t *testing.T) { home := t.TempDir() project := model.Project{Hash: "projhash1234"} - run := model.RunOptions{Tool: "codex"} + run := model.RunOptions{Tool: "custom"} storeRoot := config.HostStoreConfigRootDir(home, run.Tool, project.Hash) - for _, key := range []string{"default", "session-a", "session-b"} { + memoryRoot := config.HostProjectMemoryDir(home, project.Hash, run.Tool) + for _, key := range []string{"default", "session-a", "throwaway"} { if err := os.MkdirAll(filepath.Join(storeRoot, key), 0o700); err != nil { - t.Fatalf("mkdir %q: %v", key, err) + t.Fatalf("mkdir store %q: %v", key, err) + } + } + for _, key := range []string{"default", "session-a"} { + if err := os.MkdirAll(filepath.Join(memoryRoot, key), 0o700); err != nil { + t.Fatalf("mkdir memory %q: %v", key, err) } } - dirs := resolveEphemeralStoreDirs(run, model.CleanupOptions{}, home, project) - if len(dirs) != 2 { - t.Fatalf("resolveEphemeralStoreDirs() returned %d entries, want 2", len(dirs)) + paths := writeScopedToolSpec(t, run.Tool, model.MemoryScopeSession) + for _, tc := range []struct { + name string + cleanup model.CleanupOptions + want []cleanupDir + }{ + // Entries are sorted by path, and config-store/ sorts before memory/. + { + name: "default", + want: []cleanupDir{ + {Kind: ephemeralKind, Path: filepath.Join(storeRoot, "session-a")}, + {Kind: ephemeralKind, Path: filepath.Join(storeRoot, "throwaway")}, + {Kind: memoryKind, Path: filepath.Join(memoryRoot, "session-a")}, + }, + }, + { + // Keeping memory keeps the config store indexing it, but the + // throwaway store has no memory to keep and still goes. + name: "keep memory", + cleanup: model.CleanupOptions{CleanupKeepMemory: true}, + want: []cleanupDir{{Kind: ephemeralKind, Path: filepath.Join(storeRoot, "throwaway")}}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + dirs := resolveEphemeralStoreDirs(run, tc.cleanup, home, project, newMemoryScopeResolver(paths, nil)) + if !reflect.DeepEqual(dirs, tc.want) { + t.Fatalf("ephemeral dirs = %+v, want %+v", dirs, tc.want) + } + }) } - want := map[string]bool{ - filepath.Join(storeRoot, "session-a"): true, - filepath.Join(storeRoot, "session-b"): true, +} + +// TestCleanupToleratesUninstalledTools covers state left behind by a tool that +// has since been removed from the extension tree: its directories still carry +// ephemeral stores, and cleanup must plan their removal instead of failing on +// the missing spec. +func TestCleanupToleratesUninstalledTools(t *testing.T) { + home := t.TempDir() + project := model.Project{Hash: "projhash1234"} + run := model.RunOptions{Tool: "gone"} + + storeRoot := config.HostStoreConfigRootDir(home, run.Tool, project.Hash) + if err := os.MkdirAll(filepath.Join(storeRoot, "session-a"), 0o700); err != nil { + t.Fatal(err) } - for _, dir := range dirs { - if dir.Kind != "ephemeral" { - t.Fatalf("unexpected kind %q for %q", dir.Kind, dir.Path) - } - if !want[dir.Path] { - t.Fatalf("unexpected ephemeral store dir %q", dir.Path) + + scopes := newMemoryScopeResolver(model.Paths{UserToolsDir: t.TempDir()}, nil) + dirs := resolveEphemeralStoreDirs(run, model.CleanupOptions{}, home, project, scopes) + want := []cleanupDir{{Kind: ephemeralKind, Path: filepath.Join(storeRoot, "session-a")}} + if !reflect.DeepEqual(dirs, want) { + t.Fatalf("ephemeral dirs = %+v, want %+v", dirs, want) + } + + scope, err := scopes.scopeFor(run.Tool) + if err != nil { + t.Fatalf("scopeFor() with no spec: %v", err) + } + if scope != model.MemoryScopeProject { + t.Fatalf("scope = %q, want %q", scope, model.MemoryScopeProject) + } +} + +// TestEphemeralSweepToleratesUnloadableSpec pins the per-tool fallback for a +// spec that exists but does not load: without --keep memory the tool's stores +// are still removed and its session memory is left in place, and with +// --keep memory the tool's stores are skipped, since the scope decides what +// would have been kept. +func TestEphemeralSweepToleratesUnloadableSpec(t *testing.T) { + t.Parallel() + + home := t.TempDir() + project := model.Project{Hash: "projhash1234"} + run := model.RunOptions{Tool: "broken"} + + storeRoot := config.HostStoreConfigRootDir(home, run.Tool, project.Hash) + memoryRoot := config.HostProjectMemoryDir(home, project.Hash, run.Tool) + for _, dir := range []string{filepath.Join(storeRoot, "session-a"), filepath.Join(memoryRoot, "session-a")} { + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) } } + + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, run.Tool), 0o700); err != nil { + t.Fatal(err) + } + spec := `{"schemaVersion":"1","kind":"sandbox","name":"broken","sandbox":{ + "configDir":".broken", "memoryScope":"global"}}` + if err := os.WriteFile(filepath.Join(root, run.Tool, config.SpecFilenameJSON), []byte(spec), 0o600); err != nil { + t.Fatal(err) + } + paths := model.Paths{UserToolsDir: root} + + dirs := resolveEphemeralStoreDirs(run, model.CleanupOptions{}, home, project, newMemoryScopeResolver(paths, nil)) + want := []cleanupDir{{Kind: ephemeralKind, Path: filepath.Join(storeRoot, "session-a")}} + if !reflect.DeepEqual(dirs, want) { + t.Fatalf("fallback dirs = %+v, want %+v", dirs, want) + } + + kept := resolveEphemeralStoreDirs(run, model.CleanupOptions{CleanupKeepMemory: true}, home, project, newMemoryScopeResolver(paths, nil)) + if len(kept) != 0 { + t.Fatalf("--keep memory with an unloadable spec should skip the tool's stores, got %+v", kept) + } +} + +// TestScopeForResolvesUndeclaredScope pins that a spec declaring no memoryScope +// answers the same as no spec at all. The two arrive by different routes, one +// through profile normalization and the other through the resolver's own +// fallback, and a caller comparing against model.MemoryScopeProject must not be +// able to tell them apart. +func TestScopeForResolvesUndeclaredScope(t *testing.T) { + t.Parallel() + + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "plain"), 0o700); err != nil { + t.Fatal(err) + } + spec := `{"schemaVersion":"1","kind":"sandbox","name":"plain","sandbox":{"configDir":".plain"}}` + if err := os.WriteFile(filepath.Join(root, "plain", config.SpecFilenameJSON), []byte(spec), 0o600); err != nil { + t.Fatal(err) + } + + declared, err := newMemoryScopeResolver(model.Paths{UserToolsDir: root}, nil).scopeFor("plain") + if err != nil { + t.Fatal(err) + } + absent, err := newMemoryScopeResolver(model.Paths{UserToolsDir: t.TempDir()}, nil).scopeFor("plain") + if err != nil { + t.Fatal(err) + } + if declared != model.MemoryScopeProject || absent != model.MemoryScopeProject { + t.Fatalf("scope without memoryScope = %q, scope without spec = %q, want %q for both", + declared, absent, model.MemoryScopeProject) + } +} + +// TestUnloadableSpecOnlyBlocksScopeSensitivePlans covers the extension the user +// is most likely cleaning up after: one whose spec this binary cannot load. +// Refusing to plan is right only when the scope could still change what is +// removed; otherwise cleanup would strand the very state it exists to delete. +func TestUnloadableSpecOnlyBlocksScopeSensitivePlans(t *testing.T) { + t.Parallel() + + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "broken"), 0o700); err != nil { + t.Fatal(err) + } + spec := `{"schemaVersion":"1","kind":"sandbox","name":"broken","sandbox":{ + "configDir":".broken", "memoryScope":"global"}}` + if err := os.WriteFile(filepath.Join(root, "broken", config.SpecFilenameJSON), []byte(spec), 0o600); err != nil { + t.Fatal(err) + } + if _, err := newMemoryScopeResolver(model.Paths{UserToolsDir: root}, nil).scopeFor("broken"); err == nil { + t.Fatal("expected an unloadable spec to report an error") + } + + for _, tc := range []struct { + name string + cleanup model.CleanupOptions + wantAborts bool + }{ + {name: "default"}, + {name: "all", cleanup: model.CleanupOptions{CleanupAll: true}}, + {name: "all keep auth", cleanup: model.CleanupOptions{CleanupAll: true, CleanupKeepAuth: true}}, + {name: "all keep memory", cleanup: model.CleanupOptions{CleanupAll: true, CleanupKeepMemory: true}}, + {name: "keep cache", cleanup: model.CleanupOptions{CleanupKeepCache: true}}, + {name: "keep memory", cleanup: model.CleanupOptions{CleanupKeepMemory: true}, wantAborts: true}, + {name: "keep history", cleanup: model.CleanupOptions{CleanupKeepHist: true}, wantAborts: true}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := memoryScopeAffectsPlan(tc.cleanup); got != tc.wantAborts { + t.Fatalf("memoryScopeAffectsPlan() = %v, want %v", got, tc.wantAborts) + } + if tc.wantAborts { + return + } + // The plan the run falls back to must be the one the default scope + // produces, so continuing past the error cannot delete more or less + // than a working spec would have. + home := t.TempDir() + project := model.Project{Hash: "projhash1234"} + run := model.RunOptions{Tool: "broken"} + fallback := cleanupDirsForRemoval(run, tc.cleanup, home, project, model.MemoryScopeProject) + for _, scope := range []string{model.MemoryScopeProject, model.MemoryScopeSession} { + dirs := cleanupDirsForRemoval(run, tc.cleanup, home, project, scope) + if !reflect.DeepEqual(dirs, fallback) { + t.Fatalf("scope %q changes a plan it should not: %+v, want %+v", scope, dirs, fallback) + } + } + }) + } } func TestResolveCleanupDirsAllIncludesAuthStores(t *testing.T) { @@ -183,7 +439,7 @@ func TestResolveCleanupDirsAllIncludesAuthStores(t *testing.T) { hasAuth := func(dirs []cleanupDir) bool { for _, dir := range dirs { - if dir.Kind == "auth" { + if dir.Kind == authKind { return true } } @@ -193,11 +449,11 @@ func TestResolveCleanupDirsAllIncludesAuthStores(t *testing.T) { if !hasAuth(resolveCleanupDirs(run, all, home, model.Project{})) { t.Fatal("expected auth store dirs in full cleanup plan") } - if !hasAuth(cleanupDirsForRemoval(run, all, home, model.Project{})) { + if !hasAuth(cleanupDirsForRemoval(run, all, home, model.Project{}, model.MemoryScopeProject)) { t.Fatal("expected auth store dirs to be removed by default full cleanup") } keepAuth := model.CleanupOptions{CleanupAll: true, CleanupKeepAuth: true} - if hasAuth(cleanupDirsForRemoval(run, keepAuth, home, model.Project{})) { + if hasAuth(cleanupDirsForRemoval(run, keepAuth, home, model.Project{}, model.MemoryScopeProject)) { t.Fatal("--keep-auth should preserve auth store dirs") } } diff --git a/internal/app/command_run.go b/internal/app/command_run.go index 2a1c7776..05584bac 100644 --- a/internal/app/command_run.go +++ b/internal/app/command_run.go @@ -338,8 +338,9 @@ func resolveSessionActionArgs(action string, profile model.Profile) ([]string, s return nil, "", false, nil } - continueArgs := compactProfileArgs(profile.ContinueArgs) - resumeArgs := compactProfileArgs(profile.ResumeArgs) + // Profile arg lists are trimmed and compacted at load. + continueArgs := profile.ContinueArgs + resumeArgs := profile.ResumeArgs switch action { case actionContinue: @@ -364,21 +365,3 @@ func resolveSessionActionArgs(action string, profile model.Profile) ([]string, s profile.Name, ) } - -func compactProfileArgs(args []string) []string { - if len(args) == 0 { - return nil - } - result := make([]string, 0, len(args)) - for _, arg := range args { - trimmed := strings.TrimSpace(arg) - if trimmed == "" { - continue - } - result = append(result, trimmed) - } - if len(result) == 0 { - return nil - } - return result -} diff --git a/internal/app/command_run_session_test.go b/internal/app/command_run_session_test.go index 13c9aeb9..4f26399c 100644 --- a/internal/app/command_run_session_test.go +++ b/internal/app/command_run_session_test.go @@ -101,11 +101,3 @@ func TestResolveSessionActionArgs(t *testing.T) { }) } } - -func TestCompactProfileArgsTrimsAndSkipsEmpty(t *testing.T) { - got := compactProfileArgs([]string{" --resume ", "", " ", "--last"}) - want := []string{"--resume", "--last"} - if !reflect.DeepEqual(got, want) { - t.Fatalf("expected %v, got %v", want, got) - } -} diff --git a/internal/backend/docker/overlay_test.go b/internal/backend/docker/overlay_test.go index dd6a6c45..a105562d 100644 --- a/internal/backend/docker/overlay_test.go +++ b/internal/backend/docker/overlay_test.go @@ -19,6 +19,60 @@ import ( yaruntime "enclave/internal/runtime" ) +// TestOverlayConfigDirPreservesDeclaredStatePaths runs the overlay against the +// real preserve-path policy for a profile pinning both shapes a tool can +// declare: directory trees (one of them nested and tool-managed, like a git +// repository) and globs matching a database plus its sidecar files. The shapes +// are Codex's, but the profile is local, so tuning that extension cannot +// silently change what this asserts. +func TestOverlayConfigDirPreservesDeclaredStatePaths(t *testing.T) { + t.Parallel() + + targetDir, sourceDir := t.TempDir(), t.TempDir() + pinned := []string{ + "memories/", "memories_*.sqlite*", "thread-writer-locks/", + "state_*.sqlite*", "thread_history_*.sqlite*", + } + profile := model.Profile{ConfigDir: ".tool", StatePaths: pinned} + statePaths := []string{ + "memories_1.sqlite", "memories_1.sqlite-wal", "memories_1.sqlite-shm", + "memories_2.sqlite", "memories_2.sqlite-journal", + "state_5.sqlite", "state_5.sqlite-wal", "state_5.sqlite-shm", + "thread_history_1.sqlite", "thread_history_1.sqlite-wal", "thread_history_1.sqlite-shm", + "thread-writer-locks/thread.lock", "memories/.git/HEAD", "memories/memory_summary.md", + } + for _, rel := range statePaths { + for _, dir := range []string{targetDir, sourceDir} { + p := filepath.Join(dir, rel) + if err := os.MkdirAll(filepath.Dir(p), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(dir), 0o600); err != nil { + t.Fatal(err) + } + } + } + settings := []byte("[features]\nmemories = false\n") + if err := os.WriteFile(filepath.Join(sourceDir, "config.toml"), settings, 0o600); err != nil { + t.Fatal(err) + } + for range 2 { + if err := overlayConfigDir(targetDir, sourceDir, yaruntime.ConfigSourcePreservePaths(profile)); err != nil { + t.Fatal(err) + } + for _, rel := range statePaths { + got, err := os.ReadFile(filepath.Join(targetDir, rel)) + if err != nil || string(got) != targetDir { + t.Fatalf("preserved %s = %q, err %v", rel, got, err) + } + } + got, err := os.ReadFile(filepath.Join(targetDir, "config.toml")) + if err != nil || string(got) != string(settings) { + t.Fatalf("overlaid settings = %q, err %v", got, err) + } + } +} + func requireJQ(t *testing.T) { t.Helper() if _, err := exec.LookPath("jq"); err != nil { diff --git a/internal/config/host_config_passthrough.go b/internal/config/host_config_passthrough.go index 0bfeecf5..f6b38b94 100644 --- a/internal/config/host_config_passthrough.go +++ b/internal/config/host_config_passthrough.go @@ -39,13 +39,20 @@ func HostConfigPassthroughDefaults(profile model.Profile) []string { return subtractDeniedHostConfigPaths(profile, profile.PassthroughPaths) } +// HostConfigPassthroughDeniedPaths lists the config-relative paths the host +// config may never contribute: the tool's auth files, the non-configurable +// backstop, and the tool's declared runtime state. State paths belong here +// because they name what the container's own store owns. Passing the host's +// copy in would overwrite live session state with a foreign one. func HostConfigPassthroughDeniedPaths(profile model.Profile) []string { - denied := make([]string, 0, len(profile.RuntimeAuthFiles())+len(hostConfigPassthroughBackstop)+1) - denied = append(denied, profile.RuntimeAuthFiles()...) + authFiles := profile.RuntimeAuthFiles() + denied := make([]string, 0, len(authFiles)+len(hostConfigPassthroughBackstop)+len(profile.StatePaths)+1) + denied = append(denied, authFiles...) if credentials := normalizeHostConfigPath(profile.HostCredentialsFile); credentials != "" { denied = append(denied, credentials) } denied = append(denied, hostConfigPassthroughBackstop...) + denied = append(denied, profile.StatePaths...) return normalizeHostConfigPathList(denied) } diff --git a/internal/config/host_config_passthrough_test.go b/internal/config/host_config_passthrough_test.go index e041908e..bb8e7f41 100644 --- a/internal/config/host_config_passthrough_test.go +++ b/internal/config/host_config_passthrough_test.go @@ -85,6 +85,22 @@ func TestHostConfigPathMatchesGlobAgainstBasename(t *testing.T) { } } +func TestHostConfigPassthroughBlocksCodexMemoryState(t *testing.T) { + profile, err := LoadProfile(realRepoPaths(t), "codex") + if err != nil { + t.Fatal(err) + } + profile.PassthroughPaths = []string{ + "config.toml", "memories/", "memories_1.sqlite", "memories_1.sqlite-wal", + "memories_1.sqlite-shm", "state_5.sqlite", "thread_history_1.sqlite", + "thread-writer-locks/", + } + got := HostConfigPassthroughDefaults(profile) + if want := []string{"config.toml"}; !reflect.DeepEqual(got, want) { + t.Fatalf("passthrough paths = %v, want %v", got, want) + } +} + func TestHostConfigPassthroughBlocksPiAgentSessions(t *testing.T) { profile := model.Profile{ PassthroughPaths: []string{"agent/settings.json", "agent/sessions/"}, diff --git a/internal/config/host_paths.go b/internal/config/host_paths.go index f56beae8..681a014a 100644 --- a/internal/config/host_paths.go +++ b/internal/config/host_paths.go @@ -143,6 +143,14 @@ func HostProjectMemoryDir(home string, projectHash string, tool string) string { return filepath.Join(HostProjectToolDir(home, projectHash, tool), "memory") } +// HostProjectMemorySessionDir is the host directory backing a tool's +// session-scoped agent memory for one config-store key. The layout mirrors +// HostStoreConfigDir's keys, pairing each memory directory with the config +// store that coordinates its writers. +func HostProjectMemorySessionDir(home string, projectHash string, tool string, key string) string { + return filepath.Join(HostProjectMemoryDir(home, projectHash, tool), key) +} + func HostProjectHomeConfigDir(home string, projectHash string, tool string) string { return filepath.Join(HostProjectToolDir(home, projectHash, tool), model.HomeConfigDirName) } diff --git a/internal/config/profile.go b/internal/config/profile.go index c1e038e3..b47317e1 100644 --- a/internal/config/profile.go +++ b/internal/config/profile.go @@ -73,18 +73,47 @@ func validateAndNormalizeProfile(profile *model.Profile) error { if err := validateAndNormalizePorts(profile); err != nil { return err } - if err := validateAndNormalizeMemoryDir(profile); err != nil { + if err := validateAndNormalizeMemoryPolicy(profile); err != nil { return err } + if err := validateAndNormalizeStatePaths(profile); err != nil { + return err + } + profile.ContinueArgs = compactSpecArgs(profile.ContinueArgs) + profile.ResumeArgs = compactSpecArgs(profile.ResumeArgs) return validateAndNormalizeProviderSecurestorage(profile.Providers) } -// validateAndNormalizeMemoryDir checks the container-home-relative memory -// mount target. The path must be relative, free of traversal, and resolve to a -// concrete location (not "."). The cleaned value is written back to the profile. -func validateAndNormalizeMemoryDir(profile *model.Profile) error { +// validateAndNormalizeMemoryPolicy checks the memory scope, the disable +// arguments, and the container-home-relative memory mount target they apply to. +// Session scope keys memory by config-store key, so it needs both a memory dir +// to mount and a config dir to derive the key from. The path must be relative, +// free of traversal, and resolve to a concrete location (not "."). The cleaned +// values are written back to the profile, including the resolved scope, so no +// consumer has to treat an undeclared scope as the default itself. A tool +// without a memory dir keeps its scope undeclared. Scope only decides how the +// memory mount is keyed, so resolving it there would put a meaningless field +// into the serialized profile. +func validateAndNormalizeMemoryPolicy(profile *model.Profile) error { + switch profile.MemoryScope { + case "": + case model.MemoryScopeProject: + if profile.MemoryDir == "" { + return fmt.Errorf("memory_scope requires memory_dir") + } + case model.MemoryScopeSession: + if profile.MemoryDir == "" || profile.ConfigDir == "" { + return fmt.Errorf("memory_scope session requires memory_dir and config_dir") + } + default: + return fmt.Errorf("memory_scope must be project or session") + } + if profile.MemoryDir != "" { + profile.MemoryScope = model.ResolveMemoryScope(profile.MemoryScope) + } + profile.NoMemoryArgs = compactSpecArgs(profile.NoMemoryArgs) if profile.MemoryDir != "" { - cleaned, err := cleanMemoryPath(profile.MemoryDir) + cleaned, err := cleanRelativeConfigPath(profile.MemoryDir) if err != nil { return fmt.Errorf("memory_dir: %w", err) } @@ -93,6 +122,32 @@ func validateAndNormalizeMemoryDir(profile *model.Profile) error { return nil } +// validateAndNormalizeStatePaths checks the config-relative runtime-state +// patterns a tool pins against the config overlay and host config passthrough. +// Both consumers match them with HostConfigPathMatches, so the accepted shapes +// are that matcher's: an exact path, a directory prefix (trailing "/"), or a +// glob. Selection directives are rejected because these are a fixed policy, not +// a user-tunable allow-list that "+"/"-" could amend. +func validateAndNormalizeStatePaths(profile *model.Profile) error { + if len(profile.StatePaths) > 0 && profile.ConfigDir == "" { + return fmt.Errorf("state_paths requires config_dir") + } + for i, raw := range profile.StatePaths { + value := strings.TrimSpace(filepathToSlash(raw)) + if strings.HasPrefix(value, "+") || strings.HasPrefix(value, "-") { + return fmt.Errorf("state_paths[%d]: selection directives are not supported", i) + } + if _, err := cleanRelativeConfigPath(value); err != nil { + return fmt.Errorf("state_paths[%d]: %w", i, err) + } + if _, err := path.Match(value, ""); err != nil { + return fmt.Errorf("state_paths[%d]: invalid glob: %w", i, err) + } + profile.StatePaths[i] = normalizeHostConfigPath(value) + } + return nil +} + // validateAndNormalizeSettingsFile enforces the settings_file naming contract // at load, so every consumer can join the raw value onto a templates directory: // the config-source composition on the host and the container-side template @@ -138,11 +193,32 @@ func containerProfilePath(value string) string { return path.Join(model.ContainerHome, value) } -func cleanMemoryPath(path string) (string, error) { +// compactSpecArgs trims a spec-declared argv and drops its blank entries, so a +// stray list item cannot pass an empty argument through to the agent. +func compactSpecArgs(args []string) []string { + if len(args) == 0 { + return nil + } + compacted := make([]string, 0, len(args)) + for _, arg := range args { + if trimmed := strings.TrimSpace(arg); trimmed != "" { + compacted = append(compacted, trimmed) + } + } + if len(compacted) == 0 { + return nil + } + return compacted +} + +// cleanRelativeConfigPath rejects a spec-declared path that does not name a +// concrete location below the directory it is relative to, and returns the +// cleaned form of one that does. +func cleanRelativeConfigPath(path string) (string, error) { if path == "" { return "", fmt.Errorf("path is empty") } - if filepath.IsAbs(path) { + if filepath.IsAbs(path) || strings.HasPrefix(path, "/") { return "", fmt.Errorf("path must be relative: %s", path) } if util.HasPathTraversal(path) { diff --git a/internal/config/spec.go b/internal/config/spec.go index 7390a0dd..d2ad04c4 100644 --- a/internal/config/spec.go +++ b/internal/config/spec.go @@ -67,6 +67,9 @@ type specSandbox struct { ConfigDir string `json:"configDir,omitempty"` SkillsDir string `json:"skillsDir,omitempty"` MemoryDir string `json:"memoryDir,omitempty"` + MemoryScope string `json:"memoryScope,omitempty"` + NoMemoryArgs []string `json:"noMemoryArgs,omitempty"` + StatePaths []string `json:"statePaths,omitempty"` SettingsFile string `json:"settingsFile,omitempty"` SettingsTarget string `json:"settingsTarget,omitempty"` YoloFlag string `json:"yoloFlag,omitempty"` diff --git a/internal/config/spec_loader_test.go b/internal/config/spec_loader_test.go index 134feb71..db7b6781 100644 --- a/internal/config/spec_loader_test.go +++ b/internal/config/spec_loader_test.go @@ -11,12 +11,112 @@ import ( "errors" "os" "path/filepath" + "reflect" "strings" "testing" "enclave/internal/model" ) +func TestLoadProfileMemoryPolicyFromThirdPartySpec(t *testing.T) { + root := t.TempDir() + toolDir := filepath.Join(root, "custom") + if err := os.MkdirAll(toolDir, 0o700); err != nil { + t.Fatal(err) + } + spec := `{"schemaVersion":"1","kind":"sandbox","name":"custom","sandbox":{ + "configDir":".custom", "memoryDir":".custom/memory", "memoryScope":"session", + "noMemoryArgs":["--config", "memory=false"], + "statePaths":["memory/", "memory_*.sqlite*"]}}` + if err := os.WriteFile(filepath.Join(toolDir, SpecFilenameJSON), []byte(spec), 0o600); err != nil { + t.Fatal(err) + } + profile, err := LoadProfile(model.Paths{UserToolsDir: root}, "custom") + if err != nil { + t.Fatal(err) + } + if profile.MemoryScope != model.MemoryScopeSession || profile.MemoryDir != ".custom/memory" { + t.Fatalf("memory policy = %+v", profile) + } + if !reflect.DeepEqual(profile.NoMemoryArgs, []string{"--config", "memory=false"}) { + t.Fatalf("no-memory args = %q", profile.NoMemoryArgs) + } + if !reflect.DeepEqual(profile.StatePaths, []string{"memory/", "memory_*.sqlite*"}) { + t.Fatalf("state paths = %q", profile.StatePaths) + } + profile.PassthroughPaths = []string{"config.toml", "memory/", "memory_1.sqlite-wal"} + if got := HostConfigPassthroughDefaults(profile); !reflect.DeepEqual(got, []string{"config.toml"}) { + t.Fatalf("passthrough = %v", got) + } +} + +// TestNormalizeMemoryPolicyAtLoad pins the values a consumer would otherwise +// have to sanitize itself: an undeclared scope resolves to the default, and a +// ragged spec argv (noMemoryArgs/continueArgs/resumeArgs) cannot carry a blank +// entry through to the agent's command line. +func TestNormalizeMemoryPolicyAtLoad(t *testing.T) { + profile := model.Profile{ + ConfigDir: ".custom", + MemoryDir: ".custom/memories", + NoMemoryArgs: []string{" -c ", "", " ", "memories=false"}, + ContinueArgs: []string{" resume ", "", "--last"}, + ResumeArgs: []string{"", " resume "}, + } + if err := validateAndNormalizeProfile(&profile); err != nil { + t.Fatal(err) + } + if profile.MemoryScope != model.MemoryScopeProject { + t.Errorf("memory scope = %q, want %q", profile.MemoryScope, model.MemoryScopeProject) + } + + // A tool without a memory dir keeps its scope undeclared, so the field + // stays out of its serialized profile. + memoryless := model.Profile{ConfigDir: ".custom"} + if err := validateAndNormalizeProfile(&memoryless); err != nil { + t.Fatal(err) + } + if memoryless.MemoryScope != "" { + t.Errorf("memory-less scope = %q, want empty", memoryless.MemoryScope) + } + if want := []string{"-c", "memories=false"}; !reflect.DeepEqual(profile.NoMemoryArgs, want) { + t.Errorf("no-memory args = %q, want %q", profile.NoMemoryArgs, want) + } + if want := []string{"resume", "--last"}; !reflect.DeepEqual(profile.ContinueArgs, want) { + t.Errorf("continue args = %q, want %q", profile.ContinueArgs, want) + } + if want := []string{"resume"}; !reflect.DeepEqual(profile.ResumeArgs, want) { + t.Errorf("resume args = %q, want %q", profile.ResumeArgs, want) + } + + blank := model.Profile{ConfigDir: ".custom", NoMemoryArgs: []string{"", " "}} + if err := validateAndNormalizeProfile(&blank); err != nil { + t.Fatal(err) + } + if blank.NoMemoryArgs != nil { + t.Errorf("all-blank no-memory args = %q, want nil", blank.NoMemoryArgs) + } +} + +func TestRejectInvalidMemoryPolicy(t *testing.T) { + for _, profile := range []model.Profile{ + {MemoryScope: "global"}, + {MemoryScope: model.MemoryScopeProject}, + {MemoryScope: model.MemoryScopeSession}, + {MemoryScope: model.MemoryScopeSession, MemoryDir: ".custom/memory"}, + {StatePaths: []string{"memory/"}}, + {ConfigDir: ".custom", StatePaths: []string{"../memory"}}, + {ConfigDir: ".custom", StatePaths: []string{"/memory"}}, + {ConfigDir: ".custom", StatePaths: []string{"."}}, + {ConfigDir: ".custom", StatePaths: []string{""}}, + {ConfigDir: ".custom", StatePaths: []string{"["}}, + {ConfigDir: ".custom", StatePaths: []string{"+memory"}}, + } { + if err := validateAndNormalizeProfile(&profile); err == nil { + t.Errorf("accepted invalid memory policy: %+v", profile) + } + } +} + // testPathsWithExtensions builds a model.Paths whose ToolsDir/FeaturesDir // point at internal/config//{tools,features}, with no user // override dirs configured. diff --git a/internal/config/spec_map.go b/internal/config/spec_map.go index 2b4e29aa..6944bc71 100644 --- a/internal/config/spec_map.go +++ b/internal/config/spec_map.go @@ -342,6 +342,9 @@ func specToProfile(doc specDocument) model.Profile { p.ConfigDir = sb.ConfigDir p.SkillsDir = sb.SkillsDir p.MemoryDir = sb.MemoryDir + p.MemoryScope = sb.MemoryScope + p.NoMemoryArgs = sb.NoMemoryArgs + p.StatePaths = sb.StatePaths p.SettingsFile = sb.SettingsFile p.SettingsTarget = sb.SettingsTarget p.PassthroughPaths = sb.PassthroughPaths diff --git a/internal/config/spec_summary.go b/internal/config/spec_summary.go index b99f05e4..b23d88df 100644 --- a/internal/config/spec_summary.go +++ b/internal/config/spec_summary.go @@ -107,6 +107,10 @@ type SpecSummary struct { EntrypointOverride string ContinueArgs string ResumeArgs string + NoMemoryArgs string + MemoryDir string + MemoryScope string + StatePaths []string PostStartOpenIDE string Providers []SpecProviderSummary HostExposure SpecHostExposure @@ -227,11 +231,19 @@ func summarizeSpecDocument(doc specDocument) SpecSummary { if doc.Sandbox.Entrypoint != nil { summary.EntrypointOverride = entrypointCommand(doc.Sandbox.Entrypoint) } - // continueArgs/resumeArgs are appended to the agent's argv for - // `--continue`/`--resume`, so they grant whatever those flags grant — - // including approval bypass — without going through sandbox.yoloFlag. + // continueArgs/resumeArgs/noMemoryArgs are appended to the agent's argv + // for `--continue`/`--resume`/`--no-memory`, so they grant whatever + // those flags grant, including approval bypass, without going through + // sandbox.yoloFlag. summary.ContinueArgs = strings.Join(doc.Sandbox.ContinueArgs, " ") summary.ResumeArgs = strings.Join(doc.Sandbox.ResumeArgs, " ") + summary.NoMemoryArgs = strings.Join(doc.Sandbox.NoMemoryArgs, " ") + // memoryDir is a writable host bind mount the agent keeps between runs, + // and statePaths pins store content the config overlay may not replace, + // so both persist beyond a single session. + summary.MemoryDir = strings.TrimSpace(doc.Sandbox.MemoryDir) + summary.MemoryScope = strings.TrimSpace(doc.Sandbox.MemoryScope) + summary.StatePaths = doc.Sandbox.StatePaths summary.HostExposure.PassthroughPaths = doc.Sandbox.PassthroughPaths summary.HostExposure.HostConfigDir = doc.Sandbox.HostConfigDir summary.HostExposure.HostCredentialsFile = doc.Sandbox.HostCredentials diff --git a/internal/config/testdata/golden/tool-claude.json b/internal/config/testdata/golden/tool-claude.json index 6c75fbf0..a347c119 100644 --- a/internal/config/testdata/golden/tool-claude.json +++ b/internal/config/testdata/golden/tool-claude.json @@ -12,6 +12,11 @@ "config_dir": ".claude", "skills_dir": ".claude/skills", "memory_dir": ".claude/memory", + "memory_scope": "project", + "no_memory_args": [ + "--settings", + "{\"autoMemoryEnabled\": false}" + ], "settings_file": "claude-settings.json", "settings_target": ".claude/settings.json", "passthrough_paths": [ diff --git a/internal/config/testdata/golden/tool-codex.json b/internal/config/testdata/golden/tool-codex.json index 16fe6990..576fd4b5 100644 --- a/internal/config/testdata/golden/tool-codex.json +++ b/internal/config/testdata/golden/tool-codex.json @@ -12,6 +12,19 @@ "yolo_enabled": true, "config_dir": ".codex", "skills_dir": ".codex/skills", + "memory_dir": ".codex/memories", + "memory_scope": "session", + "no_memory_args": [ + "-c", + "features.memories=false" + ], + "state_paths": [ + "memories/", + "memories_*.sqlite*", + "thread-writer-locks/", + "state_*.sqlite*", + "thread_history_*.sqlite*" + ], "settings_file": "codex-config.toml", "settings_target": ".codex/config.toml", "passthrough_paths": [ diff --git a/internal/extinstall/capabilities.go b/internal/extinstall/capabilities.go index 714389cb..d608a730 100644 --- a/internal/extinstall/capabilities.go +++ b/internal/extinstall/capabilities.go @@ -9,6 +9,7 @@ package extinstall import ( "bufio" + "fmt" "io/fs" "os" "path/filepath" @@ -300,6 +301,18 @@ func (c capabilities) allPorts() []string { return dedupeSorted(ports) } +// describeMemory renders the agent-memory grant: a writable host directory the +// tool keeps between sessions. The scope is included because it decides how +// widely that directory is shared: one per project, or one per config store. +func describeMemory(spec config.SpecSummary) string { + if spec.MemoryDir == "" { + return "" + } + // The summary reports the spec document as written, so an undeclared scope + // still reaches here and is resolved for display. + return fmt.Sprintf("%s (scope %s)", spec.MemoryDir, model.ResolveMemoryScope(spec.MemoryScope)) +} + func describeProvider(p config.SpecProviderSummary) string { parts := []string{p.Name} if len(p.AuthFiles) > 0 { diff --git a/internal/extinstall/capabilities_diff.go b/internal/extinstall/capabilities_diff.go index 7ba9d467..bea2953e 100644 --- a/internal/extinstall/capabilities_diff.go +++ b/internal/extinstall/capabilities_diff.go @@ -41,6 +41,7 @@ func diffCapabilities(before capabilities, after capabilities) []string { changes = append(changes, diffScalar("entrypoint override", before.Spec.EntrypointOverride, after.Spec.EntrypointOverride)...) changes = append(changes, diffScalar("continue args", before.Spec.ContinueArgs, after.Spec.ContinueArgs)...) changes = append(changes, diffScalar("resume args", before.Spec.ResumeArgs, after.Spec.ResumeArgs)...) + changes = append(changes, diffScalar("no-memory args", before.Spec.NoMemoryArgs, after.Spec.NoMemoryArgs)...) changes = append(changes, diffScalar("post-start IDE launch", before.Spec.PostStartOpenIDE, after.Spec.PostStartOpenIDE)...) changes = append(changes, diffList("startup script", before.StartupScripts, after.StartupScripts)...) changes = append(changes, diffList("startup command", before.Spec.StartupCommands, after.Spec.StartupCommands)...) @@ -57,6 +58,8 @@ func diffCapabilities(before capabilities, after capabilities) []string { changes = append(changes, diffList("provider", mapDescribe(before.Spec.Providers, describeProvider), mapDescribe(after.Spec.Providers, describeProvider))...) + changes = append(changes, diffScalar("agent memory", describeMemory(before.Spec), describeMemory(after.Spec))...) + changes = append(changes, diffList("preserved state path", before.Spec.StatePaths, after.Spec.StatePaths)...) beforeHosts, afterHosts := before.Spec.HostExposure, after.Spec.HostExposure changes = append(changes, diffList("passthrough path", beforeHosts.PassthroughPaths, afterHosts.PassthroughPaths)...) changes = append(changes, diffScalar("host config dir", beforeHosts.HostConfigDir, afterHosts.HostConfigDir)...) diff --git a/internal/extinstall/capabilities_render.go b/internal/extinstall/capabilities_render.go index 2479b2f5..10b367f2 100644 --- a/internal/extinstall/capabilities_render.go +++ b/internal/extinstall/capabilities_render.go @@ -52,6 +52,7 @@ func (c capabilities) render(w io.Writer, style Style, source string) { // they grant depends on the flags they carry. row("continue args", c.Spec.ContinueArgs) row("resume args", c.Spec.ResumeArgs) + row("no-memory args", c.Spec.NoMemoryArgs) skipsApproval := "" if c.yoloActive() { skipsApproval = fmt.Sprintf("%s (agent executes actions on its own, without asking you to approve each one)", c.Spec.YoloFlag) @@ -98,6 +99,10 @@ func (c capabilities) render(w io.Writer, style Style, source string) { postStart = c.Spec.PostStartOpenIDE + " (launched on the host once the container is running; the session runs detached)" } row("post-start", postStart) + // Reported with the host-exposure rows: both name host state the extension + // keeps between sessions rather than anything it runs. + row("agent memory", describeMemory(c.Spec)) + row("preserved state", strings.Join(c.Spec.StatePaths, ", ")) row("passthrough paths", strings.Join(c.Spec.HostExposure.PassthroughPaths, ", ")) row("host config dir", c.Spec.HostExposure.HostConfigDir) row("host credentials file", c.Spec.HostExposure.HostCredentialsFile) diff --git a/internal/extinstall/capabilities_test.go b/internal/extinstall/capabilities_test.go index e59f3b27..02efa650 100644 --- a/internal/extinstall/capabilities_test.go +++ b/internal/extinstall/capabilities_test.go @@ -33,6 +33,11 @@ sandbox: run: [foo, --serve] passthroughPaths: - agents/ + memoryDir: .config/foo/memories + memoryScope: session + noMemoryArgs: [-c, memories=false] + statePaths: + - memories/ hostConfigDir: .foo hostCredentialsFile: .credentials.json hostOauthJson: .foo.json @@ -132,6 +137,15 @@ func TestInspectReportsCapabilities(t *testing.T) { if caps.Spec.EntrypointOverride != "foo --serve" { t.Errorf("EntrypointOverride = %q, want %q", caps.Spec.EntrypointOverride, "foo --serve") } + if caps.Spec.NoMemoryArgs != "-c memories=false" { + t.Errorf("NoMemoryArgs = %q", caps.Spec.NoMemoryArgs) + } + if want := ".config/foo/memories (scope session)"; describeMemory(caps.Spec) != want { + t.Errorf("describeMemory() = %q, want %q", describeMemory(caps.Spec), want) + } + if len(caps.Spec.StatePaths) != 1 || caps.Spec.StatePaths[0] != "memories/" { + t.Errorf("StatePaths = %v", caps.Spec.StatePaths) + } if len(caps.Spec.EnvironmentVars) != 1 || caps.Spec.EnvironmentVars[0] != "NODE_TLS_REJECT_UNAUTHORIZED=0" { t.Errorf("EnvironmentVars = %v", caps.Spec.EnvironmentVars) } @@ -193,6 +207,7 @@ func TestRenderIncludesEveryReportedCapability(t *testing.T) { "foo --serve", "NODE_TLS_REJECT_UNAUTHORIZED=0", "X-Acme-Token", "~/.acme/token", "json", "ACME_SECURESTORAGE_DIR", "1455", "agents/", ".credentials.json", ".foo.json", ".config/foo", "hosts.yml", "README.md", ".gitconfig", + "-c memories=false", ".config/foo/memories (scope session)", "memories/", } { if !strings.Contains(rendered, want) { t.Errorf("rendered summary is missing %q:\n%s", want, rendered) @@ -215,6 +230,7 @@ func TestRenderOmitsEmptyLines(t *testing.T) { "host config dir", "host credentials file", "host oauth json", "mixin config dir", "mixin auth files", "workspace files", "home files", "credential →", "credential file", "skips approval", + "no-memory args", "agent memory", "preserved state", } { if strings.Contains(out.String(), unwanted) { t.Errorf("rendered summary should omit %q:\n%s", unwanted, out.String()) @@ -660,6 +676,17 @@ func capabilityFieldCases() map[string]capabilityFieldCase { "12345 (published on the host)", "port 12345 (published on the host)"}, "Spec.ContinueArgs": {func(c *capabilities) { c.Spec.ContinueArgs = "resume --last" }, "resume --last", "continue args"}, "Spec.ResumeArgs": {func(c *capabilities) { c.Spec.ResumeArgs = "resume" }, "resume", "resume args"}, + "Spec.NoMemoryArgs": {func(c *capabilities) { c.Spec.NoMemoryArgs = "-c memories=false" }, "-c memories=false", "no-memory args"}, + "Spec.MemoryDir": {func(c *capabilities) { c.Spec.MemoryDir = ".tool/memories" }, + ".tool/memories (scope project)", "agent memory"}, + // A scope alone grants nothing, so the memory row is keyed on memoryDir + // and the setter sets both; the markers below are the scope's own. + "Spec.MemoryScope": {func(c *capabilities) { + c.Spec.MemoryDir = ".tool/memories" + c.Spec.MemoryScope = "session" + }, "scope session", "agent memory"}, + "Spec.StatePaths": {func(c *capabilities) { c.Spec.StatePaths = []string{"memories/"} }, + "memories/", "preserved state path memories/"}, "Spec.PostStartOpenIDE": {func(c *capabilities) { c.Spec.PostStartOpenIDE = "theia" }, "launched on the host", "post-start IDE launch"}, "Spec.CredentialEnv": {func(c *capabilities) { c.Spec.CredentialEnv = []string{"MY_TOKEN"} }, "MY_TOKEN", "credential MY_TOKEN"}, diff --git a/internal/model/types.go b/internal/model/types.go index 7cdc0ea7..c4eb2cfe 100644 --- a/internal/model/types.go +++ b/internal/model/types.go @@ -7,7 +7,10 @@ package model -import "sort" +import ( + "sort" + "strings" +) type RunOptions struct { Tool string @@ -45,6 +48,14 @@ type RunOptions struct { Force bool } +// MemoryDisabled reports whether this run must not use or generate agent +// memory: an explicit --no-memory, or an ephemeral session, whose writes would +// otherwise land in the discarded per-session config store. The memory mount +// and the tool's noMemoryArgs both follow this single predicate. +func (r RunOptions) MemoryDisabled() bool { + return r.NoMemory || r.Ephemeral +} + type AuthOptions struct { ResetAuth bool NoAPIKey bool @@ -201,6 +212,21 @@ type ConfigView struct { JSON bool } +const ( + MemoryScopeProject = "project" + MemoryScopeSession = "session" +) + +// ResolveMemoryScope maps an undeclared memory scope onto the default, so no +// consumer has to re-derive what an empty value means. Profiles are normalized +// at load; this also serves the readers that see the raw spec document. +func ResolveMemoryScope(scope string) string { + if strings.TrimSpace(scope) == "" { + return MemoryScopeProject + } + return scope +} + type Profile struct { Name string `json:"name"` Command string `json:"command"` @@ -211,6 +237,9 @@ type Profile struct { ConfigDir string `json:"config_dir"` SkillsDir string `json:"skills_dir,omitempty"` MemoryDir string `json:"memory_dir,omitempty"` + MemoryScope string `json:"memory_scope,omitempty"` + NoMemoryArgs []string `json:"no_memory_args,omitempty"` + StatePaths []string `json:"state_paths,omitempty"` SettingsFile string `json:"settings_file"` SettingsTarget string `json:"settings_target"` PassthroughPaths []string `json:"passthrough_paths,omitempty"` diff --git a/internal/runtime/command_builder.go b/internal/runtime/command_builder.go index c01a8880..93c56b1f 100644 --- a/internal/runtime/command_builder.go +++ b/internal/runtime/command_builder.go @@ -41,6 +41,9 @@ func (b commandBuilder) Build() []string { if b.yoloEnabled && b.profile.YoloFlag != "" { agentArgs = append(agentArgs, strings.Fields(b.profile.YoloFlag)...) } + if b.run.MemoryDisabled() { + agentArgs = append(agentArgs, b.profile.NoMemoryArgs...) + } agentArgs = append(agentArgs, b.run.CmdArgs...) // bash -c uses the next argument as $0; the rest become $1.. for exec "$@". diff --git a/internal/runtime/memory_mount_test.go b/internal/runtime/memory_mount_test.go index bb3313fd..4c3de8ef 100644 --- a/internal/runtime/memory_mount_test.go +++ b/internal/runtime/memory_mount_test.go @@ -9,12 +9,103 @@ package runtime import ( "os" + "path/filepath" + "reflect" "testing" + "enclave/internal/backend" "enclave/internal/config" "enclave/internal/model" ) +func TestSessionMemoryMountFollowsConfigStoreKey(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + run model.RunOptions + worktree bool + concurrent bool + wantKey string + }{ + {name: "default", wantKey: "default"}, + {name: "named", run: model.RunOptions{SessionName: "review"}, wantKey: "review"}, + {name: "background", run: model.RunOptions{Background: true, SessionName: "worker"}, wantKey: "worker"}, + {name: "worktree", run: model.RunOptions{HostConfig: model.HostConfigPassthrough}, worktree: true}, + {name: "concurrent", concurrent: true, wantKey: "2"}, + } { + t.Run(tc.name, func(t *testing.T) { + project := model.Project{Dir: "/tmp/repo", RealDir: "/tmp/repo"} + project.Hash = projectPathHash(project) + if tc.worktree { + project.Dir = "/tmp/repo-feature" + project.RealDir = project.Dir + tc.wantKey = projectPathHash(project) + } + r := &Runtime{ + host: model.Host{Home: t.TempDir()}, + project: project, + run: tc.run, + profile: model.Profile{Name: "custom", ConfigDir: ".custom", MemoryDir: ".custom/memories", MemoryScope: model.MemoryScopeSession}, + containerHome: "/home/agent", + backend: &fakeBackend{configKeyInUse: func(backend.SessionMeta, string) (bool, error) { + return tc.concurrent, nil + }}, + } + containerName := "enclave-custom-project" + if tc.concurrent { + containerName += "-2" + } + r.setConfigVolumeSuffix(containerName, "enclave-custom-project") + acc := newMountAccumulator(nil, nil) + r.addMemoryMounts(acc) + got, ok := lookupMountSource(acc.Mounts(), "/home/agent/.custom/memories") + want := filepath.Join(config.HostProjectMemoryDir(r.host.Home, project.Hash, "custom"), tc.wantKey) + if !ok || got != want { + t.Fatalf("memory mount = %q, want %q", got, want) + } + }) + } +} + +func TestMemoryDisableArgsAtLaunch(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + run model.RunOptions + disabled bool + }{ + {name: "normal"}, + {name: "no-memory", run: model.RunOptions{NoMemory: true}, disabled: true}, + {name: "ephemeral", run: model.RunOptions{Ephemeral: true}, disabled: true}, + {name: "both", run: model.RunOptions{NoMemory: true, Ephemeral: true}, disabled: true}, + } { + t.Run(tc.name, func(t *testing.T) { + for _, args := range [][]string{nil, {"resume", "--last"}, {"exec", "a prompt"}} { + r := &Runtime{ + profile: model.Profile{Name: "custom", Command: "custom", NoMemoryArgs: []string{"-c", "features.memories=false"}}, + run: tc.run, + } + r.run.CmdArgs = args + // Assert the complete command, including the wrapper's $0 argument. + want := []string{"bash", "-c", agentWrapperTemplate, "bash", "custom"} + if tc.disabled { + want = append(want, "-c", "features.memories=false") + } + want = append(want, args...) + if got := newCommandBuilder(r).Build(); !reflect.DeepEqual(got, want) { + t.Fatalf("command = %q, want %q", got, want) + } + r.profile.NoMemoryArgs = nil + want = append([]string{"bash", "-c", agentWrapperTemplate, "bash", "custom"}, args...) + if got := newCommandBuilder(r).Build(); !reflect.DeepEqual(got, want) { + t.Fatalf("tool without memory controls = %q, want %q", got, want) + } + } + }) + } +} + func TestAddMemoryMountsSkipsWhenMemoryDirUnset(t *testing.T) { t.Parallel() diff --git a/internal/runtime/runtime.go b/internal/runtime/runtime.go index 7fb72609..b5d4cfc5 100644 --- a/internal/runtime/runtime.go +++ b/internal/runtime/runtime.go @@ -1198,18 +1198,16 @@ func (r *Runtime) addHistoryMounts(mounts *mountAccumulator) { } func (r *Runtime) addMemoryMounts(mounts *mountAccumulator) { - if r.run.NoMemory { - return - } - // Ephemeral sessions must not persist memory: skip the host bind mount so - // the agent's writes land in the discarded per-session config store. - if r.run.Ephemeral { + if r.run.MemoryDisabled() { return } if r.profile.MemoryDir == "" { return } memDir := config.HostProjectMemoryDir(r.host.Home, r.project.Hash, r.profile.Name) + if r.profile.MemoryScope == model.MemoryScopeSession { + memDir = config.HostProjectMemorySessionDir(r.host.Home, r.project.Hash, r.profile.Name, r.sessionGeneratedKey()) + } if err := os.MkdirAll(memDir, 0o700); err != nil { logx.Warnf("Failed to create memory directory %s: %v", memDir, err) return