fix(boatstack): resolve managed-PR task graph in both feature layouts - #137
Merged
Conversation
managedPRSources hardcoded <feature>/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.
… 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.
Contributor
Author
|
Correction: this lab change does require a release note (CI's Generated distribution gate, not part of |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
managedPRSources(pr.go) hardcoded the task graph as<feature>/compiled/tasks.jsonwith no fallback and handed it toCheckApprovalLock, which hashes whateverTasksPathit is given (plan.go—SHA256File(options.TasksPath)). A feature whoseplan.lock.jsonwas written in the older feature-root layout (ActivatePlanwithOutDir = <feature>, sotasks.jsonsits at the feature root and there is nocompiled/dir — e.g.cta-transport-feedback) has nocompiled/tasks.json.SHA256Filefails, thetask_graphlabel mismatches, and managed PR preparation returns "managed PR requires a current build lock" — blocking the ship-gate even though build/test/review all passed.The very next block already resolved evidence across both layouts; the task graph was the only artifact missing that two-layout resolution.
Fix
Introduce one shared
featureArtifactPathresolver (canonical location first, alternate as fallback) and route both the task graph (compiled canonical → root fallback) and evidence (root canonical → compiled fallback) through it, so the two layout resolutions can never silently diverge again. Evidence behavior is preserved exactly.Scope guard honored: no change to
plan.go'sCheckApprovalLock(it correctly hashes whateverTasksPathit is handed) or toActivatePlan/OutDirwiring. The defect was solelymanagedPRSourcesassuming one layout.Conformance tests (root up)
TestFeatureArtifactPathResolvesBothLayouts— table-driven unit test of the resolution rule itself: only-compiled → compiled; only-root → root; both → canonical; neither → last candidate.TestManagedPRSourcesAcceptsBothTaskGraphLayouts— table-driven integration over{compiled, feature-root}; the feature-root case asserts the fixture is genuinely the old layout (roottasks.json, nocompiled/) and thatmanagedPRSourcessucceeds (lock'stask_graphhash matches the resolved root file). Existing compiled-layout managed-PR tests stay green.Refactored the existing
activateManagedFeaturehelper intoactivateManagedFeatureLayout(t, repo, feature, compiled bool); the old signature remains as a thin wrapper so every existing caller is unchanged.Verification
From
product-engineering-loop:go build ./...,go vet ./...,go test ./...all pass (new unit + integration cases plus the full existing suite, includingconfig_documentation_test.go— no doc drift, so no release note required).