diff --git a/src/Polyphony/Commands/NextReadyObservationScope.cs b/src/Polyphony/Commands/NextReadyObservationScope.cs new file mode 100644 index 00000000..1bc49e51 --- /dev/null +++ b/src/Polyphony/Commands/NextReadyObservationScope.cs @@ -0,0 +1,98 @@ +using Polyphony.Infrastructure.Processes; + +namespace Polyphony.Commands; + +/// +/// Per-item observation context shared by every requirement-kind composer +/// inside . Built once per work item; +/// passed to each composer so observers SHARE underlying I/O (slug +/// resolution, plan-PR list, plan-PR poll) instead of issuing them N +/// times — see the smoking-gun perf concern in +/// files/closed-loop-state-plan.md §3.2. +/// +/// +/// +/// Plug-in contract for sibling observers (PR #3 ChildrenSeeded, +/// PR #4 Implementation, …). When adding a new requirement-kind +/// observer that needs a per-item signal: +/// +/// +/// Add the signal as a property on this record (e.g. +/// PlannedTagPresent for ChildrenSeeded, LatestImplPr for +/// ImplementationMerged). +/// Fetch it inside +/// wrapped in +/// its own try/catch — failures must degrade to "no signal" with the +/// error captured in the corresponding FetchError property, +/// never thrown past the scope. state next-ready is a +/// routing-style verb and must always exit 0 with a structured +/// envelope. +/// Add a static composer on +/// that maps the scope's signals to a +/// (Disposition, Reason) tuple, mirroring +/// ComposePlanAuthored/ComposePlanReviewed/ +/// ComposePlanPromoted. +/// Wire the composer into +/// . +/// +/// +/// No re-architecture of the verb's outer shape is required for the next +/// two PRs in the closed-loop fix series. The fields below are mutable +/// by design — +/// fills them in piecewise as it issues each I/O call, then the scope is +/// frozen by being passed to the (read-only) composers. +/// +/// +internal sealed class NextReadyObservationScope +{ + /// The work item being observed. + public required int ItemId { get; init; } + + /// The resolved root work-item id (= for + /// an apex item; ancestor's id walked via parent chain otherwise). + /// Drives . + public required int RootId { get; init; } + + /// Canonical plan branch name for this item: plan/{root} + /// when item == root, else plan/{root}-{item}. Empty when + /// or are non-positive. + public required string PlanBranch { get; init; } + + // ── Plan-kind shared signals ──────────────────────────────────────── + + /// GitHub owner/repo slug from + /// git remote get-url origin. Empty when the remote could not + /// be parsed (no origin, non-GitHub URL, transient git failure). + /// When empty, all plan-kind composers degrade to Needed with a + /// "no slug" reason. + public string Slug { get; set; } = string.Empty; + + /// Result of git ls-remote --heads origin + /// refs/heads/{plan_branch}. False on absence OR transient + /// ls-remote failure (failures degrade to "no signal" — observer + /// posture from PR #1). + public bool BranchExistsOnOrigin { get; set; } + + /// Highest-numbered plan PR for + /// (open, closed, or merged), or null when none exist or the + /// gh pr list call failed. + public PullRequestSummary? LatestPlanPr { get; set; } + + /// Rich poll-data for 's number, + /// fetched once and reused by plan_authored / plan_reviewed / + /// plan_promoted. Null when no PR exists or gh pr view + /// failed. + public GhPullRequestPollData? PlanPrPoll { get; set; } + + /// Captured error message from the gh pr list call + /// (or the underlying slug resolution), or null on success. When + /// non-null, plan-kind composers force a Needed disposition with the + /// error surfaced in the reason — distinguishing + /// "couldn't observe" from "observed: no PR". + public string? PlanPrFetchError { get; set; } + + /// Captured error message from the gh pr view call, + /// or null on success. Same semantics as + /// . + public string? PlanPrPollError { get; set; } +} diff --git a/src/Polyphony/Commands/StateCommands.NextReady.cs b/src/Polyphony/Commands/StateCommands.NextReady.cs index 42bf53c8..fa3c4b2d 100644 --- a/src/Polyphony/Commands/StateCommands.NextReady.cs +++ b/src/Polyphony/Commands/StateCommands.NextReady.cs @@ -1,7 +1,9 @@ using System.Text.Json; using ConsoleAppFramework; using Polyphony.Annotations; +using Polyphony.Infrastructure.Processes; using Polyphony.Sdlc; +using Polyphony.Sdlc.Observers; namespace Polyphony.Commands; @@ -17,7 +19,10 @@ public sealed partial class StateCommands /// Compute the next-ready requirements for a work item. /// /// ADO work item ID to inspect. - /// Directory glob root for filesystem plan discovery (default docs/projects). + /// Reserved — formerly drove filesystem plan + /// discovery before took over the + /// plan_authored signal. Accepted for backward-compatibility with + /// existing workflow callers; ignored by the verb. /// Cancellation token. [Command("next-ready")] [VerbResult(typeof(StateNextReadyResult))] @@ -26,6 +31,7 @@ public async Task NextReady( string planRoot = "docs/projects", CancellationToken ct = default) { + _ = planRoot; if (RequiredInput.HaltIfMissing("state next-ready", ("--work-item", workItem == RequiredInput.MissingInt)) is { } halt) return halt; @@ -72,11 +78,11 @@ public async Task NextReady( } // Compute observable state and reduce. - var observed = await ComputeObservedAsync(workItem, item, children, planRoot, ct).ConfigureAwait(false); + var (observed, reasons) = await ComputeObservedAsync(workItem, item, children, ct).ConfigureAwait(false); var reduced = RequirementSetReducer.Apply(derived, observed); var status = ClassifyStatus(reduced); - EmitNextReadyResult(workItem, workItemType, reduced, resolved, status); + EmitNextReadyResult(workItem, workItemType, reduced, resolved, status, reasons); return ExitCodes.Success; } @@ -85,7 +91,8 @@ private void EmitNextReadyResult( string workItemType, RequirementSet set, ResolvedRequirementInputs resolved, - string status) + string status, + IReadOnlyDictionary? observationReasons = null) { var byDisp = set.Items .GroupBy(r => r.Disposition, StringComparer.Ordinal) @@ -103,6 +110,7 @@ private void EmitNextReadyResult( Needed = byDisp.GetValueOrDefault(Disposition.Needed) ?? [], ResolvedInputs = resolved, AnyInputInferred = resolved.AnyInferred, + ObservationReasons = observationReasons is { Count: > 0 } ? observationReasons : null, }; Console.WriteLine(JsonSerializer.Serialize( @@ -171,19 +179,34 @@ private static string ClassifyStatus(RequirementSet set) return "blocked"; } - private async Task ComputeObservedAsync( + private async Task<(ObservedRequirementState Observed, IReadOnlyDictionary Reasons)> ComputeObservedAsync( int workItem, Twig.Domain.Aggregates.WorkItem item, IReadOnlyList children, - string planRoot, CancellationToken ct) { - var (planStatus, _, _) = DiscoverPlan(workItem, "", planRoot); - var planAuthoredDisposition = planStatus == "complete" - ? Disposition.Satisfied - : Disposition.Needed; + var reasons = new Dictionary(StringComparer.Ordinal); - // children_seeded: every non-Done child has tasks of its own. + // ── Plan-kind observers (PR #2 — wired here). Build the per-item + // observation scope ONCE so plan_authored / plan_reviewed / + // plan_promoted share the underlying gh/git I/O. Sibling observers + // (PR #3 ChildrenSeeded, PR #4 Implementation) plug in by extending + // NextReadyObservationScope and adding their own composer methods — + // see the XML doc on NextReadyObservationScope for the contract. + var scope = await BuildObservationScopeAsync(workItem, item, ct).ConfigureAwait(false); + + var (planAuthoredDisp, planAuthoredReason) = ComposePlanAuthored(scope); + reasons[RequirementKind.PlanAuthored] = planAuthoredReason; + + var (planReviewedDisp, planReviewedReason) = ComposePlanReviewed(scope); + reasons[RequirementKind.PlanReviewed] = planReviewedReason; + + var (planPromotedDisp, planPromotedReason) = ComposePlanPromoted(scope); + reasons[RequirementKind.PlanPromoted] = planPromotedReason; + + // ── Legacy children_seeded observer (PR #3 will replace with + // PlanObserver.ObserveChildrenSeededAsync — the polyphony:planned + // tag check). Today: heuristic from cached child set. var childCount = children.Count; var anyChildMissingTasks = childCount > 0 && children.Any(c => c.State != "Done"); var allChildrenDone = childCount > 0 && children.All(c => c.State == "Done"); @@ -194,10 +217,9 @@ private async Task ComputeObservedAsync( _ => Disposition.Fulfilling, }; - // implementation_merged: best-effort from item state + child state. - // Authoritative PR/branch inspection lives in state detect; here we - // approximate from observable work-item state. State detect will - // overlay its richer signal via BuildObservedFromDetectSignals. + // ── Legacy implementation_merged observer (PR #4 will replace with + // an ImplementationObserver reading impl-PR / MG-PR state). + // Today: best-effort from item state + child state. var implDisposition = (item.State, childCount, allChildrenDone) switch { ("Done", _, _) => Disposition.Satisfied, @@ -206,21 +228,248 @@ private async Task ComputeObservedAsync( _ => Disposition.Needed, }; - await Task.CompletedTask.ConfigureAwait(false); // async-shape for future I/O - return BuildObservedFromSignals(planAuthoredDisposition, seedDisposition, implDisposition); + var observed = BuildObservedFromSignals( + planAuthoredDisp, planReviewedDisp, planPromotedDisp, + seedDisposition, implDisposition); + return (observed, reasons); + } + + /// + /// Build the per-item for the + /// PR #2 plan-kind observers. Fetches each shared signal exactly once + /// and degrades to "no signal" (with the error captured on the scope) + /// on any failure — never throws past the scope. state next-ready + /// is a routing-style verb and must always exit 0 with a structured + /// envelope. + /// + private async Task BuildObservationScopeAsync( + int workItem, + Twig.Domain.Aggregates.WorkItem item, + CancellationToken ct) + { + var rootId = await ResolveRootIdAsync(item, ct).ConfigureAwait(false); + var planBranch = PlanObserver.ResolvePlanBranch(rootId, workItem); + + var scope = new NextReadyObservationScope + { + ItemId = workItem, + RootId = rootId, + PlanBranch = planBranch, + }; + + // PlanObserver.TryResolveSlugAsync swallows internally, but defend + // again here so a future tightening of the observer does not break + // the verb's "always exit 0" contract. + try + { + scope.Slug = await planObserver.TryResolveSlugAsync(ct).ConfigureAwait(false); + } + catch + { + scope.Slug = string.Empty; + } + + if (string.IsNullOrEmpty(planBranch)) + { + // Non-positive root or item id — nothing more to observe. Plan + // composers will degrade with the empty plan branch. + return scope; + } + + try + { + scope.BranchExistsOnOrigin = await planObserver.CheckPlanBranchExistsAsync(planBranch, ct) + .ConfigureAwait(false); + } + catch + { + scope.BranchExistsOnOrigin = false; + } + + if (string.IsNullOrEmpty(scope.Slug)) + { + // Without a slug we cannot list PRs; record the gap as a + // structured fetch error so plan composers say "could not + // resolve repo slug" rather than "no PR opened". + scope.PlanPrFetchError = "could not resolve repo slug from origin remote"; + return scope; + } + + try + { + scope.LatestPlanPr = await planObserver.GetLatestPlanPrAsync(scope.Slug, planBranch, ct) + .ConfigureAwait(false); + } + catch (ExternalToolTimeoutException ex) + { + scope.PlanPrFetchError = ex.FormatErrorMessage("gh pr list"); + } + catch (ExternalToolException ex) + { + scope.PlanPrFetchError = $"gh pr list failed: {ex.Message}"; + } + catch (Exception ex) + { + scope.PlanPrFetchError = $"gh pr list failed: {ex.Message}"; + } + + if (scope.LatestPlanPr is null) return scope; + + try + { + scope.PlanPrPoll = await planObserver.GetPlanPrPollAsync(scope.Slug, scope.LatestPlanPr.Number, ct) + .ConfigureAwait(false); + } + catch (ExternalToolTimeoutException ex) + { + scope.PlanPrPollError = ex.FormatErrorMessage("gh pr view"); + } + catch (ExternalToolException ex) + { + scope.PlanPrPollError = $"gh pr view failed: {ex.Message}"; + } + catch (Exception ex) + { + scope.PlanPrPollError = $"gh pr view failed: {ex.Message}"; + } + + return scope; + } + + /// + /// Walk from + /// until we hit a node with no parent — that + /// is the root for branch-naming purposes. Returns 's + /// own id when it is already the root, when the chain breaks, or when + /// we exceed an internal cycle cap (50 ancestors). The cap matches + /// plan derive-ancestor-chain's posture. + /// + private async Task ResolveRootIdAsync(Twig.Domain.Aggregates.WorkItem item, CancellationToken ct) + { + const int AncestorWalkLimit = 50; + + var cursor = item; + var visited = new HashSet { item.Id }; + for (var step = 0; step < AncestorWalkLimit; step++) + { + var parentId = cursor.ParentId; + if (parentId is null || parentId == 0) return cursor.Id; + if (!visited.Add(parentId.Value)) + { + // Cycle — fall back to the highest id we have walked. The + // safe choice for branch naming is the current cursor (the + // last well-formed link before the cycle closes). + return cursor.Id; + } + var parent = await repository.GetByIdAsync(parentId.Value, ct).ConfigureAwait(false); + if (parent is null) return cursor.Id; + cursor = parent; + } + // Reached the cap without finding a top — degrade to the deepest + // ancestor we did reach. Branch naming will still produce a valid + // plan/{root}-{item} string. + return cursor.Id; + } + + /// + /// Map the plan-kind shared signals on to a + /// (Disposition, Reason) tuple for the plan_authored + /// requirement. Failed I/O (recorded on + /// / + /// ) forces + /// Needed with the captured error in the reason — distinguishing + /// "couldn't observe" from "observed: no PR" per the closed-loop spec. + /// + private static (string Disposition, string Reason) ComposePlanAuthored(NextReadyObservationScope scope) + { + if (scope.PlanPrFetchError is not null) + { + return (Disposition.Needed, scope.PlanPrFetchError); + } + if (scope.LatestPlanPr is not null && scope.PlanPrPollError is not null) + { + return (Disposition.Needed, scope.PlanPrPollError); + } + + var prState = scope.PlanPrPoll?.State?.ToUpperInvariant(); + var observation = PlanObserver.MapPlanAuthored( + scope.PlanBranch, scope.BranchExistsOnOrigin, scope.LatestPlanPr, prState); + return ValidateDisposition(observation.Disposition, observation.Reason); + } + + private static (string Disposition, string Reason) ComposePlanReviewed(NextReadyObservationScope scope) + { + if (scope.PlanPrFetchError is not null) + { + return (Disposition.Needed, scope.PlanPrFetchError); + } + if (scope.LatestPlanPr is not null && scope.PlanPrPollError is not null) + { + return (Disposition.Needed, scope.PlanPrPollError); + } + + var observation = PlanObserver.MapPlanReviewed(scope.LatestPlanPr, scope.PlanPrPoll); + return ValidateDisposition(observation.Disposition, observation.Reason); + } + + private static (string Disposition, string Reason) ComposePlanPromoted(NextReadyObservationScope scope) + { + if (scope.PlanPrFetchError is not null) + { + return (Disposition.Needed, scope.PlanPrFetchError); + } + if (scope.LatestPlanPr is not null && scope.PlanPrPollError is not null) + { + return (Disposition.Needed, scope.PlanPrPollError); + } + + var prState = scope.PlanPrPoll?.State?.ToUpperInvariant(); + var observation = PlanObserver.MapPlanPromoted(scope.LatestPlanPr, prState); + return ValidateDisposition(observation.Disposition, observation.Reason); + } + + /// + /// Guard against an observer ever returning a string that is not one of + /// the four canonical values. Per the + /// closed-loop spec we throw rather than silently swallow — silent + /// fallback would re-introduce the exact "everything Needed" lie the + /// PR set is fixing. + /// + private static (string Disposition, string Reason) ValidateDisposition(string disposition, string reason) + { + if (!Disposition.IsValid(disposition)) + { + throw new InvalidOperationException( + $"Observer returned unknown disposition '{disposition}'. " + + "Valid values: needed, ready, fulfilling, satisfied."); + } + if (disposition == Disposition.Ready) + { + throw new InvalidOperationException( + "Observer returned disposition 'ready'; readiness is reducer-derived, " + + "observers must emit only needed/fulfilling/satisfied."); + } + return (disposition, reason); } - /// Build an from the three - /// signal dispositions we currently track. Plan_reviewed, plan_promoted, - /// action_satisfied, and evidence_accepted have no observable signals yet - /// (Phase 3 / Phase 6 work) and default to . + /// Build an from the + /// composed signal dispositions. Kinds at + /// are omitted because the reducer treats absence as Needed by default; + /// stronger dispositions are surfaced explicitly so the reducer can + /// promote downstream gates. After PR #4 lands this becomes the only + /// composer of observed plan/seed/impl signals — the per-kind reasons + /// dictionary travels alongside via ComputeObservedAsync. private static ObservedRequirementState BuildObservedFromSignals( string planAuthored, + string planReviewed, + string planPromoted, string childrenSeeded, string implementationMerged) { var dict = new Dictionary(StringComparer.Ordinal); if (planAuthored != Disposition.Needed) dict[RequirementKind.PlanAuthored] = planAuthored; + if (planReviewed != Disposition.Needed) dict[RequirementKind.PlanReviewed] = planReviewed; + if (planPromoted != Disposition.Needed) dict[RequirementKind.PlanPromoted] = planPromoted; if (childrenSeeded != Disposition.Needed) dict[RequirementKind.ChildrenSeeded] = childrenSeeded; if (implementationMerged != Disposition.Needed) dict[RequirementKind.ImplementationMerged] = implementationMerged; return new ObservedRequirementState { Observed = dict }; diff --git a/src/Polyphony/Commands/StateCommands.PlanDiscovery.cs b/src/Polyphony/Commands/StateCommands.PlanDiscovery.cs deleted file mode 100644 index 68fc7e52..00000000 --- a/src/Polyphony/Commands/StateCommands.PlanDiscovery.cs +++ /dev/null @@ -1,76 +0,0 @@ -using System.Text.RegularExpressions; - -namespace Polyphony.Commands; - -/// -/// Filesystem plan-document discovery used by state next-ready. -/// Mirrors the priority chain of the legacy detect-state.ps1 script: -/// explicit override → frontmatter scan → legacy table. -/// -public sealed partial class StateCommands -{ - private static readonly Regex YamlFrontmatterRegex = - new(@"^---\s*\r?\n(.*?)\r?\n---", RegexOptions.Compiled | RegexOptions.Singleline); - private static readonly Regex WorkItemIdRegex = - new(@"work_item_id:\s*(\d+)", RegexOptions.Compiled); - private static readonly Regex LegacyWorkItemRowRegex = - new(@"\|\s*\*{0,2}Work\s*Item\*{0,2}\s*\|\s*#(\d+)", RegexOptions.Compiled); - private static readonly Regex LegacyAnyLabelRowRegex = - new(@"\|\s*\*{0,2}[^|*]+\*{0,2}\s*\|\s*#(\d+)", RegexOptions.Compiled); - - /// - /// Resolve a work item's plan document via filesystem fallback. - /// - /// - /// Tuple of (status, source, path) where status is - /// none | complete | ambiguous. - /// - public static (string Status, string Source, string Path) DiscoverPlan( - int workItemId, string explicitPath, string planRoot) - { - if (!string.IsNullOrEmpty(explicitPath)) - { - return File.Exists(explicitPath) - ? ("complete", "explicit_override", Path.GetFullPath(explicitPath)) - : ("none", "none", ""); - } - - if (!Directory.Exists(planRoot)) return ("none", "none", ""); - - var matches = new List(); - foreach (var file in Directory.EnumerateFiles(planRoot, "*.plan.md")) - { - string content; - try { content = File.ReadAllText(file); } catch { continue; } - if (PlanMatchesWorkItem(content, workItemId)) matches.Add(file); - } - - return matches.Count switch - { - 1 => ("complete", "filesystem_fallback", matches[0]), - > 1 => ("ambiguous", "none", ""), - _ => ("none", "none", ""), - }; - } - - private static bool PlanMatchesWorkItem(string content, int workItemId) - { - var fm = YamlFrontmatterRegex.Match(content); - if (fm.Success) - { - var idMatch = WorkItemIdRegex.Match(fm.Groups[1].Value); - if (idMatch.Success && int.TryParse(idMatch.Groups[1].Value, out var id) && id == workItemId) - return true; - } - - var rowMatch = LegacyWorkItemRowRegex.Match(content); - if (rowMatch.Success && int.TryParse(rowMatch.Groups[1].Value, out var rid) && rid == workItemId) - return true; - - var anyRowMatch = LegacyAnyLabelRowRegex.Match(content); - if (anyRowMatch.Success && int.TryParse(anyRowMatch.Groups[1].Value, out var arid) && arid == workItemId) - return true; - - return false; - } -} diff --git a/src/Polyphony/Commands/StateCommands.cs b/src/Polyphony/Commands/StateCommands.cs index caf3bc4b..2d481be6 100644 --- a/src/Polyphony/Commands/StateCommands.cs +++ b/src/Polyphony/Commands/StateCommands.cs @@ -3,6 +3,7 @@ using Polyphony.Annotations; using Polyphony.Configuration; using Polyphony.Infrastructure.Processes; +using Polyphony.Sdlc.Observers; using Polyphony.Versioning; using Twig.Domain.Interfaces; @@ -30,7 +31,8 @@ public sealed partial class StateCommands( IGhClient gh, IProcessRunner runner, IWorkItemRepository repository, - ProcessConfig processConfig) + ProcessConfig processConfig, + PlanObserver planObserver) { private const string DotnetExe = "dotnet"; diff --git a/src/Polyphony/Models/StateNextReadyResult.cs b/src/Polyphony/Models/StateNextReadyResult.cs index 170bcd05..123e99fe 100644 --- a/src/Polyphony/Models/StateNextReadyResult.cs +++ b/src/Polyphony/Models/StateNextReadyResult.cs @@ -55,6 +55,20 @@ public sealed record StateNextReadyResult /// True when any deriver input was resolved by inference rather than explicit config. public required bool AnyInputInferred { get; init; } + /// + /// Per-kind diagnostic reasons captured by the observers for the + /// kinds with an observable signal (today: plan_authored, + /// plan_reviewed, plan_promoted — wired in the closed-loop + /// PR #2). Maps a string to a + /// short human-readable reason such as + /// "plan PR #204 merged" or "gh pr list failed: timed out". + /// Omitted from JSON when null or empty (no observed kinds, e.g. + /// the verb errored out before reaching the observers). Sibling + /// observers (PR #3 ChildrenSeeded, PR #4 Implementation) extend + /// this dictionary as they ship. + /// + public IReadOnlyDictionary? ObservationReasons { get; init; } + /// Error message when is error; null otherwise. public string? Error { get; init; } } diff --git a/src/Polyphony/PolyphonyJsonContext.cs b/src/Polyphony/PolyphonyJsonContext.cs index 2d70a40e..33ccc563 100644 --- a/src/Polyphony/PolyphonyJsonContext.cs +++ b/src/Polyphony/PolyphonyJsonContext.cs @@ -120,6 +120,7 @@ namespace Polyphony; [JsonSerializable(typeof(RetiredMergeGroupRecord))] [JsonSerializable(typeof(MergedPlanPrEntry))] [JsonSerializable(typeof(Dictionary))] +[JsonSerializable(typeof(IReadOnlyDictionary))] [JsonSerializable(typeof(RunLock))] [JsonSerializable(typeof(AcquireLockResult))] [JsonSerializable(typeof(ReleaseLockResult))] diff --git a/tests/Polyphony.Tests/Commands/JsonOutputContractTests.cs b/tests/Polyphony.Tests/Commands/JsonOutputContractTests.cs index e1594cf5..e269df81 100644 --- a/tests/Polyphony.Tests/Commands/JsonOutputContractTests.cs +++ b/tests/Polyphony.Tests/Commands/JsonOutputContractTests.cs @@ -1330,10 +1330,19 @@ private StateCommands CreateStateCommands() { var config = CreateConfigBuilder().Build(); var runner = new FakeProcessRunner(); + // Baseline shell-out stubs: PlanObserver issues git remote get-url + // and (potentially) git ls-remote / gh pr list during NextReady. + // Returning empty payloads degrades the plan-kind observers to + // Needed without disturbing the JSON-shape contract these tests + // verify. + runner.WhenStartsWith("git", ["remote", "get-url"], new ProcessResult(0, "", "")); + runner.WhenStartsWith("git", ["ls-remote"], new ProcessResult(0, "", "")); + runner.WhenStartsWith("gh", ["pr", "list"], new ProcessResult(0, "[]", "")); var twig = new TwigClient(runner); var git = new GitClient(runner); var gh = new GhClient(runner); - return new StateCommands(twig, git, gh, runner, Repository, config); + var planObserver = new Polyphony.Sdlc.Observers.PlanObserver(git, gh, twig); + return new StateCommands(twig, git, gh, runner, Repository, config, planObserver); } private EdgesCommands CreateEdgesCommands() diff --git a/tests/Polyphony.Tests/Commands/StateCommandsPreflightTests.cs b/tests/Polyphony.Tests/Commands/StateCommandsPreflightTests.cs index 2d30d4d1..1cf3a5cd 100644 --- a/tests/Polyphony.Tests/Commands/StateCommandsPreflightTests.cs +++ b/tests/Polyphony.Tests/Commands/StateCommandsPreflightTests.cs @@ -17,7 +17,8 @@ public sealed class StateCommandsPreflightTests : CommandTestBase var twig = new TwigClient(runner); var git = new GitClient(runner); var gh = new GhClient(runner); - return (new StateCommands(twig, git, gh, runner, Repository, Config), runner); + var planObserver = new Polyphony.Sdlc.Observers.PlanObserver(git, gh, twig); + return (new StateCommands(twig, git, gh, runner, Repository, Config, planObserver), runner); } // End of CreateCommand // (Removed duplicate orphaned block) diff --git a/tests/Polyphony.Tests/Commands/StateNextReadyPlanIntegrationTests.cs b/tests/Polyphony.Tests/Commands/StateNextReadyPlanIntegrationTests.cs new file mode 100644 index 00000000..e60ae17f --- /dev/null +++ b/tests/Polyphony.Tests/Commands/StateNextReadyPlanIntegrationTests.cs @@ -0,0 +1,328 @@ +using System.Text.Json; +using Polyphony.Commands; +using Polyphony.Configuration; +using Polyphony.Infrastructure.Processes; +using Polyphony.Sdlc; +using Polyphony.Sdlc.Observers; +using Polyphony.Tests.Infrastructure.Processes; +using Polyphony.Tests.TestFixtures; +using Shouldly; +using Xunit; + +namespace Polyphony.Tests.Commands; + +/// +/// Integration tests for after +/// closed-loop PR #2: drives the +/// plan_authored, plan_reviewed, and plan_promoted +/// dispositions from live PR state instead of the (broken) *.plan.md +/// filesystem glob. +/// +/// +/// Mocks the git remote get-url, git ls-remote, +/// gh pr list, and gh pr view shell-outs through +/// — same pattern as +/// shipped in PR #1. +/// +public sealed class StateNextReadyPlanIntegrationTests : CommandTestBase +{ + private const int ApexId = 3043; + private const string PlanBranch = "plan/3043"; + private const string OriginUrl = "https://github.com/acme/repo.git"; + + private static FakeProcessRunner NewRunnerWithRemote() + { + var runner = new FakeProcessRunner(); + runner.WhenExact("git", ["remote", "get-url", "origin"], new ProcessResult(0, OriginUrl + "\n", "")); + return runner; + } + + private StateCommands CreateCommand(FakeProcessRunner runner, ProcessConfig? configOverride = null) + { + var config = configOverride ?? Config; + var twig = new TwigClient(runner); + var git = new GitClient(runner); + var gh = new GhClient(runner); + var planObserver = new PlanObserver(git, gh, twig); + return new StateCommands(twig, git, gh, runner, Repository, config, planObserver); + } + + private static void StubLsRemote(FakeProcessRunner runner, string branch, bool exists) + => runner.WhenExact("git", ["ls-remote", "--heads", "origin", $"refs/heads/{branch}"], + new ProcessResult(0, exists ? $"abc123\trefs/heads/{branch}\n" : "", "")); + + private static void StubPrListEmpty(FakeProcessRunner runner) + => runner.WhenStartsWith("gh", ["pr", "list"], new ProcessResult(0, "[]", "")); + + private static void StubPrListSingle(FakeProcessRunner runner, int prNumber, string headRef) + => runner.WhenStartsWith("gh", ["pr", "list"], + new ProcessResult(0, + $$"""[{"number":{{prNumber}},"headRefName":"{{headRef}}","url":"https://gh/pr/{{prNumber}}"}]""", + "")); + + private static void StubPrPoll( + FakeProcessRunner runner, + int prNumber, + string state, + string headRef, + string reviewDecision = "REVIEW_REQUIRED") + { + var json = $$""" + { + "number": {{prNumber}}, + "state": "{{state}}", + "reviewDecision": "{{reviewDecision}}", + "mergeable": "MERGEABLE", + "headRefName": "{{headRef}}", + "headRefOid": "abc123", + "baseRefName": "feature/3043", + "mergedAt": null, + "mergeCommit": null, + "body": "", + "reviews": [] + } + """; + runner.WhenStartsWith("gh", ["pr", "view", prNumber.ToString()], new ProcessResult(0, json, "")); + } + + private async Task SeedApexAsync() + { + var item = new WorkItemBuilder() + .WithId(ApexId).WithType("Issue").WithTitle("Apex 3043").WithState("Doing").Build(); + await SeedAsync(item); + } + + // ─── Plan-authored: no plan branch ────────────────────────────────── + + [Fact] + public async Task NextReady_NoPlanBranch_PlanAuthored_Needed_WithReason() + { + await SeedApexAsync(); + var runner = NewRunnerWithRemote(); + StubLsRemote(runner, PlanBranch, exists: false); + StubPrListEmpty(runner); + + var cmd = CreateCommand(runner); + var (exit, output) = await CaptureConsoleAsync(() => cmd.NextReady(workItem: ApexId)); + exit.ShouldBe(ExitCodes.Success); + + var result = JsonSerializer.Deserialize(output, PolyphonyJsonContext.Default.StateNextReadyResult)!; + // plan_authored is the entry in the plan-kind chain — with no + // prerequisites it is promoted from observed Needed to reducer-derived + // Ready (and thus appears in Next, not Needed). plan_reviewed and + // plan_promoted depend on plan_authored being Satisfied first, so + // they stay Needed. + result.Next.ShouldContain(RequirementKind.PlanAuthored); + result.Needed.ShouldContain(RequirementKind.PlanReviewed); + result.Needed.ShouldContain(RequirementKind.PlanPromoted); + + result.ObservationReasons.ShouldNotBeNull(); + result.ObservationReasons!.ShouldContainKey(RequirementKind.PlanAuthored); + result.ObservationReasons[RequirementKind.PlanAuthored].ShouldNotBeNullOrWhiteSpace(); + result.ObservationReasons[RequirementKind.PlanAuthored].ShouldContain("no plan branch"); + } + + // ─── Plan-authored: open plan PR (Fulfilling) ─────────────────────── + + [Fact] + public async Task NextReady_OpenPlanPr_AllPlanKinds_Fulfilling() + { + await SeedApexAsync(); + var runner = NewRunnerWithRemote(); + StubLsRemote(runner, PlanBranch, exists: true); + StubPrListSingle(runner, 204, PlanBranch); + StubPrPoll(runner, 204, state: "OPEN", headRef: PlanBranch, reviewDecision: "REVIEW_REQUIRED"); + + var cmd = CreateCommand(runner); + var (exit, output) = await CaptureConsoleAsync(() => cmd.NextReady(workItem: ApexId)); + exit.ShouldBe(ExitCodes.Success); + + var result = JsonSerializer.Deserialize(output, PolyphonyJsonContext.Default.StateNextReadyResult)!; + // OPEN PR + REVIEW_REQUIRED: + // plan_authored Fulfilling (PR exists, not merged) + // plan_reviewed Fulfilling (review pending) + // plan_promoted Fulfilling (awaiting merge) + result.Fulfilling.ShouldContain(RequirementKind.PlanAuthored); + result.Fulfilling.ShouldContain(RequirementKind.PlanReviewed); + result.Fulfilling.ShouldContain(RequirementKind.PlanPromoted); + result.Satisfied.ShouldNotContain(RequirementKind.PlanAuthored); + + result.ObservationReasons.ShouldNotBeNull(); + result.ObservationReasons![RequirementKind.PlanAuthored].ShouldContain("204"); + result.ObservationReasons[RequirementKind.PlanAuthored].ShouldContain("open"); + } + + // ─── Smoking-gun #3043: merged plan PR → all 3 Satisfied ──────────── + + [Fact] + public async Task NextReady_MergedPlanPr_AllPlanKinds_Satisfied_FixesSmokingGun() + { + // Reproduces the apex 3043 smoking-gun from + // files/closed-loop-state-plan.md §2: plan PR merged → all three + // plan-kind requirements should be Satisfied. Pre-PR-#2 the verb + // returned all three as Needed because *.plan.md no longer exists. + await SeedApexAsync(); + var runner = NewRunnerWithRemote(); + StubLsRemote(runner, PlanBranch, exists: true); + StubPrListSingle(runner, 204, PlanBranch); + StubPrPoll(runner, 204, state: "MERGED", headRef: PlanBranch, reviewDecision: "APPROVED"); + + var cmd = CreateCommand(runner); + var (exit, output) = await CaptureConsoleAsync(() => cmd.NextReady(workItem: ApexId)); + exit.ShouldBe(ExitCodes.Success); + + var result = JsonSerializer.Deserialize(output, PolyphonyJsonContext.Default.StateNextReadyResult)!; + result.Satisfied.ShouldContain(RequirementKind.PlanAuthored); + result.Satisfied.ShouldContain(RequirementKind.PlanReviewed); + result.Satisfied.ShouldContain(RequirementKind.PlanPromoted); + + // Regression guard: the pre-PR-#2 verb returned all three as Needed + // even when the PR was merged. Lock that in. + result.Needed.ShouldNotContain(RequirementKind.PlanAuthored); + result.Needed.ShouldNotContain(RequirementKind.PlanReviewed); + result.Needed.ShouldNotContain(RequirementKind.PlanPromoted); + + result.ObservationReasons!.ShouldContainKey(RequirementKind.PlanPromoted); + result.ObservationReasons![RequirementKind.PlanPromoted].ShouldContain("merged"); + } + + // ─── Approved-but-unmerged: plan_reviewed Satisfied, promoted Fulfilling ─ + + [Fact] + public async Task NextReady_OpenApprovedPlanPr_ReviewedSatisfied_PromotedFulfilling() + { + await SeedApexAsync(); + var runner = NewRunnerWithRemote(); + StubLsRemote(runner, PlanBranch, exists: true); + StubPrListSingle(runner, 204, PlanBranch); + StubPrPoll(runner, 204, state: "OPEN", headRef: PlanBranch, reviewDecision: "APPROVED"); + + var cmd = CreateCommand(runner); + var (exit, output) = await CaptureConsoleAsync(() => cmd.NextReady(workItem: ApexId)); + exit.ShouldBe(ExitCodes.Success); + + var result = JsonSerializer.Deserialize(output, PolyphonyJsonContext.Default.StateNextReadyResult)!; + // OPEN+APPROVED: + // plan_authored Fulfilling (PR exists, not merged) + // plan_reviewed Satisfied (approved) + // plan_promoted Fulfilling (awaiting merge) + result.Fulfilling.ShouldContain(RequirementKind.PlanAuthored); + result.Satisfied.ShouldContain(RequirementKind.PlanReviewed); + result.Fulfilling.ShouldContain(RequirementKind.PlanPromoted); + } + + // ─── Failure handling: gh pr list errors → Needed with reason, no throw ─ + + [Fact] + public async Task NextReady_GhPrListFailure_DegradesToNeeded_NoException() + { + await SeedApexAsync(); + var runner = NewRunnerWithRemote(); + StubLsRemote(runner, PlanBranch, exists: true); + // gh pr list fails — verb must NOT throw, must return all plan + // kinds as Needed (per closed-loop spec §3.1). + runner.WhenStartsWith("gh", ["pr", "list"], new ProcessResult(1, "", "boom")); + + var cmd = CreateCommand(runner); + var (exit, output) = await CaptureConsoleAsync(() => cmd.NextReady(workItem: ApexId)); + exit.ShouldBe(ExitCodes.Success); + + var result = JsonSerializer.Deserialize(output, PolyphonyJsonContext.Default.StateNextReadyResult)!; + // gh pr list failure → plan composers all return Needed with the + // captured reason. Reducer promotes plan_authored → Ready (Next), + // plan_reviewed / plan_promoted remain Needed (prerequisites unmet). + result.Next.ShouldContain(RequirementKind.PlanAuthored); + result.Needed.ShouldContain(RequirementKind.PlanReviewed); + result.Needed.ShouldContain(RequirementKind.PlanPromoted); + + result.ObservationReasons.ShouldNotBeNull(); + result.ObservationReasons![RequirementKind.PlanAuthored].ShouldNotBeNullOrWhiteSpace(); + } + + // ─── Failure handling: missing origin → all plan kinds Needed (slug gap) ─ + + [Fact] + public async Task NextReady_NoOriginRemote_PlanKindsNeeded_WithSlugReason() + { + await SeedApexAsync(); + // Simulate no origin remote; PlanObserver.TryResolveSlugAsync + // returns "" and our scope captures the slug gap in + // PlanPrFetchError so composers say "could not resolve repo slug" + // rather than "no PR opened". + var runner = new FakeProcessRunner(); + runner.WhenExact("git", ["remote", "get-url", "origin"], + new ProcessResult(128, "", "fatal: No such remote 'origin'")); + StubLsRemote(runner, PlanBranch, exists: false); + + var cmd = CreateCommand(runner); + var (exit, output) = await CaptureConsoleAsync(() => cmd.NextReady(workItem: ApexId)); + exit.ShouldBe(ExitCodes.Success); + + var result = JsonSerializer.Deserialize(output, PolyphonyJsonContext.Default.StateNextReadyResult)!; + // Slug-resolution failure surfaces via PlanPrFetchError → all plan + // composers return Needed with the captured reason. plan_authored + // is promoted to Ready (Next) by the reducer. + result.Next.ShouldContain(RequirementKind.PlanAuthored); + result.Needed.ShouldContain(RequirementKind.PlanReviewed); + result.Needed.ShouldContain(RequirementKind.PlanPromoted); + + result.ObservationReasons.ShouldNotBeNull(); + result.ObservationReasons![RequirementKind.PlanAuthored].ShouldContain("slug"); + } + + // ─── Closed-unmerged plan PR → all three Needed (replan posture) ──── + + [Fact] + public async Task NextReady_ClosedUnmergedPlanPr_AllPlanKinds_Needed() + { + await SeedApexAsync(); + var runner = NewRunnerWithRemote(); + StubLsRemote(runner, PlanBranch, exists: true); + StubPrListSingle(runner, 204, PlanBranch); + StubPrPoll(runner, 204, state: "CLOSED", headRef: PlanBranch, reviewDecision: "REVIEW_REQUIRED"); + + var cmd = CreateCommand(runner); + var (exit, output) = await CaptureConsoleAsync(() => cmd.NextReady(workItem: ApexId)); + exit.ShouldBe(ExitCodes.Success); + + var result = JsonSerializer.Deserialize(output, PolyphonyJsonContext.Default.StateNextReadyResult)!; + // Closed-unmerged plan PR → all three composers return Needed (replan + // posture). plan_authored promoted to Ready by reducer. + result.Next.ShouldContain(RequirementKind.PlanAuthored); + result.Needed.ShouldContain(RequirementKind.PlanReviewed); + result.Needed.ShouldContain(RequirementKind.PlanPromoted); + result.ObservationReasons!.Values + .Any(v => v.Contains("closed", StringComparison.OrdinalIgnoreCase)) + .ShouldBeTrue(); + } + + // ─── Descendant item → walks parent chain to discover plan/{root}-{item} ─ + + [Fact] + public async Task NextReady_DescendantItem_ResolvesRootId_AndUsesHyphenPlanBranch() + { + // Apex 3043 with a descendant 3050; the verb must walk parents to + // discover that 3043 is the root and inspect plan/3043-3050, not + // plan/3050. + var apex = new WorkItemBuilder().WithId(3043).WithType("Issue") + .WithTitle("Apex").WithState("Doing").Build(); + var child = new WorkItemBuilder().WithId(3050).WithType("Issue") + .WithTitle("Child").WithState("To Do").WithParentId(3043).Build(); + await SeedAsync(apex, child); + + const string descPlanBranch = "plan/3043-3050"; + var runner = NewRunnerWithRemote(); + StubLsRemote(runner, descPlanBranch, exists: true); + StubPrListSingle(runner, 250, descPlanBranch); + StubPrPoll(runner, 250, state: "MERGED", headRef: descPlanBranch, reviewDecision: "APPROVED"); + + var cmd = CreateCommand(runner); + var (exit, output) = await CaptureConsoleAsync(() => cmd.NextReady(workItem: 3050)); + exit.ShouldBe(ExitCodes.Success); + + var result = JsonSerializer.Deserialize(output, PolyphonyJsonContext.Default.StateNextReadyResult)!; + result.Satisfied.ShouldContain(RequirementKind.PlanAuthored); + result.Satisfied.ShouldContain(RequirementKind.PlanReviewed); + result.Satisfied.ShouldContain(RequirementKind.PlanPromoted); + } +} diff --git a/tests/Polyphony.Tests/Commands/StateNextReadyTests.cs b/tests/Polyphony.Tests/Commands/StateNextReadyTests.cs index 0560860c..78541740 100644 --- a/tests/Polyphony.Tests/Commands/StateNextReadyTests.cs +++ b/tests/Polyphony.Tests/Commands/StateNextReadyTests.cs @@ -3,6 +3,7 @@ using Polyphony.Configuration; using Polyphony.Infrastructure.Processes; using Polyphony.Sdlc; +using Polyphony.Sdlc.Observers; using Polyphony.Tests.Infrastructure.Processes; using Polyphony.Tests.TestFixtures; using Shouldly; @@ -29,10 +30,28 @@ private StateCommands CreateCommand(ProcessConfig? configOverride = null) { var config = configOverride ?? Config; var runner = new FakeProcessRunner(); + // Default: every git/gh shell-out resolves to a "no signal" response + // so the plan-kind observers degrade cleanly to Needed without + // requiring per-test stubs. Tests that exercise specific observed + // states use their own runner. + StubBaselineNoSignal(runner); var twig = new TwigClient(runner); var git = new GitClient(runner); var gh = new GhClient(runner); - return new StateCommands(twig, git, gh, runner, Repository, config); + var planObserver = new PlanObserver(git, gh, twig); + return new StateCommands(twig, git, gh, runner, Repository, config, planObserver); + } + + private static void StubBaselineNoSignal(FakeProcessRunner runner) + { + // git remote get-url origin → empty stdout; PlanObserver returns "" slug. + runner.WhenStartsWith("git", ["remote", "get-url"], new ProcessResult(0, "", "")); + // git ls-remote → empty stdout; PlanObserver returns false. + runner.WhenStartsWith("git", ["ls-remote"], new ProcessResult(0, "", "")); + // gh pr list → no PRs. + runner.WhenStartsWith("gh", ["pr", "list"], new ProcessResult(0, "[]", "")); + // twig show → empty payload (legacy children_seeded path doesn't call this; harmless safety net). + runner.WhenStartsWith("twig", ["show"], new ProcessResult(0, """{"id":0,"tags":""}""", "")); } [Fact] diff --git a/tests/Polyphony.Tests/Commands/StateValidateInputsTests.cs b/tests/Polyphony.Tests/Commands/StateValidateInputsTests.cs index ff4f9fac..057847b6 100644 --- a/tests/Polyphony.Tests/Commands/StateValidateInputsTests.cs +++ b/tests/Polyphony.Tests/Commands/StateValidateInputsTests.cs @@ -20,7 +20,8 @@ public sealed class StateValidateInputsTests gh: null!, runner: null!, repository: null!, - processConfig: null!); + processConfig: null!, + planObserver: null!); [Fact] public async Task ValidateInputs_MissingWorkflowYamlArg_EmitsHaltEnvelope()