From 8f622a255bb50f945603fa64735401ceae41dc95 Mon Sep 17 00:00:00 2001 From: ContextMatrix Runner Date: Tue, 12 May 2026 22:28:32 +0000 Subject: [PATCH 1/2] feat(gitops): add PullFastForward and startup pull for task-skills - Add `Manager.PullFastForward(ctx)`: runs `git pull --ff-only origin`, reloads go-git in-memory state on success, returns error on divergent history. - Add `dirHasGit(dir)` helper: reports whether `/.git` exists before NewManager can PlainInit an empty directory. - Add `startupPullTaskSkills(hadGit, remoteURL, mgr)`: best-effort startup pull; logs warn on error, never propagates to the caller. - Wire startup pull into main.go after task-skills git manager init. - Tests: TestPullFastForward_NoRemote/FetchesNewCommits/NonFastForwardReturnsError in manager_test.go; TestDirHasGit_PresentAbsent and TestStartupPullTaskSkills_* in cmd/contextmatrix/main_test.go. --- cmd/contextmatrix/main.go | 40 +++++++++++ cmd/contextmatrix/main_test.go | 75 ++++++++++++++++++++ internal/gitops/manager.go | 23 ++++++ internal/gitops/manager_test.go | 121 ++++++++++++++++++++++++++++++++ 4 files changed, 259 insertions(+) create mode 100644 cmd/contextmatrix/main_test.go diff --git a/cmd/contextmatrix/main.go b/cmd/contextmatrix/main.go index 290d0eb..f07cf2e 100644 --- a/cmd/contextmatrix/main.go +++ b/cmd/contextmatrix/main.go @@ -12,6 +12,7 @@ import ( "net/http/pprof" "os" "os/signal" + "path/filepath" "strconv" "strings" "sync" @@ -143,6 +144,10 @@ func main() { taskSkillsCloneURL = cfg.TaskSkills.GitRemoteURL } + // Capture whether the task-skills dir already has a .git before NewManager + // runs PlainInit on an empty directory. + taskSkillsHadGit := dirHasGit(cfg.TaskSkills.Dir) + taskSkillsGit, err := gitops.NewManager( cfg.TaskSkills.Dir, taskSkillsCloneURL, @@ -156,6 +161,8 @@ func main() { slog.Info("task-skills git manager initialized", "repo_path", cfg.TaskSkills.Dir) + startupPullTaskSkills(taskSkillsHadGit, cfg.TaskSkills.GitRemoteURL, taskSkillsGit) + // Initialize storage store, err := storage.NewFilesystemStore(cfg.Boards.Dir) if err != nil { @@ -523,6 +530,39 @@ func main() { } } +// dirHasGit reports whether /.git exists (as a file or directory). +// Returns false for an empty dir string. +func dirHasGit(dir string) bool { + if dir == "" { + return false + } + + _, err := os.Stat(filepath.Join(dir, ".git")) + + return err == nil +} + +// startupPullTaskSkills performs a fast-forward pull of the task-skills repo +// at server startup. It is a best-effort operation: pull failures are logged +// as warnings but do not prevent the server from starting. +func startupPullTaskSkills(hadGit bool, remoteURL string, mgr *gitops.Manager) { + if !hadGit || remoteURL == "" || mgr == nil { + return + } + + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + + if err := mgr.PullFastForward(ctx); err != nil { + slog.Warn("task-skills startup pull failed; serving cached copy", + "dir", mgr.RepoPath(), "error", err) + + return + } + + slog.Info("task-skills startup pull: ok", "dir", mgr.RepoPath()) +} + // waitSyncer wraps a blocking Wait() call with a context deadline. It runs // Wait() in a goroutine and returns nil as soon as it returns, or ctx.Err() // if the deadline fires first. The goroutine is leaked in the timeout case — diff --git a/cmd/contextmatrix/main_test.go b/cmd/contextmatrix/main_test.go new file mode 100644 index 0000000..1b41807 --- /dev/null +++ b/cmd/contextmatrix/main_test.go @@ -0,0 +1,75 @@ +package main + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + githubauth "github.com/mhersson/contextmatrix-githubauth" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mhersson/contextmatrix/internal/gitops" +) + +func staticMainTestProvider(t *testing.T) githubauth.TokenGenerator { + t.Helper() + + p, err := githubauth.NewPATProvider("test-token") + require.NoError(t, err) + + return p +} + +func TestDirHasGit_PresentAbsent(t *testing.T) { + tmpDir := t.TempDir() + + // No .git yet — should return false. + assert.False(t, dirHasGit(tmpDir), "dir without .git should return false") + + // Create .git directory. + gitDir := filepath.Join(tmpDir, ".git") + require.NoError(t, os.MkdirAll(gitDir, 0o755)) + + assert.True(t, dirHasGit(tmpDir), "dir with .git directory should return true") + + // Empty string — should return false. + assert.False(t, dirHasGit(""), "empty string should return false") +} + +func TestStartupPullTaskSkills_SkipsWhenGitMissing(t *testing.T) { + // hadGit=false: should return immediately without touching mgr (nil is safe). + startupPullTaskSkills(false, "https://example.com/repo.git", nil) + // If we reach here without panic the test passes. +} + +func TestStartupPullTaskSkills_SkipsWhenRemoteEmpty(t *testing.T) { + // remoteURL="": should return immediately without touching mgr (nil is safe). + startupPullTaskSkills(true, "", nil) +} + +func TestStartupPullTaskSkills_SkipsWhenMgrNil(t *testing.T) { + // mgr=nil: should return immediately. + startupPullTaskSkills(true, "https://example.com/repo.git", nil) +} + +// TestStartupPullTaskSkills_SwallowsError verifies that a pull failure +// (unreachable remote) is logged as a warning but does not panic or propagate. +func TestStartupPullTaskSkills_SwallowsError(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git binary not found, skipping") + } + + tmpDir := t.TempDir() + + mgr, err := gitops.NewManager(tmpDir, "", "test", staticMainTestProvider(t)) + require.NoError(t, err) + + // Add a bogus remote so hasRemote() returns true and PullFastForward is attempted. + bogusURL := "https://127.0.0.1:1/does-not-exist.git" + require.NoError(t, mgr.AddRemote(t.Context(), "origin", bogusURL)) + + // Must not panic; error is swallowed with a Warn log. + startupPullTaskSkills(true, bogusURL, mgr) +} diff --git a/internal/gitops/manager.go b/internal/gitops/manager.go index 8aa0206..2c95846 100644 --- a/internal/gitops/manager.go +++ b/internal/gitops/manager.go @@ -302,6 +302,29 @@ func (m *Manager) Pull(ctx context.Context) error { return nil } +// PullFastForward fetches and fast-forwards from the origin remote using +// shell git. Returns nil immediately if no remote is configured. +// Non-fast-forward situations (divergent history) return a non-nil error +// so the caller can log and continue without modifying the working tree. +func (m *Manager) PullFastForward(ctx context.Context) error { + m.mu.Lock() + defer m.mu.Unlock() + + if !m.hasRemote() { + return nil + } + + if err := m.runGit(ctx, "pull", "--ff-only", "origin"); err != nil { + return fmt.Errorf("pull --ff-only: %w", err) + } + + if err := m.reloadRepo(); err != nil { + return fmt.Errorf("reload after pull: %w", err) + } + + return nil +} + // Push pushes commits to the origin remote using shell git. // Uses "git push --set-upstream origin HEAD" so it works whether or not // the current branch already has a tracking upstream configured. diff --git a/internal/gitops/manager_test.go b/internal/gitops/manager_test.go index b25b970..5c1cc73 100644 --- a/internal/gitops/manager_test.go +++ b/internal/gitops/manager_test.go @@ -329,6 +329,127 @@ func TestPull_AutoReloadsGoGit(t *testing.T) { assert.Equal(t, "post-pull commit", strings.TrimSpace(msg)) } +// TestPullFastForward_NoRemote verifies that PullFastForward returns nil +// immediately when no origin remote is configured. +func TestPullFastForward_NoRemote(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git binary not found, skipping") + } + + tmpDir := t.TempDir() + mgr, err := NewManager(tmpDir, "", "test", staticTestProvider(t)) + require.NoError(t, err) + + err = mgr.PullFastForward(context.Background()) + assert.NoError(t, err) +} + +// TestPullFastForward_FetchesNewCommits verifies that PullFastForward brings +// in new commits from the remote and reloads the go-git in-memory state. +func TestPullFastForward_FetchesNewCommits(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git binary not found, skipping") + } + + ctx := context.Background() + + // Create a bare remote and two working copies. + bareDir := t.TempDir() + _, err := git.PlainInit(bareDir, true) + require.NoError(t, err) + + workDir := t.TempDir() + mgr, err := NewManager(workDir, "", "test", staticTestProvider(t)) + require.NoError(t, err) + mgr.SetAuthor("Test User", "test@example.com") + + require.NoError(t, os.WriteFile(filepath.Join(workDir, "init.txt"), []byte("init"), 0o644)) + require.NoError(t, mgr.CommitFile(ctx, "init.txt", "initial commit")) + require.NoError(t, mgr.AddRemote(ctx, "origin", "file://"+bareDir)) + require.NoError(t, mgr.Push(ctx)) + + // Clone and push a new commit from a second working copy. + cloneDir := t.TempDir() + _, err = git.PlainClone(cloneDir, false, &git.CloneOptions{URL: "file://" + bareDir}) + require.NoError(t, err) + cloneMgr, err := NewManager(cloneDir, "", "test", staticTestProvider(t)) + require.NoError(t, err) + cloneMgr.SetAuthor("Clone User", "clone@example.com") + require.NoError(t, os.WriteFile(filepath.Join(cloneDir, "new.txt"), []byte("new content"), 0o644)) + require.NoError(t, cloneMgr.CommitFile(ctx, "new.txt", "remote commit")) + require.NoError(t, cloneMgr.Push(ctx)) + + // PullFastForward in the original repo should bring in the new commit. + require.NoError(t, mgr.PullFastForward(ctx)) + + // new.txt should exist in the original worktree. + _, err = os.Stat(filepath.Join(workDir, "new.txt")) + require.NoError(t, err, "new.txt should exist after PullFastForward") + + // go-git in-memory state should reflect the new commit (reloadRepo ran). + msg, err := mgr.GetLastCommitMessage() + require.NoError(t, err) + assert.Equal(t, "remote commit", strings.TrimSpace(msg), + "go-git should see remote commit after PullFastForward auto-reload") +} + +// TestPullFastForward_NonFastForwardReturnsError verifies that a divergent +// history causes PullFastForward to return a non-nil error without modifying +// the original working tree. +func TestPullFastForward_NonFastForwardReturnsError(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git binary not found, skipping") + } + + ctx := context.Background() + + // Create a bare remote and two working copies. + bareDir := t.TempDir() + _, err := git.PlainInit(bareDir, true) + require.NoError(t, err) + + workDir := t.TempDir() + mgr, err := NewManager(workDir, "", "test", staticTestProvider(t)) + require.NoError(t, err) + mgr.SetAuthor("Test User", "test@example.com") + + require.NoError(t, os.WriteFile(filepath.Join(workDir, "init.txt"), []byte("init"), 0o644)) + require.NoError(t, mgr.CommitFile(ctx, "init.txt", "initial commit")) + require.NoError(t, mgr.AddRemote(ctx, "origin", "file://"+bareDir)) + require.NoError(t, mgr.Push(ctx)) + + // Original makes a local commit (diverges from remote). + require.NoError(t, os.WriteFile(filepath.Join(workDir, "local.txt"), []byte("local"), 0o644)) + require.NoError(t, mgr.CommitFile(ctx, "local.txt", "local-side commit")) + + // Clone and force-push a different commit on top of the shared base, + // making the remote head diverge from the original's local head. + cloneDir := t.TempDir() + _, err = git.PlainClone(cloneDir, false, &git.CloneOptions{URL: "file://" + bareDir}) + require.NoError(t, err) + cloneMgr, err := NewManager(cloneDir, "", "test", staticTestProvider(t)) + require.NoError(t, err) + cloneMgr.SetAuthor("Clone User", "clone@example.com") + require.NoError(t, os.WriteFile(filepath.Join(cloneDir, "remote.txt"), []byte("remote"), 0o644)) + require.NoError(t, cloneMgr.CommitFile(ctx, "remote.txt", "remote-side commit")) + + // Force-push so the remote head is now a different commit than what the + // original has locally — a true divergence. + cmd := exec.Command("git", "push", "--force", "origin", "HEAD") + cmd.Dir = cloneDir + require.NoError(t, cmd.Run()) + + // PullFastForward should fail because the histories have diverged. + err = mgr.PullFastForward(ctx) + require.Error(t, err, "PullFastForward must fail on divergent history") + + // The original's HEAD must still be the local-side commit. + msg, getMsgErr := mgr.GetLastCommitMessage() + require.NoError(t, getMsgErr) + assert.Equal(t, "local-side commit", strings.TrimSpace(msg), + "original HEAD must be unchanged after failed PullFastForward") +} + // TestPush_AutoReloadsGoGit verifies that after Push, go-git's in-memory // state is refreshed so subsequent go-git operations see correct refs. func TestPush_AutoReloadsGoGit(t *testing.T) { From 327801a548f113c15031647c808166e28e83c423 Mon Sep 17 00:00:00 2001 From: ContextMatrix Runner Date: Tue, 12 May 2026 22:30:05 +0000 Subject: [PATCH 2/2] docs(remote-execution): document server startup pull for task-skills - Add "Server startup pull" section to docs/remote-execution.md cross- referencing the existing "On-trigger pull" runner behaviour. - Note fail-open semantics: warnings logged, startup not blocked. - Clarify the two log lines operators should watch for. - Update README config table: task_skills.git_remote_url now also governs the startup pull, not only clone-on-empty. --- README.md | 2 +- docs/remote-execution.md | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 73d6286..98ee945 100644 --- a/README.md +++ b/README.md @@ -701,7 +701,7 @@ format. | `workflow_skills_dir` | `/workflow-skills` | Path to the workflow skill markdown directory (lifecycle skills served via MCP prompts). When empty, resolves to a `workflow-skills` directory next to `config.yaml` (XDG-resolved). | | `task_skills.dir` | `/task-skills` | Path to the curated task-skills repo (specialist skills mounted into runner workers) | | `task_skills.git_clone_on_empty` | `false` | Clone on first start when `task_skills.dir` is empty | -| `task_skills.git_remote_url` | `""` | HTTPS URL used for clone-on-empty | +| `task_skills.git_remote_url` | `""` | HTTPS URL used for clone-on-empty and for the startup `git pull --ff-only` | | `token_costs` | --- | Per-model token cost rates (see example below) | | `mcp_api_key` | `""` | Bearer token for MCP endpoint authentication (empty = no auth) | | `boards.dir` | --- | Path to boards git repo (required) | diff --git a/docs/remote-execution.md b/docs/remote-execution.md index 716d264..bd48db8 100644 --- a/docs/remote-execution.md +++ b/docs/remote-execution.md @@ -204,6 +204,21 @@ Before constructing the container config, the runner runs `git pull --ff-only` on `task_skills_dir`. On failure, the runner logs and continues with the existing local clone — the trigger never aborts because of a sync issue. +### Server startup pull + +The ContextMatrix server mirrors this behaviour on startup. If `task_skills.dir` +already contains a `.git` directory **and** `task_skills.git_remote_url` is +non-empty, the server runs `git pull --ff-only` (60-second timeout) immediately +after initialising the task-skills git manager and before opening any network +listeners. Failures are logged as warnings and do not prevent startup — the +cached on-disk copy is used as-is. A non-fast-forward situation (divergent local +history) also surfaces as a warning; the working tree is not modified. + +The `task-skills startup pull: ok` log line confirms a successful pull; absence +of the line means either the preconditions were not met (no `.git`, no remote +URL) or the pull was skipped/failed (see `task-skills startup pull failed` warn +log). + ### Required tool The container's `--allowed-tools` allowlist must include `Skill` for Claude