Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,9 @@ gitcontribute configure --output-format json
Authentication sources are `none`, `env`, `gh-cli`, and `keyring`. Tokens are
resolved at runtime and are never stored in the corpus or logs. Use
`gitcontribute status`, `metadata`, and `doctor` to inspect the local setup.
Use `gitcontribute doctor --strict` in automation when unhealthy required
checks should produce a non-zero exit status. Write contention is reported as
an optional availability warning rather than database corruption.

See [the onboarding design](docs/onboarding.md) for the full contract and
environment-variable reference.
Expand Down
10 changes: 8 additions & 2 deletions docs/onboarding.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ Interactive setup follows an explicit sequence:
4. Produce a dry-run plan. Planning never invokes npm or writes configuration,
corpus, client, or repository-source state.
5. Ask for confirmation, defaulting to apply, then apply the selected effects.
6. Verify the resulting local installation with `doctor`.
6. Verify the applied plan: the corpus is readable and current, its integrity
check passes, Git is available, and selected MCP registrations exactly match
the installed command. Normal contention from another corpus writer does not
make setup fail.

Interactive setup uses inline terminal forms rather than an alternate-screen
application. Active operations may show a spinner that settles into a durable
Expand Down Expand Up @@ -108,7 +111,10 @@ Both uses the verified global CLI executable:

No mode stores `npx`, `@latest`, or an npm-cache executable in coding-agent
configuration. Re-running MCP setup with a newer release installs that release
under its own versioned path and updates the selected registrations.
under its own versioned path and updates the selected registrations. When a
registration changes, setup reports the affected clients in
`restart_clients`; their active sessions must restart to replace older MCP
processes with the configured runtime.
`gitcontribute remove` deletes only selected coding-agent registrations. It
does not delete versioned private runtimes, uninstall the global CLI, or remove
application configuration or corpus data. Use
Expand Down
31 changes: 17 additions & 14 deletions internal/app/control.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
clientsetup "github.com/morluto/gitcontribute/internal/setup"
)

const databaseIntegrityTimeout = 2 * time.Second

