Skip to content

refactor: separate scope/project commands, eliminate shared globals#50

Merged
ewega merged 1 commit into
mainfrom
refactor/scope-project-separation
Feb 19, 2026
Merged

refactor: separate scope/project commands, eliminate shared globals#50
ewega merged 1 commit into
mainfrom
refactor/scope-project-separation

Conversation

@ewega

@ewega ewega commented Feb 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Architectural refactor that decouples scope and project commands, eliminates ~18 shared mutable package-level variables, and fixes orchestrator commands.

This is the core of v0.3.7 -- addresses the root cause of the scope/project entanglement.

Key Changes

Scope command -- scope-only (#35, #39)

  • Accepts ScopeOpts struct instead of reading package globals
  • Only PUTs scopes to connections -- no longer creates projects
  • Removed project-related flags (--project-name, --cron, --time-after, etc.)
  • Prints hint to run configure project after scoping

Project command -- project-only (#37, #41)

  • Accepts ProjectOpts struct instead of reading package globals
  • Uses new ListScopes API to discover existing scopes on connections
  • No longer PUTs scopes -- references existing ones in blueprints
  • Removed fall-through to runConfigureScopes when connection IDs passed as flags

Full command -- clean phase chaining (#42)

  • 3-phase flow: connections -> scopes -> project
  • Passes DevLake client/state from Phase 1 to Phase 3 (no redundant discovery)
  • Each phase uses its own opts struct, no shared mutable state

Init wizard -- fixed delegation (#40)

  • 4-phase flow: deploy -> connections -> scopes -> project
  • Phase 4 calls runConfigureProjects (not runConfigureScopes)
  • Passes connection results via opts structs

Client API (#46)

  • Added ListScopes method: GET /plugins/{plugin}/connections/{id}/scopes
  • Added ScopeListEntry and ScopeListResponse types

Tests (#45)

  • Tests use ScopeOpts struct -- zero global mutation
  • Previously-failing TestRunConfigureScopes_PluginFlag now passes
  • All go test ./... green

Testing

  • go build -- clean
  • go test ./... -- ALL PASS (including previously-failing test)

Closes #35, #37, #39, #40, #41, #42, #45, #46

Architectural refactor for v0.3.7 — decouples scope and project commands,
eliminates ~18 shared mutable package-level variables, and fixes orchestrator
commands (full, init) to properly chain phases.

## Scope command (configure_scopes.go)
- Accepts ScopeOpts struct instead of reading package globals
- Only PUTs scopes to connections — no longer creates projects (#35)
- Removed project-related flags (--project-name, --cron, --time-after, etc.)
- After scoping, prints hint to run 'configure project'

## Project command (configure_projects.go)
- Accepts ProjectOpts struct instead of reading package globals
- Uses new ListScopes API to discover existing scopes (#37)
- No longer PUTs scopes — references existing ones in blueprints
- Removed fall-through to runConfigureScopes in flag mode (#41)
- Owns finalizeProject, ensureProjectWithFlags, triggerAndPoll

## Full command (configure_full.go)
- 3-phase flow: connections -> scopes -> project
- Passes DevLake client/state from Phase 1 to Phase 3 (#42)
- Each phase uses its own opts struct, no shared mutable state

## Init wizard (init.go)
- 4-phase flow: deploy -> connections -> scopes -> project
- Phase 4 calls runConfigureProjects (not runConfigureScopes) (#40)
- Passes connection results via opts structs

## Client (internal/devlake/client.go)
- Added ListScopes method: GET /plugins/{plugin}/connections/{id}/scopes (#46)

## Types (internal/devlake/types.go)
- Added ScopeListEntry and ScopeListResponse types (#46)

## Tests (configure_scopes_test.go)
- Tests use ScopeOpts struct — no global mutation (#45)
- Previously-failing TestRunConfigureScopes_PluginFlag now passes

Closes #35, #37, #39, #40, #41, #42, #45, #46
Copilot AI review requested due to automatic review settings February 19, 2026 20:27
@ewega ewega added this to the v0.3.7 milestone Feb 19, 2026
@ewega ewega merged commit a2dd90d into main Feb 19, 2026
3 checks passed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements a major architectural refactoring that successfully decouples scope and project commands by eliminating ~18 shared mutable package-level variables and establishing clear separation of concerns.

Changes:

  • Introduces opts structs (ScopeOpts, ProjectOpts) to replace package-level scope* variables, making commands independently testable
  • Separates scope command (now only PUTs scopes) from project command (now uses ListScopes API to reference existing scopes)
  • Fixes orchestrator commands (full, init) to properly chain phases and pass client/state forward to avoid redundant discovery
  • Adds ListScopes client method and supporting types for the project command to discover existing scopes

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
internal/devlake/types.go Adds ScopeListEntry and ScopeListResponse types for GET /scopes API
internal/devlake/client.go Adds ListScopes method to query existing scopes on connections
cmd/configure_scopes_test.go Updates tests to use ScopeOpts struct instead of mutating globals
cmd/configure_scopes.go Refactors to accept ScopeOpts, removes project creation, only PUTs scopes
cmd/configure_projects.go Refactors to accept ProjectOpts, uses ListScopes instead of scoping connections
cmd/configure_full.go Implements 3-phase flow (connections→scopes→project) passing client/state forward
cmd/init.go Implements 4-phase flow, now correctly calls runConfigureProjects in Phase 4
Comments suppressed due to low confidence (14)

cmd/init.go:135

  • According to the terminal output standards, phase banners need blank lines on both sides. Currently, there's a blank line before the phase banner but not after. Add fmt.Println() after line 134 to properly separate the banner from the following content.

This issue also appears in the following locations of the same file:

  • line 191
  • line 91
  • line 67
fmt.Println("\n\u2554\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2557")
fmt.Println("\u2551  PHASE 3: Configure Scopes           \u2551")
fmt.Println("\u255a\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u255d")

cmd/init.go:194

  • According to the terminal output standards, phase banners need blank lines on both sides. Currently, there's a blank line before the phase banner but not after. Add fmt.Println() after line 193 to properly separate the banner from the following content.
fmt.Println("\n\u2554\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2557")
fmt.Println("\u2551  PHASE 4: Project Setup              \u2551")
fmt.Println("\u255a\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u255d")

cmd/configure_full.go:105

  • According to the terminal output standards, phase banners need blank lines on both sides. Currently, there's a blank line before the phase banner but not after. Add fmt.Println() after line 104 to properly separate the banner from the following content.

This issue also appears in the following locations of the same file:

  • line 168
  • line 113
fmt.Println("\n\u2554\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2557")
fmt.Println("\u2551  PHASE 1: Configure Connections      \u2551")
fmt.Println("\u255a\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u255d")

cmd/configure_full.go:171

  • According to the terminal output standards, phase banners need blank lines on both sides. Currently, there's a blank line before the phase banner but not after. Add fmt.Println() after line 170 to properly separate the banner from the following content.
fmt.Println("\n\u2554\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2557")
fmt.Println("\u2551  PHASE 3: Project Setup               \u2551")
fmt.Println("\u255a\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u255d")

cmd/init.go:94

  • According to the terminal output standards, phase banners need blank lines on both sides. Currently, there's a blank line before the phase banner but not after. Add fmt.Println() after line 93 to properly separate the banner from the following content.
fmt.Println("\n\u2554\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2557")
fmt.Println("\u2551  PHASE 2: Configure Connections      \u2551")
fmt.Println("\u255a\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u255d")

cmd/configure_scopes.go:276

  • According to the terminal output standards, separators need blank lines before AND after them. Currently there's a \n prefix before the opening separator (line 267) but no blank line after the closing separator (line 275). The pattern should be: blank line before opening separator → content → closing separator → blank line. Add fmt.Println() after line 275 to properly separate the summary section from the "Next step" hint.
fmt.Println("\n" + strings.Repeat("\u2500", 50))
fmt.Println("\u2705 Scopes configured successfully!")
if !opts.SkipGitHub {
fmt.Printf("   GitHub connection %d: scopes added\n", ghConnID)
}
if !opts.SkipCopilot && copilotConnID > 0 {
fmt.Printf("   Copilot connection %d: scope added\n", copilotConnID)
}
fmt.Println(strings.Repeat("\u2500", 50))
fmt.Println("\nNext step:")

cmd/init.go:144

  • According to the terminal output standards, there should be a blank line before interactive prompts. Add fmt.Println() before line 144 to properly separate the prompt from the preceding output.
if !prompt.Confirm("Use these defaults?") {

cmd/configure_projects.go:404

  • According to the terminal output standards, separators need blank lines before AND after them. Currently there's a \n prefix before the opening separator (line 394) but no blank line after the closing separator (line 403). Add fmt.Println() after line 403 to provide proper spacing.
fmt.Println("\n" + strings.Repeat("\u2500", 50))
fmt.Println("\u2705 Project configured successfully!")
fmt.Printf("   Project: %s\n", opts.ProjectName)
if len(opts.Repos) > 0 {
fmt.Printf("   Repos:   %s\n", strings.Join(opts.Repos, ", "))
}
if opts.HasCopilot {
fmt.Printf("   Copilot: %s\n", opts.Org)
}
fmt.Println(strings.Repeat("\u2500", 50))

cmd/configure_full.go:116

  • According to the terminal output standards, phase banners need blank lines on both sides. Currently, there's a blank line before the phase banner but not after. Add fmt.Println() after line 115 to properly separate the banner from the following content.
fmt.Println("\n\u2554\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2557")
fmt.Println("\u2551  PHASE 2: Configure Scopes            \u2551")
fmt.Println("\u255a\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u255d")

cmd/init.go:70

  • According to the terminal output standards, phase banners need blank lines on both sides. Currently, there's a blank line before the phase banner but not after. Add fmt.Println() after line 69 to properly separate the banner from the following content.
fmt.Println("\n\u2554\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2557")
fmt.Println("\u2551  PHASE 1: Deploy DevLake             \u2551")
fmt.Println("\u255a\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u255d")

cmd/configure_scopes.go:36

  • The ScopeOpts struct contains several unused project-related fields: ProjectName, TimeAfter, Cron, SkipSync, Wait, and Timeout. Since the scope command no longer creates projects or triggers syncs (per the architectural separation in this PR), these fields are dead code and should be removed. Keep only the fields actually used by the scope command: Org, Enterprise, Plugin, Repos, ReposFile, GHConnID, CopilotConnID, DeployPattern, ProdPattern, IncidentLabel, SkipCopilot, and SkipGitHub.
type ScopeOpts struct {
Org           string
Enterprise    string
Plugin        string
Repos         string
ReposFile     string
GHConnID      int
CopilotConnID int
ProjectName   string
DeployPattern string
ProdPattern   string
IncidentLabel string
TimeAfter     string
Cron          string
SkipSync      bool
SkipCopilot   bool
SkipGitHub    bool
Wait          bool
Timeout       string
}

cmd/init.go:212

  • According to the terminal output standards, completion banners need blank lines after them. Currently there's a blank line before the opening banner (line 209) but no blank line after the closing banner (line 211). The standard requires fmt.Println() after the closing banner. Line 212 already has this, so this banner correctly follows the standard.
fmt.Println("\n\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550")
fmt.Println("  \u2705 DevLake is ready!")
fmt.Println("\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550")
fmt.Println()

cmd/configure_full.go:276

  • According to the terminal output standards, separators need blank lines before AND after them. Currently there's a \n prefix before the opening separator (line 266) but no blank line after the closing separator (line 275). Add fmt.Println() after line 275 to provide proper spacing before the return statement.
fmt.Println("\n" + strings.Repeat("\u2500", 50))
fmt.Println("\u2705 Connections configured successfully!")
for _, r := range results {
name := r.Plugin
if def := FindConnectionDef(r.Plugin); def != nil {
name = def.DisplayName
}
fmt.Printf("   %-18s  ID=%d  %q\n", name, r.ConnectionID, r.Name)
}
fmt.Println(strings.Repeat("\u2500", 50))

cmd/configure_full.go:193

  • According to the terminal output standards, completion banners need blank lines after them. Line 193 already provides this with fmt.Println(). This banner correctly follows the standard.
fmt.Println("\n\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550")
fmt.Println("  \u2705 Full configuration complete!")
fmt.Println("\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550")
fmt.Println()

@ewega ewega deleted the refactor/scope-project-separation branch March 4, 2026 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

configure scope should not create projects — scope and project are independent concerns

2 participants