Skip to content

Commit 24e976e

Browse files
committed
fix(setup): verify applied installation state
1 parent b377730 commit 24e976e

13 files changed

Lines changed: 421 additions & 71 deletions

README.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,9 @@ gitcontribute configure --output-format json
350350
Authentication sources are `none`, `env`, `gh-cli`, and `keyring`. Tokens are
351351
resolved at runtime and are never stored in the corpus or logs. Use
352352
`gitcontribute status`, `metadata`, and `doctor` to inspect the local setup.
353+
Use `gitcontribute doctor --strict` in automation when unhealthy required
354+
checks should produce a non-zero exit status. Write contention is reported as
355+
an optional availability warning rather than database corruption.
353356

354357
See [the onboarding design](docs/onboarding.md) for the full contract and
355358
environment-variable reference.

docs/onboarding.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,10 @@ Interactive setup follows an explicit sequence:
6363
4. Produce a dry-run plan. Planning never invokes npm or writes configuration,
6464
corpus, client, or repository-source state.
6565
5. Ask for confirmation, defaulting to apply, then apply the selected effects.
66-
6. Verify the resulting local installation with `doctor`.
66+
6. Verify the applied plan: the corpus is readable and current, its integrity
67+
check passes, Git is available, and selected MCP registrations exactly match
68+
the installed command. Normal contention from another corpus writer does not
69+
make setup fail.
6770

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

109112
No mode stores `npx`, `@latest`, or an npm-cache executable in coding-agent
110113
configuration. Re-running MCP setup with a newer release installs that release
111-
under its own versioned path and updates the selected registrations.
114+
under its own versioned path and updates the selected registrations. When a
115+
registration changes, setup reports the affected clients in
116+
`restart_clients`; their active sessions must restart to replace older MCP
117+
processes with the configured runtime.
112118
`gitcontribute remove` deletes only selected coding-agent registrations. It
113119
does not delete versioned private runtimes, uninstall the global CLI, or remove
114120
application configuration or corpus data. Use

internal/app/control.go

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import (
1818
clientsetup "github.com/morluto/gitcontribute/internal/setup"
1919
)
2020