// Metadata reports deterministic application and local capability metadata.
// It neither opens the corpus nor performs network access.
func (s *Service) Metadata(ctx context.Context) (*cli.MetadataResult, error) {
Expand Down Expand Up @@ -178,15 +180,11 @@ func (s *Service) ControlStatus(ctx context.Context) (*cli.ControlStatusResult,
// Doctor performs bounded local diagnostics. It reports authentication source
// availability but never returns credential values or command output.
func (s *Service) Doctor(ctx context.Context) (*cli.DoctorResult, error) {
return s.doctor(ctx, true)
return s.doctor(ctx)
}

// doctor optionally verifies credentials. Setup uses the local-only mode so
// its post-apply checks cannot read environment secrets, invoke gh auth token,
// or trigger a keyring access prompt. The explicit doctor command retains the
// credential check.
func (s *Service) doctor(ctx context.Context, verifyCredentials bool) (*cli.DoctorResult, error) {
checks := make([]cli.DoctorCheck, 0, 9)
func (s *Service) doctor(ctx context.Context) (*cli.DoctorResult, error) {
checks := make([]cli.DoctorCheck, 0, 10)
add := func(name string, required bool, err error, success string) {
check := cli.DoctorCheck{Name: name, Required: required, Status: "ok", Message: success}
if err != nil {
Expand All @@ -209,12 +207,17 @@ func (s *Service) doctor(ctx context.Context, verifyCredentials bool) (*cli.Doct
c, dbErr := s.openCorpus(ctx)
add("database", true, dbErr, "corpus is readable")
if dbErr == nil {
_, schemaErr := c.SchemaVersion(ctx)
add("schema", true, schemaErr, "schema is current")
lockCtx, cancel := context.WithTimeout(ctx, 2*time.Second)
lockErr := c.CheckIntegrity(lockCtx)
current, target, schemaErr := c.SchemaVersions(ctx)
if schemaErr == nil && current != target {
schemaErr = fmt.Errorf("database schema version %d does not match expected version %d", current, target)
}
add("schema", true, schemaErr, fmt.Sprintf("schema is current at version %d", current))
integrityCtx, cancel := context.WithTimeout(ctx, databaseIntegrityTimeout)
integrityErr := c.CheckIntegrity(integrityCtx)
cancel()
add("database_lock", true, lockErr, "integrity and write lock checks passed")
add("database_integrity", true, integrityErr, "database quick check passed")
writeErr := c.CheckWriteAccess(ctx)
add("database_write", false, writeErr, "database is ready for writes")
}

gitErr := commandAvailable(ctx, "git", "--version")
Expand All @@ -224,9 +227,9 @@ func (s *Service) doctor(ctx context.Context, verifyCredentials bool) (*cli.Doct
authSuccess := "GitHub authentication source is configured; credentials were not read"
if cfg == nil {
authErr = errors.New("authentication source unavailable because configuration is invalid")
} else if cfg.TokenSource.Method == "none" && verifyCredentials {
} else if cfg.TokenSource.Method == "none" {
authErr = errors.New("no GitHub authentication source configured; public reads remain available")
} else if verifyCredentials {
} else {
authErr = checkAuthSource(ctx, cfg, tokenSource(cfg))
authSuccess = "GitHub authentication source is available"
}
Expand Down
102 changes: 84 additions & 18 deletions internal/app/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"os"
"os/exec"
"runtime"
"strings"

"github.com/morluto/gitcontribute/internal/cli"
Expand Down Expand Up @@ -61,17 +62,18 @@ func (s *Service) setup(ctx context.Context, opts cli.SetupOptions, observer cli
}

type setupRun struct {
service *Service
ctx context.Context
opts cli.SetupOptions
observer cli.SetupObserver
operation clientsetup.Operation
report *cli.SetupReport
clientOptions clientsetup.Options
clientReport clientsetup.Report
managedRuntime string
mcpCommandPending bool
configurationOK bool
service *Service
ctx context.Context
opts cli.SetupOptions
observer cli.SetupObserver
operation clientsetup.Operation
report *cli.SetupReport
clientOptions clientsetup.Options
clientReport clientsetup.Report
managedRuntime string
installedExecutable string
mcpCommandPending bool
configurationOK bool
}

func (s *Service) newSetupRun(ctx context.Context, opts cli.SetupOptions, observer cli.SetupObserver) (*setupRun, error) {
Expand Down Expand Up @@ -173,6 +175,7 @@ func (r *setupRun) setupRuntime() error {
step, executable := installCLI(r.ctx, r.opts.Version, r.opts.DryRun)
r.report.Steps = append(r.report.Steps, step)
setupCompleted(r.observer, step)
r.installedExecutable = executable
if executable == "" {
if !r.opts.DryRun {
r.mcpCommandPending = false
Expand Down Expand Up @@ -203,6 +206,7 @@ func (r *setupRun) installManagedRuntime() error {
return nil
}
setupStarted(r.observer, cli.SetupPhaseMCPRuntime)
r.installedExecutable = r.managedRuntime
source := r.opts.Executable
if source == "" {
var err error
Expand Down Expand Up @@ -333,6 +337,9 @@ func (r *setupRun) appendClientResults() {
for _, result := range r.clientReport.Results {
step := cli.SetupStep{Name: string(result.Client), Path: result.Path, Status: result.Status, Message: result.Error}
r.report.Steps = append(r.report.Steps, step)
if !r.opts.DryRun && r.operation == clientsetup.Configure && (result.Status == "configured" || result.Status == "updated") {
r.report.RestartClients = append(r.report.RestartClients, string(result.Client))
}
setupCompleted(r.observer, step)
}
}
Expand Down Expand Up @@ -362,20 +369,79 @@ func (r *setupRun) verify() {
return
}
setupStarted(r.observer, cli.SetupPhaseVerification)
diagnostics, err := r.service.doctor(r.ctx, false)
step := cli.SetupStep{Name: "verification", Status: "verified"}
if err != nil || diagnostics == nil || !diagnostics.Healthy {
if err := r.verifyAppliedSetup(); err != nil {
step.Status = "failed"
if err != nil {
step.Message = err.Error()
} else {
step.Message = "required installation checks failed"
}
step.Message = err.Error()
}
r.report.Steps = append(r.report.Steps, step)
setupCompleted(r.observer, step)
}

func (r *setupRun) verifyAppliedSetup() error {
failures := make([]string, 0, 5)
if executableErr := verifySetupExecutable(r.installedExecutable); executableErr != nil {
failures = append(failures, "executable: "+executableErr.Error())
}
c, err := r.service.openCorpus(r.ctx)
if err != nil {
failures = append(failures, "database: "+err.Error())
} else {
current, target, schemaErr := c.SchemaVersions(r.ctx)
if schemaErr != nil {
failures = append(failures, "schema: "+schemaErr.Error())
} else if current != target {
failures = append(failures, fmt.Sprintf("schema: database version %d does not match expected version %d", current, target))
}
integrityCtx, cancel := context.WithTimeout(r.ctx, databaseIntegrityTimeout)
integrityErr := c.CheckIntegrity(integrityCtx)
cancel()
if integrityErr != nil {
failures = append(failures, "database_integrity: "+integrityErr.Error())
}
}
if gitErr := commandAvailable(r.ctx, "git", "--version"); gitErr != nil {
failures = append(failures, "git: "+redactDiagnostic(gitErr.Error()))
}
if r.configuresClients() {
opts := r.clientOptions
opts.DryRun = true
report, clientErr := clientsetup.Run(opts)
if clientErr != nil {
failures = append(failures, "mcp registration: "+clientErr.Error())
} else {
for _, result := range report.Results {
if result.Error != "" {
failures = append(failures, string(result.Client)+": "+result.Error)
} else if result.Status != "already configured" {
failures = append(failures, fmt.Sprintf("%s: registration does not match the configured MCP command", result.Client))
}
}
}
}
if len(failures) == 0 {
return nil
}
return errors.New(strings.Join(failures, "; "))
}

func verifySetupExecutable(path string) error {
if strings.TrimSpace(path) == "" {
return errors.New("installed command path is unavailable")
}
info, err := os.Stat(path)
if err != nil {
return fmt.Errorf("inspect installed command: %w", err)
}
if !info.Mode().IsRegular() {
return fmt.Errorf("installed command is not a regular file: %s", path)
}
if runtime.GOOS != "windows" && info.Mode().Perm()&0o111 == 0 {
return fmt.Errorf("installed command is not executable: %s", path)
}
return nil
}

func setupStarted(observer cli.SetupObserver, phase cli.SetupPhase) {
if observer != nil {
observer.SetupStarted(phase)
Expand Down
34 changes: 0 additions & 34 deletions internal/app/setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,40 +415,6 @@ func TestSetupDoesNotInferClientMutationFromDetection(t *testing.T) {
}
}

func TestSetupVerificationDoesNotResolveCredentials(t *testing.T) {
home := t.TempDir()
t.Setenv("GITCONTRIBUTE_TEST_MISSING_TOKEN", "")
paths := config.NewPaths(&config.Env{Home: home, Vars: map[string]string{
"HOME": home, "XDG_CONFIG_HOME": filepath.Join(home, "config"),
"XDG_DATA_HOME": filepath.Join(home, "data"),
}})
svc, err := New(paths, "1.2.3", nil)
if err != nil {
t.Fatal(err)
}
defer svc.Close()

report, err := svc.Setup(context.Background(), cli.SetupOptions{
Mode: cli.SetupModeMCP, Clients: []string{"codex"}, TokenSource: "env", TokenSourceKey: "GITCONTRIBUTE_TEST_MISSING_TOKEN",
Executable: writeTestExecutable(t, filepath.Join(home, "bin")),
})
if err != nil {
t.Fatal(err)
}
if report.Authentication == nil || report.Authentication.Method != "env" || report.Authentication.Key != "GITCONTRIBUTE_TEST_MISSING_TOKEN" {
t.Fatalf("authentication report = %+v", report.Authentication)
}
for _, step := range report.Steps {
if step.Name == "verification" {
if step.Status != "verified" || strings.Contains(step.Message, "optional warning") {
t.Fatalf("verification resolved the missing credential: %+v", step)
}
return
}
}
t.Fatalf("verification step missing: %+v", report)
}

func TestSetupCLIOnlyDryRunNeedsNoDetectedClientOrNPMProcess(t *testing.T) {
home := t.TempDir()
paths := config.NewPaths(&config.Env{Home: home, Vars: map[string]string{
Expand Down
Loading
Loading