From 30be5aaca83e35210ad6b567465fdaa0d75e293c Mon Sep 17 00:00:00 2001 From: Alex Dunmow Date: Sun, 9 Aug 2026 10:16:13 +0800 Subject: [PATCH] fix: discover active BlockNinja workspace --- AGENTS.md | 2 +- README.md | 19 ++--- ...ocation-independent-workspace-discovery.md | 36 ++++++++++ lint_pipeline.go | 12 ++-- main.go | 2 + pathutil.go | 72 ++++++++++++++++++- plugin_sdk_versions.go | 4 -- registry_test.go | 2 +- testdata/golden/clean/expected.stdout | 2 +- testdata/golden/nomod/expected.stdout | 2 +- workspace_test.go | 64 +++++++++++++++++ 11 files changed, 194 insertions(+), 23 deletions(-) create mode 100644 docs/adr/0001-location-independent-workspace-discovery.md create mode 100644 workspace_test.go diff --git a/AGENTS.md b/AGENTS.md index 8d0b0ff..9b2b7dd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,7 +19,7 @@ check-safety/ ## Invariants when editing - This is **standalone** — no imports from sibling repos (CMS, orchestrator, plugins). The two vendored packages above are the only CMS surfaces it depends on, and they are intentional copies. -- `blockNinjaRepoRoot()` and `orchestratorRepoRoot()` in `lint_pipeline.go` hardcode the consolidated layout (`~/src/blockninja/{cms,orchestrator}`). Update them if the tree layout changes. +- `blockNinjaRepoRoot()` and `orchestratorRepoRoot()` discover the consolidated workspace from the current directory, executable, or source location, with `~/src/blockninja` retained only as a legacy fallback. Keep discovery location-independent. - Golden tests are characterisation tests — if you intentionally change a check's output, run `make test-update` and commit the new fixture. - The CMS Makefile's `safety-check` and `install-safety-checker` targets shell out into this directory (`cd ../check-safety`). Keep the CLI contract stable: `check-safety [--flags]`. diff --git a/README.md b/README.md index 63bbb5e..818fdf2 100644 --- a/README.md +++ b/README.md @@ -1,9 +1,9 @@ # check-safety -Static safety checker for the BlockNinja codebase. Walks a target tree, runs 33 invariant +Static safety checker for the BlockNinja codebase. Walks a target tree, runs 34 invariant checks across Go and frontend sources, and exits non-zero on any violation. -Lives at `~/src/blockninja/check-safety/` as a standalone Go module, alongside `cms/`, +Lives in the consolidated BlockNinja workspace as a standalone Go module, alongside `cms/`, `orchestrator/`, `core/`, the plugins, and the sites. It is intentionally standalone — no imports from sibling repos except two vendored packages (see [Vendored packages](#how-it-stays-in-sync-with-cms)). @@ -29,7 +29,7 @@ If you run it against the top of the consolidated BlockNinja repo, it prints a h | Flag | Meaning | |------|---------| | `` (positional) | Repo/subtree to scan. Defaults to `.`. If it contains `plugin.mod`, it is auto-registered as a plugin root so plugin checks run. | -| `--orchestrator` | Scan the orchestrator backend only (resolved from the hardcoded consolidated layout), regardless of the positional target. | +| `--orchestrator` | Scan the orchestrator backend only (resolved from the discovered consolidated workspace), regardless of the positional target. | | `--plugin-dir [more…]` | Register one or more plugin roots (dirs with `plugin.mod`). Alias: `--plugin-dirs`. Consumes args until the next `--flag`. | | `--plugin-pages [more…]` | Register frontend source dirs directly as plugin page targets (for the frontend checks). Consumes args until the next `--flag`. | | `--verbose` / `-v` | Print every check (including `OK`/`SKIP`), not just failures and warnings. | @@ -46,7 +46,7 @@ line with their findings indented beneath, followed by one tally line. A clean r ``` check-safety /home/alex/src/blockninja/cms -32 checks: 20 ok 12 skip -> OK +34 checks: 20 ok 14 skip -> OK ``` A run with problems: @@ -58,7 +58,7 @@ FAIL 15 1 err.Error() leak(s) to HTTP clients — log via slog.Error() and retur WARN 2e 1 any usage in changed lines (showing 1, --all-any to scan all) web/src/api.ts:12 [go] data: any -32 checks: 27 ok 3 skip 1 warn 1 fail -> FAIL +34 checks: 29 ok 3 skip 1 warn 1 fail -> FAIL ``` Pass `--verbose` to see every check's status. Exit code is `1` on any `FAIL`, `2` on a hard @@ -124,6 +124,9 @@ tool prints in each check header): | 21 | Preset validation | Plugin `presets.json` type-safe-unmarshals against `theme.Theme`. | | 22 | HTML sanitization | No hand-rolled HTML sanitization — use bluemonday. | | 28 | Admin toolbar | Public page handlers inject the admin toolbar data. | +| 29 | No legacy Go plugins | No `-buildmode=plugin` targets remain after the wasm migration gate is enabled. | +| 30 | Render performance | Home-page render p50 remains within the recorded baseline. | +| 32 | Schema purity | Every table in `schema.sql` originates from a core migration (31 is retired). | > The canonical numbered list also lives in the comment block at the top of `main.go`; keep > the two in sync when adding or renumbering a check. @@ -136,9 +139,9 @@ modules, so they are copied in rather than imported. When CMS changes the theme `LogDeferredError`, re-copy them — that drift is precisely what the preset-validation check (21) surfaces. -`blockNinjaRepoRoot()` and `orchestratorRepoRoot()` in `lint_pipeline.go` hardcode the -consolidated layout (`~/src/blockninja/{cms,orchestrator}`). Update them if the tree layout -changes. +`blockNinjaRepoRoot()` and `orchestratorRepoRoot()` discover the consolidated workspace by +walking upward from the current directory, executable, and compiled source location. The +historical `~/src/blockninja` layout remains a last-resort fallback for installed binaries. ## Adding a new check diff --git a/docs/adr/0001-location-independent-workspace-discovery.md b/docs/adr/0001-location-independent-workspace-discovery.md new file mode 100644 index 0000000..7c94159 --- /dev/null +++ b/docs/adr/0001-location-independent-workspace-discovery.md @@ -0,0 +1,36 @@ +# Location-independent workspace discovery + +Decided 2026-08-09. The safety runner located the CMS and orchestrator through +hard-coded `$HOME/src/blockninja` paths and compared repositories as raw +absolute strings. The shared workspace can also be mounted at paths such as +`/srv/agent-work/blockninja`. Even when both paths address the same files, the +string mismatch caused an explicit orchestrator scan to be classified as CMS; +the CMS protobuf namespace filter then removed every orchestrator RPC. + +## Decision + +- Discover the consolidated workspace by walking upward from the current + directory, executable directory, and compiled source location. +- Recognize a workspace only when the independent `cms`, `orchestrator`, and + `check-safety` Go modules are present. +- Retain `$HOME/src/blockninja` only as the final compatibility fallback for an + installed binary launched outside the workspace. +- Compare existing paths with filesystem identity and resolved symlinks before + falling back to cleaned absolute strings. +- Cover non-home workspaces and aliased paths with regression tests. +- Reconcile the registry assertion, golden snapshots, and check catalog with + the already-shipped 34th schema-purity check so the runner's own suite is a + reliable validation gate again. +- Remove the obsolete core-SDK version helper left behind when check 2c moved + to the published plugin SDK, restoring strict unused-code linting. + +## Consequences + +- `check-safety ../orchestrator` selects the `orchestrator.*` RPC namespace from + any consolidated workspace location. +- Default CMS scans and `--orchestrator` resolve siblings from the active + checkout instead of silently crossing into another checkout. +- Symlink, bind-mount, and alternate mount-path aliases are treated as the same + repository when the operating system reports the same underlying directory. + +Keywords: check-safety, workspace discovery, /srv/agent-work, $HOME/src/blockninja, orchestrator, CMS, RPC namespace, samePath, symlink, bind mount diff --git a/lint_pipeline.go b/lint_pipeline.go index 3a0e730..178fbfe 100644 --- a/lint_pipeline.go +++ b/lint_pipeline.go @@ -427,19 +427,19 @@ func copyFile(srcPath, dstPath string, mode os.FileMode) error { } func blockNinjaRepoRoot() string { - home, err := os.UserHomeDir() - if err != nil { + workspace := consolidatedWorkspaceRoot() + if workspace == "" { return "" } - return filepath.Join(home, "src", "blockninja", "cms") + return filepath.Join(workspace, "cms") } func orchestratorRepoRoot() string { - home, err := os.UserHomeDir() - if err != nil { + workspace := consolidatedWorkspaceRoot() + if workspace == "" { return "" } - return filepath.Join(home, "src", "blockninja", "orchestrator") + return filepath.Join(workspace, "orchestrator") } func ensureDefaultFrontendConfigs(repoRoot, packageRoot string) error { diff --git a/main.go b/main.go index 79509a3..16d8192 100644 --- a/main.go +++ b/main.go @@ -35,6 +35,8 @@ // 27. No hand-rolled HTML sanitization — use bluemonday // 28. Public page handlers inject admin toolbar data // 29. No -buildmode=plugin targets in ported plugin repos (disabled until WO-WZ-017) +// 30. Home-page render performance stays within its recorded baseline +// 32. schema.sql contains only tables created by core migrations (31 retired) package main import ( diff --git a/pathutil.go b/pathutil.go index 0ed14e4..7b65309 100644 --- a/pathutil.go +++ b/pathutil.go @@ -3,6 +3,7 @@ package main import ( "os" "path/filepath" + "runtime" "slices" "strings" ) @@ -66,7 +67,76 @@ func samePath(a, b string) bool { return false } - return absA == absB + infoA, errA := os.Stat(absA) + infoB, errB := os.Stat(absB) + if errA == nil && errB == nil && os.SameFile(infoA, infoB) { + return true + } + + realA, errA := filepath.EvalSymlinks(absA) + realB, errB := filepath.EvalSymlinks(absB) + if errA == nil && errB == nil { + return realA == realB + } + return filepath.Clean(absA) == filepath.Clean(absB) +} + +// consolidatedWorkspaceRoot locates the parent that owns the independent CMS, +// orchestrator, and check-safety repositories. The checkout may live anywhere +// (for example /srv/agent-work/blockninja), so discovery starts from runtime +// locations before retaining the historical ~/src/blockninja fallback. +func consolidatedWorkspaceRoot() string { + var candidates []string + if cwd, err := os.Getwd(); err == nil { + candidates = append(candidates, cwd) + } + if executable, err := os.Executable(); err == nil { + candidates = append(candidates, filepath.Dir(executable)) + } + if _, sourceFile, _, ok := runtime.Caller(0); ok { + candidates = append(candidates, filepath.Dir(sourceFile)) + } + if home, err := os.UserHomeDir(); err == nil { + candidates = append(candidates, filepath.Join(home, "src", "blockninja")) + } + + seen := make(map[string]bool, len(candidates)) + for _, candidate := range candidates { + root := findConsolidatedWorkspaceRoot(candidate) + if root == "" || seen[root] { + continue + } + seen[root] = true + return root + } + + if home, err := os.UserHomeDir(); err == nil { + return filepath.Join(home, "src", "blockninja") + } + return "" +} + +func findConsolidatedWorkspaceRoot(start string) string { + current, err := filepath.Abs(start) + if err != nil { + return "" + } + if info, statErr := os.Stat(current); statErr == nil && !info.IsDir() { + current = filepath.Dir(current) + } + + for { + if fileExists(filepath.Join(current, "cms", "backend", "go.mod")) && + fileExists(filepath.Join(current, "orchestrator", "backend", "go.mod")) && + fileExists(filepath.Join(current, "check-safety", "go.mod")) { + return current + } + parent := filepath.Dir(current) + if parent == current { + return "" + } + current = parent + } } func normalizeDisplayLabel(label string) string { diff --git a/plugin_sdk_versions.go b/plugin_sdk_versions.go index 1c90b23..e94a02b 100644 --- a/plugin_sdk_versions.go +++ b/plugin_sdk_versions.go @@ -16,10 +16,6 @@ type pluginGoModViolation struct { detail string } -func currentCMSCoreSDKVersion() (string, error) { - return readRequiredModuleVersion(filepath.Join(blockNinjaRepoRoot(), "backend", "go.mod"), blockCoreImportPrefix) -} - // currentCMSPluginSDKVersion anchors the fleet's pluginsdk pin to the CMS // backend's. Empty (no error) while the CMS itself hasn't migrated to // pluginsdk yet — version enforcement is skipped during that transition. diff --git a/registry_test.go b/registry_test.go index e2ad017..ae21b66 100644 --- a/registry_test.go +++ b/registry_test.go @@ -18,7 +18,7 @@ func TestRegistryOrder(t *testing.T) { "1", "2", "2b", "2c", "2d", "2e", "2f", "3", "3b", "4", "5", "6", "7", "8", "9", "10", "10b", "10disc", "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "21", "22", - "28", "29", "30", + "28", "29", "30", "32", } ordered := make([]Check, len(registry)) diff --git a/testdata/golden/clean/expected.stdout b/testdata/golden/clean/expected.stdout index c780c9e..fc4d2ef 100644 --- a/testdata/golden/clean/expected.stdout +++ b/testdata/golden/clean/expected.stdout @@ -1,2 +1,2 @@ check-safety FIXTURE_DIR -33 checks: 20 ok 13 skip -> OK +34 checks: 20 ok 14 skip -> OK diff --git a/testdata/golden/nomod/expected.stdout b/testdata/golden/nomod/expected.stdout index f43a6f7..a9ab573 100644 --- a/testdata/golden/nomod/expected.stdout +++ b/testdata/golden/nomod/expected.stdout @@ -10,4 +10,4 @@ FAIL 15 1 err.Error() leak(s) to HTTP clients — log via slog.Error() and retur FAIL 17 1 TODO marker(s) found — ship explicit behavior, not placeholders internal/service/handler.go:14 // TODO: add proper initialisation -33 checks: 14 ok 14 skip 5 fail -> FAIL +34 checks: 14 ok 15 skip 5 fail -> FAIL diff --git a/workspace_test.go b/workspace_test.go new file mode 100644 index 0000000..e7ee50a --- /dev/null +++ b/workspace_test.go @@ -0,0 +1,64 @@ +package main + +import ( + "os" + "path/filepath" + "slices" + "testing" +) + +func TestRepoRootsFollowConsolidatedWorkspaceContainingCWD(t *testing.T) { + workspace := scaffoldConsolidatedWorkspace(t) + t.Chdir(filepath.Join(workspace, "check-safety")) + + if got, want := blockNinjaRepoRoot(), filepath.Join(workspace, "cms"); got != want { + t.Fatalf("blockNinjaRepoRoot() = %q, want %q", got, want) + } + if got, want := orchestratorRepoRoot(), filepath.Join(workspace, "orchestrator"); got != want { + t.Fatalf("orchestratorRepoRoot() = %q, want %q", got, want) + } + + orchestratorBackend := filepath.Join(workspace, "orchestrator", "backend") + if got := inferAllowedPackagePrefixes(filepath.Join(workspace, "orchestrator"), orchestratorBackend); !slices.Equal(got, []string{"orchestrator."}) { + t.Fatalf("orchestrator prefixes = %v, want [orchestrator.]", got) + } +} + +func TestSamePathRecognizesFilesystemAliases(t *testing.T) { + realDir := filepath.Join(t.TempDir(), "real") + if err := os.Mkdir(realDir, 0755); err != nil { + t.Fatalf("mkdir real directory: %v", err) + } + aliasDir := filepath.Join(t.TempDir(), "alias") + if err := os.Symlink(realDir, aliasDir); err != nil { + t.Fatalf("symlink alias: %v", err) + } + + if !samePath(realDir, aliasDir) { + t.Fatalf("samePath(%q, %q) = false, want true", realDir, aliasDir) + } +} + +func scaffoldConsolidatedWorkspace(t *testing.T) string { + t.Helper() + workspace := t.TempDir() + for _, dir := range []string{ + filepath.Join(workspace, "cms", "backend"), + filepath.Join(workspace, "orchestrator", "backend"), + filepath.Join(workspace, "check-safety"), + } { + if err := os.MkdirAll(dir, 0755); err != nil { + t.Fatalf("mkdir %s: %v", dir, err) + } + } + for _, path := range []string{ + filepath.Join(workspace, "cms", "backend", "go.mod"), + filepath.Join(workspace, "orchestrator", "backend", "go.mod"), + filepath.Join(workspace, "check-safety", "go.mod"), + } { + if err := os.WriteFile(path, []byte("module example.com/test\n"), 0644); err != nil { + t.Fatalf("write %s: %v", path, err) + } + } + return workspace +}