From e28aa2d122017d583690f752c34a792d1690fa32 Mon Sep 17 00:00:00 2001 From: bigboateng Date: Thu, 23 Jul 2026 17:22:27 +0100 Subject: [PATCH 1/2] fix(boatstack): resolve managed-PR task graph in both feature layouts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit managedPRSources hardcoded /compiled/tasks.json with no fallback and handed it to CheckApprovalLock, which hashes whatever TasksPath it is given. Features whose plan.lock.json was written in the older feature-root layout (tasks.json at the feature root, no compiled/ dir) have no compiled/tasks.json, so SHA256File fails, the task_graph label mismatches, and managed PR preparation reports "requires a current build lock" — blocking the ship-gate even though build/test/review all passed. Introduce one shared featureArtifactPath resolver (canonical location first, alternate as fallback) and route both the task graph (compiled canonical) and evidence (root canonical) through it, so the two layout resolutions can never silently diverge again. No change to CheckApprovalLock or ActivatePlan/OutDir wiring. Adds a table-driven unit test of the resolver and a table-driven managed PR integration test over both {compiled, feature-root} layouts. --- .../product-engineering-loop/pr.go | 26 ++++- .../product-engineering-loop/pr_test.go | 96 ++++++++++++++++++- 2 files changed, 116 insertions(+), 6 deletions(-) diff --git a/labs/12-product-engineering-loop/product-engineering-loop/pr.go b/labs/12-product-engineering-loop/product-engineering-loop/pr.go index 6119b9525..bb96c4262 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/pr.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/pr.go @@ -391,6 +391,25 @@ func relativeSource(repo, path, kind string) (PRSource, error) { return PRSource{Kind: kind, Path: relative, SHA256: hash}, nil } +// featureArtifactPath resolves a feature artifact that may live in either the +// newer compiled/ subdirectory or the older feature-root layout. Candidates are +// tried in priority order (each artifact's canonical location first); the first +// that exists wins. When none exist the last candidate is returned so the +// downstream check reports a clear, canonical error path rather than a guessed +// one. This keeps the task graph and evidence resolution on one shared rule so +// the two layouts can never silently diverge. +func featureArtifactPath(directory string, candidates ...string) string { + var last string + for _, name := range candidates { + path := filepath.Join(directory, name) + last = path + if fileExists(path) { + return path + } + } + return last +} + func managedPRSources(repo, feature string) ([]PRSource, map[string]string, error) { directory := filepath.Join(repo, ".product-loop", "features", feature) planPath := filepath.Join(directory, "plan.md") @@ -411,7 +430,7 @@ func managedPRSources(repo, feature string) ([]PRSource, map[string]string, erro return nil, nil, fmt.Errorf("managed PR requires current approval: %w", err) } } - tasksPath := filepath.Join(directory, "compiled", "tasks.json") + tasksPath := featureArtifactPath(directory, filepath.Join("compiled", "tasks.json"), "tasks.json") if err := CheckApprovalLock(ApprovalOptions{ SourcePlanPath: check.SourcePlanPath, SpecPath: check.SpecPath, @@ -422,10 +441,7 @@ func managedPRSources(repo, feature string) ([]PRSource, map[string]string, erro }); err != nil { return nil, nil, fmt.Errorf("managed PR requires a current build lock: %w", err) } - evidencePath := filepath.Join(directory, "evidence.md") - if !fileExists(evidencePath) { - evidencePath = filepath.Join(directory, "compiled", "evidence.md") - } + evidencePath := featureArtifactPath(directory, "evidence.md", filepath.Join("compiled", "evidence.md")) if err := checkNonEmptyFile(evidencePath, "feature evidence"); err != nil { return nil, nil, err } diff --git a/labs/12-product-engineering-loop/product-engineering-loop/pr_test.go b/labs/12-product-engineering-loop/product-engineering-loop/pr_test.go index 1c75eb955..32c7a8bd9 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/pr_test.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/pr_test.go @@ -156,6 +156,15 @@ func TestAdHocPRContextAndPreviewAreEvidenceLimited(t *testing.T) { } func activateManagedFeature(t *testing.T, repo, feature string) string { + t.Helper() + return activateManagedFeatureLayout(t, repo, feature, true) +} + +// activateManagedFeatureLayout activates a feature in either the newer compiled/ +// layout (compiled=true → OutDir=/compiled) or the older feature-root +// layout (compiled=false → OutDir=, tasks.json at the feature root, no +// compiled/ dir), so managed-PR resolution can be exercised against both. +func activateManagedFeatureLayout(t *testing.T, repo, feature string, compiled bool) string { t.Helper() directory := filepath.Join(repo, ".product-loop", "features", feature) if err := os.MkdirAll(directory, 0o755); err != nil { @@ -193,9 +202,13 @@ func activateManagedFeature(t *testing.T, repo, feature string) string { approvalPath = filepath.Join(directory, "approval.md") writeApprovalReceipt(t, approvalPath, check.Fingerprint) } + outDir := directory + if compiled { + outDir = filepath.Join(directory, "compiled") + } if err := ActivatePlan(ActivationOptions{ PlanPath: filepath.Join(directory, "plan.md"), ApprovalPath: approvalPath, - OutDir: filepath.Join(directory, "compiled"), OutputPath: filepath.Join(directory, "plan.lock.json"), + OutDir: outDir, OutputPath: filepath.Join(directory, "plan.lock.json"), SourceCommit: runGit(t, repo, "rev-parse", "HEAD"), }); err != nil { t.Fatal(err) @@ -257,6 +270,87 @@ No migration; revert the feature commit. return directory } +// TestFeatureArtifactPathResolvesBothLayouts pins the layout-resolution rule +// from the root up, independent of PR wiring: the canonical location is tried +// first, the alternate is the fallback, and when neither exists the last +// candidate is returned for a clear downstream error path. +func TestFeatureArtifactPathResolvesBothLayouts(t *testing.T) { + for _, test := range []struct { + name string + present []string + want string + }{ + {name: "only compiled", present: []string{filepath.Join("compiled", "tasks.json")}, want: filepath.Join("compiled", "tasks.json")}, + {name: "only root", present: []string{"tasks.json"}, want: "tasks.json"}, + {name: "both prefer canonical", present: []string{filepath.Join("compiled", "tasks.json"), "tasks.json"}, want: filepath.Join("compiled", "tasks.json")}, + {name: "neither returns last", present: nil, want: "tasks.json"}, + } { + t.Run(test.name, func(t *testing.T) { + directory := t.TempDir() + for _, rel := range test.present { + path := filepath.Join(directory, rel) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + } + got := featureArtifactPath(directory, filepath.Join("compiled", "tasks.json"), "tasks.json") + if got != filepath.Join(directory, test.want) { + t.Fatalf("resolved %q, want %q", got, filepath.Join(directory, test.want)) + } + }) + } +} + +// TestManagedPRSourcesAcceptsBothTaskGraphLayouts is the conformance guard for +// the build-lock layout bug: a feature activated in the older feature-root +// layout (tasks.json at the feature root, no compiled/ dir) must resolve its +// build lock exactly like the newer compiled layout, so the ship-gate is not +// blocked after build/test/review all passed. +func TestManagedPRSourcesAcceptsBothTaskGraphLayouts(t *testing.T) { + for _, test := range []struct { + name string + compiled bool + }{ + {name: "compiled layout", compiled: true}, + {name: "feature-root layout", compiled: false}, + } { + t.Run(test.name, func(t *testing.T) { + repo := prTestRepo(t) + feature := "cta-transport-feedback" + directory := activateManagedFeatureLayout(t, repo, feature, test.compiled) + + if !test.compiled { + if !fileExists(filepath.Join(directory, "tasks.json")) { + t.Fatal("feature-root fixture must place tasks.json at the feature root") + } + if fileExists(filepath.Join(directory, "compiled", "tasks.json")) { + t.Fatal("feature-root fixture must not have a compiled/tasks.json") + } + } + + sources, statuses, err := managedPRSources(repo, feature) + if err != nil { + t.Fatalf("managed PR sources must resolve for the %s: %v", test.name, err) + } + if statuses["test"] != "PASS" || statuses["review"] != "PASS_WITH_GAPS" { + t.Fatalf("unexpected gate statuses: %+v", statuses) + } + found := false + for _, source := range sources { + if source.Kind == "plan_lock" { + found = true + } + } + if !found { + t.Fatalf("managed PR sources missing plan_lock: %+v", sources) + } + }) + } +} + func TestManagedPRRechecksCurrentAuthorizationAndGapPolicy(t *testing.T) { t.Run("policy activation omits approval source", func(t *testing.T) { repo := prTestRepoConfigured(t, func(config *ProjectConfig) { From c9e110ae06ef139d8ae336262558bc8bf21a1e2a Mon Sep 17 00:00:00 2001 From: bigboateng Date: Thu, 23 Jul 2026 17:32:58 +0100 Subject: [PATCH 2/2] docs(boatstack): require a release note per lab change; add AGENTS.md guard CI's 'Generated distribution' check (scripts/release_notes.py check-policy) blocks any PR that touches labs/12-product-engineering-loop without adding a new release note. That check is not part of go test, so a green local test run hides it and costs a fail/push/re-PR round trip. Add AGENTS.md in product-engineering-loop documenting the always-required release note, the fragment contract, and the local preflight command that mirrors CI. Also add the missing release note for the task-graph layout fix. --- ...2026-07-23-managed-pr-task-graph-layout.md | 5 ++ .../product-engineering-loop/AGENTS.md | 62 +++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-23-managed-pr-task-graph-layout.md create mode 100644 labs/12-product-engineering-loop/product-engineering-loop/AGENTS.md diff --git a/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-23-managed-pr-task-graph-layout.md b/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-23-managed-pr-task-graph-layout.md new file mode 100644 index 000000000..caf978dba --- /dev/null +++ b/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-23-managed-pr-task-graph-layout.md @@ -0,0 +1,5 @@ +### Resolve managed-PR build lock across both feature layouts + +Boatstack managed PR preparation no longer blocks the ship gate for features whose plan lock was written in the older feature-root layout. Previously `managedPRSources` looked for the task graph only at `/compiled/tasks.json`; a feature activated with `tasks.json` at the feature root (no `compiled/` directory) failed the build-lock check with `task_graph` mismatch and reported "managed PR requires a current build lock" even after build, test, and review had passed. + +The task graph is now resolved across both layouts through a single shared resolver that also backs evidence resolution, preferring each artifact's canonical location and falling back to the alternate. Features activated in either layout prepare their managed PR identically; no migration is required. diff --git a/labs/12-product-engineering-loop/product-engineering-loop/AGENTS.md b/labs/12-product-engineering-loop/product-engineering-loop/AGENTS.md new file mode 100644 index 000000000..96c520df7 --- /dev/null +++ b/labs/12-product-engineering-loop/product-engineering-loop/AGENTS.md @@ -0,0 +1,62 @@ +# Agent guide — Boatstack product-engineering-loop + +Read this before opening a PR that touches anything under +`labs/12-product-engineering-loop/`. + +## Every PR that changes this lab REQUIRES a new release note + +CI runs `scripts/release_notes.py check-policy` (the **Generated distribution** +check). It fails the PR if the diff touches **any** file under +`labs/12-product-engineering-loop/` — source, tests, docs, scripts, anything — +without **adding** a new release-note fragment. This check is **not** part of +`go test ./...`, so a green local test run does **not** mean you are done. Skipping +the note costs a full CI round trip (fail → add note → push → re-run). + +### Add the note in the same commit as your change + +Create one new file per PR: + +``` +labs/12-product-engineering-loop/boatstack-distribution/release-notes/YYYY-MM-DD-.md +``` + +Contract (enforced by `validate_release_note`): + +- **Name:** `YYYY-MM-DD-.md`, slug lowercase `[a-z0-9]` words joined by `-`. +- **First line:** a level-three Markdown heading — `### ` (no leading blank line). +- **Body:** at least one non-empty line after the heading describing **user impact** + (what changed for someone using Boatstack, not the code mechanics). +- **Encoding/EOL:** UTF-8, and the file must end with a trailing newline. +- **Append-only:** never edit or delete an existing note. To correct a shipped + note, add a new correction fragment. Only added (`A`) files under + `release-notes/` are allowed in the diff. + +### Verify locally before you push — avoid the CI round trip + +Commit your change **and** the note, then run the same policy CI runs: + +``` +# format check on the notes directory +python3 labs/12-product-engineering-loop/scripts/release_notes.py \ + validate --root labs/12-product-engineering-loop/boatstack-distribution/release-notes + +# append-only + "note present for lab changes" against origin/main (needs a clean, +# committed tree — it inspects the committed PR diff, not the working tree) +python3 labs/12-product-engineering-loop/scripts/release_notes.py \ + preflight --repo . --base-branch main +``` + +`preflight` fetches `origin/main` and checks the committed diff. `PASS` means the +**Generated distribution** check will pass; `BLOCKED` prints exactly what to fix. + +## Other checks that are not in `go test` + +- **Repository conformance** and **Runtime** (windows/macos/ubuntu) run in CI. + Locally, always run `go build ./...`, `go vet ./...`, and `go test ./...` from + `product-engineering-loop`, plus `python3 -m unittest tests.test_product_loop` + from `labs/12-product-engineering-loop` for the Python surface. + +## PR body honesty + +Do not write "no release note required" for a change under this lab — a note is +always required. State which note you added.