diff --git a/AGENTS.md b/AGENTS.md index d36b24b..3b62dce 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -26,6 +26,7 @@ - A module.yml may declare `key: ` to become multi-instance: it can be installed several times per workspace (e.g. one `ssh` entry per host, one `aws` entry per profile), each distinguished by that prompt's value. The instance identity is `name:keyvalue` (`ModuleInstance.InstanceID`); the key prompt must be required and non-secret. Without `key` a module is a singleton and a second install replaces the first. The install/uninstall scripts must be instance-scoped by the key (e.g. ssh writes `~/.ssh/config.d/$OCM_HOST.conf`) so multiple entries coexist and each uninstall removes only its own. - The `key` prompt of a multi-instance module may carry an `optionsCommand` (the only non-select prompt allowed to). It runs on the host to list accounts importable as new instances (e.g. `aws`/`outscale` list profiles, `ssh` lists `~/.ssh/config` host aliases). In the editor's add flow this drives an import picker: selecting one or more host accounts creates one instance each storing only the key value, with the actual credentials pulled by the module's `resolve` hook at install time (so secrets never reach the manifest); the picker also offers an "Add manually…" option that opens a single-page form (all fields at once) for an account not present on the host, whose typed values are stored as usual. The key prompt stays a free-text `string` so manual entry still works; `resolve` must emit nothing for an account it does not find on the host, leaving the manually-typed values in place. - A module.yml may declare `restartServer: false` to mark itself environment-neutral: it writes only its own config files (e.g. `~/.kube/config`, `~/.aws/credentials`) that tools read live, never `~/.env`. Such a module is never bounced (the manager skips the server restart even if `~/.env` somehow changed — and logs a warning, since that means the manifest lies) and, crucially, its add/remove is allowed from the editor **while a task is running**, because nothing interrupts the in-flight session. Omitting the key (or setting `restartServer: true`) keeps the conservative default: the module is assumed to touch `~/.env`, so the manager bounces the server when `~/.env` changes and the editor blocks the edit until the workspace is idle. All current built-ins (`aws`, `git`, `kubernetes`, `outscale`, `ssh`) are `restartServer: false`. +- A module.yml may declare `mounts` (`source` absolute or `~/`-relative on the host, absolute `target`, `readOnly`, `optional`). They are bind-mounted only into the containers of workspaces that have the module installed: `provision` collects them from the manifest's modules (`installedModuleMounts`), creates their mount points inside the workspace home on the host, and records a fingerprint in `OCM_MODULE_MOUNTS_SHA256`, so `containerSpecDrift` recreates the container when the set changes. `AddModule` records such a module in the manifest before starting the container, so its install runs with the mounts in place; on failure the manifest entry is rolled back and the container recreated without the mounts; `RemoveModule` recreates the container after the uninstall. A module with mounts cannot be `restartServer: false`, since recreation interrupts a running task. An `optional` mount with a missing source is skipped; a required one fails the start. - Workspace containers are long-lived and tied to the workspace lifecycle. - Shared OpenCode config lives in `~/.config/opencode-manager/opencode/` and is copied one-way into each workspace's `home/.config/opencode/` on provision, startup reconcile, and source changes while the manager runs. A per-workspace journal permits safe removal of previously managed source entries. The shared source must not contain generated top-level state (`.gitignore`, package manifests/lockfiles, or `node_modules`); those are preserved per workspace. Existing legacy root templates migrate only when `opencode/` is absent or empty. - Opt-in `selfImprovement.enabled` creates an OpenCode-only internal instance under `/internal/self-improvement`, outside `Registry.List`. `ocm improve` or dashboard `i` launches a fresh local OpenCode TUI with an analysis prompt. It mounts the entire manager configuration read-write, ordinary workspace homes read-only, and named `selfImprovement.directories` roots. Its port still participates in allocation. Agents/commands/helpers are manager-owned; personal `self-improvement/AGENTS.md` extends or replaces the built-in protocol. `sessions.ts` enforces a persistent initial baseline, incremental chunk budgets and checkpoints; `memory.ts` retains cumulative evidence and decisions and applies accepted proposals with hash checks/backups, independently of Git. See `docs/self-improvement.md`; regressions run with `bun test internal/workspace/improvement_sessions.test.ts` (also part of `npm test`). diff --git a/docs/writing-modules.md b/docs/writing-modules.md index 8e13345..c00b899 100644 --- a/docs/writing-modules.md +++ b/docs/writing-modules.md @@ -59,6 +59,7 @@ prompts: | `restartServer` | `true` (default) if the module touches `~/.env`; `false` if it only writes its own config files. | | `key` | Name of a prompt that makes the module **multi-instance** (one install per distinct value). | | `prompts` | The values to collect from the user. | +| `mounts` | Optional host paths bind-mounted into the containers of the workspaces that have the module (see [Mounts](#mounts)). | ### Prompt types @@ -101,6 +102,42 @@ and the editor blocks edits while a task is running. If your module only writes its own config files that tools read live (like `~/.aws` or `~/.kube/config`), set `restartServer: false` — it is never bounced and can be edited mid-task. +## Mounts + +A module can bind-mount host paths into the container of each workspace it is +installed in — and only those. Unlike the global +[`extraMounts`](configuration.md#extramounts), this lets you pick per workspace +which ones get access to a host file, such as a login shared with the host +instead of copied: + +```yaml +name: claude-auth +version: 1 +description: Share this host's Claude Code login with the workspace. +mounts: + - source: ~/.claude/.credentials.json + target: /home/debian/.claude/.credentials.json + optional: true +``` + +| Field | Meaning | +| --- | --- | +| `source` | Host path: absolute, or starting with `~/` for the host user's home. | +| `target` | Absolute path inside the container. Cannot be `/` or the workspace home itself. | +| `readOnly` | `true` to prevent container writes. Default `false`. | +| `optional` | `true` to skip the mount while `source` does not exist instead of failing the workspace start; it is added on the first start after it appears. | + +The mounts are part of the container's spec, so adding or removing such a +module **recreates the workspace container** (the home and the other modules +are preserved and reinstalled). On add, the module is recorded and the container +recreated before `install` runs, so the script already sees the mounted paths; on +removal the container is recreated after `uninstall`, so access ends right away. +Because recreation interrupts a running task, a module with mounts cannot set +`restartServer: false`. + +Mount points inside the workspace home are created on the host first (an empty +file or a directory, owned by you), so the runtime never creates them as root. + ## Host hooks (optional) Two optional hooks run **on the host** (with full host access), as part of an @@ -138,5 +175,7 @@ leaving any manually-typed values in place. - [ ] `uninstall` fully reverses `install` for that instance. - [ ] Secrets resolved on the host via `resolve` rather than stored in the manifest, where applicable. +- [ ] `mounts` only when the container must use the host path itself (live, + shared state) rather than a copy made by `resolve`. - [ ] Tested by adding/removing the module on a running workspace from the editor. diff --git a/internal/module/module.go b/internal/module/module.go index b9199a3..45bfb4d 100644 --- a/internal/module/module.go +++ b/internal/module/module.go @@ -72,6 +72,37 @@ type Module struct { // lives under (e.g. "cloud" for modules/cloud/aws). It organizes the module // browser in the TUI and is mirrored in the container mount path. Category string + // Mounts are host paths bind-mounted into the container of every workspace + // that has this module installed, and only those. Adding or removing such a + // module recreates the workspace container, so it always restarts the server. + Mounts []Mount +} + +// Mount is a host path a module bind-mounts into the workspace container. +type Mount struct { + // Source is the host path: absolute, or relative to the host user's home + // when it starts with "~/". + Source string `yaml:"source"` + // Target is the absolute path inside the container. + Target string `yaml:"target"` + ReadOnly bool `yaml:"readOnly"` + // Optional skips the mount while Source does not exist on the host instead + // of failing the workspace start. The mount is added on the first start + // after it appears. + Optional bool `yaml:"optional"` +} + +// HostSource returns Source with a leading "~/" expanded to the host user's +// home directory. +func (m Mount) HostSource() (string, error) { + if !strings.HasPrefix(m.Source, "~/") { + return m.Source, nil + } + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("find user home directory: %w", err) + } + return filepath.Join(home, m.Source[2:]), nil } // Multi reports whether the module can be installed as multiple independent @@ -136,6 +167,7 @@ type definition struct { Key string `yaml:"key"` RestartServer *bool `yaml:"restartServer"` Prompts []Prompt `yaml:"prompts"` + Mounts []Mount `yaml:"mounts"` } // HasResolveHook reports whether the module ships a host-side resolve script. @@ -163,6 +195,7 @@ func Load(dir string) (Module, error) { Prompts: def.Prompts, Key: def.Key, RestartServer: def.RestartServer == nil || *def.RestartServer, + Mounts: def.Mounts, Dir: dir, // The category is the name of the directory the module lives under, e.g. // modules/cloud/aws -> "cloud". @@ -172,6 +205,11 @@ func Load(dir string) (Module, error) { if err := mod.validate(); err != nil { return Module{}, fmt.Errorf("invalid module %q: %w", path, err) } + // A module with mounts recreates the container on add and remove, which + // interrupts a running task just like a server bounce does. + if len(mod.Mounts) > 0 && !mod.RestartServer { + return Module{}, fmt.Errorf("invalid module %q: a module with mounts cannot set restartServer: false (adding or removing it recreates the container)", path) + } return mod, nil } @@ -253,6 +291,27 @@ func (m Module) validate() error { } } + targets := map[string]bool{} + for _, mount := range m.Mounts { + if mount.Source == "" || mount.Target == "" { + return fmt.Errorf("mount source and target are required") + } + if !filepath.IsAbs(mount.Source) && !strings.HasPrefix(mount.Source, "~/") { + return fmt.Errorf("mount source %q must be absolute or start with ~/", mount.Source) + } + if !filepath.IsAbs(mount.Target) { + return fmt.Errorf("mount target %q must be absolute", mount.Target) + } + target := filepath.Clean(mount.Target) + if target == "/" || target == "/home/debian" { + return fmt.Errorf("mount target %q cannot replace %s", mount.Target, target) + } + if targets[target] { + return fmt.Errorf("mount target %q is duplicated", mount.Target) + } + targets[target] = true + } + if err := checkExecutable(filepath.Join(m.Dir, InstallScript)); err != nil { return err } diff --git a/internal/module/module_test.go b/internal/module/module_test.go index b40871d..9c2a381 100644 --- a/internal/module/module_test.go +++ b/internal/module/module_test.go @@ -402,3 +402,57 @@ func TestBuiltinModulesAreValid(t *testing.T) { t.Fatal("kubernetes context prompt should expose an optionsCommand for the import picker") } } + +func TestLoadMounts(t *testing.T) { + root := t.TempDir() + dir := writeModule(t, root, "claude", `name: claude +version: 1 +mounts: + - source: ~/.claude/.credentials.json + target: /home/debian/.claude/.credentials.json + optional: true + - { source: /etc/hosts, target: /etc/host-hosts, readOnly: true } +`) + + mod, err := Load(dir) + if err != nil { + t.Fatalf("Load: %v", err) + } + if len(mod.Mounts) != 2 || !mod.Mounts[0].Optional || !mod.Mounts[1].ReadOnly { + t.Fatalf("unexpected mounts: %+v", mod.Mounts) + } + if !mod.RestartServer { + t.Fatal("a module with mounts must restart the server") + } + + home, err := os.UserHomeDir() + if err != nil { + t.Fatal(err) + } + source, err := mod.Mounts[0].HostSource() + if err != nil { + t.Fatalf("HostSource: %v", err) + } + if want := filepath.Join(home, ".claude/.credentials.json"); source != want { + t.Fatalf("HostSource = %q, want %q", source, want) + } +} + +func TestLoadRejectsInvalidMounts(t *testing.T) { + cases := map[string]string{ + "relative source": "mounts: [{ source: foo, target: /a }]", + "relative target": "mounts: [{ source: /foo, target: a }]", + "missing target": "mounts: [{ source: /foo }]", + "home target": "mounts: [{ source: /foo, target: /home/debian/ }]", + "duplicate target": "mounts: [{ source: /foo, target: /a }, { source: /bar, target: /a }]", + "no server restart": "restartServer: false\nmounts: [{ source: /foo, target: /a }]", + } + for name, extra := range cases { + t.Run(name, func(t *testing.T) { + dir := writeModule(t, t.TempDir(), "m", "name: m\nversion: 1\n"+extra+"\n") + if _, err := Load(dir); err == nil { + t.Fatal("expected an error") + } + }) + } +} diff --git a/internal/workspace/lifecycle.go b/internal/workspace/lifecycle.go index beeb30e..28475af 100644 --- a/internal/workspace/lifecycle.go +++ b/internal/workspace/lifecycle.go @@ -371,6 +371,12 @@ func (l Lifecycle) provisionWithOptions(ctx context.Context, summary Summary, re // scripts are runnable inside the container. mounts = append(mounts, moduleMounts(l.cfg)...) mounts = append(mounts, extraMounts(l.cfg)...) + // Mounts declared by the modules installed in this workspace only. + installedMounts, installedMountsFingerprint, err := l.installedModuleMounts(manifest) + if err != nil { + return runtime.StatusUnknown, runtime.ContainerSpec{}, err + } + mounts = append(mounts, installedMounts...) if l.isImprovement(summary) { internalMounts, err := l.improvementMounts() if err != nil { @@ -417,6 +423,9 @@ func (l Lifecycle) provisionWithOptions(ctx context.Context, summary Summary, re if fingerprint := extraMountsFingerprint(l.cfg.ExtraMounts); fingerprint != "" { env[extraMountsFingerprintEnv] = fingerprint } + if installedMountsFingerprint != "" { + env[moduleMountsFingerprintEnv] = installedMountsFingerprint + } if l.isImprovement(summary) { privateMounts := make([]config.ExtraMount, 0, len(mounts)) for _, mount := range mounts { @@ -557,6 +566,7 @@ const openCodeAuthRelPath = ".local/share/opencode/auth.json" const ( extraCACertificateFingerprintEnv = "OCM_EXTRA_CA_CERTIFICATE_SHA256" extraMountsFingerprintEnv = "OCM_EXTRA_MOUNTS_SHA256" + moduleMountsFingerprintEnv = "OCM_MODULE_MOUNTS_SHA256" workspaceEnvKeysEnv = "OCM_WORKSPACE_ENV_KEYS" ) @@ -892,6 +902,11 @@ func (l Lifecycle) containerSpecDrift(ctx context.Context, manifest Manifest, sp return true } + if rc.Env[moduleMountsFingerprintEnv] != spec.Env[moduleMountsFingerprintEnv] { + slog.Debug("container module mounts differ from desired", "workspace", manifest.Name, "container", manifest.ContainerName) + return true + } + if rc.Env[workspaceEnvKeysEnv] != spec.Env[workspaceEnvKeysEnv] { slog.Debug("container workspace environment keys differ from desired", "workspace", manifest.Name, "container", manifest.ContainerName) return true diff --git a/internal/workspace/lifecycle_drift_test.go b/internal/workspace/lifecycle_drift_test.go index f3113dd..52653b5 100644 --- a/internal/workspace/lifecycle_drift_test.go +++ b/internal/workspace/lifecycle_drift_test.go @@ -89,6 +89,18 @@ func TestContainerSpecDrift(t *testing.T) { spec: runtime.ContainerSpec{Env: map[string]string{extraMountsFingerprintEnv: "new"}}, want: true, }, + { + name: "module mount added", + rc: runtime.ContainerRuntimeConfig{NetworkMode: "bridge", Env: map[string]string{OpenCodePortEnv: "4097"}}, + spec: runtime.ContainerSpec{Env: map[string]string{moduleMountsFingerprintEnv: "new"}}, + want: true, + }, + { + name: "module mount removed", + rc: runtime.ContainerRuntimeConfig{NetworkMode: "bridge", Env: map[string]string{OpenCodePortEnv: "4097", moduleMountsFingerprintEnv: "old"}}, + spec: runtime.ContainerSpec{Env: map[string]string{}}, + want: true, + }, { name: "workspace environment changed", rc: runtime.ContainerRuntimeConfig{NetworkMode: "bridge", Env: map[string]string{OpenCodePortEnv: "4097", workspaceEnvKeysEnv: "API_TOKEN", "API_TOKEN": "old"}}, diff --git a/internal/workspace/modulemounts_test.go b/internal/workspace/modulemounts_test.go new file mode 100644 index 0000000..63a0e45 --- /dev/null +++ b/internal/workspace/modulemounts_test.go @@ -0,0 +1,222 @@ +package workspace + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/mickael-menu/opencode-manager/internal/config" + "github.com/mickael-menu/opencode-manager/internal/runtime" +) + +// writeMountingModule creates a module in /tools/ declaring mounts. +func writeMountingModule(t *testing.T, root, name, mounts string) { + t.Helper() + dir := filepath.Join(root, "tools", name) + writeTestFile(t, filepath.Join(dir, "module.yml"), []byte("name: "+name+"\nversion: 1\nmounts:\n"+mounts)) + for _, script := range []string{"install", "uninstall"} { + if err := os.WriteFile(filepath.Join(dir, script), []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + } +} + +func TestInstalledModuleMountsOnlyForInstalledModules(t *testing.T) { + modules := t.TempDir() + hostDir := t.TempDir() + creds := filepath.Join(hostDir, "credentials.json") + writeTestFile(t, creds, []byte("{}")) + + writeMountingModule(t, modules, "claude-auth", " - { source: "+creds+", target: /home/debian/.claude/.credentials.json }\n") + writeMountingModule(t, modules, "other", " - { source: "+hostDir+", target: /opt/other }\n") + writeMountingModule(t, modules, "maybe", " - { source: "+filepath.Join(hostDir, "missing")+", target: /opt/maybe, optional: true }\n") + + l := Lifecycle{cfg: config.Config{ModuleDirs: []string{modules}}} + home := t.TempDir() + manifest := Manifest{HomeDir: home} + + mounts, fingerprint, err := l.installedModuleMounts(manifest) + if err != nil || mounts != nil || fingerprint != "" { + t.Fatalf("no modules: got %v %q %v", mounts, fingerprint, err) + } + + manifest.Modules = []ModuleInstance{ + {Name: "claude-auth", Category: "tools", Version: 1}, + {Name: "maybe", Category: "tools", Version: 1}, + } + mounts, fingerprint, err = l.installedModuleMounts(manifest) + if err != nil { + t.Fatalf("installedModuleMounts: %v", err) + } + if len(mounts) != 1 || mounts[0].Source != creds || mounts[0].Target != "/home/debian/.claude/.credentials.json" || mounts[0].ReadOnly { + t.Fatalf("unexpected mounts: %+v", mounts) + } + if fingerprint == "" { + t.Fatal("expected a fingerprint") + } + info, err := os.Stat(filepath.Join(home, ".claude", ".credentials.json")) + if err != nil || info.IsDir() { + t.Fatalf("mount point not created as a file in the workspace home: %v", err) + } + + // The optional source appearing changes the fingerprint, so the container is + // recreated with the new mount. + if err := os.Mkdir(filepath.Join(hostDir, "missing"), 0o755); err != nil { + t.Fatal(err) + } + mounts, withOptional, err := l.installedModuleMounts(manifest) + if err != nil || len(mounts) != 2 || withOptional == fingerprint { + t.Fatalf("optional mount: got %+v %q (was %q) %v", mounts, withOptional, fingerprint, err) + } +} + +func TestInstalledModuleMountsRequiredSourceMissing(t *testing.T) { + modules := t.TempDir() + writeMountingModule(t, modules, "need", " - { source: /nonexistent/ocm-test, target: /opt/need }\n") + l := Lifecycle{cfg: config.Config{ModuleDirs: []string{modules}}} + manifest := Manifest{HomeDir: t.TempDir(), Modules: []ModuleInstance{{Name: "need", Category: "tools", Version: 1}}} + if _, _, err := l.installedModuleMounts(manifest); err == nil { + t.Fatal("expected an error for a missing required mount source") + } +} + +func TestAddMountingModuleRecordsBeforeInstall(t *testing.T) { + modules := t.TempDir() + creds := filepath.Join(t.TempDir(), "credentials.json") + writeTestFile(t, creds, []byte("{}")) + writeMountingModule(t, modules, "claude-auth", " - { source: "+creds+", target: /home/debian/.claude/.credentials.json }\n") + + workspacePath := t.TempDir() + home := filepath.Join(workspacePath, "home") + if err := os.MkdirAll(filepath.Join(home, ".config", "opencode"), 0o755); err != nil { + t.Fatal(err) + } + manifest := Manifest{Name: "demo", Runtime: "docker", ImageName: "img", ContainerName: "c", HomeDir: home} + if err := SaveManifest(filepath.Join(workspacePath, ManifestFile), manifest); err != nil { + t.Fatal(err) + } + + fake := &fakeDriver{output: func(args []string) []byte { return nil }} + l := Lifecycle{cfg: config.Config{ModuleDirs: []string{modules}}, driver: fake} + catalog, err := l.Catalog() + if err != nil || len(catalog) != 1 { + t.Fatalf("Catalog: %v %v", catalog, err) + } + + if err := l.AddModule(context.Background(), Summary{Manifest: manifest, Path: workspacePath}, catalog[0], nil); err != nil { + t.Fatalf("AddModule: %v", err) + } + + saved, err := LoadManifest(filepath.Join(workspacePath, ManifestFile)) + if err != nil { + t.Fatal(err) + } + if len(saved.Modules) != 1 || saved.Modules[0].Name != "claude-auth" { + t.Fatalf("manifest modules: %+v", saved.Modules) + } + installs := 0 + for _, args := range fake.gotArgs { + if contains(args, "/opt/opencode-manager/modules/tools/claude-auth/install") { + installs++ + } + } + // The fake marker never records it, so reconcile and the explicit run both + // install; what matters is that it ran after the manifest listed it. + if installs == 0 { + t.Fatal("install script never ran") + } +} + +// mountDriver keeps the spec of the container it last created, reports it back +// as the running container's config, and fails the scripts matching failExec. +type mountDriver struct { + *fakeDriver + current *runtime.ContainerSpec + created int + failExec string +} + +func (d *mountDriver) ContainerStatus(context.Context, string) (string, error) { + if d.current == nil { + return runtime.StatusMissing, nil + } + return runtime.StatusRunning, nil +} +func (d *mountDriver) ContainerRuntimeConfig(context.Context, string) (runtime.ContainerRuntimeConfig, error) { + if d.current == nil { + return runtime.ContainerRuntimeConfig{}, errors.New("no container") + } + return runtime.ContainerRuntimeConfig{Env: d.current.Env}, nil +} +func (d *mountDriver) CreateContainer(_ context.Context, spec runtime.ContainerSpec) error { + d.current = &spec + d.created++ + return nil +} +func (d *mountDriver) RemoveContainer(context.Context, string) error { d.current = nil; return nil } +func (d *mountDriver) Exec(ctx context.Context, spec runtime.ExecSpec) ([]byte, error) { + if d.failExec != "" && contains(spec.Args, d.failExec) { + return nil, errors.New("install failed") + } + return d.fakeDriver.Exec(ctx, spec) +} + +func TestAddMountingModuleFailedInstallRemovesMount(t *testing.T) { + modules := t.TempDir() + creds := filepath.Join(t.TempDir(), "credentials.json") + writeTestFile(t, creds, []byte("{}")) + writeMountingModule(t, modules, "claude-auth", " - { source: "+creds+", target: /home/debian/.claude/.credentials.json }\n") + + workspacePath := t.TempDir() + home := filepath.Join(workspacePath, "home") + if err := os.MkdirAll(filepath.Join(home, ".config", "opencode"), 0o755); err != nil { + t.Fatal(err) + } + manifest := Manifest{Name: "demo", Runtime: "docker", ImageName: "img", ContainerName: "c", HomeDir: home, OpenCodePort: 4100} + manifestPath := filepath.Join(workspacePath, ManifestFile) + if err := SaveManifest(manifestPath, manifest); err != nil { + t.Fatal(err) + } + + d := &mountDriver{ + fakeDriver: &fakeDriver{output: func([]string) []byte { return nil }}, + failExec: "/opt/opencode-manager/modules/tools/claude-auth/install", + } + l := Lifecycle{cfg: config.Config{ModuleDirs: []string{modules}}, driver: d} + summary := Summary{Manifest: manifest, Path: workspacePath} + if err := l.ensureStarted(context.Background(), summary, false); err != nil { + t.Fatalf("ensureStarted: %v", err) + } + catalog, err := l.Catalog() + if err != nil || len(catalog) != 1 { + t.Fatalf("Catalog: %v %v", catalog, err) + } + + err = l.AddModule(context.Background(), summary, catalog[0], nil) + if err == nil || !strings.Contains(err.Error(), "install failed") { + t.Fatalf("AddModule error = %v, want the install failure", err) + } + // The container was recreated with the mount for the install, then again + // without it after the rollback. + if d.created != 3 { + t.Fatalf("containers created = %d, want 3", d.created) + } + for _, m := range d.current.Mounts { + if m.Source == creds { + t.Fatalf("failed module mount %+v still in the container", m) + } + } + if _, ok := d.current.Env[moduleMountsFingerprintEnv]; ok { + t.Fatal("container still carries the module mounts fingerprint") + } + saved, err := LoadManifest(manifestPath) + if err != nil { + t.Fatal(err) + } + if len(saved.Modules) != 0 { + t.Fatalf("manifest modules after rollback: %+v", saved.Modules) + } +} diff --git a/internal/workspace/modules.go b/internal/workspace/modules.go index 0f5484e..1ee512a 100644 --- a/internal/workspace/modules.go +++ b/internal/workspace/modules.go @@ -94,6 +94,109 @@ func (l Lifecycle) Catalog() ([]module.Module, error) { return module.Catalog([]string{dir}) } +// installedModuleMounts returns the bind mounts declared by the modules installed +// in the workspace, and a fingerprint of them that changes whenever the set does, +// so a workspace container is recreated exactly when one of its mounting modules +// is added or removed (or an optional source appears). Mount points under the +// workspace home are created on the host first, owned by the host user, so the +// runtime does not create them as root. +func (l Lifecycle) installedModuleMounts(manifest Manifest) ([]runtime.Mount, string, error) { + if len(manifest.Modules) == 0 { + return nil, "", nil + } + catalog, err := l.Catalog() + if err != nil { + return nil, "", err + } + byName := make(map[string]module.Module, len(catalog)) + for _, mod := range catalog { + byName[mod.Name] = mod + } + + var mounts []runtime.Mount + targets := map[string]string{} + for _, inst := range manifest.Modules { + mod, ok := byName[inst.Name] + if !ok { + continue + } + for _, declared := range mod.Mounts { + source, err := declared.HostSource() + if err != nil { + return nil, "", err + } + info, err := os.Stat(source) + if err != nil { + if os.IsNotExist(err) && declared.Optional { + slog.Debug("skipping optional module mount with a missing source", "module", mod.Name, "source", source) + continue + } + return nil, "", fmt.Errorf("module %s mount source %q: %w", mod.Name, source, err) + } + target := filepath.Clean(declared.Target) + if previous, ok := targets[target]; ok { + // Several instances of one module declare the same mount. + if previous == source { + continue + } + return nil, "", fmt.Errorf("module %s mounts %q onto %s, already used by %q", mod.Name, source, target, previous) + } + targets[target] = source + if err := ensureHomeMountPoint(manifest.HomeDir, target, info.IsDir()); err != nil { + return nil, "", err + } + mounts = append(mounts, runtime.Mount{Source: source, Target: target, ReadOnly: declared.ReadOnly}) + } + } + if len(mounts) == 0 { + return nil, "", nil + } + data, err := json.Marshal(mounts) + if err != nil { + return nil, "", fmt.Errorf("encode module mounts: %w", err) + } + sum := sha256.Sum256(data) + return mounts, hex.EncodeToString(sum[:]), nil +} + +// ensureHomeMountPoint creates the host-side mount point of a target inside the +// workspace home (a directory or an empty file), with its parent directories. +// Targets outside the home are left to the runtime. +func ensureHomeMountPoint(homeDir, target string, dir bool) error { + rel, err := filepath.Rel(openCodeHomeDir, target) + if homeDir == "" || err != nil || rel == "." || strings.HasPrefix(rel, "..") { + return nil + } + path := filepath.Join(homeDir, rel) + if dir { + if err := os.MkdirAll(path, 0o700); err != nil { + return fmt.Errorf("create module mount point %q: %w", path, err) + } + return nil + } + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + return fmt.Errorf("create module mount point %q: %w", filepath.Dir(path), err) + } + if _, err := os.Lstat(path); err == nil { + return nil + } + file, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, 0o600) + if err != nil { + return fmt.Errorf("create module mount point %q: %w", path, err) + } + return file.Close() +} + +// moduleHasMounts reports whether an installed module declares mounts. +func (l Lifecycle) moduleHasMounts(category, name string) bool { + dir := primaryModuleDir(l.cfg) + if dir == "" || category == "" { + return false + } + mod, err := module.Load(filepath.Join(dir, category, name)) + return err == nil && len(mod.Mounts) > 0 +} + // AddModule installs a module into the workspace and records it in the manifest. // The install script runs inside the container as the workspace user (which has // passwordless sudo) with the prompt values passed as OCM_* environment @@ -106,6 +209,9 @@ func (l Lifecycle) AddModule(ctx context.Context, summary Summary, mod module.Mo } defer unlock() slog.Info("adding module to workspace", "workspace", summary.Manifest.Name, "module", mod.Name) + if len(mod.Mounts) > 0 { + return l.addMountingModule(ctx, summary, mod, values) + } if err := l.ensureStarted(ctx, summary, false); err != nil { return err } @@ -145,6 +251,85 @@ func (l Lifecycle) AddModule(ctx context.Context, summary Summary, mod module.Mo return nil } +// addMountingModule adds a module that declares mounts. The mounts must exist +// before its install script runs, so the module is recorded in the manifest +// first and the container recreated with them; the start's reconcile then runs +// the install. When the install does not succeed the manifest entry is rolled +// back and the container reconciled with it, so the workspace does not keep a +// mount that no installed module grants. +func (l Lifecycle) addMountingModule(ctx context.Context, summary Summary, mod module.Module, values map[string]string) error { + id := mod.InstanceID(values) + manifestPath := filepath.Join(summary.Path, ManifestFile) + manifest, err := LoadManifest(manifestPath) + if err != nil { + return err + } + var previous *ModuleInstance + for _, m := range manifest.Modules { + if m.InstanceID() == id { + m := m + previous = &m + } + } + manifest.Modules = upsertModule(manifest.Modules, id, mod.Name, mod.Category, mod.Version, values) + manifest.UpdatedAt = time.Now().UTC() + if err := SaveManifest(manifestPath, manifest); err != nil { + return err + } + + rollback := func(cause error) error { + current, err := LoadManifest(manifestPath) + if err != nil { + return fmt.Errorf("%w (roll back manifest: %v)", cause, err) + } + current.Modules = removeModule(current.Modules, id) + if previous != nil { + current.Modules = append(current.Modules, *previous) + } + current.UpdatedAt = time.Now().UTC() + if err := SaveManifest(manifestPath, current); err != nil { + return fmt.Errorf("%w (roll back manifest: %v)", cause, err) + } + // The container may already run with the module's mounts; recreate it + // with the restored mount set. Failing that, stop it: the next start + // recreates it from the manifest. + if err := l.ensureStarted(ctx, Summary{Manifest: current, Path: summary.Path}, false); err != nil { + if stopErr := l.driver.StopContainer(ctx, current.ContainerName); stopErr != nil { + return fmt.Errorf("%w (remove module mounts: %v; stop container: %v)", cause, err, stopErr) + } + return fmt.Errorf("%w (remove module mounts: %v; container stopped)", cause, err) + } + return cause + } + + summary.Manifest = manifest + if err := l.ensureStarted(ctx, summary, false); err != nil { + return rollback(err) + } + + // Reconcile already installed it unless the container was not recreated (the + // mounts were unchanged) or its install failed; run it here then, so the + // error reaches the user. + if v, ok := l.readMarker(ctx, manifest.ContainerName)[id]; !ok || v != mod.Version { + homeDir := manifest.HomeDir + before := envHash(homeDir) + installVals, err := l.resolveInstallValues(ctx, mod.Category, mod.Name, values) + if err != nil { + return rollback(err) + } + if err := l.runModuleScript(ctx, summary, mod.Category, mod.Name, module.InstallScript, "install", installVals); err != nil { + return rollback(err) + } + if err := l.writeMarker(ctx, manifest.ContainerName, manifest.Modules); err != nil { + return err + } + l.maybeBounce(ctx, manifest.ContainerName, mod.Name, mod.RestartServer, before, envHash(homeDir)) + } + + slog.Info("module added", "workspace", manifest.Name, "module", mod.Name) + return nil +} + // RemoveModule runs a module's uninstall script and drops it from the manifest. // id is the instance identity (ModuleInstance.InstanceID): the module name for a // singleton, or "name:keyvalue" for one entry of a multi-instance module. @@ -170,6 +355,7 @@ func (l Lifecycle) RemoveModule(ctx context.Context, summary Summary, id string) } } restartServer := l.moduleRestartServer(category, modName) + hasMounts := l.moduleHasMounts(category, modName) homeDir := summary.Manifest.HomeDir before := envHash(homeDir) @@ -194,7 +380,16 @@ func (l Lifecycle) RemoveModule(ctx context.Context, summary Summary, id string) return err } - l.maybeBounce(ctx, manifest.ContainerName, modName, restartServer, before, envHash(homeDir)) + if hasMounts { + // Recreate the container without the module's mounts, so the workspace + // loses access to them right away. The fresh server reads ~/.env anyway. + summary.Manifest = manifest + if err := l.ensureStarted(ctx, summary, false); err != nil { + return err + } + } else { + l.maybeBounce(ctx, manifest.ContainerName, modName, restartServer, before, envHash(homeDir)) + } slog.Info("module removed", "workspace", manifest.Name, "module", id) return nil