|
| 1 | +# Design: Workspaces — Multi-Project Support |
| 2 | + |
| 3 | +**status**: success |
| 4 | +**next_recommended**: tasks |
| 5 | + |
| 6 | +## Technical Approach |
| 7 | + |
| 8 | +A new `internal/pkg/workspace` package owns the workspace file loader (strict yaml.v3 with `KnownFields(true)`, `go-arch.workspace.yaml`), validation, discovery (explicit `--workspace` flag + upward walk), and path resolution. Workspace commands use the **chdir-hybrid** model: resolve the service path, `os.Chdir(path)` + defer restore (the codebase's established pattern — `cmd/upgrade.go:47`, `mcp/server.go:429-436`), then run the existing command logic unchanged — generate/hooks see the correct CWD automatically (`manifestDir()` returns "." with a manifest present; hooks `ProjectPath` from `os.Getwd()` is correct under chdir; `ResolveConfigPath` follows `viper.ConfigFileUsed()`, so config reload lands on the service). `Upgrade` gains a `WithRoot(root string)` UpgradeOption (mirrors `WithResolver` in `upgrade_opts.go`). Config isolation is opt-in: workspace commands reload the service's `.go-arch.yaml` via viper snapshot/restore; single-project flows are untouched (byte-identical). **`workspace upgrade` runs the FULL standard upgrade logic per service — including `plan.Apply()` under `--yes`, `WriteVersionField`, and `WithResolver` — so pack-sourced files re-render.** Multi-service commands continue on error with a summary; single `--service` preserves fail-fast. |
| 9 | + |
| 10 | +## Architecture Decisions |
| 11 | + |
| 12 | +| Decision | Options | Chosen | Rationale | |
| 13 | +|---|---|---|---| |
| 14 | +| Package layout | extend cmd vs new package | **New `internal/pkg/workspace/`** | Loader/schema/paths is a distinct concern; mirrors packs/generators precedent | |
| 15 | +| Path resolution | chdir vs root-injection vs hybrid | **chdir-hybrid** | chdir is the proven pattern; generate/hooks already CWD-correct; WithRoot adds an injection escape for upgrade | |
| 16 | +| Workspace file discovery | flag-only vs upward walk vs both | **Both** (`--workspace` persistent flag wins, upward walk fallback) | Spec workspace-discovery | |
| 17 | +| Upgrade root | hardcoded "." vs WithRoot option | **`WithRoot(root string)` UpgradeOption** | Mirrors `WithResolver`; variadic = backward-compatible; default "." preserves ADR-7 | |
| 18 | +| Config isolation | viper reload vs config injection | **Opt-in viper snapshot/restore** (see contract below) | Only workspace/`--service` paths trigger it; single-project byte-identical | |
| 19 | +| Multi-service failure | fail-fast vs continue-on-error | **Continue-on-error + summary** | Workspace batch semantics; single `--service` stays fail-fast | |
| 20 | +| Service path check | lazy at command time | **Validate at workspace load + re-check at command time** | Missing dir is an operational error, not a load error | |
| 21 | +| Duplicate name code | workspace_invalid vs service_duplicate | **`service_duplicate` at load**; `workspace_invalid` reserved for structural/schema errors | Spec self-contradiction (workspace-file vs workspace-errors) resolved: semantic duplicate is a service error | |
| 22 | +| Workspace file | required for --service, optional otherwise | **Optional until a workspace command/flag uses it** | Single-project flow never reads it | |
| 23 | + |
| 24 | +## Package Layout |
| 25 | + |
| 26 | +``` |
| 27 | +internal/pkg/workspace/ |
| 28 | + workspace.go — Workspace{Dir, Services []Service}, Service{Name, Path, Template}; Find(name) |
| 29 | + loader.go — Load(path) strict yaml.v3 with Decoder.KnownFields(true), Validate |
| 30 | + discover.go — Discover(cwd) — upward walk; explicit --workspace flag handled in cmd |
| 31 | + errors.go — oops codes: workspace_not_found, workspace_invalid, service_not_found, |
| 32 | + service_path_missing, service_duplicate, service_no_manifest |
| 33 | + loader_test.go, discover_test.go |
| 34 | +``` |
| 35 | + |
| 36 | +## Data Flow |
| 37 | + |
| 38 | +### workspace upgrade (FULL standard logic per service) |
| 39 | + |
| 40 | +``` |
| 41 | +cmd/workspace.go |
| 42 | + │ ws := workspace.Load(flag or Discover(cwd)) ← workspace_not_found if absent |
| 43 | + │ for svc := range ws.Services (sequential): |
| 44 | + │ path := filepath.Join(ws.Dir, svc.Path) |
| 45 | + │ if !dirExists(path): report service_path_missing; continue |
| 46 | + │ oldWd, _ := os.Getwd() |
| 47 | + │ os.Chdir(path) ← chdir into service |
| 48 | + │ loadServiceConfig() ← viper snapshot + reload (defer restore) |
| 49 | + │ if !ManifestExists("."): ← legacy service |
| 50 | + │ report service_no_manifest (naming service); run upgradeLegacy non-interactively; continue |
| 51 | + │ plan, err := scaffold.Upgrade(cfg, |
| 52 | + │ scaffold.WithResolver(scaffold.DefaultResolver{}), ← pack-source re-render works |
| 53 | + │ scaffold.WithRoot(".")) ← default; explicit for clarity |
| 54 | + │ if err: summary[svc.Name] = err; os.Chdir(oldWd); continue |
| 55 | + │ print plan summary |
| 56 | + │ if yes: applied, err := plan.Apply() ← DRY-RUN by default, --yes applies |
| 57 | + │ if err: summary[svc.Name] = err |
| 58 | + │ else: _ = scaffold.WriteVersionField(".go-arch.yaml", Version) ← surgical ADR-4 |
| 59 | + │ os.Chdir(oldWd); restoreConfig() |
| 60 | + │ print per-service summary; exit non-zero if any failed |
| 61 | +``` |
| 62 | + |
| 63 | +Batch apply mode: **dry-run by default, `--yes` applies** (mirrors standalone `cmd/upgrade.go:88-98`). Legacy per-file interactive prompting is **non-interactive under batch** — legacy services apply fully with `--yes`, report `service_no_manifest`, no TTY prompt. |
| 64 | + |
| 65 | +### generate --service |
| 66 | + |
| 67 | +``` |
| 68 | +cmd/generate.go (--service flag set) |
| 69 | + │ ws := workspace.Load(flag or Discover(cwd)) ← workspace_not_found if absent |
| 70 | + │ svc := ws.Find(name) ← service_not_found if absent |
| 71 | + │ chdir into svc.Path (defer restore) |
| 72 | + │ loadServiceConfig() ← viper snapshot + reload (defer restore) |
| 73 | + │ run existing generate dispatch (unchanged, incl. --route) |
| 74 | +``` |
| 75 | + |
| 76 | +### Upgrade WithRoot |
| 77 | + |
| 78 | +``` |
| 79 | +scaffold.Upgrade(cfg, WithRoot(root), WithResolver(resolver)) |
| 80 | + │ upgradeConfig.Root = root (default ".") ← all filepath.Join(root, ...) use it; ADR-7 preserved |
| 81 | +``` |
| 82 | + |
| 83 | +## Interfaces / Contracts |
| 84 | + |
| 85 | +```go |
| 86 | +// workspace package |
| 87 | +func Load(path string) (*Workspace, error) // workspace_not_found / workspace_invalid |
| 88 | +func Discover(cwd string) (string, error) // upward walk; workspace_not_found |
| 89 | +func (w *Workspace) Find(name string) (*Service, bool) |
| 90 | + |
| 91 | +// scaffold upgrade (modified) |
| 92 | +type UpgradeOption func(*upgradeConfig) // existing |
| 93 | +func WithRoot(root string) UpgradeOption // NEW — default "." |
| 94 | + |
| 95 | +// cmd helpers (filed in cmd/workspace_helpers.go) |
| 96 | +func resolveWorkspace(flag string) (*workspace.Workspace, error) // flag wins, else Discover(cwd) |
| 97 | +func withService(w *workspace.Workspace, name string, fn func() error) error // chdir + viper snapshot/restore + defer |
| 98 | +func loadServiceConfig() func() // snapshot/restore closure |
| 99 | +``` |
| 100 | + |
| 101 | +### viper snapshot/restore contract (MED-4 fix) |
| 102 | + |
| 103 | +``` |
| 104 | +snapshot: |
| 105 | + prev := viper.ConfigFileUsed() |
| 106 | +reload (per service): |
| 107 | + viper.Reset() |
| 108 | + if service config exists: viper.SetConfigFile(absServiceConfig); viper.ReadInConfig() // best-effort — skip on missing |
| 109 | + else: viper.AddConfigPath("."); viper.SetConfigName(".go-arch"); viper.ReadInConfig() // best-effort |
| 110 | +restore (defer after service op): |
| 111 | + viper.Reset() |
| 112 | + if prev != "": viper.SetConfigFile(prev); viper.ReadInConfig() |
| 113 | + else: viper.AddConfigPath("."); viper.SetConfigName(".go-arch"); viper.ReadInConfig() // best-effort |
| 114 | +``` |
| 115 | + |
| 116 | +Best-effort semantics: any `ReadInConfig` error on missing files is ignored (unlike `cmd/upgrade.go:58-62` which treats it as fatal — workspace reloads must not). |
| 117 | + |
| 118 | +## File Change Plan |
| 119 | + |
| 120 | +| File | Action | What | |
| 121 | +|------|--------|------| |
| 122 | +| `internal/pkg/workspace/workspace.go` | Create | Types (Dir, Services, Find) | |
| 123 | +| `internal/pkg/workspace/loader.go` | Create | Load/Validate — `yaml.v3` `Decoder.KnownFields(true)` (note: packs/manifest.go:228 is NOT strict; workspace loader is stricter by design) | |
| 124 | +| `internal/pkg/workspace/discover.go` | Create | Discover upward walk | |
| 125 | +| `internal/pkg/workspace/errors.go` | Create | 6 oops codes | |
| 126 | +| `internal/pkg/workspace/loader_test.go` | Create | Valid/duplicate→service_duplicate/unknown-key table | |
| 127 | +| `internal/pkg/workspace/discover_test.go` | Create | Upward walk / none found | |
| 128 | +| `internal/pkg/scaffold/upgrade_opts.go` | Modify | Add WithRoot + Root field (default ".") | |
| 129 | +| `internal/pkg/scaffold/upgrade.go` | Modify | Use upgradeConfig.Root in ManifestExists/LoadManifest/filepath.Join/plan.ProjectRoot | |
| 130 | +| `cmd/workspace.go` | Create | Parent command + upgrade/check subcommands; self-registers in its own `init()` (repo convention) | |
| 131 | +| `cmd/workspace_helpers.go` | Create | resolveWorkspace, withService, loadServiceConfig | |
| 132 | +| `cmd/root.go` | Modify | Persistent `--workspace` flag | |
| 133 | +| `cmd/generate.go` | Modify | `--service` flag + chdir + config reload | |
| 134 | +| `cmd/check.go` | Modify | `--service` flag + chdir + config reload | |
| 135 | +| `cmd/upgrade.go` | Modify | `--service` flag + chdir + config reload (reuse existing Upgrade flow) | |
| 136 | +| `docs/workspaces.md` | Create | Reference | |
| 137 | +| `docs/COMMANDS.md`, `README.md` | Modify | Workspace docs | |
| 138 | + |
| 139 | +## Testing Strategy |
| 140 | + |
| 141 | +- **Unit — workspace package**: loader table (valid, duplicate→service_duplicate, unknown key→workspace_invalid, missing name/path, bad slug); discovery (upward walk, none found). |
| 142 | +- **Unit — upgrade**: `WithRoot` (root used; default "." preserved — ADR-7 regression test). |
| 143 | +- **Integration — workspace upgrade**: t.TempDir monorepo with 2 manifest services + 1 pack-source service; `workspace upgrade` (dry-run) → plans printed, nothing written; `workspace upgrade --yes` → files actually applied, version field written, **pack-source entry re-rendered (WithResolver)**, summary printed, continue-on-error with a failing service. Legacy service → `service_no_manifest` reported, legacy apply works with `--yes`. |
| 144 | +- **Integration — workspace check**: both services checked, per-service summary. |
| 145 | +- **Integration — --service**: `generate crud User --service orders` → files land in services/orders, CWD restored; unknown service → service_not_found; no workspace → error naming the flag + hint `--workspace`. |
| 146 | +- **Integration — hooks CWD**: service with post-generate hook writing a marker → marker lands in the service dir; PROJECT_PATH points at the service. |
| 147 | +- **Integration — config isolation**: monorepo-root config vs service config with different settings → service settings used; config restored after (subsequent command sees prior config). |
| 148 | +- **Live smoke (verify phase)**: real chdir across services, pack-source re-render, Windows path notes. |
| 149 | + |
| 150 | +## Key Risks |
| 151 | + |
| 152 | +- **viper global-state snapshot/restore** (HIGH slice): initConfig runs once at startup; the opt-in snapshot/restore bounds it; best-effort reload must NOT be fatal on missing files. Live smoke verifies single-project byte-identical behavior. |
| 153 | +- **os.Chdir global state**: sequential-only + defer restore (MCP precedent); no concurrent service ops in v1. |
| 154 | +- **Manifest path collisions**: service manifests are relative; integration test verifies upgrade never writes monorepo-root files when chdir'd. |
| 155 | +- **Batch apply semantics**: legacy interactive prompting is disabled under batch (non-interactive) — documented; `--yes` required to write anything. |
| 156 | +- **Windows**: filepath semantics in YAML paths, chdir behavior — CI coverage note. |
| 157 | +- **Workspace file schema is a new public contract**: strict KnownFields(true) validation + small v1 scope mitigate. |
0 commit comments