diff --git a/internal/app/contribution.go b/internal/app/contribution.go index 5d0711f..c353148 100644 --- a/internal/app/contribution.go +++ b/internal/app/contribution.go @@ -199,14 +199,14 @@ func (s *Service) workspaceDiff(ctx context.Context, workspaceID string, inv *in if err != nil { return "", err } - hasUntracked, err := mgr.HasUntrackedByPath(ctx, ws.Path) + hasUntracked, err := mgr.HasUntrackedWorkspace(ctx, ws) if err != nil { return "", err } if hasUntracked { return "", errors.New("workspace has untracked files; stage them or provide an explicit --changes summary") } - return mgr.DiffByPath(ctx, ws.Path, ws.BaseSHA) + return mgr.DiffWorkspace(ctx, ws) } // PrepareReviewReportInput scopes a review report to an opportunity and/or workspace. diff --git a/internal/app/mcp_scalable_operations.go b/internal/app/mcp_scalable_operations.go index 9573c5f..2e88f21 100644 --- a/internal/app/mcp_scalable_operations.go +++ b/internal/app/mcp_scalable_operations.go @@ -408,7 +408,7 @@ func (r *MCPReader) CheckMergeConflicts(ctx context.Context, in mcpserver.CheckM out.Items[index] = item continue } - result, err := manager.CheckMerge(ctx, ws.Path, current.BaseOID, current.HeadOID) + result, err := manager.CheckMergeWorkspace(ctx, ws, current.BaseOID, current.HeadOID) if err != nil { item.Status, item.Reason, item.Message = "failed", "merge_check_failed", err.Error() out.Items[index] = item diff --git a/internal/app/mcp_v1.go b/internal/app/mcp_v1.go index ef9576e..53a40a8 100644 --- a/internal/app/mcp_v1.go +++ b/internal/app/mcp_v1.go @@ -322,6 +322,20 @@ func (r *MCPReader) CreateWorkspace(ctx context.Context, in mcpserver.CreateWork return queuedJobReference(id, "create_workspace", "workspace creation job started"), nil } +// AdoptWorkspace records an existing worktree synchronously without exposing +// its host path or remote URL in the protocol result. +func (r *MCPReader) AdoptWorkspace(ctx context.Context, in mcpserver.AdoptWorkspaceInput) (mcpserver.AdoptWorkspaceOutput, error) { + res, err := r.Service.AdoptWorkspace(ctx, in.InvestigationID, cli.WorkspaceAdoptOptions{Path: in.Path, BaseRef: in.BaseRef, Name: in.Name}) + if err != nil { + return mcpserver.AdoptWorkspaceOutput{}, err + } + return mcpserver.AdoptWorkspaceOutput{ + ID: res.ID, InvestigationID: res.InvestigationID, Owner: res.Repo.Owner, Repo: res.Repo.Repo, + BaseSHA: res.BaseSHA, CandidateSHA: res.CandidateSHA, MergeBase: res.MergeBase, + Dirty: res.Dirty, HasUntracked: res.HasUntracked, Ownership: res.Ownership, + }, nil +} + // RunValidation submits a durable validation run. func (r *MCPReader) RunValidation(ctx context.Context, in mcpserver.RunValidationInput) (mcpserver.JobReference, error) { runKind := evidence.RunKind(in.Kind) diff --git a/internal/app/workspace.go b/internal/app/workspace.go index 4bf980b..b5f08d3 100644 --- a/internal/app/workspace.go +++ b/internal/app/workspace.go @@ -12,6 +12,7 @@ import ( "github.com/google/uuid" "github.com/morluto/gitcontribute/internal/cli" + "github.com/morluto/gitcontribute/internal/gitremote" "github.com/morluto/gitcontribute/internal/workspace" ) @@ -27,6 +28,55 @@ func (s *Service) workspaceManager(ctx context.Context) (*workspace.Manager, err return workspace.NewManager(root, nil) } +// AdoptWorkspace records an existing local worktree without fetching, +// changing its refs, or taking ownership of its files. +func (s *Service) AdoptWorkspace(ctx context.Context, investigationID string, opts cli.WorkspaceAdoptOptions) (*cli.WorkspaceResult, error) { + invSvc, err := s.writeInvestigationSvc(ctx) + if err != nil { + return nil, err + } + inv, err := invSvc.GetInvestigation(ctx, investigationID) + if err != nil { + return nil, mapInvestigationError(err) + } + requestedName := strings.TrimSpace(opts.Name) + name := requestedName + if name == "" { + name = uuid.NewString() + } + mgr, err := s.workspaceManager(ctx) + if err != nil { + return nil, err + } + ws, err := mgr.Adopt(ctx, workspace.AdoptOptions{Path: opts.Path, BaseRef: opts.BaseRef, Name: name}) + if err != nil { + return nil, fmt.Errorf("adopt worktree: %w", err) + } + identity, err := gitremote.ParseRepositoryIdentity(ws.Remote) + if err != nil { + return nil, fmt.Errorf("identify origin repository: %w", err) + } + if !strings.EqualFold(identity.Host, "github.com") || !strings.EqualFold(identity.Owner, inv.Repo.Owner) || !strings.EqualFold(identity.Repo, inv.Repo.Repo) { + return nil, fmt.Errorf("origin repository %s/%s does not match investigation repository %s/%s", identity.Owner, identity.Repo, inv.Repo.Owner, inv.Repo.Repo) + } + ws.InvestigationID, ws.RepoOwner, ws.RepoName = inv.ID, inv.Repo.Owner, inv.Repo.Repo + c, err := s.openCorpus(ctx) + if err != nil { + return nil, err + } + bound, inserted, err := c.BindWorkspacePath(ctx, ws) + if errors.Is(err, workspace.ErrExists) { + return nil, fmt.Errorf("workspace name %q already exists", name) + } + if err != nil { + return nil, err + } + if !inserted && (bound.InvestigationID != inv.ID || (requestedName != "" && bound.Name != name)) { + return nil, fmt.Errorf("worktree path is already bound to workspace %q", bound.Name) + } + return workspaceResult(bound), nil +} + func (s *Service) workspaceReader() (*workspace.Manager, error) { dataDir, err := s.paths.DataDir() if err != nil { @@ -121,11 +171,14 @@ func (s *Service) ShowWorkspace(ctx context.Context, id string) (*cli.WorkspaceR } mgr, err := s.workspaceReader() - if err == nil { - if st, err := mgr.StatusByPath(ctx, ws.Path); err == nil { - ws.Dirty = st.Dirty - } + if err != nil { + return nil, err + } + st, err := mgr.StatusWorkspace(ctx, ws) + if err != nil { + return nil, fmt.Errorf("read workspace status: %w", err) } + ws.Dirty = st.Dirty return workspaceResult(ws), nil } @@ -170,22 +223,22 @@ func (s *Service) WorkspaceDiff(ctx context.Context, id string) (*WorkspaceDiffR return nil, err } - st, err := mgr.StatusByPath(ctx, ws.Path) + st, err := mgr.StatusWorkspace(ctx, ws) if err != nil { return nil, err } - hasUntracked, err := mgr.HasUntrackedByPath(ctx, ws.Path) + hasUntracked, err := mgr.HasUntrackedWorkspace(ctx, ws) if err != nil { return nil, err } - diff, err := mgr.DiffByPath(ctx, ws.Path, ws.BaseSHA) + diff, err := mgr.DiffWorkspace(ctx, ws) if err != nil { return nil, err } - files, err := mgr.ChangedFilesByPath(ctx, ws.Path, ws.BaseSHA) + files, err := mgr.ChangedFilesWorkspace(ctx, ws) if err != nil { return nil, err } @@ -276,6 +329,8 @@ func workspaceResult(ws *workspace.Workspace) *cli.WorkspaceResult { CandidateSHA: ws.CandidateSHA, MergeBase: ws.MergeBase, Dirty: ws.Dirty, + HasUntracked: ws.HasUntracked, + Ownership: string(ws.Ownership), CreatedAt: formatTime(ws.CreatedAt), } } diff --git a/internal/app/workspace_security_test.go b/internal/app/workspace_security_test.go index 6aa88af..c54d816 100644 --- a/internal/app/workspace_security_test.go +++ b/internal/app/workspace_security_test.go @@ -92,3 +92,76 @@ func TestWorkspaceCreateAndShow(t *testing.T) { t.Fatalf("workspace roundtrip failed: %+v", shown) } } + +func TestAdoptWorkspaceIsIdempotentAndUsable(t *testing.T) { + t.Parallel() + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + ctx := context.Background() + remote, baseSHA, candidateSHA := setupAppGitRemote(t) + source := filepath.Join(filepath.Dir(remote), "src") + external := filepath.Join(t.TempDir(), "existing") + runGitApp(t, source, "worktree", "add", "--detach", external, candidateSHA) + runGitApp(t, source, "remote", "set-url", "origin", "https://github.com/owner/repo.git") + writeAppFile(t, filepath.Join(external, "feature.txt"), "changed") + writeAppFile(t, filepath.Join(external, "untracked.txt"), "new") + + paths := config.NewPaths(&config.Env{Home: t.TempDir()}) + svc, err := New(paths, "test", nil) + if err != nil { + t.Fatal(err) + } + defer func() { _ = svc.Close() }() + if _, err := svc.Init(ctx); err != nil { + t.Fatal(err) + } + inv, err := svc.StartInvestigation(ctx, cli.RepoRef{Owner: "owner", Repo: "repo"}, candidateSHA, "") + if err != nil { + t.Fatal(err) + } + first, err := svc.AdoptWorkspace(ctx, inv.ID, cli.WorkspaceAdoptOptions{Path: external, BaseRef: "master"}) + if err != nil { + t.Fatal(err) + } + second, err := svc.AdoptWorkspace(ctx, inv.ID, cli.WorkspaceAdoptOptions{Path: external, BaseRef: "master"}) + if err != nil { + t.Fatal(err) + } + if first.ID != second.ID || first.Ownership != "external" || first.BaseSHA != baseSHA || !first.Dirty || !first.HasUntracked { + t.Fatalf("unexpected adoption results: first=%+v second=%+v", first, second) + } + diff, err := svc.WorkspaceDiff(ctx, first.ID) + if err != nil { + t.Fatal(err) + } + if diff.Diff == "" || !diff.HasUntracked { + t.Fatalf("adopted diff missing state: %+v", diff) + } +} + +func TestAdoptWorkspaceRejectsRepositoryMismatch(t *testing.T) { + t.Parallel() + ctx := context.Background() + remote, _, candidateSHA := setupAppGitRemote(t) + source := filepath.Join(filepath.Dir(remote), "src") + external := filepath.Join(t.TempDir(), "existing") + runGitApp(t, source, "worktree", "add", "--detach", external, candidateSHA) + runGitApp(t, source, "remote", "set-url", "origin", "https://github.com/other/repo.git") + paths := config.NewPaths(&config.Env{Home: t.TempDir()}) + svc, err := New(paths, "test", nil) + if err != nil { + t.Fatal(err) + } + defer func() { _ = svc.Close() }() + if _, err := svc.Init(ctx); err != nil { + t.Fatal(err) + } + inv, err := svc.StartInvestigation(ctx, cli.RepoRef{Owner: "owner", Repo: "repo"}, candidateSHA, "") + if err != nil { + t.Fatal(err) + } + if _, err := svc.AdoptWorkspace(ctx, inv.ID, cli.WorkspaceAdoptOptions{Path: external, BaseRef: "master"}); err == nil || !strings.Contains(err.Error(), "does not match") { + t.Fatalf("AdoptWorkspace mismatch error = %v", err) + } +} diff --git a/internal/cli/cli.go b/internal/cli/cli.go index ec518e4..2bb1ef6 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -892,14 +892,6 @@ func (c *CLI) investigationService() (InvestigationService, error) { return service, nil } -func (c *CLI) workspaceService() (WorkspaceService, error) { - service, ok := c.svc.(WorkspaceService) - if !ok { - return nil, NewCLIError(ExitNotWired, ErrNotWired) - } - return service, nil -} - func (c *CLI) validationService() (ValidationService, error) { service, ok := c.svc.(ValidationService) if !ok { @@ -1392,47 +1384,6 @@ func (c *CLI) runCheck(ctx context.Context, command, kind string, cmd *checkCmd) return c.render(cmd.Check.JSON, result) } -func (c *CLI) runWorkspace(ctx context.Context, command string, cmd *workspaceCmd) error { - service, err := c.workspaceService() - if err != nil { - return err - } - switch command { - case "workspace create": - fmt.Fprintf(c.stderr, "creating workspace for investigation %s...\n", cmd.Create.InvestigationID) - result, err := service.CreateWorkspace(ctx, cmd.Create.InvestigationID, WorkspaceCreateOptions{ - Remote: cmd.Create.Remote, - BaseRef: cmd.Create.Base, - CandidateRef: cmd.Create.Candidate, - Name: cmd.Create.Name, - }) - if err != nil { - return c.mapError(err) - } - return c.render(cmd.Create.JSON, result) - case "workspace show": - result, err := service.ShowWorkspace(ctx, cmd.Show.ID) - if err != nil { - return c.mapError(err) - } - return c.render(cmd.Show.JSON, result) - default: - return NewCLIError(ExitUsage, fmt.Errorf("unknown workspace command: %s", command)) - } -} - -func (c *CLI) runDiff(ctx context.Context, cmd *diffCmd) error { - service, err := c.workflowExtensionService() - if err != nil { - return err - } - result, err := service.WorkspaceDiffForCLI(ctx, cmd.WorkspaceID) - if err != nil { - return c.mapError(err) - } - return c.render(cmd.JSON, result) -} - func formatCommand(args []string) string { quoted := make([]string, len(args)) for i, arg := range args { diff --git a/internal/cli/extended_test.go b/internal/cli/extended_test.go index 778d8cf..03ad917 100644 --- a/internal/cli/extended_test.go +++ b/internal/cli/extended_test.go @@ -2,6 +2,7 @@ package cli_test import ( "context" + "path/filepath" "strings" "testing" "time" @@ -13,6 +14,7 @@ type fakeExtendedService struct { *fakeService createWorkspaceCalled bool + adoptWorkspaceCalled bool showWorkspaceCalled bool defineValidationCalled bool runValidationCalled bool @@ -36,6 +38,7 @@ type fakeExtendedService struct { lastWorkspaceInvestigation string lastCreateWorkspaceOpts cli.WorkspaceCreateOptions + lastAdoptWorkspaceOpts cli.WorkspaceAdoptOptions lastShowWorkspaceID string lastValidationInvestigation string lastDefineValidationOpts cli.DefineValidationOptions @@ -61,6 +64,13 @@ func (f *fakeExtendedService) CreateWorkspace(ctx context.Context, investigation return f.workspaceResult, f.err } +func (f *fakeExtendedService) AdoptWorkspace(_ context.Context, investigationID string, opts cli.WorkspaceAdoptOptions) (*cli.WorkspaceResult, error) { + f.adoptWorkspaceCalled = true + f.lastWorkspaceInvestigation = investigationID + f.lastAdoptWorkspaceOpts = opts + return f.workspaceResult, f.err +} + func (f *fakeExtendedService) ShowWorkspace(ctx context.Context, id string) (*cli.WorkspaceResult, error) { f.showWorkspaceCalled = true f.lastShowWorkspaceID = id @@ -156,6 +166,15 @@ func TestWorkspaceCreateAndShow(t *testing.T) { t.Fatalf("stdout=%q stderr=%q", stdout.String(), stderr.String()) } + stdout.Reset() + err = c.Run(context.Background(), []string{"workspace", "adopt", "inv-1", "--path", "/tmp/existing", "--base", "main", "--name", "external"}) + requireNoErr(t, err) + expectedAdoptPath, err := filepath.Abs("/tmp/existing") + requireNoErr(t, err) + if !svc.adoptWorkspaceCalled || svc.lastAdoptWorkspaceOpts.Path != expectedAdoptPath || svc.lastAdoptWorkspaceOpts.BaseRef != "main" { + t.Fatalf("adopt workspace args = %+v", svc.lastAdoptWorkspaceOpts) + } + stdout.Reset() err = c.Run(context.Background(), []string{"workspace", "show", "ws-1"}) requireNoErr(t, err) diff --git a/internal/cli/interfaces.go b/internal/cli/interfaces.go index c0ce701..6245af3 100644 --- a/internal/cli/interfaces.go +++ b/internal/cli/interfaces.go @@ -415,9 +415,17 @@ type HealthService interface { // WorkspaceService is the optional workspace management capability used by the CLI. type WorkspaceService interface { CreateWorkspace(ctx context.Context, investigationID string, opts WorkspaceCreateOptions) (*WorkspaceResult, error) + AdoptWorkspace(ctx context.Context, investigationID string, opts WorkspaceAdoptOptions) (*WorkspaceResult, error) ShowWorkspace(ctx context.Context, id string) (*WorkspaceResult, error) } +// WorkspaceAdoptOptions identifies an existing local Git worktree. +type WorkspaceAdoptOptions struct { + Path string + BaseRef string + Name string +} + // WorkspaceCreateOptions carries explicit local-write intent for workspace creation. type WorkspaceCreateOptions struct { Remote string @@ -437,6 +445,8 @@ type WorkspaceResult struct { CandidateSHA string `json:"candidate_sha"` MergeBase string `json:"merge_base"` Dirty bool `json:"dirty"` + HasUntracked bool `json:"has_untracked"` + Ownership string `json:"ownership"` CreatedAt string `json:"created_at"` } diff --git a/internal/cli/workspace_cli.go b/internal/cli/workspace_cli.go new file mode 100644 index 0000000..5fa6518 --- /dev/null +++ b/internal/cli/workspace_cli.go @@ -0,0 +1,68 @@ +package cli + +import ( + "context" + "fmt" +) + +func (c *CLI) workspaceService() (WorkspaceService, error) { + service, ok := c.svc.(WorkspaceService) + if !ok { + return nil, NewCLIError(ExitNotWired, ErrNotWired) + } + return service, nil +} + +func (c *CLI) runWorkspace(ctx context.Context, command string, cmd *workspaceCmd) error { + service, err := c.workspaceService() + if err != nil { + return err + } + switch command { + case "workspace create": + if _, err := fmt.Fprintf(c.stderr, "creating workspace for investigation %s...\n", cmd.Create.InvestigationID); err != nil { + return err + } + result, err := service.CreateWorkspace(ctx, cmd.Create.InvestigationID, WorkspaceCreateOptions{ + Remote: cmd.Create.Remote, + BaseRef: cmd.Create.Base, + CandidateRef: cmd.Create.Candidate, + Name: cmd.Create.Name, + }) + if err != nil { + return c.mapError(err) + } + return c.render(cmd.Create.JSON, result) + case "workspace adopt": + if _, err := fmt.Fprintf(c.stderr, "adopting local worktree for investigation %s...\n", cmd.Adopt.InvestigationID); err != nil { + return err + } + result, err := service.AdoptWorkspace(ctx, cmd.Adopt.InvestigationID, WorkspaceAdoptOptions{ + Path: cmd.Adopt.Path, BaseRef: cmd.Adopt.Base, Name: cmd.Adopt.Name, + }) + if err != nil { + return c.mapError(err) + } + return c.render(cmd.Adopt.JSON, result) + case "workspace show": + result, err := service.ShowWorkspace(ctx, cmd.Show.ID) + if err != nil { + return c.mapError(err) + } + return c.render(cmd.Show.JSON, result) + default: + return NewCLIError(ExitUsage, fmt.Errorf("unknown workspace command: %s", command)) + } +} + +func (c *CLI) runDiff(ctx context.Context, cmd *diffCmd) error { + service, err := c.workflowExtensionService() + if err != nil { + return err + } + result, err := service.WorkspaceDiffForCLI(ctx, cmd.WorkspaceID) + if err != nil { + return c.mapError(err) + } + return c.render(cmd.JSON, result) +} diff --git a/internal/cli/workspace_commands.go b/internal/cli/workspace_commands.go index 32d2d94..2a9ba4a 100644 --- a/internal/cli/workspace_commands.go +++ b/internal/cli/workspace_commands.go @@ -2,6 +2,7 @@ package cli type workspaceCmd struct { Create createWorkspaceCmd `cmd:"" help:"Create a workspace for an investigation"` + Adopt adoptWorkspaceCmd `cmd:"" help:"Adopt an existing local Git worktree"` Show showWorkspaceCmd `cmd:"" help:"Show a workspace"` } @@ -19,6 +20,14 @@ type showWorkspaceCmd struct { JSON bool `name:"json" help:"Print the result as JSON"` } +type adoptWorkspaceCmd struct { + InvestigationID string `arg:"" help:"Investigation ID"` + Path string `name:"path" required:"" help:"Existing worktree root" type:"path"` + Base string `name:"base" required:"" help:"Base ref already available in the repository"` + Name string `name:"name" help:"Workspace name (defaults to generated ID)"` + JSON bool `name:"json" help:"Print the result as JSON"` +} + type diffCmd struct { WorkspaceID string `arg:"" help:"Workspace ID"` JSON bool `name:"json" help:"Print the result as JSON"` diff --git a/internal/corpus/workflow.go b/internal/corpus/workflow.go index 8d2fcfd..4026968 100644 --- a/internal/corpus/workflow.go +++ b/internal/corpus/workflow.go @@ -65,7 +65,7 @@ func listWorkflowPayloads[T any](ctx context.Context, db *sql.DB, operation, que return out, nil } -// SaveWorkspace inserts or replaces a managed workspace record. +// SaveWorkspace inserts or replaces a workspace record. func (c *Corpus) SaveWorkspace(ctx context.Context, item *workspace.Workspace) error { if item == nil || item.Name == "" { return errors.New("workspace name is required") @@ -86,7 +86,7 @@ func (c *Corpus) SaveWorkspace(ctx context.Context, item *workspace.Workspace) e return nil } -// GetWorkspace returns a managed workspace by ID, or nil when absent. +// GetWorkspace returns a workspace by ID. func (c *Corpus) GetWorkspace(ctx context.Context, id string) (*workspace.Workspace, error) { var payload string var createdAt int64 @@ -105,6 +105,77 @@ func (c *Corpus) GetWorkspace(ctx context.Context, id string) (*workspace.Worksp return &item, nil } +// BindWorkspacePath atomically returns an existing exact path binding or +// inserts item. The immediate transaction prevents concurrent adopters from +// assigning the same external path to different workspace IDs. +func (c *Corpus) BindWorkspacePath(ctx context.Context, item *workspace.Workspace) (bound *workspace.Workspace, inserted bool, err error) { + if item == nil || item.Name == "" || item.Path == "" { + return nil, false, errors.New("workspace name and path are required") + } + conn, err := c.db.Conn(ctx) + if err != nil { + return nil, false, fmt.Errorf("reserve workspace binding connection: %w", err) + } + defer func() { + if closeErr := conn.Close(); err == nil && closeErr != nil { + err = fmt.Errorf("close workspace binding connection: %w", closeErr) + } + }() + if _, err := conn.ExecContext(ctx, `BEGIN IMMEDIATE`); err != nil { + return nil, false, fmt.Errorf("begin workspace binding: %w", err) + } + committed := false + defer func() { + if !committed { + if _, rollbackErr := conn.ExecContext(context.WithoutCancel(ctx), `ROLLBACK`); err == nil && rollbackErr != nil { + err = fmt.Errorf("rollback workspace binding: %w", rollbackErr) + } + } + }() + + var payload string + var createdAt int64 + err = conn.QueryRowContext(ctx, ` + SELECT payload, created_at FROM workspaces + WHERE json_extract(payload, '$.Path')=? + LIMIT 1 + `, item.Path).Scan(&payload, &createdAt) + if err == nil { + var existing workspace.Workspace + if err := unmarshalWorkflow(payload, &existing); err != nil { + return nil, false, err + } + existing.CreatedAt = scanTime(createdAt) + return &existing, false, nil + } + if !errors.Is(err, sql.ErrNoRows) { + return nil, false, fmt.Errorf("find workspace by path: %w", err) + } + var exists int + if err := conn.QueryRowContext(ctx, `SELECT EXISTS(SELECT 1 FROM workspaces WHERE id=?)`, item.Name).Scan(&exists); err != nil { + return nil, false, fmt.Errorf("check workspace name: %w", err) + } + if exists != 0 { + return nil, false, workspace.ErrExists + } + payload, err = marshalWorkflow(item) + if err != nil { + return nil, false, err + } + if _, err := conn.ExecContext(ctx, ` + INSERT INTO workspaces (id, investigation_id, payload, created_at) + VALUES (?, ?, ?, ?) + `, item.Name, item.InvestigationID, payload, encodeTime(item.CreatedAt)); err != nil { + return nil, false, fmt.Errorf("bind workspace path: %w", err) + } + if _, err := conn.ExecContext(ctx, `COMMIT`); err != nil { + return nil, false, fmt.Errorf("commit workspace binding: %w", err) + } + committed = true + result := *item + return &result, true, nil +} + // SaveInvestigation inserts or updates an investigation record. func (c *Corpus) SaveInvestigation(ctx context.Context, item *investigation.Investigation) error { if item == nil || item.ID == "" { diff --git a/internal/corpus/workflow_test.go b/internal/corpus/workflow_test.go index a630231..3140d1e 100644 --- a/internal/corpus/workflow_test.go +++ b/internal/corpus/workflow_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "path/filepath" + "sync" "testing" "time" @@ -318,3 +319,52 @@ func TestWorkspacePersistsAcrossReopen(t *testing.T) { t.Fatalf("expected ErrNotFound, got %v", err) } } + +func TestBindWorkspacePathSerializesCompetingNames(t *testing.T) { + t.Parallel() + ctx := context.Background() + c, err := Open(ctx, filepath.Join(t.TempDir(), "workspace.db")) + if err != nil { + t.Fatal(err) + } + defer func() { _ = c.Close() }() + + start := make(chan struct{}) + type result struct { + bound *workspace.Workspace + inserted bool + err error + } + results := make(chan result, 2) + var wg sync.WaitGroup + for _, name := range []string{"first", "second"} { + wg.Add(1) + go func() { + defer wg.Done() + <-start + bound, inserted, err := c.BindWorkspacePath(ctx, &workspace.Workspace{Name: name, Path: "/tmp/shared", CreatedAt: time.Now().UTC()}) + results <- result{bound: bound, inserted: inserted, err: err} + }() + } + close(start) + wg.Wait() + close(results) + insertions := 0 + var boundName string + for got := range results { + if got.err != nil { + t.Fatal(got.err) + } + if got.inserted { + insertions++ + } + if boundName == "" { + boundName = got.bound.Name + } else if got.bound.Name != boundName { + t.Fatalf("path bound to different names: %q and %q", boundName, got.bound.Name) + } + } + if insertions != 1 { + t.Fatalf("insertions = %d, want 1", insertions) + } +} diff --git a/internal/gitremote/validate.go b/internal/gitremote/validate.go index ab064a5..09b53d5 100644 --- a/internal/gitremote/validate.go +++ b/internal/gitremote/validate.go @@ -14,6 +14,18 @@ import ( // credentials. var ErrInvalid = errors.New("invalid Git remote") +// ErrRepositoryIdentity indicates that a valid remote does not encode a +// hosted OWNER/REPOSITORY identity. +var ErrRepositoryIdentity = errors.New("git remote has no hosted repository identity") + +// RepositoryIdentity is the credential-free hosted identity encoded by a +// remote URL. +type RepositoryIdentity struct { + Host string + Owner string + Repo string +} + // Validate accepts local absolute paths, file URLs, HTTPS URLs, SSH URLs, and // SCP-like SSH remotes. HTTPS userinfo and SSH passwords are never accepted; // SSH usernames remain supported because they are not secrets. @@ -44,6 +56,43 @@ func Validate(remote string) error { } } +// ParseRepositoryIdentity extracts a hosted repository identity from an HTTPS, +// SSH URL, or SCP-like SSH remote. Local paths and file URLs are valid remotes +// but do not establish a hosted repository identity. +func ParseRepositoryIdentity(remote string) (RepositoryIdentity, error) { + remote = strings.TrimSpace(remote) + if err := Validate(remote); err != nil { + return RepositoryIdentity{}, err + } + if filepath.IsAbs(remote) || path.IsAbs(remote) || strings.HasPrefix(remote, "file://") { + return RepositoryIdentity{}, ErrRepositoryIdentity + } + + var host, repoPath string + if strings.HasPrefix(remote, "https://") || strings.HasPrefix(remote, "ssh://") { + u, err := url.Parse(remote) + if err != nil { + return RepositoryIdentity{}, ErrRepositoryIdentity + } + host, repoPath = u.Hostname(), u.Path + } else { + colon := strings.IndexByte(remote, ':') + host, repoPath = remote[:colon], remote[colon+1:] + if at := strings.LastIndexByte(host, '@'); at >= 0 { + host = host[at+1:] + } + } + parts := strings.Split(strings.Trim(repoPath, "/"), "/") + if host == "" || len(parts) != 2 { + return RepositoryIdentity{}, ErrRepositoryIdentity + } + repo := strings.TrimSuffix(parts[1], ".git") + if parts[0] == "" || repo == "" { + return RepositoryIdentity{}, ErrRepositoryIdentity + } + return RepositoryIdentity{Host: strings.ToLower(host), Owner: parts[0], Repo: repo}, nil +} + func validateFileURL(remote string) error { u, err := url.Parse(remote) if err != nil || u.User != nil { diff --git a/internal/gitremote/validate_test.go b/internal/gitremote/validate_test.go index fed7cce..0cc9115 100644 --- a/internal/gitremote/validate_test.go +++ b/internal/gitremote/validate_test.go @@ -1,6 +1,7 @@ package gitremote import ( + "errors" "strings" "testing" ) @@ -56,3 +57,25 @@ func TestValidate(t *testing.T) { }) } } + +func TestParseRepositoryIdentity(t *testing.T) { + t.Parallel() + for _, remote := range []string{ + "https://github.com/owner/repo.git", + "ssh://git@github.com/owner/repo.git", + "git@github.com:owner/repo.git", + } { + identity, err := ParseRepositoryIdentity(remote) + if err != nil { + t.Fatalf("ParseRepositoryIdentity(%q): %v", remote, err) + } + if identity.Host != "github.com" || identity.Owner != "owner" || identity.Repo != "repo" { + t.Fatalf("ParseRepositoryIdentity(%q) = %+v", remote, identity) + } + } + for _, remote := range []string{"/tmp/repo", "file:///tmp/repo", "https://github.com/owner/group/repo.git"} { + if _, err := ParseRepositoryIdentity(remote); !errors.Is(err, ErrRepositoryIdentity) { + t.Fatalf("ParseRepositoryIdentity(%q) error = %v", remote, err) + } + } +} diff --git a/internal/mcpserver/capabilities_test.go b/internal/mcpserver/capabilities_test.go index 5169b48..1abcfcd 100644 --- a/internal/mcpserver/capabilities_test.go +++ b/internal/mcpserver/capabilities_test.go @@ -88,6 +88,8 @@ type completeTestReader struct { Operator ConcernReader ConcernOperator + WorkspaceCreator + WorkspaceAdopter } func completeFakeReader(base *fakeReader) Reader { @@ -99,5 +101,6 @@ func completeFakeReader(base *fakeReader) Reader { CommitPlannerReader: base, PortfolioOperator: optional, Operator: base, ConcernReader: base, ConcernOperator: base, + WorkspaceCreator: base, WorkspaceAdopter: base, } } diff --git a/internal/mcpserver/catalog.go b/internal/mcpserver/catalog.go index b4175dc..5790a5e 100644 --- a/internal/mcpserver/catalog.go +++ b/internal/mcpserver/catalog.go @@ -44,6 +44,7 @@ const ( ToolPlanSemanticCommits = "workspace.plan_semantic_commits" ToolQueryDeepWiki = "research.query_deepwiki" ToolCreateWorkspace = "workspace.create" + ToolAdoptWorkspace = "workspace.adopt" ToolDefineValidation = "validation.define" ToolRunValidation = "validation.run" ToolRunRepeatedValidation = "validation.run_repeated" @@ -94,6 +95,7 @@ var canonicalToolNames = []string{ ToolPlanSemanticCommits, ToolQueryDeepWiki, ToolCreateWorkspace, + ToolAdoptWorkspace, ToolDefineValidation, ToolRunValidation, ToolRunRepeatedValidation, @@ -189,7 +191,7 @@ var toolsets = map[string][]string{ ToolListConcerns, ToolCreateConcern, }, "code": { - ToolSearchCode, ToolIndexRepositories, ToolCreateWorkspace, ToolCheckMergeConflicts, + ToolSearchCode, ToolIndexRepositories, ToolCreateWorkspace, ToolAdoptWorkspace, ToolCheckMergeConflicts, ToolInspectCommitChanges, ToolPlanSemanticCommits, ToolDefineValidation, ToolRunValidation, ToolRunRepeatedValidation, ToolGetJob, ToolCancelJob, }, diff --git a/internal/mcpserver/catalog_test.go b/internal/mcpserver/catalog_test.go index 5d4fe63..f2abe56 100644 --- a/internal/mcpserver/catalog_test.go +++ b/internal/mcpserver/catalog_test.go @@ -62,7 +62,13 @@ func TestCanonicalToolCatalogIsNamespacedAndUnambiguous(t *testing.T) { defer closeSessions() if len(tools) != len(canonicalToolNames) { - t.Fatalf("listed %d tools, want canonical catalog of %d", len(tools), len(canonicalToolNames)) + var missing []string + for _, name := range canonicalToolNames { + if tools[name] == nil { + missing = append(missing, name) + } + } + t.Fatalf("listed %d tools, want canonical catalog of %d; missing %v", len(tools), len(canonicalToolNames), missing) } titles := make(map[string]string) for _, name := range canonicalToolNames { @@ -250,6 +256,7 @@ func TestToolSchemasExposeMachineReadableContracts(t *testing.T) { assertSchemaValue(t, tools[ToolHydrateThreads].InputSchema, []string{"properties", "max_pages", "default"}, float64(3)) assertSchemaValue(t, tools[ToolRankThreads].InputSchema, []string{"required"}, []any{"repositories"}) assertSchemaValue(t, tools[ToolCreateWorkspace].InputSchema, []string{"required"}, []any{"investigation_id"}) + assertSchemaValue(t, tools[ToolAdoptWorkspace].InputSchema, []string{"required"}, []any{"investigation_id", "path", "base_ref"}) assertSchemaValue(t, tools[ToolRankThreads].OutputSchema, []string{"properties", "total", "type"}, "integer") assertSchemaValue(t, tools[ToolRankThreads].OutputSchema, []string{"properties", "truncated", "type"}, "boolean") assertSchemaValue(t, tools[ToolRunValidation].InputSchema, []string{"properties", "execute", "const"}, true) diff --git a/internal/mcpserver/server.go b/internal/mcpserver/server.go index d2e9f7e..b6e8dd4 100644 --- a/internal/mcpserver/server.go +++ b/internal/mcpserver/server.go @@ -84,6 +84,17 @@ type MergeConflictReader interface { CheckMergeConflicts(context.Context, CheckMergeConflictsInput) (CheckMergeConflictsOutput, error) } +// WorkspaceCreator exposes managed workspace creation separately from the +// broader contribution workflow capability. +type WorkspaceCreator interface { + CreateWorkspace(context.Context, CreateWorkspaceInput) (JobReference, error) +} + +// WorkspaceAdopter exposes non-owning external-worktree registration. +type WorkspaceAdopter interface { + AdoptWorkspace(context.Context, AdoptWorkspaceInput) (AdoptWorkspaceOutput, error) +} + // ResearchReader exposes external derived repository context. type ResearchReader interface { DeepWiki(context.Context, DeepWikiInput) (DeepWikiOutput, error) @@ -97,7 +108,6 @@ type Operator interface { CheckDuplicates(context.Context, CheckDuplicatesInput) (CheckOutput, error) CheckCollisions(context.Context, CheckCollisionsInput) (CheckOutput, error) PromoteOpportunity(context.Context, PromoteOpportunityInput) (OpportunityOutput, error) - CreateWorkspace(context.Context, CreateWorkspaceInput) (JobReference, error) DefineValidation(context.Context, DefineValidationInput) (ValidationOutput, error) RunValidation(context.Context, RunValidationInput) (JobReference, error) RunRepeatedValidation(context.Context, RunRepeatedValidationInput) (JobReference, error) diff --git a/internal/mcpserver/server_test.go b/internal/mcpserver/server_test.go index cf928b6..9942e2d 100644 --- a/internal/mcpserver/server_test.go +++ b/internal/mcpserver/server_test.go @@ -16,6 +16,9 @@ type fakeReader struct { repeatInput RunRepeatedValidationInput } +var _ WorkspaceCreator = (*fakeReader)(nil) +var _ WorkspaceAdopter = (*fakeReader)(nil) + func (f *fakeReader) Search(ctx context.Context, in SearchInput) (SearchOutput, error) { if in.Query == "block" { close(f.searchStarted) @@ -271,6 +274,10 @@ func (*fakeReader) PromoteConcern(_ context.Context, in PromoteConcernInput) (Co return ConcernOutput{ID: in.ID, Status: "promoted", Freshness: "unknown", Promotion: &ConcernPromotionOutput{Kind: in.Kind, InvestigationID: "inv-1", HypothesisID: "hyp-1"}}, nil } +func (*fakeReader) AdoptWorkspace(_ context.Context, in AdoptWorkspaceInput) (AdoptWorkspaceOutput, error) { + return AdoptWorkspaceOutput{ID: in.Name, InvestigationID: in.InvestigationID, Ownership: "external"}, nil +} + func (*fakeReader) DefineValidation(_ context.Context, in DefineValidationInput) (ValidationOutput, error) { return ValidationOutput{ID: "val-1", InvestigationID: in.InvestigationID, Kind: in.Kind, Command: []string{"echo"}}, nil } @@ -707,8 +714,8 @@ func TestV1ParityToolsAndResources(t *testing.T) { for _, name := range []string{ ToolSearchRepositories, ToolSearchThreads, ToolGetRepositoryDossier, ToolExplainMatch, ToolGetJob, ToolGetReadiness, ToolBuildRepositoryDossier, - ToolCreateWorkspace, ToolRunValidation, ToolStartInvestigation, ToolRecordHypothesis, - ToolRunRepeatedValidation, + ToolCreateWorkspace, ToolAdoptWorkspace, ToolRunValidation, ToolRunRepeatedValidation, + ToolStartInvestigation, ToolRecordHypothesis, ToolCheckDuplicates, ToolFindCompetingWork, ToolPromoteOpportunity, ToolDefineValidation, ToolPrepareContribution, ToolCancelJob, } { @@ -744,6 +751,7 @@ func TestV1ParityToolsAndResources(t *testing.T) { }{ {ToolBuildRepositoryDossier, map[string]any{"owner": "acme", "repo": "rocket"}}, {ToolCreateWorkspace, map[string]any{"investigation_id": "inv-1"}}, + {ToolAdoptWorkspace, map[string]any{"investigation_id": "inv-1", "path": "/tmp/worktree", "base_ref": "main", "name": "external"}}, {ToolRunValidation, map[string]any{"id": "val-1", "kind": "base", "execute": true}}, {ToolRunRepeatedValidation, map[string]any{"id": "val-1", "target": "both", "execute": true}}, {ToolStartInvestigation, map[string]any{"owner": "acme", "repo": "rocket", "commit_sha": "abc123"}}, diff --git a/internal/mcpserver/v1.go b/internal/mcpserver/v1.go index 4982682..ec1a55b 100644 --- a/internal/mcpserver/v1.go +++ b/internal/mcpserver/v1.go @@ -133,15 +133,6 @@ type JobReference struct { // BuildRepositoryDossierInput selects a repository for durable dossier generation. type BuildRepositoryDossierInput RepoInput -// CreateWorkspaceInput configures a durable managed-workspace creation job. -type CreateWorkspaceInput struct { - InvestigationID string `json:"investigation_id" jsonschema:"Investigation ID"` - Remote string `json:"remote,omitempty" jsonschema:"Git remote URL to clone; defaults to the investigation repository"` - BaseRef string `json:"base_ref,omitempty" jsonschema:"Base ref to resolve; defaults to the remote HEAD"` - CandidateRef string `json:"candidate_ref,omitempty" jsonschema:"Candidate ref to resolve; defaults to the investigation commit"` - Name string `json:"name,omitempty" jsonschema:"Workspace name; defaults to a generated ID"` -} - // StartInvestigationInput creates a local investigation for a repository revision. type StartInvestigationInput struct { Owner string `json:"owner" jsonschema:"GitHub repository owner"` @@ -273,9 +264,15 @@ func (s *Server) registerV1() { addCatalogTool(s, catalogTool[CreateWorkspaceInput, JobReference]{ name: ToolCreateWorkspace, title: "Create managed Git workspace", description: "Start an asynchronous job that clones the specified remote and creates a managed worktree for an investigation. This performs network reads, Git process execution, filesystem writes, and local metadata writes, but never mutates GitHub.", - annotations: networkReadAnnotations(), supportedBy: supports[Operator], input: inputSchema[CreateWorkspaceInput](noSchemaCustomization), + annotations: networkReadAnnotations(), supportedBy: supports[WorkspaceCreator], input: inputSchema[CreateWorkspaceInput](noSchemaCustomization), output: outputSchema[JobReference]("Reference to a newly queued workspace creation job."), handler: s.createWorkspace, }) + addCatalogTool(s, catalogTool[AdoptWorkspaceInput, AdoptWorkspaceOutput]{ + name: ToolAdoptWorkspace, title: "Adopt existing Git worktree", + description: "Inspect and record an existing local Git worktree for an investigation. This runs read-only Git commands and writes local metadata; it never fetches, changes refs or files, takes deletion ownership, or contacts GitHub.", + annotations: localWriteAnnotations(true), supportedBy: supports[WorkspaceAdopter], input: inputSchema[AdoptWorkspaceInput](noSchemaCustomization), + output: outputSchema[AdoptWorkspaceOutput]("Persisted external-worktree identity without host paths or remote URLs."), handler: s.adoptWorkspace, + }) addCatalogTool(s, catalogTool[RunValidationInput, JobReference]{ name: ToolRunValidation, title: "Run stored validation command", description: "Execute one stored shell-free validation command against its base or candidate workspace and persist the run asynchronously. This can modify the workspace or host through the authorized command and requires execute=true.", @@ -476,22 +473,6 @@ func (s *Server) buildRepositoryDossier(ctx context.Context, _ *mcp.CallToolRequ return nil, out, err } -func (s *Server) createWorkspace(ctx context.Context, _ *mcp.CallToolRequest, in CreateWorkspaceInput) (*mcp.CallToolResult, JobReference, error) { - if _, err := normalizeID("investigation_id", in.InvestigationID); err != nil { - return nil, JobReference{}, err - } - in.Remote = strings.TrimSpace(in.Remote) - in.BaseRef = strings.TrimSpace(in.BaseRef) - in.CandidateRef = strings.TrimSpace(in.CandidateRef) - in.Name = strings.TrimSpace(in.Name) - operator, ok := s.reader.(Operator) - if !ok { - return nil, JobReference{}, errors.New("workspace creation is not available") - } - out, err := operator.CreateWorkspace(ctx, in) - return nil, out, err -} - func (s *Server) startInvestigation(ctx context.Context, _ *mcp.CallToolRequest, in StartInvestigationInput) (*mcp.CallToolResult, InvestigationOutput, error) { if err := validateRepo(RepoInput{Owner: in.Owner, Repo: in.Repo}); err != nil { return nil, InvestigationOutput{}, err diff --git a/internal/mcpserver/workspace_v1.go b/internal/mcpserver/workspace_v1.go new file mode 100644 index 0000000..a49db67 --- /dev/null +++ b/internal/mcpserver/workspace_v1.go @@ -0,0 +1,74 @@ +package mcpserver + +import ( + "context" + "errors" + "strings" + + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// CreateWorkspaceInput configures a durable managed-workspace creation job. +type CreateWorkspaceInput struct { + InvestigationID string `json:"investigation_id" jsonschema:"Investigation ID"` + Remote string `json:"remote,omitempty" jsonschema:"Git remote URL to clone; defaults to the investigation repository"` + BaseRef string `json:"base_ref,omitempty" jsonschema:"Base ref to resolve; defaults to the remote HEAD"` + CandidateRef string `json:"candidate_ref,omitempty" jsonschema:"Candidate ref to resolve; defaults to the investigation commit"` + Name string `json:"name,omitempty" jsonschema:"Workspace name; defaults to a generated ID"` +} + +// AdoptWorkspaceInput identifies an existing local worktree and an already +// available base revision. Adoption never fetches or changes the worktree. +type AdoptWorkspaceInput struct { + InvestigationID string `json:"investigation_id" jsonschema:"Investigation ID"` + Path string `json:"path" jsonschema:"Existing local worktree root"` + BaseRef string `json:"base_ref" jsonschema:"Base ref already available in the repository"` + Name string `json:"name,omitempty" jsonschema:"Workspace name; defaults to a generated ID"` +} + +// AdoptWorkspaceOutput deliberately omits host paths and remote URLs. +type AdoptWorkspaceOutput struct { + ID string `json:"id" jsonschema:"Workspace ID"` + InvestigationID string `json:"investigation_id" jsonschema:"Investigation ID"` + Owner string `json:"owner" jsonschema:"Repository owner"` + Repo string `json:"repo" jsonschema:"Repository name"` + BaseSHA string `json:"base_sha" jsonschema:"Resolved base commit"` + CandidateSHA string `json:"candidate_sha" jsonschema:"Worktree HEAD observed during adoption"` + MergeBase string `json:"merge_base" jsonschema:"Merge base of base and candidate commits"` + Dirty bool `json:"dirty" jsonschema:"Whether tracked or untracked changes were observed"` + HasUntracked bool `json:"has_untracked" jsonschema:"Whether untracked non-ignored files were observed"` + Ownership string `json:"ownership" jsonschema:"Workspace ownership classification"` +} + +func (s *Server) adoptWorkspace(ctx context.Context, _ *mcp.CallToolRequest, in AdoptWorkspaceInput) (*mcp.CallToolResult, AdoptWorkspaceOutput, error) { + var err error + if in.InvestigationID, err = normalizeID("investigation_id", in.InvestigationID); err != nil { + return nil, AdoptWorkspaceOutput{}, err + } + in.Path, in.BaseRef, in.Name = strings.TrimSpace(in.Path), strings.TrimSpace(in.BaseRef), strings.TrimSpace(in.Name) + if in.Path == "" || in.BaseRef == "" { + return nil, AdoptWorkspaceOutput{}, errors.New("path and base_ref are required") + } + operator, ok := s.reader.(WorkspaceAdopter) + if !ok { + return nil, AdoptWorkspaceOutput{}, errors.New("workspace adoption is not available") + } + out, err := operator.AdoptWorkspace(ctx, in) + return nil, out, err +} + +func (s *Server) createWorkspace(ctx context.Context, _ *mcp.CallToolRequest, in CreateWorkspaceInput) (*mcp.CallToolResult, JobReference, error) { + if _, err := normalizeID("investigation_id", in.InvestigationID); err != nil { + return nil, JobReference{}, err + } + in.Remote = strings.TrimSpace(in.Remote) + in.BaseRef = strings.TrimSpace(in.BaseRef) + in.CandidateRef = strings.TrimSpace(in.CandidateRef) + in.Name = strings.TrimSpace(in.Name) + operator, ok := s.reader.(WorkspaceCreator) + if !ok { + return nil, JobReference{}, errors.New("workspace creation is not available") + } + out, err := operator.CreateWorkspace(ctx, in) + return nil, out, err +} diff --git a/internal/workspace/adopt.go b/internal/workspace/adopt.go new file mode 100644 index 0000000..db94575 --- /dev/null +++ b/internal/workspace/adopt.go @@ -0,0 +1,244 @@ +package workspace + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "time" +) + +// Adopt inspects an existing worktree without fetching, changing refs, or +// taking ownership of the path. The returned administrative identities are +// revalidated by later workspace operations. +func (m *Manager) Adopt(ctx context.Context, opts AdoptOptions) (*Workspace, error) { + if err := validateName(opts.Name); err != nil { + return nil, err + } + if strings.TrimSpace(opts.BaseRef) == "" { + return nil, errors.New("base ref is required") + } + canonical, err := canonicalDirectory(opts.Path) + if err != nil { + return nil, err + } + record, err := m.findWorktreeRecord(ctx, canonical) + if err != nil { + return nil, err + } + inside, err := m.git(ctx, canonical, "rev-parse", "--is-inside-work-tree") + if err != nil || strings.TrimSpace(inside) != "true" { + return nil, errors.New("path is not a Git worktree") + } + bare, err := m.git(ctx, canonical, "rev-parse", "--is-bare-repository") + if err != nil || strings.TrimSpace(bare) != "false" { + return nil, errors.New("bare repositories cannot be adopted") + } + topLevel, err := m.git(ctx, canonical, "rev-parse", "--path-format=absolute", "--show-toplevel") + if err != nil { + return nil, fmt.Errorf("resolve worktree root: %w", err) + } + if cleanGitPath(topLevel) != canonical { + return nil, errors.New("path must be the worktree root") + } + gitDir, err := m.gitPath(ctx, canonical, "--absolute-git-dir") + if err != nil { + return nil, err + } + commonDir, err := m.gitPath(ctx, canonical, "--git-common-dir") + if err != nil { + return nil, err + } + candidateSHA, err := m.resolvePathRef(ctx, canonical, "HEAD") + if err != nil { + return nil, fmt.Errorf("resolve candidate: %w", err) + } + if record.head != candidateSHA { + return nil, fmt.Errorf("%w: worktree HEAD changed during adoption", ErrPathChanged) + } + baseSHA, err := m.resolvePathRef(ctx, canonical, opts.BaseRef) + if err != nil { + return nil, fmt.Errorf("resolve base: %w", err) + } + mergeBase, err := m.git(ctx, canonical, "merge-base", baseSHA, candidateSHA) + if err != nil { + return nil, fmt.Errorf("merge-base: %w", err) + } + remote, err := m.validateRemotes(ctx, canonical) + if err != nil { + return nil, err + } + status, err := m.status(ctx, canonical) + if err != nil { + return nil, err + } + hasUntracked, err := m.hasUntracked(ctx, canonical) + if err != nil { + return nil, err + } + return &Workspace{ + Name: opts.Name, Path: canonical, Remote: remote, + BaseSHA: baseSHA, CandidateSHA: candidateSHA, MergeBase: strings.TrimSpace(mergeBase), + Dirty: status.Dirty, HasUntracked: hasUntracked, Ownership: OwnershipExternal, + GitDir: gitDir, GitCommonDir: commonDir, CreatedAt: time.Now().UTC(), + }, nil +} + +type worktreeRecord struct { + path string + head string +} + +func (m *Manager) findWorktreeRecord(ctx context.Context, canonical string) (worktreeRecord, error) { + out, err := m.git(ctx, canonical, "worktree", "list", "--porcelain", "-z") + if err != nil { + return worktreeRecord{}, fmt.Errorf("list Git worktrees: %w", err) + } + var current worktreeRecord + for _, field := range strings.Split(out, "\x00") { + switch { + case field == "": + if current.path == canonical && current.head != "" { + return current, nil + } + current = worktreeRecord{} + case strings.HasPrefix(field, "worktree "): + path, err := canonicalDirectory(strings.TrimPrefix(field, "worktree ")) + if err == nil { + current.path = path + } + case strings.HasPrefix(field, "HEAD "): + current.head = strings.TrimPrefix(field, "HEAD ") + case field == "bare" || strings.HasPrefix(field, "prunable"): + current = worktreeRecord{} + } + } + return worktreeRecord{}, errors.New("path is not a registered Git worktree") +} + +func canonicalDirectory(path string) (string, error) { + if strings.TrimSpace(path) == "" { + return "", errors.New("workspace path is required") + } + abs, err := filepath.Abs(path) + if err != nil { + return "", fmt.Errorf("resolve workspace path: %w", err) + } + resolved, err := filepath.EvalSymlinks(filepath.Clean(abs)) + if err != nil { + return "", fmt.Errorf("resolve workspace path symlinks: %w", err) + } + info, err := os.Stat(resolved) + if err != nil { + return "", fmt.Errorf("stat workspace path: %w", err) + } + if !info.IsDir() { + return "", errors.New("workspace path is not a directory") + } + return resolved, nil +} + +func cleanGitPath(output string) string { + return filepath.Clean(strings.TrimSuffix(strings.TrimSuffix(output, "\n"), "\r")) +} + +func (m *Manager) gitPath(ctx context.Context, worktree, arg string) (string, error) { + out, err := m.git(ctx, worktree, "rev-parse", "--path-format=absolute", arg) + if err != nil { + return "", fmt.Errorf("resolve Git administrative path: %w", err) + } + path := cleanGitPath(out) + if !filepath.IsAbs(path) { + path = filepath.Join(worktree, path) + } + resolved, err := filepath.EvalSymlinks(path) + if err != nil { + return "", fmt.Errorf("resolve Git administrative path symlinks: %w", err) + } + return filepath.Clean(resolved), nil +} + +func (m *Manager) resolvePathRef(ctx context.Context, path, ref string) (string, error) { + out, err := m.git(ctx, path, "rev-parse", "--verify", "--end-of-options", strings.TrimSpace(ref)+"^{commit}") + if err != nil { + return "", ErrNotFound + } + return strings.TrimSpace(out), nil +} + +func (m *Manager) validateRemotes(ctx context.Context, path string) (string, error) { + out, err := m.git(ctx, path, "remote") + if err != nil { + return "", fmt.Errorf("list remotes: %w", err) + } + var origin string + for _, name := range strings.Fields(out) { + urls, err := m.git(ctx, path, "remote", "get-url", "--all", name) + if err != nil { + return "", fmt.Errorf("read remote %q: %w", name, err) + } + for _, remote := range strings.Split(strings.TrimSpace(urls), "\n") { + if err := validateRemote(remote); err != nil { + return "", fmt.Errorf("%w: remote %q contains an unsafe URL", ErrInvalidRemote, name) + } + if name == "origin" && origin == "" { + origin = remote + } + } + } + if origin == "" { + return "", errors.New("origin remote is required for adoption") + } + return origin, nil +} + +// ValidateWorkspace revalidates the authority recorded for a persisted +// workspace before it is used. +func (m *Manager) ValidateWorkspace(ctx context.Context, ws *Workspace) error { + _, err := m.authorizedPath(ctx, ws) + return err +} + +func (m *Manager) authorizedPath(ctx context.Context, ws *Workspace) (string, error) { + if ws == nil { + return "", ErrNotFound + } + if ws.Ownership == "" || ws.Ownership == OwnershipManaged { + return m.managedPath(ws.Path) + } + if ws.Ownership != OwnershipExternal { + return "", fmt.Errorf("%w: unknown ownership %q", ErrNotManaged, ws.Ownership) + } + canonical, err := canonicalDirectory(ws.Path) + if err != nil { + return "", fmt.Errorf("%w: %w", ErrPathChanged, err) + } + if canonical != ws.Path { + return "", fmt.Errorf("%w: workspace root", ErrPathChanged) + } + record, err := m.findWorktreeRecord(ctx, canonical) + if err != nil { + return "", fmt.Errorf("%w: %w", ErrPathChanged, err) + } + gitDir, err := m.gitPath(ctx, canonical, "--absolute-git-dir") + if err != nil || gitDir != ws.GitDir { + return "", fmt.Errorf("%w: Git directory", ErrPathChanged) + } + commonDir, err := m.gitPath(ctx, canonical, "--git-common-dir") + if err != nil || commonDir != ws.GitCommonDir { + return "", fmt.Errorf("%w: Git common directory", ErrPathChanged) + } + if record.path != canonical { + return "", fmt.Errorf("%w: worktree registration", ErrPathChanged) + } + remote, err := m.validateRemotes(ctx, canonical) + if err != nil { + return "", err + } + if remote != ws.Remote { + return "", fmt.Errorf("%w: origin remote", ErrPathChanged) + } + return canonical, nil +} diff --git a/internal/workspace/adopt_test.go b/internal/workspace/adopt_test.go new file mode 100644 index 0000000..648f7c1 --- /dev/null +++ b/internal/workspace/adopt_test.go @@ -0,0 +1,83 @@ +package workspace + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestManagerAdoptExternalWorktreeWithoutMutation(t *testing.T) { + t.Parallel() + ctx := context.Background() + remote, baseSHA, candidateSHA := setupRemote(t) + source := filepath.Join(filepath.Dir(remote), "src") + external := filepath.Join(t.TempDir(), "existing worktree") + runGit(t, source, "worktree", "add", "--detach", external, candidateSHA) + runGit(t, source, "remote", "set-url", "origin", "https://github.com/owner/repo.git") + writeFile(t, filepath.Join(external, "feature.txt"), "modified") + writeFile(t, filepath.Join(external, "untracked.txt"), "new") + + beforeHead := strings.TrimSpace(runGit(t, external, "rev-parse", "HEAD")) + beforeRefs := runGit(t, external, "show-ref") + beforeStatus := runGit(t, external, "status", "--porcelain=v1", "-z") + + mgr := newManager(t) + ws, err := mgr.Adopt(ctx, AdoptOptions{Path: external, BaseRef: "master", Name: "external"}) + if err != nil { + t.Fatal(err) + } + if ws.Ownership != OwnershipExternal || ws.BaseSHA != baseSHA || ws.CandidateSHA != candidateSHA || !ws.Dirty || !ws.HasUntracked { + t.Fatalf("unexpected adopted workspace: %+v", ws) + } + if err := mgr.ValidateWorkspace(ctx, ws); err != nil { + t.Fatalf("ValidateWorkspace: %v", err) + } + if _, err := mgr.DiffWorkspace(ctx, ws); err != nil { + t.Fatalf("DiffWorkspace: %v", err) + } + if err := mgr.Remove(ctx, ws.Path, true); !errors.Is(err, ErrNotManaged) { + t.Fatalf("Remove external workspace = %v, want ErrNotManaged", err) + } + if got := strings.TrimSpace(runGit(t, external, "rev-parse", "HEAD")); got != beforeHead { + t.Fatalf("HEAD changed: got %s want %s", got, beforeHead) + } + if got := runGit(t, external, "show-ref"); got != beforeRefs { + t.Fatal("adoption changed refs") + } + if got := runGit(t, external, "status", "--porcelain=v1", "-z"); got != beforeStatus { + t.Fatal("adoption changed worktree state") + } +} + +func TestManagerExternalWorkspaceRejectsChangedIdentity(t *testing.T) { + t.Parallel() + ctx := context.Background() + remote, _, candidateSHA := setupRemote(t) + source := filepath.Join(filepath.Dir(remote), "src") + external := filepath.Join(t.TempDir(), "worktree") + runGit(t, source, "worktree", "add", "--detach", external, candidateSHA) + runGit(t, source, "remote", "set-url", "origin", "https://github.com/owner/repo.git") + mgr := newManager(t) + ws, err := mgr.Adopt(ctx, AdoptOptions{Path: external, BaseRef: "master", Name: "external"}) + if err != nil { + t.Fatal(err) + } + runGit(t, source, "remote", "set-url", "origin", "https://github.com/other/repo.git") + if _, err := mgr.StatusWorkspace(ctx, ws); !errors.Is(err, ErrPathChanged) { + t.Fatalf("StatusWorkspace changed origin error = %v, want ErrPathChanged", err) + } + runGit(t, source, "remote", "set-url", "origin", ws.Remote) + moved := external + "-moved" + if err := os.Rename(external, moved); err != nil { + t.Fatal(err) + } + if err := os.Symlink(moved, external); err != nil { + t.Fatal(err) + } + if _, err := mgr.StatusWorkspace(ctx, ws); !errors.Is(err, ErrPathChanged) { + t.Fatalf("StatusWorkspace replaced path error = %v, want ErrPathChanged", err) + } +} diff --git a/internal/workspace/merge.go b/internal/workspace/merge.go new file mode 100644 index 0000000..7c12326 --- /dev/null +++ b/internal/workspace/merge.go @@ -0,0 +1,51 @@ +package workspace + +import ( + "context" + "errors" + "strings" +) + +// MergeCheck is a non-mutating comparison of two already-fetched revisions. +type MergeCheck struct { + MergeBase string + Conflicted bool + Summary string +} + +// CheckMerge compares already-fetched revisions without fetching or changing +// refs, the index, or a worktree. +func (m *Manager) CheckMerge(ctx context.Context, path, baseOID, headOID string) (MergeCheck, error) { + return m.checkMerge(ctx, path, baseOID, headOID) +} + +// CheckMergeWorkspace revalidates workspace authority before comparing refs. +func (m *Manager) CheckMergeWorkspace(ctx context.Context, ws *Workspace, baseOID, headOID string) (MergeCheck, error) { + path, err := m.authorizedPath(ctx, ws) + if err != nil { + return MergeCheck{}, err + } + return m.checkMerge(ctx, path, baseOID, headOID) +} + +func (m *Manager) checkMerge(ctx context.Context, path, baseOID, headOID string) (MergeCheck, error) { + baseOID, headOID = strings.TrimSpace(baseOID), strings.TrimSpace(headOID) + if baseOID == "" || headOID == "" { + return MergeCheck{}, errors.New("base and head OIDs are required") + } + mergeBase, err := m.git(ctx, path, "merge-base", baseOID, headOID) + if err != nil { + return MergeCheck{}, err + } + mergeBase = strings.TrimSpace(mergeBase) + out, err := m.git(ctx, path, "merge-tree", mergeBase, baseOID, headOID) + if err != nil { + return MergeCheck{}, err + } + conflicted := strings.Contains(out, "changed in both") || strings.Contains(out, "<<<<<<<") || strings.Contains(out, "CONFLICT") + summary := "revisions merge cleanly" + if conflicted { + summary = "revisions have merge conflicts" + } + return MergeCheck{MergeBase: mergeBase, Conflicted: conflicted, Summary: summary}, nil +} diff --git a/internal/workspace/workspace.go b/internal/workspace/workspace.go index a4be5db..92171e6 100644 --- a/internal/workspace/workspace.go +++ b/internal/workspace/workspace.go @@ -28,6 +28,7 @@ var ( ErrInvalidName = errors.New("invalid name") ErrInvalidRemote = errors.New("invalid remote") ErrRemoteMismatch = errors.New("existing mirror remote does not match requested remote") + ErrPathChanged = errors.New("workspace path identity changed") ) const ( @@ -74,7 +75,18 @@ func (execRunner) Run(ctx context.Context, name string, args ...string) (string, // DefaultRunner returns a Runner backed by the local git executable. func DefaultRunner() Runner { return execRunner{} } -// Workspace is a product-owned record for a managed Git worktree. +// Ownership distinguishes worktrees created by GitContribute from external +// worktrees that it may inspect but never remove. +type Ownership string + +const ( + // OwnershipManaged marks a worktree created and removable by GitContribute. + OwnershipManaged Ownership = "managed" + // OwnershipExternal marks a worktree that GitContribute may inspect but never remove. + OwnershipExternal Ownership = "external" +) + +// Workspace is a product-owned record for a Git worktree. type Workspace struct { Name string InvestigationID string @@ -86,47 +98,28 @@ type Workspace struct { CandidateSHA string MergeBase string Dirty bool + HasUntracked bool + Ownership Ownership + GitDir string + GitCommonDir string CreatedAt time.Time mirror string } +// AdoptOptions identifies an existing worktree without granting ownership of +// its files or refs. +type AdoptOptions struct { + Path string + BaseRef string + Name string +} + // Status reports the dirty state of a workspace. type Status struct { Dirty bool } -// MergeCheck is a non-mutating comparison of two already-fetched revisions. -type MergeCheck struct { - MergeBase string - Conflicted bool - Summary string -} - -// CheckMerge compares already-fetched revisions without fetching or changing -// refs, the index, or a worktree. -func (m *Manager) CheckMerge(ctx context.Context, path, baseOID, headOID string) (MergeCheck, error) { - baseOID, headOID = strings.TrimSpace(baseOID), strings.TrimSpace(headOID) - if baseOID == "" || headOID == "" { - return MergeCheck{}, errors.New("base and head OIDs are required") - } - mergeBase, err := m.git(ctx, path, "merge-base", baseOID, headOID) - if err != nil { - return MergeCheck{}, err - } - mergeBase = strings.TrimSpace(mergeBase) - out, err := m.git(ctx, path, "merge-tree", mergeBase, baseOID, headOID) - if err != nil { - return MergeCheck{}, err - } - conflicted := strings.Contains(out, "changed in both") || strings.Contains(out, "<<<<<<<") || strings.Contains(out, "CONFLICT") - summary := "revisions merge cleanly" - if conflicted { - summary = "revisions have merge conflicts" - } - return MergeCheck{MergeBase: mergeBase, Conflicted: conflicted, Summary: summary}, nil -} - type mirror struct { name string remote string @@ -386,6 +379,7 @@ func (m *Manager) Create(ctx context.Context, mirrorName, baseRef, candidateRef, CandidateSHA: candidateSHA, MergeBase: mergeBase, Dirty: st.Dirty, + Ownership: OwnershipManaged, CreatedAt: time.Now().UTC(), mirror: mi.name, } @@ -576,6 +570,15 @@ func (m *Manager) StatusByPath(ctx context.Context, path string) (Status, error) return m.status(ctx, managed) } +// StatusWorkspace revalidates workspace authority before reading status. +func (m *Manager) StatusWorkspace(ctx context.Context, ws *Workspace) (Status, error) { + path, err := m.authorizedPath(ctx, ws) + if err != nil { + return Status{}, err + } + return m.status(ctx, path) +} + // DiffByPath returns the diff for a workspace path against the supplied base // SHA, including staged and unstaged changes. func (m *Manager) DiffByPath(ctx context.Context, path, baseSHA string) (string, error) { @@ -583,12 +586,25 @@ func (m *Manager) DiffByPath(ctx context.Context, path, baseSHA string) (string, if err != nil { return "", err } + return m.diffByPath(ctx, managed, baseSHA) +} + +// DiffWorkspace revalidates workspace authority before reading its diff. +func (m *Manager) DiffWorkspace(ctx context.Context, ws *Workspace) (string, error) { + path, err := m.authorizedPath(ctx, ws) + if err != nil { + return "", err + } + return m.diffByPath(ctx, path, ws.BaseSHA) +} + +func (m *Manager) diffByPath(ctx context.Context, path, baseSHA string) (string, error) { args := []string{"diff", "--no-ext-diff", "--no-textconv", "--binary", "--full-index", "--find-renames"} if baseSHA != "" { args = append(args, baseSHA) } args = append(args, "--") - out, err := m.git(ctx, managed, args...) + out, err := m.git(ctx, path, args...) if err != nil { return "", fmt.Errorf("diff: %w", err) } @@ -639,12 +655,25 @@ func (m *Manager) ChangedFilesByPath(ctx context.Context, path, baseSHA string) if err != nil { return nil, err } + return m.changedFilesByPath(ctx, managed, baseSHA) +} + +// ChangedFilesWorkspace revalidates workspace authority before listing files. +func (m *Manager) ChangedFilesWorkspace(ctx context.Context, ws *Workspace) ([]string, error) { + path, err := m.authorizedPath(ctx, ws) + if err != nil { + return nil, err + } + return m.changedFilesByPath(ctx, path, ws.BaseSHA) +} + +func (m *Manager) changedFilesByPath(ctx context.Context, path, baseSHA string) ([]string, error) { args := []string{"diff", "--name-only", "--find-renames", "-z"} if baseSHA != "" { args = append(args, baseSHA) } args = append(args, "--") - out, err := m.git(ctx, managed, args...) + out, err := m.git(ctx, path, args...) if err != nil { return nil, fmt.Errorf("list changed files: %w", err) } @@ -666,7 +695,20 @@ func (m *Manager) HasUntrackedByPath(ctx context.Context, path string) (bool, er if err != nil { return false, err } - out, err := m.git(ctx, managed, "ls-files", "--others", "--exclude-standard", "-z") + return m.hasUntracked(ctx, managed) +} + +// HasUntrackedWorkspace revalidates workspace authority before reading files. +func (m *Manager) HasUntrackedWorkspace(ctx context.Context, ws *Workspace) (bool, error) { + path, err := m.authorizedPath(ctx, ws) + if err != nil { + return false, err + } + return m.hasUntracked(ctx, path) +} + +func (m *Manager) hasUntracked(ctx context.Context, path string) (bool, error) { + out, err := m.git(ctx, path, "ls-files", "--others", "--exclude-standard", "-z") if err != nil { return false, fmt.Errorf("list untracked files: %w", err) }