diff --git a/api/v1alpha1/context_types.go b/api/v1alpha1/context_types.go index c82c81d3..90cd857e 100644 --- a/api/v1alpha1/context_types.go +++ b/api/v1alpha1/context_types.go @@ -139,6 +139,26 @@ type GitSync struct { // +optional // +kubebuilder:default=HotReload Policy GitSyncPolicy `json:"policy,omitempty"` + + // Reload asks the OpenCode server to re-scan its configuration after a + // HotReload update lands, so the change takes effect without a Pod restart. + // + // This matters for content the server snapshots at instance start — most + // notably skills, which are discovered once and cached in memory. Updating + // files on disk alone is invisible to a running server. + // + // The reload is performed by the git-sync sidecar calling the server's + // reload endpoint. To avoid interrupting work in progress, the reload is + // deferred while any session is busy and retried on the next sync cycle. + // + // Only effective with Policy: HotReload, and only for long-running Agents + // (ignored for ephemeral Task pods). + // + // This field is an opt-in for Git contexts. Skill sources always reload + // when sync is enabled, since re-scanning is the point of syncing skills; + // set sync on a skill only if you want that behavior. + // +optional + Reload bool `json:"reload,omitempty"` } // GitSecretReference references a Secret for Git authentication. diff --git a/api/v1alpha1/skill_types.go b/api/v1alpha1/skill_types.go index 30c9a051..d015aa90 100644 --- a/api/v1alpha1/skill_types.go +++ b/api/v1alpha1/skill_types.go @@ -41,6 +41,8 @@ type SkillSource struct { // GitSkillSource defines a Git repository as a skill source. // The repository should contain SKILL.md files organized as one-folder-per-skill, // following the standard skill format (Markdown with YAML frontmatter). +// +// +kubebuilder:validation:XValidation:rule="!has(self.sync) || !has(self.sync.policy) || self.sync.policy != 'Rollout'",message="sync.policy Rollout is not supported for skills; use HotReload" type GitSkillSource struct { // Repository is the Git repository URL. // Supported protocols: https://, http://, git@ (SSH). @@ -97,4 +99,23 @@ type GitSkillSource struct { // Reuses the same Secret format as context Git. // +optional SecretRef *GitSecretReference `json:"secretRef,omitempty"` + + // Sync configures periodic synchronization of the skill repository for + // long-running Agents (ignored for ephemeral Task pods, which run once). + // + // When enabled, a git-sync sidecar polls the remote and updates the cloned + // content in place. Because OpenCode discovers skills only at instance + // start, updated files alone are not enough for a running server: after a + // change is detected the sidecar asks the server to re-scan, so changed + // skills become visible without a Pod restart. + // + // Only the HotReload policy is supported for skills. Rollout would require + // comparing remote refs in the controller and is rejected here. + // + // Note: skills selected via "names" are mounted from fixed subpaths, so + // edits to those skills are picked up but newly added skill directories are + // not mounted until the Pod restarts. Omit "names" to mount the whole + // directory and pick up additions without a restart. + // +optional + Sync *GitSync `json:"sync,omitempty"` } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 305d38de..8070160b 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -926,6 +926,11 @@ func (in *GitSkillSource) DeepCopyInto(out *GitSkillSource) { *out = new(GitSecretReference) **out = **in } + if in.Sync != nil { + in, out := &in.Sync, &out.Sync + *out = new(GitSync) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GitSkillSource. diff --git a/charts/kubeopencode/crds/kubeopencode.io_agents.yaml b/charts/kubeopencode/crds/kubeopencode.io_agents.yaml index 7231395e..4adab7fb 100644 --- a/charts/kubeopencode/crds/kubeopencode.io_agents.yaml +++ b/charts/kubeopencode/crds/kubeopencode.io_agents.yaml @@ -357,6 +357,26 @@ spec: - HotReload - Rollout type: string + reload: + description: |- + Reload asks the OpenCode server to re-scan its configuration after a + HotReload update lands, so the change takes effect without a Pod restart. + + This matters for content the server snapshots at instance start — most + notably skills, which are discovered once and cached in memory. Updating + files on disk alone is invisible to a running server. + + The reload is performed by the git-sync sidecar calling the server's + reload endpoint. To avoid interrupting work in progress, the reload is + deferred while any session is busy and retried on the next sync cycle. + + Only effective with Policy: HotReload, and only for long-running Agents + (ignored for ephemeral Task pods). + + This field is an opt-in for Git contexts. Skill sources always reload + when sync is enabled, since re-scanning is the point of syncing skills; + set sync on a skill only if you want that behavior. + type: boolean type: object required: - repository @@ -6301,9 +6321,75 @@ spec: required: - name type: object + sync: + description: |- + Sync configures periodic synchronization of the skill repository for + long-running Agents (ignored for ephemeral Task pods, which run once). + + When enabled, a git-sync sidecar polls the remote and updates the cloned + content in place. Because OpenCode discovers skills only at instance + start, updated files alone are not enough for a running server: after a + change is detected the sidecar asks the server to re-scan, so changed + skills become visible without a Pod restart. + + Only the HotReload policy is supported for skills. Rollout would require + comparing remote refs in the controller and is rejected here. + + Note: skills selected via "names" are mounted from fixed subpaths, so + edits to those skills are picked up but newly added skill directories are + not mounted until the Pod restarts. Omit "names" to mount the whole + directory and pick up additions without a restart. + properties: + enabled: + description: |- + Enabled enables periodic sync of the Git repository. + When true, a sidecar container (HotReload) or controller polling (Rollout) + is used to keep the Git content up-to-date. + type: boolean + interval: + description: |- + Interval is the polling interval for checking remote changes. + Default: "5m". + type: string + policy: + default: HotReload + description: |- + Policy determines how changes are applied. + HotReload (default): sidecar pulls changes in-place, no Pod restart. + Rollout: controller detects changes and triggers Deployment rolling update. + enum: + - HotReload + - Rollout + type: string + reload: + description: |- + Reload asks the OpenCode server to re-scan its configuration after a + HotReload update lands, so the change takes effect without a Pod restart. + + This matters for content the server snapshots at instance start — most + notably skills, which are discovered once and cached in memory. Updating + files on disk alone is invisible to a running server. + + The reload is performed by the git-sync sidecar calling the server's + reload endpoint. To avoid interrupting work in progress, the reload is + deferred while any session is busy and retried on the next sync cycle. + + Only effective with Policy: HotReload, and only for long-running Agents + (ignored for ephemeral Task pods). + + This field is an opt-in for Git contexts. Skill sources always reload + when sync is enabled, since re-scanning is the point of syncing skills; + set sync on a skill only if you want that behavior. + type: boolean + type: object required: - repository type: object + x-kubernetes-validations: + - message: sync.policy Rollout is not supported for skills; + use HotReload + rule: '!has(self.sync) || !has(self.sync.policy) || self.sync.policy + != ''Rollout''' name: description: |- Name is a unique identifier for this skill source. diff --git a/charts/kubeopencode/crds/kubeopencode.io_agenttemplates.yaml b/charts/kubeopencode/crds/kubeopencode.io_agenttemplates.yaml index 162181f9..bc50b667 100644 --- a/charts/kubeopencode/crds/kubeopencode.io_agenttemplates.yaml +++ b/charts/kubeopencode/crds/kubeopencode.io_agenttemplates.yaml @@ -291,6 +291,26 @@ spec: - HotReload - Rollout type: string + reload: + description: |- + Reload asks the OpenCode server to re-scan its configuration after a + HotReload update lands, so the change takes effect without a Pod restart. + + This matters for content the server snapshots at instance start — most + notably skills, which are discovered once and cached in memory. Updating + files on disk alone is invisible to a running server. + + The reload is performed by the git-sync sidecar calling the server's + reload endpoint. To avoid interrupting work in progress, the reload is + deferred while any session is busy and retried on the next sync cycle. + + Only effective with Policy: HotReload, and only for long-running Agents + (ignored for ephemeral Task pods). + + This field is an opt-in for Git contexts. Skill sources always reload + when sync is enabled, since re-scanning is the point of syncing skills; + set sync on a skill only if you want that behavior. + type: boolean type: object required: - repository @@ -6043,9 +6063,75 @@ spec: required: - name type: object + sync: + description: |- + Sync configures periodic synchronization of the skill repository for + long-running Agents (ignored for ephemeral Task pods, which run once). + + When enabled, a git-sync sidecar polls the remote and updates the cloned + content in place. Because OpenCode discovers skills only at instance + start, updated files alone are not enough for a running server: after a + change is detected the sidecar asks the server to re-scan, so changed + skills become visible without a Pod restart. + + Only the HotReload policy is supported for skills. Rollout would require + comparing remote refs in the controller and is rejected here. + + Note: skills selected via "names" are mounted from fixed subpaths, so + edits to those skills are picked up but newly added skill directories are + not mounted until the Pod restarts. Omit "names" to mount the whole + directory and pick up additions without a restart. + properties: + enabled: + description: |- + Enabled enables periodic sync of the Git repository. + When true, a sidecar container (HotReload) or controller polling (Rollout) + is used to keep the Git content up-to-date. + type: boolean + interval: + description: |- + Interval is the polling interval for checking remote changes. + Default: "5m". + type: string + policy: + default: HotReload + description: |- + Policy determines how changes are applied. + HotReload (default): sidecar pulls changes in-place, no Pod restart. + Rollout: controller detects changes and triggers Deployment rolling update. + enum: + - HotReload + - Rollout + type: string + reload: + description: |- + Reload asks the OpenCode server to re-scan its configuration after a + HotReload update lands, so the change takes effect without a Pod restart. + + This matters for content the server snapshots at instance start — most + notably skills, which are discovered once and cached in memory. Updating + files on disk alone is invisible to a running server. + + The reload is performed by the git-sync sidecar calling the server's + reload endpoint. To avoid interrupting work in progress, the reload is + deferred while any session is busy and retried on the next sync cycle. + + Only effective with Policy: HotReload, and only for long-running Agents + (ignored for ephemeral Task pods). + + This field is an opt-in for Git contexts. Skill sources always reload + when sync is enabled, since re-scanning is the point of syncing skills; + set sync on a skill only if you want that behavior. + type: boolean + type: object required: - repository type: object + x-kubernetes-validations: + - message: sync.policy Rollout is not supported for skills; + use HotReload + rule: '!has(self.sync) || !has(self.sync.policy) || self.sync.policy + != ''Rollout''' name: description: |- Name is a unique identifier for this skill source. diff --git a/charts/kubeopencode/crds/kubeopencode.io_crontasks.yaml b/charts/kubeopencode/crds/kubeopencode.io_crontasks.yaml index 41a49c13..2fd8b9a3 100644 --- a/charts/kubeopencode/crds/kubeopencode.io_crontasks.yaml +++ b/charts/kubeopencode/crds/kubeopencode.io_crontasks.yaml @@ -291,6 +291,26 @@ spec: - HotReload - Rollout type: string + reload: + description: |- + Reload asks the OpenCode server to re-scan its configuration after a + HotReload update lands, so the change takes effect without a Pod restart. + + This matters for content the server snapshots at instance start — most + notably skills, which are discovered once and cached in memory. Updating + files on disk alone is invisible to a running server. + + The reload is performed by the git-sync sidecar calling the server's + reload endpoint. To avoid interrupting work in progress, the reload is + deferred while any session is busy and retried on the next sync cycle. + + Only effective with Policy: HotReload, and only for long-running Agents + (ignored for ephemeral Task pods). + + This field is an opt-in for Git contexts. Skill sources always reload + when sync is enabled, since re-scanning is the point of syncing skills; + set sync on a skill only if you want that behavior. + type: boolean type: object required: - repository diff --git a/charts/kubeopencode/crds/kubeopencode.io_registries.yaml b/charts/kubeopencode/crds/kubeopencode.io_registries.yaml index 837a181a..7472b33b 100644 --- a/charts/kubeopencode/crds/kubeopencode.io_registries.yaml +++ b/charts/kubeopencode/crds/kubeopencode.io_registries.yaml @@ -337,9 +337,75 @@ spec: required: - name type: object + sync: + description: |- + Sync configures periodic synchronization of the skill repository for + long-running Agents (ignored for ephemeral Task pods, which run once). + + When enabled, a git-sync sidecar polls the remote and updates the cloned + content in place. Because OpenCode discovers skills only at instance + start, updated files alone are not enough for a running server: after a + change is detected the sidecar asks the server to re-scan, so changed + skills become visible without a Pod restart. + + Only the HotReload policy is supported for skills. Rollout would require + comparing remote refs in the controller and is rejected here. + + Note: skills selected via "names" are mounted from fixed subpaths, so + edits to those skills are picked up but newly added skill directories are + not mounted until the Pod restarts. Omit "names" to mount the whole + directory and pick up additions without a restart. + properties: + enabled: + description: |- + Enabled enables periodic sync of the Git repository. + When true, a sidecar container (HotReload) or controller polling (Rollout) + is used to keep the Git content up-to-date. + type: boolean + interval: + description: |- + Interval is the polling interval for checking remote changes. + Default: "5m". + type: string + policy: + default: HotReload + description: |- + Policy determines how changes are applied. + HotReload (default): sidecar pulls changes in-place, no Pod restart. + Rollout: controller detects changes and triggers Deployment rolling update. + enum: + - HotReload + - Rollout + type: string + reload: + description: |- + Reload asks the OpenCode server to re-scan its configuration after a + HotReload update lands, so the change takes effect without a Pod restart. + + This matters for content the server snapshots at instance start — most + notably skills, which are discovered once and cached in memory. Updating + files on disk alone is invisible to a running server. + + The reload is performed by the git-sync sidecar calling the server's + reload endpoint. To avoid interrupting work in progress, the reload is + deferred while any session is busy and retried on the next sync cycle. + + Only effective with Policy: HotReload, and only for long-running Agents + (ignored for ephemeral Task pods). + + This field is an opt-in for Git contexts. Skill sources always reload + when sync is enabled, since re-scanning is the point of syncing skills; + set sync on a skill only if you want that behavior. + type: boolean + type: object required: - repository type: object + x-kubernetes-validations: + - message: sync.policy Rollout is not supported for skills; + use HotReload + rule: '!has(self.sync) || !has(self.sync.policy) || self.sync.policy + != ''Rollout''' metadata: description: Metadata provides human-readable information for the UI. diff --git a/charts/kubeopencode/crds/kubeopencode.io_tasks.yaml b/charts/kubeopencode/crds/kubeopencode.io_tasks.yaml index afb643ac..b920048a 100644 --- a/charts/kubeopencode/crds/kubeopencode.io_tasks.yaml +++ b/charts/kubeopencode/crds/kubeopencode.io_tasks.yaml @@ -210,6 +210,26 @@ spec: - HotReload - Rollout type: string + reload: + description: |- + Reload asks the OpenCode server to re-scan its configuration after a + HotReload update lands, so the change takes effect without a Pod restart. + + This matters for content the server snapshots at instance start — most + notably skills, which are discovered once and cached in memory. Updating + files on disk alone is invisible to a running server. + + The reload is performed by the git-sync sidecar calling the server's + reload endpoint. To avoid interrupting work in progress, the reload is + deferred while any session is busy and retried on the next sync cycle. + + Only effective with Policy: HotReload, and only for long-running Agents + (ignored for ephemeral Task pods). + + This field is an opt-in for Git contexts. Skill sources always reload + when sync is enabled, since re-scanning is the point of syncing skills; + set sync on a skill only if you want that behavior. + type: boolean type: object required: - repository diff --git a/cmd/kubeopencode/git_sync.go b/cmd/kubeopencode/git_sync.go index 79b1ef99..838ad9d1 100644 --- a/cmd/kubeopencode/git_sync.go +++ b/cmd/kubeopencode/git_sync.go @@ -4,7 +4,10 @@ package main import ( "context" + "encoding/json" "fmt" + "io" + "net/http" "os" "os/exec" "os/signal" @@ -19,11 +22,30 @@ import ( // Environment variable names for git-sync (unique to git-sync) const ( envSyncInterval = "GIT_SYNC_INTERVAL" + + // envReloadURL, when set, is the base URL of the OpenCode server that should + // be asked to re-scan its configuration after a content update lands. + // Only set on sidecars for mounts that request a reload (skills, or Git + // contexts with sync.reload enabled). + envReloadURL = "OPENCODE_RELOAD_URL" ) // Default values for git-sync const ( defaultSyncInterval = 300 // 5 minutes in seconds + + // sessionStatusPath reports per-session activity. A session that is + // currently generating has type "busy"; an idle server returns an empty map. + sessionStatusPath = "/session/status" + + // instanceDisposePath releases the server's current instance, causing it to + // re-scan configuration and content on the next request. This is what makes + // updated skills visible without restarting the Pod. + instanceDisposePath = "/instance/dispose" + + // reloadRequestTimeout bounds each reload-related HTTP call so a wedged + // server can never block the sync loop indefinitely. + reloadRequestTimeout = 10 * time.Second ) func init() { @@ -57,7 +79,14 @@ GitHub App authentication (takes precedence over the credentials above): GITHUB_API_URL GitHub API base URL, default: https://api.github.com Installation access tokens expire after one hour, so a fresh token is minted - before each sync cycle.`, + before each sync cycle. + +Reload (optional): + OPENCODE_RELOAD_URL Base URL of the OpenCode server to notify after a change + (e.g. http://127.0.0.1:4096). When set, the sidecar asks + the server to re-scan so the update takes effect without a + Pod restart. The reload is deferred while any session is + busy, and retried on the next cycle.`, RunE: runGitSync, } @@ -140,13 +169,73 @@ func runGitSync(cmd *cobra.Command, args []string) error { } } + // reloadURL is set only for mounts that need the server to re-scan after an + // update (skill sources, or Git contexts with sync.reload). Without it, this + // sidecar behaves exactly as before: files update, no server interaction. + reloadURL := strings.TrimRight(os.Getenv(envReloadURL), "/") + if reloadURL != "" { + fmt.Printf(" Reload: %s (deferred while sessions are busy)\n", reloadURL) + } + + // The server remembers nothing about what it has scanned, and this sidecar + // can restart at any time, so the "server is behind the working tree" + // condition is persisted on the shared volume as the commit hash the server + // last re-scanned. Comparing it against the local HEAD each cycle means an + // owed reload survives a sidecar restart instead of waiting for the next + // commit to the repository. + stateFile := filepath.Join(root, "."+link+".reload-state") + + // On a fresh Pod the server scanned the git-init clone at startup, so the + // current HEAD is already current. Seed the state to avoid a needless + // reload right after boot. Recorded before the first sync so a repository + // that is already ahead still registers as needing a reload. + if reloadURL != "" { + if _, ok := readReloadState(stateFile); !ok { + if h, err := gitRevParse(targetDir, "HEAD"); err == nil { + if err := writeReloadState(stateFile, h); err != nil { + fmt.Printf("git-sync: Warning: could not seed reload state: %v\n", err) + } + } + } + } + + // runSync performs one sync cycle and reloads the server if it is behind. + runSync := func() { + syncOnce(targetDir, fetchRef) + + if reloadURL == "" { + return + } + + localHash, err := gitRevParse(targetDir, "HEAD") + if err != nil { + fmt.Printf("git-sync: Warning: could not read local HEAD for reload check: %v\n", err) + return + } + // Already current: either nothing changed, or the reload succeeded on a + // previous cycle. Checked outside the change detection so a reload + // deferred for a busy server is retried even without a new commit. + if scanned, _ := readReloadState(stateFile); scanned == localHash { + return + } + + if err := reloadOpenCodeServer(reloadURL); err != nil { + fmt.Printf("git-sync: Reload deferred: %v\n", err) + return + } + fmt.Println("git-sync: Server re-scanned updated content") + if err := writeReloadState(stateFile, localHash); err != nil { + fmt.Printf("git-sync: Warning: could not persist reload state: %v\n", err) + } + } + // Main sync loop ticker := time.NewTicker(time.Duration(interval) * time.Second) defer ticker.Stop() // Run first sync immediately refreshCredentials() - syncOnce(targetDir, fetchRef) + runSync() for { select { @@ -155,11 +244,85 @@ func runGitSync(cmd *cobra.Command, args []string) error { return nil case <-ticker.C: refreshCredentials() - syncOnce(targetDir, fetchRef) + runSync() } } } +// reloadOpenCodeServer asks the OpenCode server to re-scan its configuration. +// +// The server caches some content at instance start (notably skills, which are +// discovered once). Updating files on disk is therefore invisible to a running +// server until its instance is disposed and rebuilt. +// +// Disposal releases the whole instance, so it must not run while a session is +// generating — that would abort the in-flight turn. This function therefore +// refuses to reload while any session is busy, returning an error that the +// caller treats as "retry next cycle" rather than a failure. +func reloadOpenCodeServer(baseURL string) error { + client := &http.Client{Timeout: reloadRequestTimeout} + + busy, err := anySessionBusy(client, baseURL) + if err != nil { + return fmt.Errorf("could not determine session activity: %w", err) + } + if busy { + return fmt.Errorf("a session is active; will retry next cycle") + } + + req, err := http.NewRequest(http.MethodPost, baseURL+instanceDisposePath, nil) + if err != nil { + return fmt.Errorf("building reload request: %w", err) + } + resp, err := client.Do(req) + if err != nil { + return fmt.Errorf("reload request failed: %w", err) + } + defer func() { _ = resp.Body.Close() }() + _, _ = io.Copy(io.Discard, resp.Body) + + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return fmt.Errorf("reload returned status %d", resp.StatusCode) + } + return nil +} + +// anySessionBusy reports whether any session on the server is currently +// generating. The status endpoint returns a map of session ID to state, where a +// busy session has type "busy". An idle server returns an empty map. +func anySessionBusy(client *http.Client, baseURL string) (bool, error) { + resp, err := client.Get(baseURL + sessionStatusPath) //nolint:gosec // URL from controlled env var + if err != nil { + return false, err + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + return false, fmt.Errorf("session status returned status %d", resp.StatusCode) + } + + body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + if err != nil { + return false, err + } + + var statuses map[string]struct { + Type string `json:"type"` + } + if err := json.Unmarshal(body, &statuses); err != nil { + // An unrecognized payload is treated as "busy" (fail safe) so an API + // change cannot cause us to abort someone's work in progress. + return true, fmt.Errorf("unexpected session status payload: %w", err) + } + + for _, s := range statuses { + if s.Type == "busy" { + return true, nil + } + } + return false, nil +} + // syncOnce performs a single sync cycle: fetch, compare, and update if needed. func syncOnce(targetDir, fetchRef string) { // Get current local HEAD @@ -216,6 +379,35 @@ func syncOnce(targetDir, fetchRef string) { fmt.Printf("git-sync: Successfully updated to %s\n", safeHash(remoteHash)) } +// readReloadState returns the commit hash the server last re-scanned. The second +// return value is false when no state has been recorded yet. +func readReloadState(path string) (string, bool) { + data, err := os.ReadFile(path) //nolint:gosec // controlled path on the shared volume + if err != nil { + return "", false + } + hash := strings.TrimSpace(string(data)) + if hash == "" { + return "", false + } + return hash, true +} + +// writeReloadState records the commit hash the server has most recently scanned. +// +// The file is made world-writable because containers may run as a random UID +// (SCC environments) and a restarted sidecar can therefore inherit a different +// UID than the one that created the file. +func writeReloadState(path, hash string) error { + if err := os.WriteFile(path, []byte(hash+"\n"), 0o666); err != nil { //nolint:gosec // not sensitive + return err + } + if err := os.Chmod(path, 0o666); err != nil { //nolint:gosec // not sensitive + return err + } + return nil +} + // gitRevParse runs git rev-parse and returns the hash. func gitRevParse(targetDir, ref string) (string, error) { cmd := exec.Command("git", "-C", targetDir, "rev-parse", ref) //nolint:gosec // controlled inputs diff --git a/cmd/kubeopencode/git_sync_reload_test.go b/cmd/kubeopencode/git_sync_reload_test.go new file mode 100644 index 00000000..dc366d18 --- /dev/null +++ b/cmd/kubeopencode/git_sync_reload_test.go @@ -0,0 +1,223 @@ +// Copyright Contributors to the KubeOpenCode project + +package main + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +// sessionStatusPayload builds a /session/status body. OpenCode reports a map of +// session ID to state, where "busy" means a turn is generating. +func sessionStatusPayload(types ...string) string { + statuses := make(map[string]map[string]string, len(types)) + for i, t := range types { + statuses["ses_"+string(rune('a'+i))] = map[string]string{"type": t} + } + body, _ := json.Marshal(statuses) + return string(body) +} + +func TestAnySessionBusy(t *testing.T) { + tests := []struct { + name string + statusCode int + body string + wantBusy bool + wantErr bool + description string + }{ + { + name: "idle server reports no sessions", + statusCode: http.StatusOK, + body: "{}", + wantBusy: false, + }, + { + name: "busy session is detected", + statusCode: http.StatusOK, + body: sessionStatusPayload("busy"), + wantBusy: true, + }, + { + name: "idle session is not busy", + statusCode: http.StatusOK, + body: sessionStatusPayload("idle"), + wantBusy: false, + }, + { + name: "one busy among several is busy", + statusCode: http.StatusOK, + body: sessionStatusPayload("idle", "busy", "idle"), + wantBusy: true, + }, + { + name: "non-200 is an error", + statusCode: http.StatusInternalServerError, + body: "", + wantErr: true, + }, + { + // Fail safe: an unrecognized payload must not be read as idle, or a + // server API change could cause reloads during active work. + name: "malformed payload is treated as busy", + statusCode: http.StatusOK, + body: "not json", + wantBusy: true, + wantErr: true, + description: "an unparseable response must fail closed", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != sessionStatusPath { + t.Errorf("expected path %q, got %q", sessionStatusPath, r.URL.Path) + } + w.WriteHeader(tc.statusCode) + _, _ = w.Write([]byte(tc.body)) + })) + defer srv.Close() + + busy, err := anySessionBusy(srv.Client(), srv.URL) + if tc.wantErr && err == nil { + t.Fatal("expected an error, got nil") + } + if !tc.wantErr && err != nil { + t.Fatalf("unexpected error: %v", err) + } + if busy != tc.wantBusy { + t.Errorf("busy = %v, want %v", busy, tc.wantBusy) + } + }) + } +} + +func TestReloadOpenCodeServer(t *testing.T) { + t.Run("disposes when idle", func(t *testing.T) { + var disposed bool + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case sessionStatusPath: + _, _ = w.Write([]byte("{}")) + case instanceDisposePath: + if r.Method != http.MethodPost { + t.Errorf("dispose must use POST, got %s", r.Method) + } + disposed = true + w.WriteHeader(http.StatusOK) + default: + t.Errorf("unexpected path %q", r.URL.Path) + } + })) + defer srv.Close() + + if err := reloadOpenCodeServer(srv.URL); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !disposed { + t.Error("expected the server to be disposed when idle") + } + }) + + t.Run("does not dispose while busy", func(t *testing.T) { + var disposed bool + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case sessionStatusPath: + _, _ = w.Write([]byte(sessionStatusPayload("busy"))) + case instanceDisposePath: + disposed = true + w.WriteHeader(http.StatusOK) + } + })) + defer srv.Close() + + err := reloadOpenCodeServer(srv.URL) + if err == nil { + t.Fatal("expected an error while a session is busy") + } + if disposed { + t.Error("must not dispose the instance while a session is busy") + } + }) + + t.Run("propagates disposal failure", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == instanceDisposePath { + w.WriteHeader(http.StatusBadGateway) + return + } + _, _ = w.Write([]byte("{}")) + })) + defer srv.Close() + + if err := reloadOpenCodeServer(srv.URL); err == nil { + t.Fatal("expected an error for a non-2xx dispose response") + } + }) + + t.Run("errors when the status endpoint is unreachable", func(t *testing.T) { + // Nothing is listening: the caller retries next cycle instead of + // reloading blindly. + if err := reloadOpenCodeServer("http://127.0.0.1:1"); err == nil { + t.Fatal("expected an error when the server is unreachable") + } + }) +} + +func TestReadWriteReloadState(t *testing.T) { + path := filepath.Join(t.TempDir(), "state") + + if _, ok := readReloadState(path); ok { + t.Error("expected no state for a missing file") + } + + if err := writeReloadState(path, "abc123"); err != nil { + t.Fatalf("failed to write state: %v", err) + } + got, ok := readReloadState(path) + if !ok { + t.Fatal("expected state to be present after writing") + } + if got != "abc123" { + t.Errorf("state = %q, want %q", got, "abc123") + } + + // The file is written world-writable because a restarted sidecar may run as + // a different random UID in SCC environments. + info, err := os.Stat(path) + if err != nil { + t.Fatalf("failed to stat state file: %v", err) + } + if perm := info.Mode().Perm(); perm != 0o666 { + t.Errorf("state file mode = %o, want 666 so a restart under a new UID can write it", perm) + } +} + +func TestReadReloadStateIgnoresEmptyFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "state") + if err := os.WriteFile(path, []byte("\n"), 0o600); err != nil { + t.Fatalf("failed to write file: %v", err) + } + if _, ok := readReloadState(path); ok { + t.Error("expected an empty file to be treated as no state") + } +} + +// TestReloadEnvVarName guards the contract between the controller, which sets +// OPENCODE_RELOAD_URL on the sidecar, and this command, which reads it. +func TestReloadEnvVarName(t *testing.T) { + if envReloadURL != "OPENCODE_RELOAD_URL" { + t.Errorf("envReloadURL = %q, want OPENCODE_RELOAD_URL", envReloadURL) + } + if !strings.HasPrefix(sessionStatusPath, "/") || !strings.HasPrefix(instanceDisposePath, "/") { + t.Error("OpenCode API paths must be absolute") + } +} diff --git a/deploy/crds/kubeopencode.io_agents.yaml b/deploy/crds/kubeopencode.io_agents.yaml index 7231395e..4adab7fb 100644 --- a/deploy/crds/kubeopencode.io_agents.yaml +++ b/deploy/crds/kubeopencode.io_agents.yaml @@ -357,6 +357,26 @@ spec: - HotReload - Rollout type: string + reload: + description: |- + Reload asks the OpenCode server to re-scan its configuration after a + HotReload update lands, so the change takes effect without a Pod restart. + + This matters for content the server snapshots at instance start — most + notably skills, which are discovered once and cached in memory. Updating + files on disk alone is invisible to a running server. + + The reload is performed by the git-sync sidecar calling the server's + reload endpoint. To avoid interrupting work in progress, the reload is + deferred while any session is busy and retried on the next sync cycle. + + Only effective with Policy: HotReload, and only for long-running Agents + (ignored for ephemeral Task pods). + + This field is an opt-in for Git contexts. Skill sources always reload + when sync is enabled, since re-scanning is the point of syncing skills; + set sync on a skill only if you want that behavior. + type: boolean type: object required: - repository @@ -6301,9 +6321,75 @@ spec: required: - name type: object + sync: + description: |- + Sync configures periodic synchronization of the skill repository for + long-running Agents (ignored for ephemeral Task pods, which run once). + + When enabled, a git-sync sidecar polls the remote and updates the cloned + content in place. Because OpenCode discovers skills only at instance + start, updated files alone are not enough for a running server: after a + change is detected the sidecar asks the server to re-scan, so changed + skills become visible without a Pod restart. + + Only the HotReload policy is supported for skills. Rollout would require + comparing remote refs in the controller and is rejected here. + + Note: skills selected via "names" are mounted from fixed subpaths, so + edits to those skills are picked up but newly added skill directories are + not mounted until the Pod restarts. Omit "names" to mount the whole + directory and pick up additions without a restart. + properties: + enabled: + description: |- + Enabled enables periodic sync of the Git repository. + When true, a sidecar container (HotReload) or controller polling (Rollout) + is used to keep the Git content up-to-date. + type: boolean + interval: + description: |- + Interval is the polling interval for checking remote changes. + Default: "5m". + type: string + policy: + default: HotReload + description: |- + Policy determines how changes are applied. + HotReload (default): sidecar pulls changes in-place, no Pod restart. + Rollout: controller detects changes and triggers Deployment rolling update. + enum: + - HotReload + - Rollout + type: string + reload: + description: |- + Reload asks the OpenCode server to re-scan its configuration after a + HotReload update lands, so the change takes effect without a Pod restart. + + This matters for content the server snapshots at instance start — most + notably skills, which are discovered once and cached in memory. Updating + files on disk alone is invisible to a running server. + + The reload is performed by the git-sync sidecar calling the server's + reload endpoint. To avoid interrupting work in progress, the reload is + deferred while any session is busy and retried on the next sync cycle. + + Only effective with Policy: HotReload, and only for long-running Agents + (ignored for ephemeral Task pods). + + This field is an opt-in for Git contexts. Skill sources always reload + when sync is enabled, since re-scanning is the point of syncing skills; + set sync on a skill only if you want that behavior. + type: boolean + type: object required: - repository type: object + x-kubernetes-validations: + - message: sync.policy Rollout is not supported for skills; + use HotReload + rule: '!has(self.sync) || !has(self.sync.policy) || self.sync.policy + != ''Rollout''' name: description: |- Name is a unique identifier for this skill source. diff --git a/deploy/crds/kubeopencode.io_agenttemplates.yaml b/deploy/crds/kubeopencode.io_agenttemplates.yaml index 162181f9..bc50b667 100644 --- a/deploy/crds/kubeopencode.io_agenttemplates.yaml +++ b/deploy/crds/kubeopencode.io_agenttemplates.yaml @@ -291,6 +291,26 @@ spec: - HotReload - Rollout type: string + reload: + description: |- + Reload asks the OpenCode server to re-scan its configuration after a + HotReload update lands, so the change takes effect without a Pod restart. + + This matters for content the server snapshots at instance start — most + notably skills, which are discovered once and cached in memory. Updating + files on disk alone is invisible to a running server. + + The reload is performed by the git-sync sidecar calling the server's + reload endpoint. To avoid interrupting work in progress, the reload is + deferred while any session is busy and retried on the next sync cycle. + + Only effective with Policy: HotReload, and only for long-running Agents + (ignored for ephemeral Task pods). + + This field is an opt-in for Git contexts. Skill sources always reload + when sync is enabled, since re-scanning is the point of syncing skills; + set sync on a skill only if you want that behavior. + type: boolean type: object required: - repository @@ -6043,9 +6063,75 @@ spec: required: - name type: object + sync: + description: |- + Sync configures periodic synchronization of the skill repository for + long-running Agents (ignored for ephemeral Task pods, which run once). + + When enabled, a git-sync sidecar polls the remote and updates the cloned + content in place. Because OpenCode discovers skills only at instance + start, updated files alone are not enough for a running server: after a + change is detected the sidecar asks the server to re-scan, so changed + skills become visible without a Pod restart. + + Only the HotReload policy is supported for skills. Rollout would require + comparing remote refs in the controller and is rejected here. + + Note: skills selected via "names" are mounted from fixed subpaths, so + edits to those skills are picked up but newly added skill directories are + not mounted until the Pod restarts. Omit "names" to mount the whole + directory and pick up additions without a restart. + properties: + enabled: + description: |- + Enabled enables periodic sync of the Git repository. + When true, a sidecar container (HotReload) or controller polling (Rollout) + is used to keep the Git content up-to-date. + type: boolean + interval: + description: |- + Interval is the polling interval for checking remote changes. + Default: "5m". + type: string + policy: + default: HotReload + description: |- + Policy determines how changes are applied. + HotReload (default): sidecar pulls changes in-place, no Pod restart. + Rollout: controller detects changes and triggers Deployment rolling update. + enum: + - HotReload + - Rollout + type: string + reload: + description: |- + Reload asks the OpenCode server to re-scan its configuration after a + HotReload update lands, so the change takes effect without a Pod restart. + + This matters for content the server snapshots at instance start — most + notably skills, which are discovered once and cached in memory. Updating + files on disk alone is invisible to a running server. + + The reload is performed by the git-sync sidecar calling the server's + reload endpoint. To avoid interrupting work in progress, the reload is + deferred while any session is busy and retried on the next sync cycle. + + Only effective with Policy: HotReload, and only for long-running Agents + (ignored for ephemeral Task pods). + + This field is an opt-in for Git contexts. Skill sources always reload + when sync is enabled, since re-scanning is the point of syncing skills; + set sync on a skill only if you want that behavior. + type: boolean + type: object required: - repository type: object + x-kubernetes-validations: + - message: sync.policy Rollout is not supported for skills; + use HotReload + rule: '!has(self.sync) || !has(self.sync.policy) || self.sync.policy + != ''Rollout''' name: description: |- Name is a unique identifier for this skill source. diff --git a/deploy/crds/kubeopencode.io_crontasks.yaml b/deploy/crds/kubeopencode.io_crontasks.yaml index 41a49c13..2fd8b9a3 100644 --- a/deploy/crds/kubeopencode.io_crontasks.yaml +++ b/deploy/crds/kubeopencode.io_crontasks.yaml @@ -291,6 +291,26 @@ spec: - HotReload - Rollout type: string + reload: + description: |- + Reload asks the OpenCode server to re-scan its configuration after a + HotReload update lands, so the change takes effect without a Pod restart. + + This matters for content the server snapshots at instance start — most + notably skills, which are discovered once and cached in memory. Updating + files on disk alone is invisible to a running server. + + The reload is performed by the git-sync sidecar calling the server's + reload endpoint. To avoid interrupting work in progress, the reload is + deferred while any session is busy and retried on the next sync cycle. + + Only effective with Policy: HotReload, and only for long-running Agents + (ignored for ephemeral Task pods). + + This field is an opt-in for Git contexts. Skill sources always reload + when sync is enabled, since re-scanning is the point of syncing skills; + set sync on a skill only if you want that behavior. + type: boolean type: object required: - repository diff --git a/deploy/crds/kubeopencode.io_registries.yaml b/deploy/crds/kubeopencode.io_registries.yaml index 837a181a..7472b33b 100644 --- a/deploy/crds/kubeopencode.io_registries.yaml +++ b/deploy/crds/kubeopencode.io_registries.yaml @@ -337,9 +337,75 @@ spec: required: - name type: object + sync: + description: |- + Sync configures periodic synchronization of the skill repository for + long-running Agents (ignored for ephemeral Task pods, which run once). + + When enabled, a git-sync sidecar polls the remote and updates the cloned + content in place. Because OpenCode discovers skills only at instance + start, updated files alone are not enough for a running server: after a + change is detected the sidecar asks the server to re-scan, so changed + skills become visible without a Pod restart. + + Only the HotReload policy is supported for skills. Rollout would require + comparing remote refs in the controller and is rejected here. + + Note: skills selected via "names" are mounted from fixed subpaths, so + edits to those skills are picked up but newly added skill directories are + not mounted until the Pod restarts. Omit "names" to mount the whole + directory and pick up additions without a restart. + properties: + enabled: + description: |- + Enabled enables periodic sync of the Git repository. + When true, a sidecar container (HotReload) or controller polling (Rollout) + is used to keep the Git content up-to-date. + type: boolean + interval: + description: |- + Interval is the polling interval for checking remote changes. + Default: "5m". + type: string + policy: + default: HotReload + description: |- + Policy determines how changes are applied. + HotReload (default): sidecar pulls changes in-place, no Pod restart. + Rollout: controller detects changes and triggers Deployment rolling update. + enum: + - HotReload + - Rollout + type: string + reload: + description: |- + Reload asks the OpenCode server to re-scan its configuration after a + HotReload update lands, so the change takes effect without a Pod restart. + + This matters for content the server snapshots at instance start — most + notably skills, which are discovered once and cached in memory. Updating + files on disk alone is invisible to a running server. + + The reload is performed by the git-sync sidecar calling the server's + reload endpoint. To avoid interrupting work in progress, the reload is + deferred while any session is busy and retried on the next sync cycle. + + Only effective with Policy: HotReload, and only for long-running Agents + (ignored for ephemeral Task pods). + + This field is an opt-in for Git contexts. Skill sources always reload + when sync is enabled, since re-scanning is the point of syncing skills; + set sync on a skill only if you want that behavior. + type: boolean + type: object required: - repository type: object + x-kubernetes-validations: + - message: sync.policy Rollout is not supported for skills; + use HotReload + rule: '!has(self.sync) || !has(self.sync.policy) || self.sync.policy + != ''Rollout''' metadata: description: Metadata provides human-readable information for the UI. diff --git a/deploy/crds/kubeopencode.io_tasks.yaml b/deploy/crds/kubeopencode.io_tasks.yaml index afb643ac..b920048a 100644 --- a/deploy/crds/kubeopencode.io_tasks.yaml +++ b/deploy/crds/kubeopencode.io_tasks.yaml @@ -210,6 +210,26 @@ spec: - HotReload - Rollout type: string + reload: + description: |- + Reload asks the OpenCode server to re-scan its configuration after a + HotReload update lands, so the change takes effect without a Pod restart. + + This matters for content the server snapshots at instance start — most + notably skills, which are discovered once and cached in memory. Updating + files on disk alone is invisible to a running server. + + The reload is performed by the git-sync sidecar calling the server's + reload endpoint. To avoid interrupting work in progress, the reload is + deferred while any session is busy and retried on the next sync cycle. + + Only effective with Policy: HotReload, and only for long-running Agents + (ignored for ephemeral Task pods). + + This field is an opt-in for Git contexts. Skill sources always reload + when sync is enabled, since re-scanning is the point of syncing skills; + set sync on a skill only if you want that behavior. + type: boolean type: object required: - repository diff --git a/docs/adr/0043-hot-reload-skills-in-agents.md b/docs/adr/0043-hot-reload-skills-in-agents.md new file mode 100644 index 00000000..b5783a20 --- /dev/null +++ b/docs/adr/0043-hot-reload-skills-in-agents.md @@ -0,0 +1,134 @@ +# ADR 0043: Hot-Reloading Skills in Long-Running Agents + +## Status + +Proposed + +## Date + +2026-09-22 + +## Context + +KubeOpenCode Agent pods run `opencode serve` as a long-lived process. Task pods +are ephemeral and restart per Task, so they always discover current content. An +Agent, however, can run for days. + +Skills are loaded and **cached when the server instance starts**. A change to a +skill repository therefore does not reach a running Agent, even though the +cloned files on disk may be current. Git contexts already had a `sync` option +(HotReload / Rollout); skill sources had none at all, so a skill catalog could +only be updated by restarting the Agent Deployment. + +Two facts, established by probing a running `opencode serve`, shape the design: + +1. **File updates alone are insufficient.** After editing and even adding a + `SKILL.md` on disk, `GET /skill` returned the cached list: a newly added skill + was not discovered, and an edited skill still served its old description. + Discovery happens once per instance. + +2. **`POST /instance/dispose` forces a re-scan without restarting the process.** + After disposal the new skill appeared and edited content was current. The + server stayed healthy, and **sessions survived** — the session list and + per-session reads were unchanged. + +The same probe showed one hazard: **disposing during an active turn aborts it**. +The assistant message was terminated with `MessageAbortedError`. Disposal +releases the whole instance, not just skill state, so it must not run while a +turn is generating. + +Measured cost of a reload (pinned OpenCode 1.17.11, several repos, with and +without MCP): + +| Measurement | Result | +|---|---| +| `POST /instance/dispose` | ~3ms | +| Turn after dispose vs. warm | ~+3.5–4.5s on the first turn | +| Idle gap of 20s, no dispose | ~+0.5–1s (so the cost is the dispose, not idleness) | +| Memory over 30 consecutive reloads | oscillates, then settles below baseline — churn, not a leak | + +The cost lands on the turn *after* the reload and is small at a 15m interval +(roughly 0.4% of one turn). There is no lighter re-scan endpoint; disposal is the +only mechanism the server exposes. + +## Decision + +Add `sync` to `GitSkillSource`, reusing the existing `GitSync` type so the field +shape matches Git contexts, and implement the re-scan in the existing `git-sync` +sidecar. + +1. **Skills sync via a sidecar that reloads.** When `sync.enabled` is set, the + mount gets a `git-sync` sidecar (as synced Git contexts do). On a detected + commit change it updates the files and then notifies the server. + +2. **Reloads are gated on session idleness.** The sidecar reads + `GET /session/status` and only disposes when no session is `busy`. A busy + agent defers the reload to the next cycle; the file update still happens + immediately. + +3. **An owed reload is persisted, not held in memory.** The commit hash last + scanned by the server is written to the shared volume, so a restarted sidecar + still knows the server is behind. Without this, a restart leaves the server + stale until the repository happens to change again — possibly days. + +4. **Skill sources always reload when syncing**; Git contexts opt in via + `sync.reload`. Re-scanning is the entire point of syncing skills, so making it + opt-in there would be a footgun. Contexts keep today's behavior by default. + +5. **Reload applies to Agent servers only.** Task pods are ephemeral and never + build `git-sync` sidecars, so the concept does not apply. + +6. **Skills default to a 15m interval** versus 5m for contexts, since each + applied change costs a reload. + +7. **`Rollout` is rejected for skills.** It would require comparing remote refs + in the controller, which is not implemented for authenticated repositories; + allowing it would present an option that silently does nothing. + +## Consequences + +### Positive + +- Skill catalogs can be updated on running Agents without a restart, closing a + gap where a change required an operator to bounce a Deployment. +- The mechanism reuses the sidecar, GitHub App credential refresh, CA bundle, and + proxy wiring that Git contexts already have. +- The idle gate means a reload never aborts a turn in progress, and the + persisted state means a deferred reload is not lost. + +### Negative + +- A reload briefly re-initializes the server instance, adding a few seconds to + the first turn after it. This is the accepted cost of live skill updates and is + why the skill interval defaults to 15m. +- The reload is an entire-instance operation, so it also discards unrelated + instance state (MCP connections, language servers), which then re-initialize. +- In-cluster behavior may differ from the loopback measurements above; memory + should be watched after rollout. + +### Limitations + +- **`names` filtering blocks additions.** A mount using `names` maps fixed + subpaths, so a newly added skill directory is not mounted until the Pod + restarts. Only edits to already-mounted skills hot-reload. Omitting `names` + makes additions dynamic. This is documented rather than worked around, as + fixing it needs a different mount strategy. +- **Plugins are out of scope.** Plugin packages are installed into an emptyDir + by `plugin-init`, so a new plugin's `node_modules` would not exist at reload + time. Reloading cannot make a new plugin available; that needs a separate + install mechanism. +- Reload depends on the `/instance/dispose` and `/session/status` endpoints, which + are present in the pinned OpenCode version but are internal-ish surfaces. + +## Alternatives Considered + +- **Watch the filesystem from the server.** OpenCode does not re-scan on file + change, so this would require upstream changes. +- **Always dispose on every sync cycle.** Simpler, but wasteful and aborts + in-flight turns. +- **Rollout (recreate the Pod).** Already available implicitly via a Deployment + change, but defeats the purpose for long-running Agents and interrupts work. +- **Track pending state in memory only.** Rejected after testing showed a + restarted sidecar could leave the server permanently stale. +- **Pre-warming the instance after dispose.** Measured as ineffective — a `/skill` + call after disposal did not reduce the next turn's latency. diff --git a/docs/adr/README.md b/docs/adr/README.md index b49875a2..bed22d6b 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -43,6 +43,8 @@ ADRs document significant architectural and design decisions along with their co | 0030 | [Graceful Task Termination on Deletion](0030-task-deletion-graceful-stop.md) | | | 0031 | [OpenTelemetry Observability for Tasks and Agents](0031-opentelemetry-observability.md) | | | 0036 | [Agent Registry — Enterprise Agent Asset Management and Visual Agent Assembly](0036-agent-registry.md) | | +| 0042 | [GitHub App Authentication for Git Operations](0042-github-app-auth-for-git.md) | | +| 0043 | [Hot-Reloading Skills in Long-Running Agents](0043-hot-reload-skills-in-agents.md) | | ## Archived ADRs diff --git a/internal/controller/context_processor.go b/internal/controller/context_processor.go index 8fe4c51a..242364df 100644 --- a/internal/controller/context_processor.go +++ b/internal/controller/context_processor.go @@ -177,6 +177,7 @@ func resolveContextContentFromReader(reader contextReader, ctx context.Context, if gm.syncInterval == 0 { gm.syncInterval = 5 * time.Minute } + gm.reloadOnSync = git.Sync.Reload } return "", nil, gm, nil diff --git a/internal/controller/git_sync_test.go b/internal/controller/git_sync_test.go index 9681f9fd..1a3eaf65 100644 --- a/internal/controller/git_sync_test.go +++ b/internal/controller/git_sync_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -169,7 +170,7 @@ func TestBuildGitSyncSidecar(t *testing.T) { } sysCfg := systemConfig{systemImage: "ghcr.io/kubeopencode/kubeopencode:latest"} - sidecar := buildGitSyncSidecar(gm, "git-context-0", 0, sysCfg) + sidecar := buildGitSyncSidecar(gm, "git-context-0", 0, sysCfg, "") if sidecar.Name != "git-sync-0" { t.Errorf("expected name 'git-sync-0', got %q", sidecar.Name) @@ -199,6 +200,65 @@ func TestBuildGitSyncSidecar(t *testing.T) { } } +func TestBuildGitSyncSidecar_Reload(t *testing.T) { + reloadMount := gitMount{ + contextName: "skill-org-skills", + repository: "https://github.com/org/skills.git", + ref: "main", + mountPath: "/skills/org-skills", + syncEnabled: true, + syncPolicy: kubeopenv1alpha1.GitSyncPolicyHotReload, + syncInterval: 15 * time.Minute, + reloadOnSync: true, + } + plainMount := reloadMount + plainMount.reloadOnSync = false + + sysCfg := defaultSystemConfig() + const serverURL = "http://127.0.0.1:4096" + + hasReloadEnv := func(envs []corev1.EnvVar) bool { + for _, e := range envs { + if e.Name == EnvOpenCodeReloadURL { + return true + } + } + return false + } + + t.Run("reload mount gets the server URL", func(t *testing.T) { + sidecar := buildGitSyncSidecar(reloadMount, "git-context-0", 0, sysCfg, serverURL) + if !hasEnvVar(sidecar.Env, EnvOpenCodeReloadURL, serverURL) { + t.Errorf("expected %s=%s on the sidecar", EnvOpenCodeReloadURL, serverURL) + } + }) + + t.Run("non-reload mount gets no server URL", func(t *testing.T) { + sidecar := buildGitSyncSidecar(plainMount, "git-context-0", 0, sysCfg, serverURL) + if hasReloadEnv(sidecar.Env) { + t.Errorf("did not expect %s on a mount without reload", EnvOpenCodeReloadURL) + } + }) + + t.Run("no server URL means no reload env", func(t *testing.T) { + // Ephemeral Task pods have no long-lived server to reload. + sidecar := buildGitSyncSidecar(reloadMount, "git-context-0", 0, sysCfg, "") + if hasReloadEnv(sidecar.Env) { + t.Errorf("did not expect %s when no server URL is provided", EnvOpenCodeReloadURL) + } + }) + + t.Run("reload does not disturb existing env", func(t *testing.T) { + sidecar := buildGitSyncSidecar(reloadMount, "git-context-0", 0, sysCfg, serverURL) + if !hasEnvVar(sidecar.Env, "GIT_SYNC_INTERVAL", "900") { + t.Error("expected the 15m interval to be passed through") + } + if !hasEnvVar(sidecar.Env, "GIT_REPO", reloadMount.repository) { + t.Error("expected GIT_REPO to be preserved") + } + }) +} + func TestTruncateHash(t *testing.T) { tests := []struct { in, want string diff --git a/internal/controller/pod_builder.go b/internal/controller/pod_builder.go index bd0d42fa..7268703c 100644 --- a/internal/controller/pod_builder.go +++ b/internal/controller/pod_builder.go @@ -177,6 +177,11 @@ type gitMount struct { syncEnabled bool // Whether auto-sync is enabled syncPolicy kubeopenv1alpha1.GitSyncPolicy // HotReload or Rollout syncInterval time.Duration // Polling interval + + // reloadOnSync asks the OpenCode server to re-scan its configuration after + // a HotReload update lands. Required for content the server caches at + // instance start (skills). Only meaningful on long-running Agent servers. + reloadOnSync bool } // resolvedContext holds a resolved context with its content and metadata @@ -311,6 +316,12 @@ const ( // The value is a JSON object mapping tool names to permission actions (allow/ask/deny). OpenCodePermissionEnvVar = "OPENCODE_PERMISSION" + // EnvOpenCodeReloadURL is the environment variable set on git-sync sidecars + // that should ask the OpenCode server to re-scan after a config update. The + // value is the base URL of the agent's OpenCode server (e.g. + // http://127.0.0.1:4096). Unset means no reload is attempted. + EnvOpenCodeReloadURL = "OPENCODE_RELOAD_URL" + // DefaultOpenCodePermission is the default permission configuration for automated execution. // In Kubernetes/CI environments, we need to allow all permissions to avoid interactive prompts // that would block task execution. Users can still restrict permissions via Agent.spec.config. @@ -536,7 +547,11 @@ func buildGitCredentialEnvVars(secretName string) []corev1.EnvVar { // buildGitSyncSidecar creates a sidecar container that periodically syncs a Git repository. // Used when sync.policy is HotReload to keep content up-to-date without Pod restart. -func buildGitSyncSidecar(gm gitMount, volumeName string, index int, sysCfg systemConfig) corev1.Container { +// +// serverReloadURL, when non-empty, is passed to the sidecar so it can ask the +// OpenCode server to re-scan after an update. It is empty for mounts that do not +// request a reload, and for ephemeral Task pods (which have no long-lived server). +func buildGitSyncSidecar(gm gitMount, volumeName string, index int, sysCfg systemConfig, serverReloadURL string) corev1.Container { ref := defaultString(gm.ref, DefaultGitRef) intervalSeconds := int(gm.syncInterval.Seconds()) if intervalSeconds <= 0 { @@ -555,6 +570,13 @@ func buildGitSyncSidecar(gm gitMount, volumeName string, index int, sysCfg syste {Name: "GIT_SYNC_INTERVAL", Value: strconv.Itoa(intervalSeconds)}, } + if gm.reloadOnSync && serverReloadURL != "" { + envVars = append(envVars, corev1.EnvVar{ + Name: EnvOpenCodeReloadURL, + Value: serverReloadURL, + }) + } + volumeMounts := []corev1.VolumeMount{ {Name: volumeName, MountPath: DefaultGitRoot}, } diff --git a/internal/controller/pod_builder_test.go b/internal/controller/pod_builder_test.go index 87c1e287..286adb2d 100644 --- a/internal/controller/pod_builder_test.go +++ b/internal/controller/pod_builder_test.go @@ -3969,7 +3969,7 @@ func TestBuildGitSyncSidecar_HomeEnv(t *testing.T) { mountPath: "/workspace", } sysCfg := defaultSystemConfig() - c := buildGitSyncSidecar(gm, "git-context-0", 0, sysCfg) + c := buildGitSyncSidecar(gm, "git-context-0", 0, sysCfg, "") if !hasEnvVar(c.Env, "HOME", DefaultHomeDir) { t.Errorf("git-sync sidecar missing HOME=%s env var for SCC compatibility", DefaultHomeDir) diff --git a/internal/controller/server_builder.go b/internal/controller/server_builder.go index bb31d338..6beac8ca 100644 --- a/internal/controller/server_builder.go +++ b/internal/controller/server_builder.go @@ -673,9 +673,13 @@ func BuildServerDeployment(agent *kubeopenv1alpha1.Agent, agentCfg agentConfig, proxyEnvs = buildProxyEnvVars(agentCfg.proxy, sysCfg.clusterDomain) } + // Base URL the git-sync sidecar uses to ask the OpenCode server process in + // this Pod to re-scan its configuration. Loopback, never proxied. + serverReloadURL := fmt.Sprintf("http://127.0.0.1:%d", port) + for i, gm := range ctxGitMounts { if gm.syncEnabled && gm.syncPolicy == kubeopenv1alpha1.GitSyncPolicyHotReload { - sidecar := buildGitSyncSidecar(gm, fmt.Sprintf("git-context-%d", i), i, sysCfg) + sidecar := buildGitSyncSidecar(gm, fmt.Sprintf("git-context-%d", i), i, sysCfg, serverReloadURL) if hasCA { sidecar.VolumeMounts = append(sidecar.VolumeMounts, sidecarCAMount) sidecar.Env = append(sidecar.Env, sidecarCAEnv) diff --git a/internal/controller/server_builder_test.go b/internal/controller/server_builder_test.go index 13bdad54..b17a5ae1 100644 --- a/internal/controller/server_builder_test.go +++ b/internal/controller/server_builder_test.go @@ -5,6 +5,7 @@ package controller import ( + "fmt" "strings" "testing" "time" @@ -2943,3 +2944,77 @@ func TestBuildServerDeployment_OTelEnableLLMTraces(t *testing.T) { t.Error("expected OPENCODE_CONFIG_CONTENT to be set when enableLLMTraces is true") } } + +// TestBuildServerDeployment_GitSyncReload verifies that a git-sync sidecar for a +// mount requesting reload is told how to reach the agent's own OpenCode server, +// and that mounts which do not request reload are left untouched. +func TestBuildServerDeployment_GitSyncReload(t *testing.T) { + newAgent := func(port int32) *kubeopenv1alpha1.Agent { + return &kubeopenv1alpha1.Agent{ + ObjectMeta: metav1.ObjectMeta{Name: "reload-agent", Namespace: "default"}, + Spec: kubeopenv1alpha1.AgentSpec{Port: port}, + } + } + cfg := agentConfig{ + executorImage: "test-executor", + agentImage: "test-agent", + workspaceDir: "/workspace", + } + // Port is deliberately non-default to catch a hardcoded URL. + const port = int32(4123) + + sidecarFor := func(t *testing.T, gm gitMount, agent *kubeopenv1alpha1.Agent) corev1.Container { + t.Helper() + deployment := BuildServerDeployment(agent, cfg, defaultSystemConfig(), nil, nil, nil, []gitMount{gm}, nil) + containers := deployment.Spec.Template.Spec.Containers + if len(containers) != 2 { + t.Fatalf("expected main + sidecar containers, got %d", len(containers)) + } + return containers[1] + } + envValue := func(container corev1.Container, name string) (string, bool) { + for _, e := range container.Env { + if e.Name == name { + return e.Value, true + } + } + return "", false + } + + t.Run("reload mount points at the agent server port", func(t *testing.T) { + sidecar := sidecarFor(t, gitMount{ + contextName: "skill-org-skills", + repository: "https://github.com/org/skills.git", + mountPath: "/skills/org-skills", + syncEnabled: true, + syncPolicy: kubeopenv1alpha1.GitSyncPolicyHotReload, + syncInterval: 15 * time.Minute, + reloadOnSync: true, + }, newAgent(port)) + + got, ok := envValue(sidecar, EnvOpenCodeReloadURL) + if !ok { + t.Fatalf("expected %s on the sidecar", EnvOpenCodeReloadURL) + } + want := fmt.Sprintf("http://127.0.0.1:%d", port) + if got != want { + t.Errorf("%s = %q, want %q", EnvOpenCodeReloadURL, got, want) + } + }) + + t.Run("non-reload mount gets no server URL", func(t *testing.T) { + sidecar := sidecarFor(t, gitMount{ + contextName: "team-prompts", + repository: "https://github.com/org/prompts.git", + mountPath: "/workspace/prompts", + syncEnabled: true, + syncPolicy: kubeopenv1alpha1.GitSyncPolicyHotReload, + syncInterval: 5 * time.Minute, + reloadOnSync: false, + }, newAgent(port)) + + if _, ok := envValue(sidecar, EnvOpenCodeReloadURL); ok { + t.Errorf("did not expect %s without reloadOnSync", EnvOpenCodeReloadURL) + } + }) +} diff --git a/internal/controller/skill_processor.go b/internal/controller/skill_processor.go index d54b728d..2b14856f 100644 --- a/internal/controller/skill_processor.go +++ b/internal/controller/skill_processor.go @@ -7,6 +7,7 @@ import ( "fmt" "path/filepath" "strings" + "time" "k8s.io/apimachinery/pkg/runtime" @@ -21,6 +22,12 @@ const ( // DefaultPluginsMountBase is the base directory where plugins are installed. // The plugin-init container runs npm install here, creating /plugins/node_modules/. DefaultPluginsMountBase = "/plugins" + + // DefaultSkillSyncInterval is the default polling interval for skill sync. + // Longer than the Git context default (5m) on purpose: each detected change + // triggers a server instance reload, which re-initializes in-flight server + // resources. Skill catalogs change far less often than prompts or docs. + DefaultSkillSyncInterval = 15 * time.Minute ) // processSkills converts SkillSource items into gitMounts and returns @@ -65,6 +72,25 @@ func processSkills(skills []kubeopenv1alpha1.SkillSource) ([]gitMount, []string) recurseSubmodules: git.RecurseSubmodules, names: git.Names, } + + // Populate sync fields if configured. + // + // Skills always request a server reload when syncing: OpenCode caches + // discovered skills at instance start, so updating the cloned files is + // not by itself visible to a long-running server. + if git.Sync != nil && git.Sync.Enabled { + gm.syncEnabled = true + gm.syncPolicy = git.Sync.Policy + if gm.syncPolicy == "" { + gm.syncPolicy = kubeopenv1alpha1.GitSyncPolicyHotReload + } + gm.syncInterval = git.Sync.Interval.Duration + if gm.syncInterval == 0 { + gm.syncInterval = DefaultSkillSyncInterval + } + gm.reloadOnSync = gm.syncPolicy == kubeopenv1alpha1.GitSyncPolicyHotReload + } + gitMounts = append(gitMounts, gm) // Compute skill paths for OpenCode discovery. diff --git a/internal/controller/skill_processor_test.go b/internal/controller/skill_processor_test.go index 2ab632dc..0eb7e457 100644 --- a/internal/controller/skill_processor_test.go +++ b/internal/controller/skill_processor_test.go @@ -5,8 +5,10 @@ package controller import ( "encoding/json" "testing" + "time" kubeopenv1alpha1 "github.com/kubeopencode/kubeopencode/api/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -192,6 +194,75 @@ func TestProcessSkills(t *testing.T) { }) } +func TestProcessSkills_Sync(t *testing.T) { + skillWithSync := func(sync *kubeopenv1alpha1.GitSync) []kubeopenv1alpha1.SkillSource { + return []kubeopenv1alpha1.SkillSource{ + { + Name: "org-skills", + Git: &kubeopenv1alpha1.GitSkillSource{ + Repository: "https://github.com/org/skills.git", + Sync: sync, + }, + }, + } + } + + t.Run("no sync leaves the mount unsynced", func(t *testing.T) { + gitMounts, _ := processSkills(skillWithSync(nil)) + gm := gitMounts[0] + if gm.syncEnabled { + t.Error("expected syncEnabled to be false without a sync block") + } + if gm.reloadOnSync { + t.Error("expected reloadOnSync to be false without a sync block") + } + }) + + t.Run("disabled sync leaves the mount unsynced", func(t *testing.T) { + gitMounts, _ := processSkills(skillWithSync(&kubeopenv1alpha1.GitSync{Enabled: false})) + if gitMounts[0].syncEnabled { + t.Error("expected syncEnabled to be false when sync is disabled") + } + }) + + t.Run("enabled sync defaults to HotReload with a 15m interval", func(t *testing.T) { + gitMounts, _ := processSkills(skillWithSync(&kubeopenv1alpha1.GitSync{Enabled: true})) + gm := gitMounts[0] + if !gm.syncEnabled { + t.Fatal("expected syncEnabled to be true") + } + if gm.syncPolicy != kubeopenv1alpha1.GitSyncPolicyHotReload { + t.Errorf("syncPolicy = %q, want HotReload", gm.syncPolicy) + } + if gm.syncInterval != DefaultSkillSyncInterval { + t.Errorf("syncInterval = %v, want %v", gm.syncInterval, DefaultSkillSyncInterval) + } + if DefaultSkillSyncInterval != 15*time.Minute { + t.Errorf("DefaultSkillSyncInterval = %v, want 15m", DefaultSkillSyncInterval) + } + }) + + t.Run("skills always reload when syncing", func(t *testing.T) { + // OpenCode caches discovered skills at instance start, so file updates + // alone are invisible to a running server. Reload is implicit for + // skills rather than an opt-in. + gitMounts, _ := processSkills(skillWithSync(&kubeopenv1alpha1.GitSync{Enabled: true})) + if !gitMounts[0].reloadOnSync { + t.Error("expected reloadOnSync to be true for a synced skill source") + } + }) + + t.Run("explicit interval is honored", func(t *testing.T) { + gitMounts, _ := processSkills(skillWithSync(&kubeopenv1alpha1.GitSync{ + Enabled: true, + Interval: metav1.Duration{Duration: 30 * time.Minute}, + })) + if got := gitMounts[0].syncInterval; got != 30*time.Minute { + t.Errorf("syncInterval = %v, want 30m", got) + } + }) +} + func TestInjectSkillsIntoConfig(t *testing.T) { t.Run("nil config creates new config", func(t *testing.T) { result, err := injectSkillsIntoConfig(nil, []string{"/skills/a"}) diff --git a/website/docs/features/context-system.md b/website/docs/features/context-system.md index 5045b8a8..9637b055 100644 --- a/website/docs/features/context-system.md +++ b/website/docs/features/context-system.md @@ -98,6 +98,7 @@ contexts: enabled: true interval: "5m" policy: HotReload # HotReload (update in-place) or Rollout (rolling restart) + reload: false # true = also re-scan the OpenCode server after an update mountPath: synced-repo ``` diff --git a/website/docs/features/git-auto-sync.md b/website/docs/features/git-auto-sync.md index 459cb52b..366b3c76 100644 --- a/website/docs/features/git-auto-sync.md +++ b/website/docs/features/git-auto-sync.md @@ -53,6 +53,36 @@ When the controller detects a remote change with Rollout policy, it checks for a New Tasks are **not blocked** during `GitSyncPending` — only the Deployment rollout is delayed. +## Reloading the Server (HotReload) + +HotReload updates files in place, which is enough for content the agent reads at +use time (prompts, docs, context files). Content that OpenCode snapshots at +instance start — most notably **skills** — needs more: the file on disk can be +current while the running server still holds the old version. + +Set `sync.reload: true` to have the sidecar ask the server to re-scan after an +update lands: + +```yaml +contexts: +- name: shared-config + type: Git + git: + repository: https://github.com/org/config.git + sync: + enabled: true + policy: HotReload + reload: true # re-scan the server after an update + mountPath: config/ +``` + +The reload is **deferred while any session is busy**, because re-scanning +re-initializes the server instance and would abort a turn in progress. It is +retried every cycle, and an owed reload survives a sidecar restart. + +Skill sources reload automatically when synced and do not need this flag — see +[Skills](skills.md#keeping-skills-up-to-date). + ## Sync Status Agent status tracks the sync state: diff --git a/website/docs/features/skills.md b/website/docs/features/skills.md index a4e6f695..321d7f46 100644 --- a/website/docs/features/skills.md +++ b/website/docs/features/skills.md @@ -147,6 +147,47 @@ skills: 3. The controller auto-injects `skills.paths` into `opencode.json` 4. OpenCode discovers SKILL.md files and makes them available as slash commands +## Keeping Skills Up to Date + +OpenCode discovers skills **once, when its server instance starts**, and caches +them. For a long-running Agent that means a change pushed to a skill repository +is invisible until the Pod restarts — even though the cloned files on disk are +updated. + +Enable `sync` to close that gap. A `git-sync` sidecar polls the repository and, +when it detects a new commit, updates the files **and asks the server to +re-scan**, so the change takes effect without a restart: + +```yaml +skills: +- name: org-skills + git: + repository: https://github.com/my-org/standards-skills.git + path: .claude/skills + sync: + enabled: true + interval: 15m # default for skills +``` + +Notes: + +- **Only `HotReload` is supported.** `Rollout` is rejected for skills, since it + would compare remote refs in the controller rather than reloading in place. +- **Reloads wait for an idle agent.** Re-scanning briefly re-initializes the + server instance, which would abort a turn that is mid-flight. If a session is + busy the reload is deferred to the next cycle, so a continuously busy agent + picks up the change at its next idle window. The file update itself still + happens immediately. +- **`sync` applies to long-running Agents only.** Task pods are ephemeral and + always start fresh, so they ignore it. +- **Skills selected with `names` are mounted from fixed subpaths.** Edits to + those skills are picked up, but a newly added skill directory is not mounted + until the Pod restarts. Omit `names` to mount the whole directory and pick up + additions without a restart. + +Git contexts can do the same with `sync.reload: true` — see +[Git Auto-Sync](git-auto-sync.md). + ## Skills in Templates Skills can be defined in AgentTemplates and inherited by Agents. Agent-level skills replace template-level skills (same merge strategy as contexts): @@ -177,3 +218,6 @@ spec: | `git.depth` | int | 1 | Clone depth (1=shallow, 0=full) | | `git.recurseSubmodules` | bool | false | Clone submodules recursively | | `git.secretRef.name` | string | - | Secret for Git authentication. Keys: `app-id`/`app-installation-id`/`app-private-key` (GitHub App, preferred), `username`/`password` (HTTPS), or `ssh-privatekey` (SSH) | +| `git.sync.enabled` | bool | false | Poll the repository and reload the server on change (Agent servers only) | +| `git.sync.interval` | duration | 15m | Polling interval for skill sync | +| `git.sync.policy` | string | HotReload | Only `HotReload` is valid for skills |