From adecad7f30088af285aae45ffe1e87e4f2b2c67f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Tue, 28 Jul 2026 15:46:28 +0100 Subject: [PATCH] fix(github): honor executor clone transport (#1998) --- .../orchestrator/executor/executor.go | 2 + .../executor/executor_credentials.go | 3 +- .../orchestrator/executor/executor_resume.go | 67 ++++- ...ecutor_resume_clone_default_branch_test.go | 2 + .../executor_resume_clone_transport_test.go | 247 ++++++++++++++++++ .../orchestrator/executor/executor_test.go | 4 + apps/backend/internal/repoclone/clone.go | 17 ++ apps/backend/internal/repoclone/clone_test.go | 59 +++++ .../github-executor-clone-transport/plan.md | 98 +++++++ ...sk-00-reconcile-managed-checkout-origin.md | 85 ++++++ docs/public/integrations.md | 4 + .../integrations/github-authentication.md | 24 +- 12 files changed, 607 insertions(+), 5 deletions(-) create mode 100644 apps/backend/internal/orchestrator/executor/executor_resume_clone_transport_test.go create mode 100644 docs/plans/github-executor-clone-transport/plan.md create mode 100644 docs/plans/github-executor-clone-transport/task-00-reconcile-managed-checkout-origin.md diff --git a/apps/backend/internal/orchestrator/executor/executor.go b/apps/backend/internal/orchestrator/executor/executor.go index e6d5038065..cb4ac4c27d 100644 --- a/apps/backend/internal/orchestrator/executor/executor.go +++ b/apps/backend/internal/orchestrator/executor/executor.go @@ -736,6 +736,8 @@ type RepoCloner interface { owner, name, credentialHost, token string, ) (string, error) ShouldRecloneForWorkspace(workspaceID, path string) bool + // SetOriginURL updates a Kandev-managed checkout remote without exposing credentials. + SetOriginURL(ctx context.Context, repositoryPath, originURL string) error // BuildCloneURL constructs a protocol-aware clone URL for the given provider/owner/name. BuildCloneURLWithHost(provider, host, owner, name string) (string, error) } diff --git a/apps/backend/internal/orchestrator/executor/executor_credentials.go b/apps/backend/internal/orchestrator/executor/executor_credentials.go index 860beb87a5..b226b16b0d 100644 --- a/apps/backend/internal/orchestrator/executor/executor_credentials.go +++ b/apps/backend/internal/orchestrator/executor/executor_credentials.go @@ -30,6 +30,7 @@ const ( gitHubCredentialHelper = "!agentctl git-credential" defaultGitHubHost = "github.com" gitLabCredentialHelper = `!f() { echo "username=oauth2"; echo "password=$GITLAB_TOKEN"; }; f` + taskGitCredentialsModeManaged = "managed" taskGitCredentialsModeExecutor = "executor" ) @@ -95,7 +96,7 @@ func (e *Executor) applyGitCredentialSnapshot( if req == nil || session == nil { return nil } - policy := TaskGitCredentialPolicy{Mode: "managed"} + policy := TaskGitCredentialPolicy{Mode: taskGitCredentialsModeManaged} if e.githubCredentialPolicyResolver != nil { resolved, err := e.githubCredentialPolicyResolver.ResolveTaskGitCredentialPolicy(ctx, req.WorkspaceID) if err != nil { diff --git a/apps/backend/internal/orchestrator/executor/executor_resume.go b/apps/backend/internal/orchestrator/executor/executor_resume.go index a55c62df90..bfa4dcead2 100644 --- a/apps/backend/internal/orchestrator/executor/executor_resume.go +++ b/apps/backend/internal/orchestrator/executor/executor_resume.go @@ -6,10 +6,12 @@ import ( "fmt" "os" "path/filepath" + "strings" "time" "github.com/kandev/kandev/internal/agent/runtime/lifecycle" "github.com/kandev/kandev/internal/common/gitref" + "github.com/kandev/kandev/internal/repoclone" "github.com/kandev/kandev/internal/task/models" "github.com/kandev/kandev/internal/worktree" v1 "github.com/kandev/kandev/pkg/api/v1" @@ -157,7 +159,7 @@ func (e *Executor) ensureRepoLocalPath(ctx context.Context, repo *models.Reposit } if repo.LocalPath != "" && isLocalGitRepo(repo.LocalPath) && (e.repoCloner == nil || !e.repoCloner.ShouldRecloneForWorkspace(repo.WorkspaceID, repo.LocalPath)) { - return nil + return e.reconcileGitHubCheckoutOrigin(ctx, repo, repo.LocalPath) } localPath, cloneErr := e.ensureRepoCloned(ctx, repo) if cloneErr != nil { @@ -205,7 +207,8 @@ func (e *Executor) ensureRepoCloned(ctx context.Context, repo *models.Repository return "", err } - // Persist the local path so future launches skip cloning + // Persist the local path before reconciliation so a remote-update failure + // does not discard a completed clone and force a re-clone on every retry. if e.repoUpdater != nil && localPath != "" { if updateErr := e.repoUpdater.UpdateRepositoryLocalPath(ctx, repo.ID, localPath); updateErr != nil { e.logger.Warn("failed to update repository local path after clone", @@ -215,6 +218,9 @@ func (e *Executor) ensureRepoCloned(ctx context.Context, repo *models.Repository // Non-fatal: the clone succeeded, we can use the path } } + if err := e.reconcileGitHubCheckoutOrigin(ctx, repo, localPath); err != nil { + return "", err + } // Note: default_branch backfill is intentionally driven from // resolveTaskRepoInfo (the caller), not here. That way it also runs for @@ -225,6 +231,63 @@ func (e *Executor) ensureRepoCloned(ctx context.Context, repo *models.Repository return localPath, nil } +func (e *Executor) reconcileGitHubCheckoutOrigin( + ctx context.Context, repo *models.Repository, localPath string, +) error { + if e.repoCloner == nil || localPath == "" || !isGitHubRepository(repo) { + return nil + } + policy := TaskGitCredentialPolicy{Mode: taskGitCredentialsModeManaged} + if e.githubCredentialPolicyResolver != nil { + resolved, err := e.githubCredentialPolicyResolver.ResolveTaskGitCredentialPolicy(ctx, repo.WorkspaceID) + if err != nil { + return fmt.Errorf("resolve task Git credential policy: %w", err) + } + policy = resolved + } + originURL, err := gitHubCheckoutOriginURL(repo, policy, e.repoCloner) + if err != nil { + return err + } + if err := e.repoCloner.SetOriginURL(ctx, localPath, originURL); err != nil { + return fmt.Errorf("set GitHub checkout origin: %w", err) + } + return nil +} + +func isGitHubRepository(repo *models.Repository) bool { + if repo == nil || repo.ProviderOwner == "" || repo.ProviderName == "" { + return false + } + // Provider == "" is treated as GitHub for legacy rows imported before + // provider tagging; this matches the clone URL fallback convention. + return repo.Provider == "" || strings.EqualFold(repo.Provider, "github") +} + +func gitHubCheckoutOriginURL( + repo *models.Repository, policy TaskGitCredentialPolicy, cloner RepoCloner, +) (string, error) { + if policy.Mode == taskGitCredentialsModeExecutor { + originURL, err := cloner.BuildCloneURLWithHost( + repo.Provider, repo.ProviderHost, repo.ProviderOwner, repo.ProviderName, + ) + if err != nil { + return "", fmt.Errorf("build executor GitHub checkout origin: %w", err) + } + if originURL == "" { + return "", errors.New("build executor GitHub checkout origin: empty URL") + } + return originURL, nil + } + originURL, err := repoclone.CloneURLWithHost( + repo.Provider, repo.ProviderHost, repo.ProviderOwner, repo.ProviderName, repoclone.ProtocolHTTPS, + ) + if err != nil { + return "", fmt.Errorf("build managed GitHub checkout origin: %w", err) + } + return originURL, nil +} + // EnsureRepositoryCloned is the host-materialization seam for provider-backed // repositories. Authentication and clone-url construction deliberately remain // inside Executor so callers never receive credentials or construct git calls. diff --git a/apps/backend/internal/orchestrator/executor/executor_resume_clone_default_branch_test.go b/apps/backend/internal/orchestrator/executor/executor_resume_clone_default_branch_test.go index f08d8407d6..6e0474b8db 100644 --- a/apps/backend/internal/orchestrator/executor/executor_resume_clone_default_branch_test.go +++ b/apps/backend/internal/orchestrator/executor/executor_resume_clone_default_branch_test.go @@ -399,6 +399,8 @@ func (f *fakeRepoCloner) EnsureWorkspaceClonedForProvider( func (f *fakeRepoCloner) ShouldRecloneForWorkspace(_, _ string) bool { return false } +func (f *fakeRepoCloner) SetOriginURL(context.Context, string, string) error { return nil } + func (f *fakeRepoCloner) BuildCloneURLWithHost(_, _, owner, name string) (string, error) { return "https://github.com/" + owner + "/" + name + ".git", nil } diff --git a/apps/backend/internal/orchestrator/executor/executor_resume_clone_transport_test.go b/apps/backend/internal/orchestrator/executor/executor_resume_clone_transport_test.go new file mode 100644 index 0000000000..01de37b190 --- /dev/null +++ b/apps/backend/internal/orchestrator/executor/executor_resume_clone_transport_test.go @@ -0,0 +1,247 @@ +package executor + +import ( + "context" + "fmt" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/kandev/kandev/internal/task/models" +) + +func TestEnsureRepoLocalPath_ReconcilesGitHubOriginForCredentialPolicy(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + + tests := []struct { + name string + policy string + origin string + wantOrigin string + }{ + { + name: "executor inheritance uses host SSH clone protocol", + policy: taskGitCredentialsModeExecutor, + origin: "https://github.com/acme/widgets.git", + wantOrigin: "git@github.com:acme/widgets.git", + }, + { + name: "managed credentials restore HTTPS origin", + policy: "managed", + origin: "git@github.com:acme/widgets.git", + wantOrigin: "https://github.com/acme/widgets.git", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + repoPath := initGitRepoWithOrigin(t, tt.origin) + exec := newTestExecutor(t, &mockAgentManager{}, newMockRepository()) + exec.SetTaskGitCredentialPolicyResolver(fakeTaskGitCredentialPolicyResolver{ + policy: TaskGitCredentialPolicy{Mode: tt.policy}, + }) + exec.SetRepoCloner(&cloneTransportTestCloner{cloneURL: "git@github.com:acme/widgets.git"}, nil) + + repository := &models.Repository{ + WorkspaceID: "workspace-1", + SourceType: "provider", + Provider: "github", + ProviderOwner: "acme", + ProviderName: "widgets", + LocalPath: repoPath, + } + if err := exec.ensureRepoLocalPath(context.Background(), repository); err != nil { + t.Fatalf("ensureRepoLocalPath() error = %v", err) + } + if got := gitOriginURL(t, repoPath); got != tt.wantOrigin { + t.Fatalf("origin = %q, want %q", got, tt.wantOrigin) + } + }) + } +} + +func TestEnsureRepoLocalPath_DoesNotRewriteUserManagedOrigin(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + + const origin = "git@github.com:acme/widgets.git" + repoPath := initGitRepoWithOrigin(t, origin) + exec := newTestExecutor(t, &mockAgentManager{}, newMockRepository()) + exec.SetTaskGitCredentialPolicyResolver(fakeTaskGitCredentialPolicyResolver{ + policy: TaskGitCredentialPolicy{Mode: "managed"}, + }) + exec.SetRepoCloner(&cloneTransportTestCloner{cloneURL: "https://github.com/acme/widgets.git"}, nil) + + repository := &models.Repository{ + WorkspaceID: "workspace-1", + SourceType: sourceTypeLocal, + Provider: "github", + ProviderOwner: "acme", + ProviderName: "widgets", + LocalPath: repoPath, + } + if err := exec.ensureRepoLocalPath(context.Background(), repository); err != nil { + t.Fatalf("ensureRepoLocalPath() error = %v", err) + } + if got := gitOriginURL(t, repoPath); got != origin { + t.Fatalf("origin = %q, want unchanged %q", got, origin) + } +} + +func TestEnsureRepoLocalPath_ReconcilesFreshGitHubCheckoutOrigin(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + + repoPath := initGitRepoWithOrigin(t, "https://github.com/acme/widgets.git") + exec := newTestExecutor(t, &mockAgentManager{}, newMockRepository()) + exec.SetTaskGitCredentialPolicyResolver(fakeTaskGitCredentialPolicyResolver{ + policy: TaskGitCredentialPolicy{Mode: taskGitCredentialsModeExecutor}, + }) + exec.SetRepoCloner(&cloneTransportTestCloner{ + cloneURL: "git@github.com:acme/widgets.git", + returnPath: repoPath, + }, nil) + + repository := &models.Repository{ + WorkspaceID: "workspace-1", + SourceType: "provider", + Provider: "github", + ProviderOwner: "acme", + ProviderName: "widgets", + } + if err := exec.ensureRepoLocalPath(context.Background(), repository); err != nil { + t.Fatalf("ensureRepoLocalPath() error = %v", err) + } + if repository.LocalPath != repoPath { + t.Fatalf("LocalPath = %q, want %q", repository.LocalPath, repoPath) + } + if got := gitOriginURL(t, repoPath); got != "git@github.com:acme/widgets.git" { + t.Fatalf("origin = %q, want SSH URL", got) + } +} + +func TestEnsureRepoLocalPath_ReturnsOriginUpdateFailure(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + + repoPath := initGitRepoWithOrigin(t, "https://github.com/acme/widgets.git") + exec := newTestExecutor(t, &mockAgentManager{}, newMockRepository()) + exec.SetTaskGitCredentialPolicyResolver(fakeTaskGitCredentialPolicyResolver{ + policy: TaskGitCredentialPolicy{Mode: taskGitCredentialsModeExecutor}, + }) + exec.SetRepoCloner(&cloneTransportTestCloner{ + cloneURL: "git@github.com:acme/widgets.git", + setOriginErr: fmt.Errorf("read-only repository"), + }, nil) + + err := exec.ensureRepoLocalPath(context.Background(), &models.Repository{ + WorkspaceID: "workspace-1", + SourceType: "provider", + Provider: "github", + ProviderOwner: "acme", + ProviderName: "widgets", + LocalPath: repoPath, + }) + if err == nil || !strings.Contains(err.Error(), "read-only repository") { + t.Fatalf("ensureRepoLocalPath() error = %v, want origin-update failure", err) + } +} + +func TestEnsureRepoLocalPath_PersistsFreshCloneBeforeOriginFailure(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + + repoPath := initGitRepoWithOrigin(t, "https://github.com/acme/widgets.git") + updater := &localPathRecordingRepoUpdater{} + exec := newTestExecutor(t, &mockAgentManager{}, newMockRepository()) + exec.SetTaskGitCredentialPolicyResolver(fakeTaskGitCredentialPolicyResolver{ + policy: TaskGitCredentialPolicy{Mode: taskGitCredentialsModeExecutor}, + }) + exec.SetRepoCloner(&cloneTransportTestCloner{ + cloneURL: "git@github.com:acme/widgets.git", + returnPath: repoPath, + setOriginErr: fmt.Errorf("read-only repository"), + }, updater) + + err := exec.ensureRepoLocalPath(context.Background(), &models.Repository{ + ID: "repo-1", + WorkspaceID: "workspace-1", + SourceType: "provider", + Provider: "github", + ProviderOwner: "acme", + ProviderName: "widgets", + }) + if err == nil || !strings.Contains(err.Error(), "read-only repository") { + t.Fatalf("ensureRepoLocalPath() error = %v, want origin-update failure", err) + } + if updater.localPath != repoPath { + t.Fatalf("persisted local path = %q, want %q", updater.localPath, repoPath) + } +} + +type localPathRecordingRepoUpdater struct { + localPath string +} + +func (u *localPathRecordingRepoUpdater) UpdateRepositoryLocalPath(_ context.Context, _, localPath string) error { + u.localPath = localPath + return nil +} + +func (u *localPathRecordingRepoUpdater) UpdateRepositoryDefaultBranch(context.Context, string, string) error { + return nil +} + +type cloneTransportTestCloner struct { + cloneURL string + returnPath string + setOriginErr error +} + +func (c *cloneTransportTestCloner) EnsureWorkspaceClonedForProvider( + context.Context, string, string, string, string, string, string, string, string, +) (string, error) { + return c.returnPath, nil +} + +func (c *cloneTransportTestCloner) ShouldRecloneForWorkspace(string, string) bool { return false } + +func (c *cloneTransportTestCloner) SetOriginURL(ctx context.Context, repositoryPath, originURL string) error { + if c.setOriginErr != nil { + return c.setOriginErr + } + cmd := exec.CommandContext(ctx, "git", "-C", repositoryPath, "remote", "set-url", "origin", "--", originURL) + if out, err := cmd.CombinedOutput(); err != nil { + return fmt.Errorf("set origin: %w: %s", err, out) + } + return nil +} + +func (c *cloneTransportTestCloner) BuildCloneURLWithHost(string, string, string, string) (string, error) { + return c.cloneURL, nil +} + +func initGitRepoWithOrigin(t *testing.T, origin string) string { + t.Helper() + repoPath := filepath.Join(t.TempDir(), "repo") + runGitInTest(t, "", "init", "--quiet", repoPath) + runGitInTest(t, repoPath, "remote", "add", "origin", origin) + return repoPath +} + +func gitOriginURL(t *testing.T, repoPath string) string { + t.Helper() + cmd := exec.Command("git", "-C", repoPath, "remote", "get-url", "origin") + out, err := cmd.Output() + if err != nil { + t.Fatalf("git remote get-url origin: %v", err) + } + return string(out[:len(out)-1]) +} diff --git a/apps/backend/internal/orchestrator/executor/executor_test.go b/apps/backend/internal/orchestrator/executor/executor_test.go index 31b46b19b7..8affe3b526 100644 --- a/apps/backend/internal/orchestrator/executor/executor_test.go +++ b/apps/backend/internal/orchestrator/executor/executor_test.go @@ -2516,6 +2516,10 @@ func (c *recordingAuthenticatedCloner) EnsureWorkspaceClonedForProvider( func (c *recordingAuthenticatedCloner) ShouldRecloneForWorkspace(_, _ string) bool { return false } +func (c *recordingAuthenticatedCloner) SetOriginURL(context.Context, string, string) error { + return nil +} + func (c *recordingAuthenticatedCloner) BuildCloneURLWithHost(_, _, _, _ string) (string, error) { return "", nil } diff --git a/apps/backend/internal/repoclone/clone.go b/apps/backend/internal/repoclone/clone.go index f391638f9a..9e8e0f5d59 100644 --- a/apps/backend/internal/repoclone/clone.go +++ b/apps/backend/internal/repoclone/clone.go @@ -327,6 +327,23 @@ func (c *Cloner) EnsureWorkspaceClonedForProvider( return c.ensureClonedAtPath(ctx, cloneURL, targetPath, auth) } +// SetOriginURL updates a managed checkout's origin without exposing credentials. +func (c *Cloner) SetOriginURL(ctx context.Context, repositoryPath, originURL string) error { + if strings.TrimSpace(repositoryPath) == "" || strings.TrimSpace(originURL) == "" { + return errors.New("repository path and origin URL are required") + } + mu := c.repoMu(repositoryPath) + mu.Lock() + defer mu.Unlock() + + cmd := exec.CommandContext(ctx, "git", "-C", repositoryPath, "remote", "set-url", "origin", "--", originURL) + configureGitCommand(cmd, nil) + if _, err := subproc.RunGitCombinedOutput(ctx, cmd); err != nil { + return fmt.Errorf("set repository origin: %w", err) + } + return nil +} + type cloneAuth struct { origin string username string diff --git a/apps/backend/internal/repoclone/clone_test.go b/apps/backend/internal/repoclone/clone_test.go index 5c21845dbf..3cc27402ab 100644 --- a/apps/backend/internal/repoclone/clone_test.go +++ b/apps/backend/internal/repoclone/clone_test.go @@ -7,6 +7,7 @@ import ( "os/exec" "path/filepath" "strings" + "sync" "testing" "github.com/kandev/kandev/internal/common/logger" @@ -53,6 +54,64 @@ func TestClone_PreservesNonDefaultRemoteBranches(t *testing.T) { } } +func TestSetOriginURL(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + + repoPath := t.TempDir() + runGit(t, repoPath, "init", "--quiet") + runGit(t, repoPath, "remote", "add", "origin", "https://github.com/acme/widgets.git") + + cloner := NewCloner(Config{}, ProtocolSSH, t.TempDir(), logger.Default()) + if err := cloner.SetOriginURL(context.Background(), repoPath, "git@github.com:acme/widgets.git"); err != nil { + t.Fatalf("SetOriginURL() error = %v", err) + } + if got := strings.TrimSpace(runGit(t, repoPath, "remote", "get-url", "origin")); got != "git@github.com:acme/widgets.git" { + t.Fatalf("origin = %q, want SSH URL", got) + } + + if err := cloner.SetOriginURL(context.Background(), "", "https://github.com/acme/widgets.git"); err == nil { + t.Fatal("SetOriginURL() error = nil, want missing repository path error") + } + if err := cloner.SetOriginURL(context.Background(), repoPath, ""); err == nil { + t.Fatal("SetOriginURL() error = nil, want missing origin URL error") + } +} + +func TestSetOriginURLSerializesConcurrentUpdates(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + + repoPath := t.TempDir() + runGit(t, repoPath, "init", "--quiet") + runGit(t, repoPath, "remote", "add", "origin", "https://github.com/acme/widgets.git") + cloner := NewCloner(Config{}, ProtocolSSH, t.TempDir(), logger.Default()) + + const workers = 32 + start := make(chan struct{}) + errs := make(chan error, workers) + var group sync.WaitGroup + group.Add(workers) + for i := 0; i < workers; i++ { + go func() { + defer group.Done() + <-start + errs <- cloner.SetOriginURL(context.Background(), repoPath, "git@github.com:acme/widgets.git") + }() + } + close(start) + group.Wait() + close(errs) + + for err := range errs { + if err != nil { + t.Fatalf("concurrent SetOriginURL() error = %v", err) + } + } +} + func TestWorkspaceRepoPathIsolatesManagedClones(t *testing.T) { t.Parallel() diff --git a/docs/plans/github-executor-clone-transport/plan.md b/docs/plans/github-executor-clone-transport/plan.md new file mode 100644 index 0000000000..12e6163cdf --- /dev/null +++ b/docs/plans/github-executor-clone-transport/plan.md @@ -0,0 +1,98 @@ +--- +spec: docs/specs/integrations/github-authentication.md +created: 2026-07-28 +status: complete +--- + +# Fix Plan: Honor Executor-Inherited GitHub Clone Transport + +## Overview + +PR #1810 made host materialization of provider-backed GitHub repositories use a workspace-scoped +credential and canonical HTTPS URL. PR #1985 later separated managed task credentials from +executor inheritance, but only changed the environment injected into task processes. The shared +managed checkout's `origin` therefore remains HTTPS and prevents SSH-oriented +`includeIf hasconfig:remote.*.url` rules from matching. + +The repair keeps backend materialization authenticated by the workspace automation connection, +then reconciles only Kandev-managed GitHub checkout remotes to the selected task policy before +Local/Worktree preparation. User-managed local repositories and remote-executor clone contracts +remain unchanged. + +## Confirmed Root Cause + +`repoclone.Cloner.workspaceCloneAuth` intentionally converts every workspace GitHub clone to +canonical HTTPS so the backend can use the workspace automation credential. +`executor.configureGitHubCredentialBrokerForRepositories` honors +`task_git_credentials_mode=executor` by removing the broker helper, but +`executor.ensureRepoLocalPath` returns immediately for an existing valid managed checkout and +never reconciles its `origin`. A newly materialized checkout also retains the HTTPS URL used for +the authenticated clone. + +The smallest reproduction is a provider-backed repository whose managed local path has +`origin=https://github.com/acme/widgets.git`, a host clone protocol of `ssh`, and workspace task +credential mode `executor`. Calling `ensureRepoLocalPath` leaves the HTTPS origin unchanged, so a +conditional include matching `git@github.com:acme/**` does not activate. + +--- + +## Backend + +### Managed checkout origin boundary + +- Add an origin-update operation to `apps/backend/internal/repoclone/clone.go` and the + `executor.RepoCloner` contract. It must invoke Git without a shell, terminate no user-controlled + option position ambiguously, and return a credential-safe error. +- In `apps/backend/internal/orchestrator/executor/executor_resume.go`, resolve the workspace task + Git credential policy before returning an existing provider-backed checkout and after a fresh + materialization. +- For GitHub-managed checkouts, derive canonical HTTPS in `managed` mode and use + `RepoCloner.BuildCloneURLWithHost` in `executor` mode so the host's startup-detected `gh` clone + protocol selects SSH or HTTPS. +- Reconcile only repositories with provider-backed ownership. Preserve the existing + `source_type=local` early return so user-owned repository remotes are never mutated. +- Treat a policy-resolution or remote-update error as repository preparation failure. Do not fall + back to the stale transport. + +## Tests + +- **What:** an existing Kandev-managed GitHub checkout changes from HTTPS to the host-selected SSH + URL in executor-inherited mode before `ensureRepoLocalPath` returns. + **File:** `apps/backend/internal/orchestrator/executor/executor_resume_clone_transport_test.go`. + **How:** filesystem-backed Git repository plus the real cloner origin updater and a fake + non-secret policy resolver. +- **What:** switching back to managed mode changes an existing managed checkout from SSH to + canonical HTTPS. + **File:** `apps/backend/internal/orchestrator/executor/executor_resume_clone_transport_test.go`. + **How:** table-driven companion case against the same real Git state boundary. +- **What:** a `source_type=local` repository keeps its original remote in either policy. + **File:** `apps/backend/internal/orchestrator/executor/executor_resume_clone_transport_test.go`. + **How:** real Git repository assertion after `ensureRepoLocalPath`. +- **What:** the repoclone origin updater changes `origin` and reports a missing/invalid repository + error without leaking credentials. + **File:** `apps/backend/internal/repoclone/clone_test.go`. + **How:** focused real Git subprocess test. + +## Documentation + +- Update `docs/public/integrations.md` to explain that Local/Worktree executor inheritance uses the + host-detected clone protocol for Kandev-managed checkout origins, allowing remote-URL conditional + includes to match. +- Keep the existing setting and terminology. No frontend, API, persistence, or migration change is + required. + +## Implementation Task + +- [x] [task-00-reconcile-managed-checkout-origin](task-00-reconcile-managed-checkout-origin.md) — done + +Execution is sequential in the primary conversation. There are no parallel candidates. + +## Risks And Out Of Scope + +- Existing active worktrees share the managed repository's Git configuration. Reconciliation is + workspace-policy-wide and occurs during later preparation; it does not rewrite user-managed + local checkouts. +- Custom SSH host aliases are not inferred from conditional includes. Executor mode follows the + existing `gh config get git_protocol` preference and canonical provider host construction. +- Remote Docker, SSH, and cloud executor clone URLs remain executor-specific and are not changed by + this repair. diff --git a/docs/plans/github-executor-clone-transport/task-00-reconcile-managed-checkout-origin.md b/docs/plans/github-executor-clone-transport/task-00-reconcile-managed-checkout-origin.md new file mode 100644 index 0000000000..e42147ee2b --- /dev/null +++ b/docs/plans/github-executor-clone-transport/task-00-reconcile-managed-checkout-origin.md @@ -0,0 +1,85 @@ +--- +id: "00-reconcile-managed-checkout-origin" +title: "Reconcile managed checkout origin" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/integrations/github-authentication.md" +--- + +# Task 00: Reconcile Managed Checkout Origin + +## Acceptance + +- Existing and newly materialized Kandev-managed GitHub checkouts use canonical HTTPS in managed + mode and the host-detected clone protocol in executor-inherited mode before Local/Worktree + preparation continues. +- A policy-resolution or origin-update failure stops preparation; no stale-transport fallback is + used. +- Repositories registered as user-managed local checkouts retain their configured origin. +- Public documentation explains the Local/Worktree transport behavior and conditional-include + effect. + +## TDD Sequence + +1. Add filesystem-backed regression cases for executor HTTPS-to-SSH reconciliation, managed + SSH-to-HTTPS reconciliation, and the local-source no-op. +2. Run the focused executor test and record the expected RED assertion that the origin remains on + the old transport. +3. Add a focused repoclone origin-update test and minimal production operation. +4. Wire policy-aware reconciliation into repository preparation. +5. Run the exact targeted backend and public-doc commands below and record GREEN results. + +## Verification + +```bash +cd apps/backend && go test ./internal/orchestrator/executor ./internal/repoclone -run 'Test.*(RepoLocalPath.*(GitHubOrigin|OriginUpdateFailure|DoesNotRewriteUserManagedOrigin)|SetOriginURL)' -count=1 +node --test scripts/validate-public-docs.test.mjs +node scripts/validate-public-docs.mjs +``` + +## Files Likely Touched + +- `apps/backend/internal/repoclone/clone.go` +- `apps/backend/internal/repoclone/clone_test.go` +- `apps/backend/internal/orchestrator/executor/executor.go` +- `apps/backend/internal/orchestrator/executor/executor_resume.go` +- `apps/backend/internal/orchestrator/executor/executor_resume_clone_transport_test.go` +- `apps/backend/internal/orchestrator/executor/executor_resume_clone_default_branch_test.go` +- `docs/public/integrations.md` +- `docs/specs/integrations/github-authentication.md` +- this task file and `plan.md` + +## Dependencies + +None. + +## Parallelism + +Sequential. The contract, implementation, tests, and documentation describe one behavior and +should land together. + +## Inputs + +- The confirmed root cause and backend boundary in `plan.md`. +- The task-policy transport scenarios in the workspace GitHub authentication spec. +- `repoclone.Cloner.BuildCloneURLWithHost` and `DetectGitProtocol`. +- `executor.ensureRepoLocalPath`, `ensureRepoCloned`, and + `configureGitHubCredentialBrokerForRepositories`. + +## Output Contract + +Report the RED failure, final origin-reconciliation behavior, files changed, exact GREEN command +results, public-doc validation, remaining risks, and task/plan status updates in the same primary +conversation. + +## Recorded Results + +- RED: `TestEnsureRepoLocalPath_ReconcilesGitHubOriginForCredentialPolicy` failed because both + policy transitions left the original remote URL unchanged. +- GREEN: the targeted Go command passed six tests across `executor` and `repoclone`. +- Public docs: `node --test scripts/validate-public-docs.test.mjs` passed 58 tests and + `node scripts/validate-public-docs.mjs` validated 41 published pages. +- Review remediation: concurrent origin updates now share the existing per-repository mutex; + `TestSetOriginURLSerializesConcurrentUpdates` fails before the lock and passes after it. diff --git a/docs/public/integrations.md b/docs/public/integrations.md index 3602417192..0b1fe88b57 100644 --- a/docs/public/integrations.md +++ b/docs/public/integrations.md @@ -104,6 +104,10 @@ controls how GitHub HTTPS and `gh` authenticate *inside newly launched task proc - **Inherit executor Git credentials** does not install Kandev's broker helper or `gh` shim. Local and Worktree tasks use credentials already visible to the host Git process (including SSH). Docker, SSH, and cloud tasks use only credentials intentionally configured in that executor. + For Kandev-managed GitHub checkouts, Local and Worktree preparation also updates `origin` to the + host's configured `gh` clone protocol. Selecting SSH therefore lets Git conditional includes that + match `remote.*.url` apply; switching back to managed credentials restores the canonical HTTPS + origin. Repositories you registered from an existing local checkout are never rewritten. Selecting a GitHub CLI workspace connection does not implicitly select host Git credentials; use the explicit inheritance mode when that is the intended boundary. The Changes panel's branch diff --git a/docs/specs/integrations/github-authentication.md b/docs/specs/integrations/github-authentication.md index b100668bed..920c9f3938 100644 --- a/docs/specs/integrations/github-authentication.md +++ b/docs/specs/integrations/github-authentication.md @@ -1,7 +1,7 @@ --- status: draft created: 2026-07-19 -amended: 2026-07-27 +amended: 2026-07-28 owner: Kandev --- @@ -50,6 +50,12 @@ automation under different GitHub Apps without operating separate Kandev deploym workspace automation connection. **Inherit executor Git credentials** injects no GitHub broker helper or `gh` shim: Local and Worktree tasks use host-visible Git/SSH credentials, while remote tasks use credentials configured in that executor. +- For Kandev-managed GitHub checkouts used by Local and Worktree tasks, the selected task policy + also controls the persisted `origin` transport. Managed routing uses canonical GitHub HTTPS. + Executor inheritance uses the host's detected `gh` clone protocol, including SSH, and reconciles + an existing managed checkout when the policy changes. This makes Git conditional includes based + on `remote.*.url` observe the same transport the task uses. Kandev never rewrites the remote of a + repository registered as a user-managed local checkout. - Under managed routing, App installation tokens are minted for the requested repository and cached only in memory. PAT/CLI tokens retain their provider-granted scope once delivered to a trusted agent subprocess. GitHub HTTPS and the broker-aware `gh` shim fail closed rather than consulting @@ -378,6 +384,9 @@ post-signature processing failures produce `failing`; a later valid successful d truncating, partially merging, or executing a different block. - If executor inheritance is selected but no usable credential exists in that executor, Git/SSH reports its normal authentication failure. Kandev does not probe or guess the actor. +- If Kandev cannot reconcile a managed checkout's `origin` with the selected task policy, Local and + Worktree preparation fails before the agent starts instead of silently using the other policy's + transport. - Deleting a registration with any workspace or personal reference returns `github_app_registration_in_use` with a non-secret binding count. - Changing workspace auth while a flow is open makes the stale callback fail without reverting the @@ -451,6 +460,15 @@ registration and never creates a global default. - **GIVEN** a workspace selects executor inheritance, **WHEN** a Local/Worktree or remote task launches, **THEN** Kandev injects no broker helper/shim and the task uses host-visible or executor-configured credentials respectively. +- **GIVEN** the host `gh` clone protocol is SSH and a Kandev-managed GitHub checkout currently has + an HTTPS `origin`, **WHEN** the workspace selects executor inheritance and launches a Local or + Worktree task, **THEN** Kandev changes that managed checkout's `origin` to the canonical SSH URL + before task preparation so matching Git conditional includes apply. +- **GIVEN** a Kandev-managed GitHub checkout currently has an SSH `origin`, **WHEN** the workspace + selects managed credentials and launches a Local or Worktree task, **THEN** Kandev changes that + managed checkout's `origin` to canonical HTTPS before task preparation. +- **GIVEN** a repository is registered from a user-managed local checkout, **WHEN** either task Git + credential policy is selected, **THEN** Kandev leaves its configured `origin` unchanged. - **GIVEN** managed mode and an explicit executor-profile GitHub token, **WHEN** a task launches, **THEN** the profile token wins and the session disclosure labels its actor runtime-selected. - **GIVEN** a managed helper cannot execute or redeem its lease, **WHEN** Git requests GitHub HTTPS @@ -510,7 +528,9 @@ registration and never creates a global default. See [the original authentication implementation plan](../../plans/github-authentication/plan.md) and the -[task Git credential policy follow-up plan](../../plans/task-git-credential-policy/plan.md). +[task Git credential policy follow-up plan](../../plans/task-git-credential-policy/plan.md), plus +the +[executor clone transport repair plan](../../plans/github-executor-clone-transport/plan.md). ## Decision