21+
const databaseIntegrityTimeout = 2 * time.Second
22+
2123
// Metadata reports deterministic application and local capability metadata.
2224
// It neither opens the corpus nor performs network access.
2325
func (s *Service) Metadata(ctx context.Context) (*cli.MetadataResult, error) {
@@ -178,15 +180,11 @@ func (s *Service) ControlStatus(ctx context.Context) (*cli.ControlStatusResult,
178180
// Doctor performs bounded local diagnostics. It reports authentication source
179181
// availability but never returns credential values or command output.
180182
func (s *Service) Doctor(ctx context.Context) (*cli.DoctorResult, error) {
181-
return s.doctor(ctx, true)
183+
return s.doctor(ctx)
182184
}
183185

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

220223
gitErr := commandAvailable(ctx, "git", "--version")
@@ -224,9 +227,9 @@ func (s *Service) doctor(ctx context.Context, verifyCredentials bool) (*cli.Doct
224227
authSuccess := "GitHub authentication source is configured; credentials were not read"
225228
if cfg == nil {
226229
authErr = errors.New("authentication source unavailable because configuration is invalid")
227-
} else if cfg.TokenSource.Method == "none" && verifyCredentials {
230+
} else if cfg.TokenSource.Method == "none" {
228231
authErr = errors.New("no GitHub authentication source configured; public reads remain available")
229-
} else if verifyCredentials {
232+
} else {
230233
authErr = checkAuthSource(ctx, cfg, tokenSource(cfg))
231234
authSuccess = "GitHub authentication source is available"
232235
}

internal/app/setup.go

Lines changed: 78 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"fmt"
88
"os"
99
"os/exec"
10+
"runtime"
1011
"strings"
1112

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

6364
type setupRun struct {
64-
service *Service
65-
ctx context.Context
66-
opts cli.SetupOptions
67-
observer cli.SetupObserver
68-
operation clientsetup.Operation
69-
report *cli.SetupReport
70-
clientOptions clientsetup.Options
71-
clientReport clientsetup.Report
72-
managedRuntime string
73-
mcpCommandPending bool
74-
configurationOK bool
65+
service *Service
66+
ctx context.Context
67+
opts cli.SetupOptions
68+
observer cli.SetupObserver
69+
operation clientsetup.Operation
70+
report *cli.SetupReport
71+
clientOptions clientsetup.Options
72+
clientReport clientsetup.Report
73+
managedRuntime string
74+
installedExecutable string
75+
mcpCommandPending bool
76+
configurationOK bool
7577
}
7678

7779
func (s *Service) newSetupRun(ctx context.Context, opts cli.SetupOptions, observer cli.SetupObserver) (*setupRun, error) {
@@ -173,6 +175,7 @@ func (r *setupRun) setupRuntime() error {
173175
step, executable := installCLI(r.ctx, r.opts.Version, r.opts.DryRun)
174176
r.report.Steps = append(r.report.Steps, step)
175177
setupCompleted(r.observer, step)
178+
r.installedExecutable = executable
176179
if executable == "" {
177180
if !r.opts.DryRun {
178181
r.mcpCommandPending = false
@@ -203,6 +206,7 @@ func (r *setupRun) installManagedRuntime() error {
203206
return nil
204207
}
205208
setupStarted(r.observer, cli.SetupPhaseMCPRuntime)
209+
r.installedExecutable = r.managedRuntime
206210
source := r.opts.Executable
207211
if source == "" {
208212
var err error
@@ -333,6 +337,9 @@ func (r *setupRun) appendClientResults() {
333337
for _, result := range r.clientReport.Results {
334338
step := cli.SetupStep{Name: string(result.Client), Path: result.Path, Status: result.Status, Message: result.Error}
335339
r.report.Steps = append(r.report.Steps, step)
340+
if !r.opts.DryRun && r.operation == clientsetup.Configure && (result.Status == "configured" || result.Status == "updated") {
341+
r.report.RestartClients = append(r.report.RestartClients, string(result.Client))
342+
}
336343
setupCompleted(r.observer, step)
337344
}
338345
}
@@ -362,34 +369,77 @@ func (r *setupRun) verify() {
362369
return
363370
}
364371
setupStarted(r.observer, cli.SetupPhaseVerification)
365-
diagnostics, err := r.service.doctor(r.ctx, false)
366372
step := cli.SetupStep{Name: "verification", Status: "verified"}
367-
if err != nil || diagnostics == nil || !diagnostics.Healthy {
373+
if err := r.verifyAppliedSetup(); err != nil {
368374
step.Status = "failed"
369-
if err != nil {
370-
step.Message = err.Error()
371-
} else {
372-
step.Message = setupVerificationFailure(diagnostics)
373-
}
375+
step.Message = err.Error()
374376
}
375377
r.report.Steps = append(r.report.Steps, step)
376378
setupCompleted(r.observer, step)
377379
}
378380

379-
func setupVerificationFailure(diagnostics *cli.DoctorResult) string {
380-
if diagnostics == nil {
381-
return "required installation checks failed"
382-
}
383-
failures := make([]string, 0, len(diagnostics.Checks))
384-
for _, check := range diagnostics.Checks {
385-
if check.Required && check.Status == "error" {
386-
failures = append(failures, check.Name+": "+check.Message)
381+
func (r *setupRun) verifyAppliedSetup() error {
382+
failures := make([]string, 0, 5)
383+
if executableErr := verifySetupExecutable(r.installedExecutable); executableErr != nil {
384+
failures = append(failures, "executable: "+executableErr.Error())
385+
}
386+
c, err := r.service.openCorpus(r.ctx)
387+
if err != nil {
388+
failures = append(failures, "database: "+err.Error())
389+
} else {
390+
current, target, schemaErr := c.SchemaVersions(r.ctx)
391+
if schemaErr != nil {
392+
failures = append(failures, "schema: "+schemaErr.Error())
393+
} else if current != target {
394+
failures = append(failures, fmt.Sprintf("schema: database version %d does not match expected version %d", current, target))
395+
}
396+
integrityCtx, cancel := context.WithTimeout(r.ctx, databaseIntegrityTimeout)
397+
integrityErr := c.CheckIntegrity(integrityCtx)
398+
cancel()
399+
if integrityErr != nil {
400+
failures = append(failures, "database_integrity: "+integrityErr.Error())
401+
}
402+
}
403+
if gitErr := commandAvailable(r.ctx, "git", "--version"); gitErr != nil {
404+
failures = append(failures, "git: "+redactDiagnostic(gitErr.Error()))
405+
}
406+
if r.configuresClients() {
407+
opts := r.clientOptions
408+
opts.DryRun = true
409+
report, clientErr := clientsetup.Run(opts)
410+
if clientErr != nil {
411+
failures = append(failures, "mcp registration: "+clientErr.Error())
412+
} else {
413+
for _, result := range report.Results {
414+
if result.Error != "" {
415+
failures = append(failures, string(result.Client)+": "+result.Error)
416+
} else if result.Status != "already configured" {
417+
failures = append(failures, fmt.Sprintf("%s: registration does not match the configured MCP command", result.Client))
418+
}
419+
}
387420
}
388421
}
389422
if len(failures) == 0 {
390-
return "required installation checks failed"
423+
return nil
424+
}
425+
return errors.New(strings.Join(failures, "; "))
426+
}
427+
428+
func verifySetupExecutable(path string) error {
429+
if strings.TrimSpace(path) == "" {
430+
return errors.New("installed command path is unavailable")
391431
}
392-
return strings.Join(failures, "; ")
432+
info, err := os.Stat(path)
433+
if err != nil {
434+
return fmt.Errorf("inspect installed command: %w", err)
435+
}
436+
if !info.Mode().IsRegular() {
437+
return fmt.Errorf("installed command is not a regular file: %s", path)
438+
}
439+
if runtime.GOOS != "windows" && info.Mode().Perm()&0o111 == 0 {
440+
return fmt.Errorf("installed command is not executable: %s", path)
441+
}
442+
return nil
393443
}
394444

395445
func setupStarted(observer cli.SetupObserver, phase cli.SetupPhase) {

0 commit comments

Comments
 (0)