From 9bd0b09e3baec53219d1ae7f595d3f93ef48ce66 Mon Sep 17 00:00:00 2001 From: Snider Date: Wed, 4 Feb 2026 13:40:16 +0000 Subject: [PATCH] refactor(core): decompose Core into serviceManager + messageBus (#282) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(core): decompose Core into serviceManager + messageBus (#215) Extract two focused, unexported components from the Core "god object": - serviceManager: owns service registry, lifecycle tracking (startables/ stoppables), and service lock - messageBus: owns IPC action dispatch, query handling, and task handling All public API methods on Core become one-line delegation wrappers. Zero consumer changes — no files outside pkg/framework/core/ modified. Co-Authored-By: Claude Opus 4.5 * fix(core): remove unused fields from test struct Co-Authored-By: Claude Opus 4.5 * fix(core): address review feedback from Gemini and Copilot - Move locked check inside mutex in registerService to fix TOCTOU race - Add mutex guards to enableLock and applyLock methods - Replace fmt.Errorf with errors.Join in action() for correct error aggregation (consistent with queryAll and lifecycle methods) - Add TestMessageBus_Action_Bad for error aggregation coverage Co-Authored-By: Claude Opus 4.5 * ci(workflows): bump host-uk/build from v3 to v4 Co-Authored-By: Claude Opus 4.5 * ci(workflows): replace Wails build with Go CLI build The build action doesn't yet support Wails v3. Comment out the GUI build step and use host-uk/build/actions/setup/go for Go toolchain setup with a plain `go build` for the CLI binary. Co-Authored-By: Claude Opus 4.5 * fix(container): check context before select in Stop to fix flaky test Stop() now checks ctx.Err() before entering the select block. When a pre-cancelled context is passed, the select could non-deterministically choose <-done over <-ctx.Done() if the process had already exited, causing TestLinuxKitManager_Stop_Good_ContextCancelled to fail on CI. Co-Authored-By: Claude Opus 4.5 * fix(ci): trim CodeQL matrix to valid languages Remove javascript-typescript and actions from CodeQL matrix — this repo contains only Go and Python. Invalid languages blocked SARIF upload and prevented merge. Co-Authored-By: Claude Opus 4.5 * feat(go): add `core go fuzz` command and wire into QA - New `core go fuzz` command discovers Fuzz* targets and runs them with configurable --duration (default 10s per target) - Fuzz added to default QA checks with 5s burst duration - Seed fuzz targets for core package: FuzzE (error constructor), FuzzServiceRegistration, FuzzMessageDispatch Co-Authored-By: Claude Opus 4.5 * ci(codeql): add workflow_dispatch trigger for manual runs Allows manual triggering of CodeQL when the automatic pull_request trigger doesn't fire. Co-Authored-By: Claude Opus 4.5 * ci(codeql): remove workflow in favour of default setup CodeQL default setup is now enabled via repo settings for go and python. The workflow-based approach uploaded results as "code quality" rather than "code scanning", which didn't satisfy the code_scanning ruleset requirement. Default setup handles this natively. Co-Authored-By: Claude Opus 4.5 * ci(workflows): add explicit permissions to all workflows - agent-verify: add issues: write (was missing, writes comments/labels) - ci: add contents: read (explicit least-privilege) - coverage: add contents: read (explicit least-privilege) All workflows now declare permissions explicitly. Repo default is read-only, so workflows without a block silently lacked write access. Co-Authored-By: Claude Opus 4.5 * ci(workflows): replace inline logic with org reusable workflow callers agent-verify.yml and auto-project.yml now delegate to centralised reusable workflows in host-uk/.github, reducing per-repo duplication. Co-Authored-By: Claude Opus 4.5 --------- Co-authored-by: Claude Opus 4.5 --- .github/workflows/agent-verify.yml | 132 +--------------- .github/workflows/alpha-release.yml | 48 ++++-- .github/workflows/auto-project.yml | 29 +--- .github/workflows/ci.yml | 3 + .github/workflows/codeql.yml | 40 ----- .github/workflows/coverage.yml | 3 + .github/workflows/pr-build.yml | 32 +++- .github/workflows/release.yml | 48 ++++-- internal/cmd/go/cmd_fuzz.go | 169 +++++++++++++++++++++ internal/cmd/go/cmd_go.go | 1 + internal/cmd/go/cmd_qa.go | 17 ++- pkg/container/linuxkit.go | 6 + pkg/framework/core/core.go | 124 +++------------ pkg/framework/core/fuzz_test.go | 107 +++++++++++++ pkg/framework/core/interfaces.go | 26 +--- pkg/framework/core/message_bus.go | 119 +++++++++++++++ pkg/framework/core/message_bus_test.go | 146 ++++++++++++++++++ pkg/framework/core/service_manager.go | 94 ++++++++++++ pkg/framework/core/service_manager_test.go | 132 ++++++++++++++++ 19 files changed, 928 insertions(+), 348 deletions(-) delete mode 100644 .github/workflows/codeql.yml create mode 100644 internal/cmd/go/cmd_fuzz.go create mode 100644 pkg/framework/core/fuzz_test.go create mode 100644 pkg/framework/core/message_bus.go create mode 100644 pkg/framework/core/message_bus_test.go create mode 100644 pkg/framework/core/service_manager.go create mode 100644 pkg/framework/core/service_manager_test.go diff --git a/.github/workflows/agent-verify.yml b/.github/workflows/agent-verify.yml index b1b3a97..d8bcb16 100644 --- a/.github/workflows/agent-verify.yml +++ b/.github/workflows/agent-verify.yml @@ -1,134 +1,10 @@ -# https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#issues -name: "Agent Verification: Issue Labeled" +name: Agent Verification on: issues: types: [labeled] jobs: - # When work is claimed, track the implementer - track-implementer: - if: github.event.label.name == 'agent:wip' - runs-on: ubuntu-latest - steps: - - name: Record implementer - run: | - echo "Implementer: ${{ github.actor }}" - # Could store in issue body or external system - - # When work is submitted for review, add to verification queue - request-verification: - if: github.event.label.name == 'agent:review' - runs-on: ubuntu-latest - steps: - - name: Add to Workstation for verification - uses: actions/add-to-project@v1.0.2 - with: - project-url: https://github.com/orgs/host-uk/projects/2 - github-token: ${{ secrets.PROJECT_TOKEN }} - - - name: Comment verification needed - uses: actions/github-script@v8 - with: - script: | - const implementer = context.payload.sender.login; - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - body: `## 🔍 Verification Required\n\nWork submitted by @${implementer}.\n\n**Rule:** A different agent must verify this work.\n\nTo verify:\n1. Review the implementation\n2. Run tests if applicable\n3. Add \`verified\` or \`verify-failed\` label\n\n_Self-verification is not allowed._` - }); - - # Block self-verification - check-verification: - if: github.event.label.name == 'verified' || github.event.label.name == 'verify-failed' - runs-on: ubuntu-latest - steps: - - name: Get issue details - id: issue - uses: actions/github-script@v8 - with: - script: | - const issue = await github.rest.issues.get({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number - }); - - // Check timeline for who added agent:wip - const timeline = await github.rest.issues.listEventsForTimeline({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - per_page: 100 - }); - - const wipEvent = timeline.data.find(e => - e.event === 'labeled' && e.label?.name === 'agent:wip' - ); - - const implementer = wipEvent?.actor?.login || 'unknown'; - const verifier = context.payload.sender.login; - - console.log(`Implementer: ${implementer}`); - console.log(`Verifier: ${verifier}`); - - if (implementer === verifier) { - core.setFailed(`Self-verification not allowed. ${verifier} cannot verify their own work.`); - } - - return { implementer, verifier }; - - - name: Record verification - if: success() - uses: actions/github-script@v8 - with: - script: | - const label = context.payload.label.name; - const verifier = context.payload.sender.login; - const status = label === 'verified' ? '✅ Verified' : '❌ Failed'; - - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - body: `## ${status}\n\nVerified by @${verifier}` - }); - - // Remove agent:review label - try { - await github.rest.issues.removeLabel({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - name: 'agent:review' - }); - } catch (e) { - console.log('agent:review label not present'); - } - - # If verification failed, reset for rework - handle-failure: - if: github.event.label.name == 'verify-failed' - runs-on: ubuntu-latest - needs: check-verification - steps: - - name: Reset for rework - uses: actions/github-script@v8 - with: - script: | - // Remove verify-failed after processing - await github.rest.issues.removeLabel({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - name: 'verify-failed' - }); - - // Add back to ready queue - await github.rest.issues.addLabels({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - labels: ['agent:ready'] - }); + verify: + uses: host-uk/.github/.github/workflows/agent-verify.yml@main + secrets: inherit diff --git a/.github/workflows/alpha-release.yml b/.github/workflows/alpha-release.yml index 1d7486a..a5b2441 100644 --- a/.github/workflows/alpha-release.yml +++ b/.github/workflows/alpha-release.yml @@ -20,25 +20,51 @@ jobs: matrix: include: - os: ubuntu-latest - platform: linux/amd64 + goos: linux + goarch: amd64 - os: ubuntu-latest - platform: linux/arm64 + goos: linux + goarch: arm64 - os: macos-latest - platform: darwin/universal + goos: darwin + goarch: arm64 - os: windows-latest - platform: windows/amd64 + goos: windows + goarch: amd64 runs-on: ${{ matrix.os }} + env: + GOOS: ${{ matrix.goos }} + GOARCH: ${{ matrix.goarch }} steps: - uses: actions/checkout@v6 - - name: Build - uses: host-uk/build@v3 + # GUI build disabled until build action supports Wails v3 + # - name: Wails Build Action + # uses: host-uk/build@v4.0.0 + # with: + # build-name: core + # build-platform: ${{ matrix.goos }}/${{ matrix.goarch }} + # build: true + # package: true + # sign: false + + - name: Setup Go + uses: host-uk/build/actions/setup/go@v4.0.0 with: - build-name: core - build-platform: ${{ matrix.platform }} - build: true - package: true - sign: false + go-version: "1.25" + + - name: Build CLI + shell: bash + run: | + EXT="" + if [ "$GOOS" = "windows" ]; then EXT=".exe"; fi + go build -o "./bin/core${EXT}" . + + - name: Upload artifact + uses: actions/upload-artifact@v4 + with: + name: core-${{ matrix.goos }}-${{ matrix.goarch }} + path: ./bin/core* release: needs: build diff --git a/.github/workflows/auto-project.yml b/.github/workflows/auto-project.yml index 47b6a7d..9244ba2 100644 --- a/.github/workflows/auto-project.yml +++ b/.github/workflows/auto-project.yml @@ -1,31 +1,10 @@ -# https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#issues -name: "Auto Project: Issue Created/Labeled" +name: Auto Project on: issues: types: [opened, labeled] jobs: - add-to-project: - runs-on: ubuntu-latest - steps: - - name: Add to Workstation (agentic label) - if: contains(github.event.issue.labels.*.name, 'agentic') - uses: actions/add-to-project@v1.0.2 - with: - project-url: https://github.com/orgs/host-uk/projects/2 - github-token: ${{ secrets.PROJECT_TOKEN }} - - - name: Add to Core.GO (lang:go label) - if: contains(github.event.issue.labels.*.name, 'lang:go') - uses: actions/add-to-project@v1.0.2 - with: - project-url: https://github.com/orgs/host-uk/projects/4 - github-token: ${{ secrets.PROJECT_TOKEN }} - - - name: Add to Core.Framework (scope:arch label) - if: contains(github.event.issue.labels.*.name, 'scope:arch') - uses: actions/add-to-project@v1.0.2 - with: - project-url: https://github.com/orgs/host-uk/projects/1 - github-token: ${{ secrets.PROJECT_TOKEN }} + project: + uses: host-uk/.github/.github/workflows/auto-project.yml@main + secrets: inherit diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 147193c..0de1733 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,6 +7,9 @@ on: branches: [dev, main] workflow_dispatch: +permissions: + contents: read + env: CORE_VERSION: dev diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml deleted file mode 100644 index 1bc44a0..0000000 --- a/.github/workflows/codeql.yml +++ /dev/null @@ -1,40 +0,0 @@ -name: CodeQL - -on: - push: - branches: [dev, main] - pull_request: - branches: [dev, main] - schedule: - - cron: "0 6 * * 1" - -jobs: - analyze: - name: Analyze (${{ matrix.language }}) - runs-on: ubuntu-latest - permissions: - actions: read - contents: read - security-events: write - - strategy: - fail-fast: false - matrix: - language: [go, javascript-typescript, python, actions] - - steps: - - name: Checkout - uses: actions/checkout@v6 - - - name: Initialize CodeQL - uses: github/codeql-action/init@v4 - with: - languages: ${{ matrix.language }} - - - name: Autobuild - uses: github/codeql-action/autobuild@v4 - - - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v4 - with: - category: "/language:${{ matrix.language }}" diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index b2bf4ae..a2cdeaa 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -7,6 +7,9 @@ on: branches: [dev, main] workflow_dispatch: +permissions: + contents: read + env: CORE_VERSION: dev diff --git a/.github/workflows/pr-build.yml b/.github/workflows/pr-build.yml index f9b1f37..c928aa5 100644 --- a/.github/workflows/pr-build.yml +++ b/.github/workflows/pr-build.yml @@ -26,21 +26,37 @@ jobs: matrix: include: - os: ubuntu-latest - platform: linux/amd64 + goos: linux + goarch: amd64 runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 with: ref: ${{ github.event.pull_request.head.sha || github.sha }} - - name: Build - uses: host-uk/build@v3 + # GUI build disabled until build action supports Wails v3 + # - name: Wails Build Action + # uses: host-uk/build@v4.0.0 + # with: + # build-name: core + # build-platform: ${{ matrix.goos }}/${{ matrix.goarch }} + # build: true + # package: true + # sign: false + + - name: Setup Go + uses: host-uk/build/actions/setup/go@v4.0.0 with: - build-name: core - build-platform: ${{ matrix.platform }} - build: true - package: true - sign: false + go-version: "1.25" + + - name: Build CLI + run: go build -o ./bin/core . + + - name: Upload artifact + uses: actions/upload-artifact@v4 + with: + name: core-${{ matrix.goos }}-${{ matrix.goarch }} + path: ./bin/core draft-release: needs: build diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 1ddf7b7..173e7c8 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -15,25 +15,51 @@ jobs: matrix: include: - os: ubuntu-latest - platform: linux/amd64 + goos: linux + goarch: amd64 - os: ubuntu-latest - platform: linux/arm64 + goos: linux + goarch: arm64 - os: macos-latest - platform: darwin/universal + goos: darwin + goarch: arm64 - os: windows-latest - platform: windows/amd64 + goos: windows + goarch: amd64 runs-on: ${{ matrix.os }} + env: + GOOS: ${{ matrix.goos }} + GOARCH: ${{ matrix.goarch }} steps: - uses: actions/checkout@v6 - - name: Build - uses: host-uk/build@v3 + # GUI build disabled until build action supports Wails v3 + # - name: Wails Build Action + # uses: host-uk/build@v4.0.0 + # with: + # build-name: core + # build-platform: ${{ matrix.goos }}/${{ matrix.goarch }} + # build: true + # package: true + # sign: false + + - name: Setup Go + uses: host-uk/build/actions/setup/go@v4.0.0 with: - build-name: core - build-platform: ${{ matrix.platform }} - build: true - package: true - sign: false + go-version: "1.25" + + - name: Build CLI + shell: bash + run: | + EXT="" + if [ "$GOOS" = "windows" ]; then EXT=".exe"; fi + go build -o "./bin/core${EXT}" . + + - name: Upload artifact + uses: actions/upload-artifact@v4 + with: + name: core-${{ matrix.goos }}-${{ matrix.goarch }} + path: ./bin/core* release: needs: build diff --git a/internal/cmd/go/cmd_fuzz.go b/internal/cmd/go/cmd_fuzz.go new file mode 100644 index 0000000..194cd1e --- /dev/null +++ b/internal/cmd/go/cmd_fuzz.go @@ -0,0 +1,169 @@ +package gocmd + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "regexp" + "strings" + "time" + + "github.com/host-uk/core/pkg/cli" + "github.com/host-uk/core/pkg/i18n" +) + +var ( + fuzzDuration time.Duration + fuzzPkg string + fuzzRun string + fuzzVerbose bool +) + +func addGoFuzzCommand(parent *cli.Command) { + fuzzCmd := &cli.Command{ + Use: "fuzz", + Short: "Run Go fuzz tests", + Long: `Run Go fuzz tests with configurable duration. + +Discovers Fuzz* functions across the project and runs each with go test -fuzz. + +Examples: + core go fuzz # Run all fuzz targets for 10s each + core go fuzz --duration=30s # Run each target for 30s + core go fuzz --pkg=./pkg/... # Fuzz specific package + core go fuzz --run=FuzzE # Run only matching fuzz targets`, + RunE: func(cmd *cli.Command, args []string) error { + return runGoFuzz(fuzzDuration, fuzzPkg, fuzzRun, fuzzVerbose) + }, + } + + fuzzCmd.Flags().DurationVar(&fuzzDuration, "duration", 10*time.Second, "Duration per fuzz target") + fuzzCmd.Flags().StringVar(&fuzzPkg, "pkg", "", "Package to fuzz (default: auto-discover)") + fuzzCmd.Flags().StringVar(&fuzzRun, "run", "", "Only run fuzz targets matching pattern") + fuzzCmd.Flags().BoolVarP(&fuzzVerbose, "verbose", "v", false, "Verbose output") + + parent.AddCommand(fuzzCmd) +} + +// fuzzTarget represents a discovered fuzz function and its package. +type fuzzTarget struct { + Pkg string + Name string +} + +func runGoFuzz(duration time.Duration, pkg, run string, verbose bool) error { + cli.Print("%s %s\n", dimStyle.Render(i18n.Label("fuzz")), i18n.ProgressSubject("run", "fuzz tests")) + cli.Blank() + + targets, err := discoverFuzzTargets(pkg, run) + if err != nil { + return cli.Wrap(err, "discover fuzz targets") + } + + if len(targets) == 0 { + cli.Print(" %s no fuzz targets found\n", dimStyle.Render("—")) + return nil + } + + cli.Print(" %s %d target(s), %s each\n", dimStyle.Render(i18n.Label("targets")), len(targets), duration) + cli.Blank() + + passed := 0 + failed := 0 + + for _, t := range targets { + cli.Print(" %s %s in %s\n", dimStyle.Render("→"), t.Name, t.Pkg) + + args := []string{ + "test", + fmt.Sprintf("-fuzz=^%s$", t.Name), + fmt.Sprintf("-fuzztime=%s", duration), + "-run=^$", // Don't run unit tests + } + if verbose { + args = append(args, "-v") + } + args = append(args, t.Pkg) + + cmd := exec.Command("go", args...) + cmd.Env = append(os.Environ(), "MACOSX_DEPLOYMENT_TARGET=26.0", "CGO_ENABLED=0") + cmd.Dir, _ = os.Getwd() + + output, runErr := cmd.CombinedOutput() + outputStr := string(output) + + if runErr != nil { + failed++ + cli.Print(" %s %s\n", errorStyle.Render(cli.Glyph(":cross:")), runErr.Error()) + if outputStr != "" { + cli.Text(outputStr) + } + } else { + passed++ + cli.Print(" %s %s\n", successStyle.Render(cli.Glyph(":check:")), i18n.T("i18n.done.pass")) + if verbose && outputStr != "" { + cli.Text(outputStr) + } + } + } + + cli.Blank() + if failed > 0 { + cli.Print("%s %d passed, %d failed\n", errorStyle.Render(cli.Glyph(":cross:")), passed, failed) + return cli.Err("fuzz: %d target(s) failed", failed) + } + + cli.Print("%s %d passed\n", successStyle.Render(cli.Glyph(":check:")), passed) + return nil +} + +// discoverFuzzTargets scans for Fuzz* functions in test files. +func discoverFuzzTargets(pkg, pattern string) ([]fuzzTarget, error) { + root := "." + if pkg != "" { + // Convert Go package pattern to filesystem path + root = strings.TrimPrefix(pkg, "./") + root = strings.TrimSuffix(root, "/...") + } + + fuzzRe := regexp.MustCompile(`^func\s+(Fuzz\w+)\s*\(\s*\w+\s+\*testing\.F\s*\)`) + var matchRe *regexp.Regexp + if pattern != "" { + var err error + matchRe, err = regexp.Compile(pattern) + if err != nil { + return nil, fmt.Errorf("invalid --run pattern: %w", err) + } + } + + var targets []fuzzTarget + err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error { + if err != nil { + return nil + } + if info.IsDir() || !strings.HasSuffix(info.Name(), "_test.go") { + return nil + } + + data, readErr := os.ReadFile(path) + if readErr != nil { + return nil + } + + dir := "./" + filepath.Dir(path) + for line := range strings.SplitSeq(string(data), "\n") { + m := fuzzRe.FindStringSubmatch(line) + if m == nil { + continue + } + name := m[1] + if matchRe != nil && !matchRe.MatchString(name) { + continue + } + targets = append(targets, fuzzTarget{Pkg: dir, Name: name}) + } + return nil + }) + return targets, err +} diff --git a/internal/cmd/go/cmd_go.go b/internal/cmd/go/cmd_go.go index 7aebd9f..1fc7e46 100644 --- a/internal/cmd/go/cmd_go.go +++ b/internal/cmd/go/cmd_go.go @@ -32,4 +32,5 @@ func AddGoCommands(root *cli.Command) { addGoInstallCommand(goCmd) addGoModCommand(goCmd) addGoWorkCommand(goCmd) + addGoFuzzCommand(goCmd) } diff --git a/internal/cmd/go/cmd_qa.go b/internal/cmd/go/cmd_qa.go index 910852f..2ac1dfc 100644 --- a/internal/cmd/go/cmd_qa.go +++ b/internal/cmd/go/cmd_qa.go @@ -42,7 +42,7 @@ func addGoQACommand(parent *cli.Command) { Short: "Run QA checks", Long: `Run comprehensive code quality checks for Go projects. -Checks available: fmt, vet, lint, test, race, vuln, sec, bench, docblock +Checks available: fmt, vet, lint, test, race, fuzz, vuln, sec, bench, docblock Examples: core go qa # Default: fmt, lint, test @@ -64,7 +64,7 @@ Examples: // Scope flags qaCmd.PersistentFlags().BoolVar(&qaChanged, "changed", false, "Only check changed files (git-aware)") qaCmd.PersistentFlags().BoolVar(&qaAll, "all", false, "Check all files (override git-aware)") - qaCmd.PersistentFlags().StringVar(&qaSkip, "skip", "", "Skip checks (comma-separated: fmt,vet,lint,test,race,vuln,sec,bench)") + qaCmd.PersistentFlags().StringVar(&qaSkip, "skip", "", "Skip checks (comma-separated: fmt,vet,lint,test,race,fuzz,vuln,sec,bench)") qaCmd.PersistentFlags().StringVar(&qaOnly, "only", "", "Only run these checks (comma-separated)") // Coverage flags @@ -313,7 +313,7 @@ func determineChecks() []string { } // Default checks - checks := []string{"fmt", "lint", "test", "docblock"} + checks := []string{"fmt", "lint", "test", "fuzz", "docblock"} // Add race if requested if qaRace { @@ -424,6 +424,9 @@ func buildCheck(name string) QACheck { case "sec": return QACheck{Name: "sec", Command: "gosec", Args: []string{"-quiet", "./..."}} + case "fuzz": + return QACheck{Name: "fuzz", Command: "_internal_"} + case "docblock": // Special internal check - handled separately return QACheck{Name: "docblock", Command: "_internal_"} @@ -524,6 +527,14 @@ func runCoverage(ctx context.Context, dir string) (float64, error) { // runInternalCheck runs internal Go-based checks (not external commands). func runInternalCheck(check QACheck) (string, error) { switch check.Name { + case "fuzz": + // Short burst fuzz in QA (5s per target) + duration := 5 * time.Second + if qaTimeout > 0 && qaTimeout < 30*time.Second { + duration = 2 * time.Second + } + return "", runGoFuzz(duration, "", "", qaVerbose) + case "docblock": result, err := qa.CheckDocblockCoverage([]string{"./..."}) if err != nil { diff --git a/pkg/container/linuxkit.go b/pkg/container/linuxkit.go index ee203b4..2f2780a 100644 --- a/pkg/container/linuxkit.go +++ b/pkg/container/linuxkit.go @@ -258,6 +258,12 @@ func (m *LinuxKitManager) Stop(ctx context.Context, id string) error { return nil } + // Honour already-cancelled contexts before waiting + if err := ctx.Err(); err != nil { + _ = process.Signal(syscall.SIGKILL) + return err + } + // Wait for graceful shutdown with timeout done := make(chan struct{}) go func() { diff --git a/pkg/framework/core/core.go b/pkg/framework/core/core.go index 2f0f70c..5f6c393 100644 --- a/pkg/framework/core/core.go +++ b/pkg/framework/core/core.go @@ -7,6 +7,12 @@ import ( "fmt" "reflect" "strings" + "sync" +) + +var ( + instance *Core + instanceMu sync.RWMutex ) // New initialises a Core instance using the provided options and performs the necessary setup. @@ -20,18 +26,18 @@ import ( // ) func New(opts ...Option) (*Core, error) { c := &Core{ - services: make(map[string]any), Features: &Features{}, + svc: newServiceManager(), } + c.bus = newMessageBus(c) + for _, o := range opts { if err := o(c); err != nil { return nil, err } } - if c.serviceLock { - c.servicesLocked = true - } + c.svc.applyLock() return c, nil } @@ -121,7 +127,7 @@ func WithAssets(fs embed.FS) Option { // prevent late-binding of services that could have unintended consequences. func WithServiceLock() Option { return func(c *Core) error { - c.serviceLock = true + c.svc.enableLock() return nil } } @@ -131,9 +137,7 @@ func WithServiceLock() Option { // ServiceStartup is the entry point for the Core service's startup lifecycle. // It is called by the GUI runtime when the application starts. func (c *Core) ServiceStartup(ctx context.Context, options any) error { - c.serviceMu.RLock() - startables := append([]Startable(nil), c.startables...) - c.serviceMu.RUnlock() + startables := c.svc.getStartables() var agg error for _, s := range startables { @@ -157,10 +161,7 @@ func (c *Core) ServiceShutdown(ctx context.Context) error { agg = errors.Join(agg, err) } - c.serviceMu.RLock() - stoppables := append([]Stoppable(nil), c.stoppables...) - c.serviceMu.RUnlock() - + stoppables := c.svc.getStoppables() for i := len(stoppables) - 1; i >= 0; i-- { if err := stoppables[i].OnShutdown(ctx); err != nil { agg = errors.Join(agg, err) @@ -173,135 +174,56 @@ func (c *Core) ServiceShutdown(ctx context.Context) error { // ACTION dispatches a message to all registered IPC handlers. // This is the primary mechanism for services to communicate with each other. func (c *Core) ACTION(msg Message) error { - c.ipcMu.RLock() - handlers := append([]func(*Core, Message) error(nil), c.ipcHandlers...) - c.ipcMu.RUnlock() - - var agg error - for _, h := range handlers { - if err := h(c, msg); err != nil { - agg = fmt.Errorf("%w; %v", agg, err) - } - } - return agg + return c.bus.action(msg) } // RegisterAction adds a new IPC handler to the Core. func (c *Core) RegisterAction(handler func(*Core, Message) error) { - c.ipcMu.Lock() - c.ipcHandlers = append(c.ipcHandlers, handler) - c.ipcMu.Unlock() + c.bus.registerAction(handler) } // RegisterActions adds multiple IPC handlers to the Core. func (c *Core) RegisterActions(handlers ...func(*Core, Message) error) { - c.ipcMu.Lock() - c.ipcHandlers = append(c.ipcHandlers, handlers...) - c.ipcMu.Unlock() + c.bus.registerActions(handlers...) } // QUERY dispatches a query to handlers until one responds. // Returns (result, handled, error). If no handler responds, handled is false. func (c *Core) QUERY(q Query) (any, bool, error) { - c.queryMu.RLock() - handlers := append([]QueryHandler(nil), c.queryHandlers...) - c.queryMu.RUnlock() - - for _, h := range handlers { - result, handled, err := h(c, q) - if handled { - return result, true, err - } - } - return nil, false, nil + return c.bus.query(q) } // QUERYALL dispatches a query to all handlers and collects all responses. // Returns all results from handlers that responded. func (c *Core) QUERYALL(q Query) ([]any, error) { - c.queryMu.RLock() - handlers := append([]QueryHandler(nil), c.queryHandlers...) - c.queryMu.RUnlock() - - var results []any - var agg error - for _, h := range handlers { - result, handled, err := h(c, q) - if err != nil { - agg = errors.Join(agg, err) - } - if handled && result != nil { - results = append(results, result) - } - } - return results, agg + return c.bus.queryAll(q) } // PERFORM dispatches a task to handlers until one executes it. // Returns (result, handled, error). If no handler responds, handled is false. func (c *Core) PERFORM(t Task) (any, bool, error) { - c.taskMu.RLock() - handlers := append([]TaskHandler(nil), c.taskHandlers...) - c.taskMu.RUnlock() - - for _, h := range handlers { - result, handled, err := h(c, t) - if handled { - return result, true, err - } - } - return nil, false, nil + return c.bus.perform(t) } // RegisterQuery adds a query handler to the Core. func (c *Core) RegisterQuery(handler QueryHandler) { - c.queryMu.Lock() - c.queryHandlers = append(c.queryHandlers, handler) - c.queryMu.Unlock() + c.bus.registerQuery(handler) } // RegisterTask adds a task handler to the Core. func (c *Core) RegisterTask(handler TaskHandler) { - c.taskMu.Lock() - c.taskHandlers = append(c.taskHandlers, handler) - c.taskMu.Unlock() + c.bus.registerTask(handler) } // RegisterService adds a new service to the Core. func (c *Core) RegisterService(name string, api any) error { - if c.servicesLocked { - return fmt.Errorf("core: service %q is not permitted by the serviceLock setting", name) - } - if name == "" { - return errors.New("core: service name cannot be empty") - } - c.serviceMu.Lock() - defer c.serviceMu.Unlock() - if _, exists := c.services[name]; exists { - return fmt.Errorf("core: service %q already registered", name) - } - c.services[name] = api - - if s, ok := api.(Startable); ok { - c.startables = append(c.startables, s) - } - if s, ok := api.(Stoppable); ok { - c.stoppables = append(c.stoppables, s) - } - - return nil + return c.svc.registerService(name, api) } // Service retrieves a registered service by name. // It returns nil if the service is not found. func (c *Core) Service(name string) any { - c.serviceMu.RLock() - api, ok := c.services[name] - c.serviceMu.RUnlock() - if !ok { - return nil - } - return api + return c.svc.service(name) } // ServiceFor retrieves a registered service by name and asserts its type to the given interface T. diff --git a/pkg/framework/core/fuzz_test.go b/pkg/framework/core/fuzz_test.go new file mode 100644 index 0000000..93972e0 --- /dev/null +++ b/pkg/framework/core/fuzz_test.go @@ -0,0 +1,107 @@ +package core + +import ( + "errors" + "testing" +) + +// FuzzE exercises the E() error constructor with arbitrary input. +func FuzzE(f *testing.F) { + f.Add("svc.Method", "something broke", true) + f.Add("", "", false) + f.Add("a.b.c.d.e.f", "unicode: \u00e9\u00e8\u00ea", true) + + f.Fuzz(func(t *testing.T, op, msg string, withErr bool) { + var underlying error + if withErr { + underlying = errors.New("wrapped") + } + + e := E(op, msg, underlying) + if e == nil { + t.Fatal("E() returned nil") + } + + s := e.Error() + if s == "" { + t.Fatal("Error() returned empty string") + } + + // Round-trip: Unwrap should return the underlying error + var coreErr *Error + if !errors.As(e, &coreErr) { + t.Fatal("errors.As failed for *Error") + } + if withErr && coreErr.Unwrap() == nil { + t.Fatal("Unwrap() returned nil with underlying error") + } + if !withErr && coreErr.Unwrap() != nil { + t.Fatal("Unwrap() returned non-nil without underlying error") + } + }) +} + +// FuzzServiceRegistration exercises service name registration with arbitrary names. +func FuzzServiceRegistration(f *testing.F) { + f.Add("myservice") + f.Add("") + f.Add("a/b/c") + f.Add("service with spaces") + f.Add("service\x00null") + + f.Fuzz(func(t *testing.T, name string) { + sm := newServiceManager() + + err := sm.registerService(name, struct{}{}) + if name == "" { + if err == nil { + t.Fatal("expected error for empty name") + } + return + } + if err != nil { + t.Fatalf("unexpected error for name %q: %v", name, err) + } + + // Retrieve should return the same service + got := sm.service(name) + if got == nil { + t.Fatalf("service %q not found after registration", name) + } + + // Duplicate registration should fail + err = sm.registerService(name, struct{}{}) + if err == nil { + t.Fatalf("expected duplicate error for name %q", name) + } + }) +} + +// FuzzMessageDispatch exercises action dispatch with concurrent registrations. +func FuzzMessageDispatch(f *testing.F) { + f.Add("hello") + f.Add("") + f.Add("test\nmultiline") + + f.Fuzz(func(t *testing.T, payload string) { + c := &Core{ + Features: &Features{}, + svc: newServiceManager(), + } + c.bus = newMessageBus(c) + + var received string + c.bus.registerAction(func(_ *Core, msg Message) error { + received = msg.(string) + return nil + }) + + err := c.bus.action(payload) + if err != nil { + t.Fatalf("action dispatch failed: %v", err) + } + if received != payload { + t.Fatalf("got %q, want %q", received, payload) + } + }) +} diff --git a/pkg/framework/core/interfaces.go b/pkg/framework/core/interfaces.go index 069d836..0bef944 100644 --- a/pkg/framework/core/interfaces.go +++ b/pkg/framework/core/interfaces.go @@ -3,7 +3,6 @@ package core import ( "context" "embed" - "sync" ) // This file defines the public API contracts (interfaces) for the services @@ -73,28 +72,13 @@ type Stoppable interface { // Core is the central application object that manages services, assets, and communication. type Core struct { - App any // GUI runtime (e.g., Wails App) - set by WithApp option - assets embed.FS - Features *Features - serviceLock bool - ipcMu sync.RWMutex - ipcHandlers []func(*Core, Message) error - queryMu sync.RWMutex - queryHandlers []QueryHandler - taskMu sync.RWMutex - taskHandlers []TaskHandler - serviceMu sync.RWMutex - services map[string]any - servicesLocked bool - startables []Startable - stoppables []Stoppable + App any // GUI runtime (e.g., Wails App) - set by WithApp option + assets embed.FS + Features *Features + svc *serviceManager + bus *messageBus } -var ( - instance *Core - instanceMu sync.RWMutex -) - // Config provides access to application configuration. type Config interface { // Get retrieves a configuration value by key and stores it in the 'out' variable. diff --git a/pkg/framework/core/message_bus.go b/pkg/framework/core/message_bus.go new file mode 100644 index 0000000..457ced2 --- /dev/null +++ b/pkg/framework/core/message_bus.go @@ -0,0 +1,119 @@ +package core + +import ( + "errors" + "sync" +) + +// messageBus owns the IPC action, query, and task dispatch. +// It is an unexported component used internally by Core. +type messageBus struct { + core *Core + + ipcMu sync.RWMutex + ipcHandlers []func(*Core, Message) error + + queryMu sync.RWMutex + queryHandlers []QueryHandler + + taskMu sync.RWMutex + taskHandlers []TaskHandler +} + +// newMessageBus creates an empty message bus bound to the given Core. +func newMessageBus(c *Core) *messageBus { + return &messageBus{core: c} +} + +// action dispatches a message to all registered IPC handlers. +func (b *messageBus) action(msg Message) error { + b.ipcMu.RLock() + handlers := append([]func(*Core, Message) error(nil), b.ipcHandlers...) + b.ipcMu.RUnlock() + + var agg error + for _, h := range handlers { + if err := h(b.core, msg); err != nil { + agg = errors.Join(agg, err) + } + } + return agg +} + +// registerAction adds a single IPC handler. +func (b *messageBus) registerAction(handler func(*Core, Message) error) { + b.ipcMu.Lock() + b.ipcHandlers = append(b.ipcHandlers, handler) + b.ipcMu.Unlock() +} + +// registerActions adds multiple IPC handlers. +func (b *messageBus) registerActions(handlers ...func(*Core, Message) error) { + b.ipcMu.Lock() + b.ipcHandlers = append(b.ipcHandlers, handlers...) + b.ipcMu.Unlock() +} + +// query dispatches a query to handlers until one responds. +func (b *messageBus) query(q Query) (any, bool, error) { + b.queryMu.RLock() + handlers := append([]QueryHandler(nil), b.queryHandlers...) + b.queryMu.RUnlock() + + for _, h := range handlers { + result, handled, err := h(b.core, q) + if handled { + return result, true, err + } + } + return nil, false, nil +} + +// queryAll dispatches a query to all handlers and collects all responses. +func (b *messageBus) queryAll(q Query) ([]any, error) { + b.queryMu.RLock() + handlers := append([]QueryHandler(nil), b.queryHandlers...) + b.queryMu.RUnlock() + + var results []any + var agg error + for _, h := range handlers { + result, handled, err := h(b.core, q) + if err != nil { + agg = errors.Join(agg, err) + } + if handled && result != nil { + results = append(results, result) + } + } + return results, agg +} + +// registerQuery adds a query handler. +func (b *messageBus) registerQuery(handler QueryHandler) { + b.queryMu.Lock() + b.queryHandlers = append(b.queryHandlers, handler) + b.queryMu.Unlock() +} + +// perform dispatches a task to handlers until one executes it. +func (b *messageBus) perform(t Task) (any, bool, error) { + b.taskMu.RLock() + handlers := append([]TaskHandler(nil), b.taskHandlers...) + b.taskMu.RUnlock() + + for _, h := range handlers { + result, handled, err := h(b.core, t) + if handled { + return result, true, err + } + } + return nil, false, nil +} + +// registerTask adds a task handler. +func (b *messageBus) registerTask(handler TaskHandler) { + b.taskMu.Lock() + b.taskHandlers = append(b.taskHandlers, handler) + b.taskMu.Unlock() +} diff --git a/pkg/framework/core/message_bus_test.go b/pkg/framework/core/message_bus_test.go new file mode 100644 index 0000000..e69ac95 --- /dev/null +++ b/pkg/framework/core/message_bus_test.go @@ -0,0 +1,146 @@ +package core + +import ( + "errors" + "sync" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMessageBus_Action_Good(t *testing.T) { + c, _ := New() + + var received []Message + c.bus.registerAction(func(_ *Core, msg Message) error { + received = append(received, msg) + return nil + }) + c.bus.registerAction(func(_ *Core, msg Message) error { + received = append(received, msg) + return nil + }) + + err := c.bus.action("hello") + assert.NoError(t, err) + assert.Len(t, received, 2) +} + +func TestMessageBus_Action_Bad(t *testing.T) { + c, _ := New() + + err1 := errors.New("handler1 failed") + err2 := errors.New("handler2 failed") + + c.bus.registerAction(func(_ *Core, msg Message) error { return err1 }) + c.bus.registerAction(func(_ *Core, msg Message) error { return nil }) + c.bus.registerAction(func(_ *Core, msg Message) error { return err2 }) + + err := c.bus.action("test") + assert.Error(t, err) + assert.ErrorIs(t, err, err1) + assert.ErrorIs(t, err, err2) +} + +func TestMessageBus_RegisterAction_Good(t *testing.T) { + c, _ := New() + + var coreRef *Core + c.bus.registerAction(func(core *Core, msg Message) error { + coreRef = core + return nil + }) + + _ = c.bus.action(nil) + assert.Same(t, c, coreRef, "handler should receive the Core reference") +} + +func TestMessageBus_Query_Good(t *testing.T) { + c, _ := New() + + c.bus.registerQuery(func(_ *Core, q Query) (any, bool, error) { + return "first", true, nil + }) + + result, handled, err := c.bus.query(TestQuery{Value: "test"}) + assert.NoError(t, err) + assert.True(t, handled) + assert.Equal(t, "first", result) +} + +func TestMessageBus_QueryAll_Good(t *testing.T) { + c, _ := New() + + c.bus.registerQuery(func(_ *Core, q Query) (any, bool, error) { + return "a", true, nil + }) + c.bus.registerQuery(func(_ *Core, q Query) (any, bool, error) { + return nil, false, nil // skips + }) + c.bus.registerQuery(func(_ *Core, q Query) (any, bool, error) { + return "b", true, nil + }) + + results, err := c.bus.queryAll(TestQuery{}) + assert.NoError(t, err) + assert.Equal(t, []any{"a", "b"}, results) +} + +func TestMessageBus_Perform_Good(t *testing.T) { + c, _ := New() + + c.bus.registerTask(func(_ *Core, t Task) (any, bool, error) { + return "done", true, nil + }) + + result, handled, err := c.bus.perform(TestTask{}) + assert.NoError(t, err) + assert.True(t, handled) + assert.Equal(t, "done", result) +} + +func TestMessageBus_ConcurrentAccess_Good(t *testing.T) { + c, _ := New() + + var wg sync.WaitGroup + const goroutines = 20 + + // Concurrent register + dispatch + for i := 0; i < goroutines; i++ { + wg.Add(2) + go func() { + defer wg.Done() + c.bus.registerAction(func(_ *Core, msg Message) error { return nil }) + }() + go func() { + defer wg.Done() + _ = c.bus.action("ping") + }() + } + + for i := 0; i < goroutines; i++ { + wg.Add(2) + go func() { + defer wg.Done() + c.bus.registerQuery(func(_ *Core, q Query) (any, bool, error) { return nil, false, nil }) + }() + go func() { + defer wg.Done() + _, _ = c.bus.queryAll(TestQuery{}) + }() + } + + for i := 0; i < goroutines; i++ { + wg.Add(2) + go func() { + defer wg.Done() + c.bus.registerTask(func(_ *Core, t Task) (any, bool, error) { return nil, false, nil }) + }() + go func() { + defer wg.Done() + _, _, _ = c.bus.perform(TestTask{}) + }() + } + + wg.Wait() +} diff --git a/pkg/framework/core/service_manager.go b/pkg/framework/core/service_manager.go new file mode 100644 index 0000000..80c208f --- /dev/null +++ b/pkg/framework/core/service_manager.go @@ -0,0 +1,94 @@ +package core + +import ( + "fmt" + "sync" +) + +// serviceManager owns the service registry and lifecycle tracking. +// It is an unexported component used internally by Core. +type serviceManager struct { + mu sync.RWMutex + services map[string]any + startables []Startable + stoppables []Stoppable + lockEnabled bool // WithServiceLock was called + locked bool // lock applied after New() completes +} + +// newServiceManager creates an empty service manager. +func newServiceManager() *serviceManager { + return &serviceManager{ + services: make(map[string]any), + } +} + +// registerService adds a named service to the registry. +// It also appends to startables/stoppables if the service implements those interfaces. +func (m *serviceManager) registerService(name string, svc any) error { + if name == "" { + return fmt.Errorf("core: service name cannot be empty") + } + m.mu.Lock() + defer m.mu.Unlock() + if m.locked { + return fmt.Errorf("core: service %q is not permitted by the serviceLock setting", name) + } + if _, exists := m.services[name]; exists { + return fmt.Errorf("core: service %q already registered", name) + } + m.services[name] = svc + + if s, ok := svc.(Startable); ok { + m.startables = append(m.startables, s) + } + if s, ok := svc.(Stoppable); ok { + m.stoppables = append(m.stoppables, s) + } + + return nil +} + +// service retrieves a registered service by name, or nil if not found. +func (m *serviceManager) service(name string) any { + m.mu.RLock() + svc, ok := m.services[name] + m.mu.RUnlock() + if !ok { + return nil + } + return svc +} + +// enableLock marks that the lock should be applied after initialisation. +func (m *serviceManager) enableLock() { + m.mu.Lock() + defer m.mu.Unlock() + m.lockEnabled = true +} + +// applyLock activates the service lock if it was enabled. +// Called once during New() after all options have been processed. +func (m *serviceManager) applyLock() { + m.mu.Lock() + defer m.mu.Unlock() + if m.lockEnabled { + m.locked = true + } +} + +// getStartables returns a snapshot copy of the startables slice. +func (m *serviceManager) getStartables() []Startable { + m.mu.RLock() + out := append([]Startable(nil), m.startables...) + m.mu.RUnlock() + return out +} + +// getStoppables returns a snapshot copy of the stoppables slice. +func (m *serviceManager) getStoppables() []Stoppable { + m.mu.RLock() + out := append([]Stoppable(nil), m.stoppables...) + m.mu.RUnlock() + return out +} diff --git a/pkg/framework/core/service_manager_test.go b/pkg/framework/core/service_manager_test.go new file mode 100644 index 0000000..fe408c4 --- /dev/null +++ b/pkg/framework/core/service_manager_test.go @@ -0,0 +1,132 @@ +package core + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestServiceManager_RegisterService_Good(t *testing.T) { + m := newServiceManager() + + err := m.registerService("svc1", &MockService{Name: "one"}) + assert.NoError(t, err) + + got := m.service("svc1") + assert.NotNil(t, got) + assert.Equal(t, "one", got.(*MockService).GetName()) +} + +func TestServiceManager_RegisterService_Bad(t *testing.T) { + m := newServiceManager() + + // Empty name + err := m.registerService("", &MockService{}) + assert.Error(t, err) + assert.Contains(t, err.Error(), "cannot be empty") + + // Duplicate + err = m.registerService("dup", &MockService{}) + assert.NoError(t, err) + err = m.registerService("dup", &MockService{}) + assert.Error(t, err) + assert.Contains(t, err.Error(), "already registered") + + // Locked + m2 := newServiceManager() + m2.enableLock() + m2.applyLock() + err = m2.registerService("late", &MockService{}) + assert.Error(t, err) + assert.Contains(t, err.Error(), "serviceLock") +} + +func TestServiceManager_ServiceNotFound_Good(t *testing.T) { + m := newServiceManager() + assert.Nil(t, m.service("nonexistent")) +} + +func TestServiceManager_Startables_Good(t *testing.T) { + m := newServiceManager() + + s1 := &MockStartable{} + s2 := &MockStartable{} + + _ = m.registerService("s1", s1) + _ = m.registerService("s2", s2) + + startables := m.getStartables() + assert.Len(t, startables, 2) + + // Verify order matches registration order + assert.Same(t, s1, startables[0]) + assert.Same(t, s2, startables[1]) + + // Verify it's a copy — mutating the slice doesn't affect internal state + startables[0] = nil + assert.Len(t, m.getStartables(), 2) + assert.NotNil(t, m.getStartables()[0]) +} + +func TestServiceManager_Stoppables_Good(t *testing.T) { + m := newServiceManager() + + s1 := &MockStoppable{} + s2 := &MockStoppable{} + + _ = m.registerService("s1", s1) + _ = m.registerService("s2", s2) + + stoppables := m.getStoppables() + assert.Len(t, stoppables, 2) + + // Stoppables are returned in registration order; Core.ServiceShutdown reverses them + assert.Same(t, s1, stoppables[0]) + assert.Same(t, s2, stoppables[1]) +} + +func TestServiceManager_Lock_Good(t *testing.T) { + m := newServiceManager() + + // Register before lock — should succeed + err := m.registerService("early", &MockService{}) + assert.NoError(t, err) + + // Enable and apply lock + m.enableLock() + m.applyLock() + + // Register after lock — should fail + err = m.registerService("late", &MockService{}) + assert.Error(t, err) + assert.Contains(t, err.Error(), "serviceLock") + + // Early service is still accessible + assert.NotNil(t, m.service("early")) +} + +func TestServiceManager_LockNotAppliedWithoutEnable_Good(t *testing.T) { + m := newServiceManager() + m.applyLock() // applyLock without enableLock should be a no-op + + err := m.registerService("svc", &MockService{}) + assert.NoError(t, err) +} + +type mockFullLifecycle struct{} + +func (m *mockFullLifecycle) OnStartup(_ context.Context) error { return nil } +func (m *mockFullLifecycle) OnShutdown(_ context.Context) error { return nil } + +func TestServiceManager_LifecycleBoth_Good(t *testing.T) { + m := newServiceManager() + + svc := &mockFullLifecycle{} + err := m.registerService("both", svc) + assert.NoError(t, err) + + // Should appear in both startables and stoppables + assert.Len(t, m.getStartables(), 1) + assert.Len(t, m.getStoppables(), 1) +}