diff --git a/src/Polyphony/Commands/CommandExecution.cs b/src/Polyphony/Commands/CommandExecution.cs new file mode 100644 index 0000000..8766a96 --- /dev/null +++ b/src/Polyphony/Commands/CommandExecution.cs @@ -0,0 +1,15 @@ +namespace Polyphony.Commands; + +/// +/// Output of an internal verb method (the typed *Async seam introduced +/// by AB#3313): pairs the typed result record the verb computed with the +/// process exit code the public shell will return. +/// +/// +/// The public verb method is a thin shell — parse envelope → call typed +/// *Async method → serialize to stdout → return +/// . The typed method itself is pure of JSON shape, +/// CLI binding, and console I/O; callers like the future PrLifecycle +/// skeleton can compose typed without round-tripping through JSON. +/// +internal sealed record CommandExecution(TResult Result, int ExitCode); diff --git a/src/Polyphony/Commands/PrCommands.MergePlanAdo.cs b/src/Polyphony/Commands/PrCommands.MergePlanAdo.cs index 6a9d933..6c4a078 100644 --- a/src/Polyphony/Commands/PrCommands.MergePlanAdo.cs +++ b/src/Polyphony/Commands/PrCommands.MergePlanAdo.cs @@ -96,53 +96,39 @@ public async Task MergePlanAdo( CreateJournalInvocation("pr_merge_plan_ado", PullRequestJournalTarget(prUrl, prNumber), rootId, itemId), async innerCt => { - var sw = new StringWriter(); - var originalOut = Console.Out; - int exitCode; - try - { - Console.SetOut(sw); - exitCode = await MergePlanAdoBodyAsync(organization, project, repository, rootId, itemId, prNumber, parentItemId, manifestPath, lockTtlHours, by, innerCt).ConfigureAwait(false); - } - finally + var execution = await MergePlanAdoAsync( + organization, project, repository, rootId, itemId, prNumber, parentItemId, manifestPath, lockTtlHours, by, innerCt) + .ConfigureAwait(false); + EmitMergePlanAdo(execution.Result); + var result = execution.Result; + payload = new PrMergePlanAdoPayload { - Console.SetOut(originalOut); - } - var output = sw.ToString(); - Console.Write(output); - PrMergePlanAdoResult? result = null; - try { result = JsonSerializer.Deserialize(output.Trim(), PolyphonyJsonContext.Default.PrMergePlanAdoResult); } - catch (JsonException) { } - payload = result is null - ? new PrMergePlanAdoPayload { RootId = rootId, ItemId = itemId, ParentItemId = parentItemId, PrNumber = prNumber, ItemKey = itemId == rootId ? "root" : (itemId > 0 ? itemId.ToString(CultureInfo.InvariantCulture) : ""), IsRootPlan = itemId == rootId, Organization = organization, Project = project, Repository = repository, HeadBranch = "", BaseBranch = "", ManifestBranch = "", ResultAction = "error", Succeeded = false, WasMutated = false, AlreadyMerged = false, Error = output.Trim() } - : new PrMergePlanAdoPayload - { - RootId = result.RootId, - ItemId = result.ItemId, - ParentItemId = result.ParentItemId, - PrNumber = result.PrNumber, - ItemKey = result.ItemKey, - IsRootPlan = result.IsRootPlan, - Organization = result.Organization, - Project = result.Project, - Repository = result.Repository, - HeadBranch = result.HeadBranch, - BaseBranch = result.BaseBranch, - ManifestBranch = result.ManifestBranch, - RepoSlug = result.RepoSlug, - PrUrl = result.PrUrl, - LockToken = result.LockToken, - MergeCommit = result.MergeCommit, - ResultAction = result.ErrorCode?.Length > 0 ? "error" : (result.AlreadyMerged ? "already_merged" : "merged"), - Succeeded = string.IsNullOrEmpty(result.ErrorCode), - WasMutated = result.Merged && !result.AlreadyMerged, - AlreadyMerged = result.AlreadyMerged, - ManifestRecorded = result.ManifestRecorded, - ManifestPushed = result.ManifestPushed, - ErrorCode = result.ErrorCode?.Length > 0 ? result.ErrorCode : null, - Error = result.Error, - }; - return exitCode; + RootId = result.RootId, + ItemId = result.ItemId, + ParentItemId = result.ParentItemId, + PrNumber = result.PrNumber, + ItemKey = result.ItemKey, + IsRootPlan = result.IsRootPlan, + Organization = result.Organization, + Project = result.Project, + Repository = result.Repository, + HeadBranch = result.HeadBranch, + BaseBranch = result.BaseBranch, + ManifestBranch = result.ManifestBranch, + RepoSlug = result.RepoSlug, + PrUrl = result.PrUrl, + LockToken = result.LockToken, + MergeCommit = result.MergeCommit, + ResultAction = result.ErrorCode?.Length > 0 ? "error" : (result.AlreadyMerged ? "already_merged" : "merged"), + Succeeded = string.IsNullOrEmpty(result.ErrorCode), + WasMutated = result.Merged && !result.AlreadyMerged, + AlreadyMerged = result.AlreadyMerged, + ManifestRecorded = result.ManifestRecorded, + ManifestPushed = result.ManifestPushed, + ErrorCode = result.ErrorCode?.Length > 0 ? result.ErrorCode : null, + Error = result.Error, + }; + return execution.ExitCode; }, outcomeSelector: exitCode => SelectJournalOutcome( exitCode, @@ -153,7 +139,14 @@ public async Task MergePlanAdo( ct: ct).ConfigureAwait(false); } - private async Task MergePlanAdoBodyAsync( + /// + /// Typed seam introduced by AB#3313. Computes the + /// envelope without any stdout side + /// effects. The public shell parses the CLI + /// envelope, calls this method, serializes the result, and feeds the + /// journal payload directly from the typed result (no JSON reparse). + /// + internal async Task> MergePlanAdoAsync( string organization, string project, string repository, @@ -174,22 +167,22 @@ private async Task MergePlanAdoBodyAsync( || string.IsNullOrWhiteSpace(project) || string.IsNullOrWhiteSpace(repository)) { - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "invalid_argument", "organization, project, and repository are required"); } if (!Branching.RootId.TryParse(rootId, out var root)) - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "invalid_argument", $"--root-id must be positive (got {rootId})"); if (!WorkItemId.TryParse(itemId, out var item)) - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "invalid_argument", $"--item-id must be positive (got {itemId})"); if (prNumber <= 0) - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "invalid_argument", $"--pr-number must be positive (got {prNumber})"); @@ -202,7 +195,7 @@ private async Task MergePlanAdoBodyAsync( if (isRootPlan) { if (parentItemId != 0) - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "invalid_argument", $"--parent-item-id must be omitted when --item-id == --root-id (got {parentItemId}); the root plan has no parent."); @@ -220,16 +213,16 @@ private async Task MergePlanAdoBodyAsync( else { if (!WorkItemId.TryParse(parentItemId, out var parentItem)) - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "invalid_argument", $"--parent-item-id must be positive (got {parentItemId})"); if (parentItemId == itemId) - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "invalid_argument", $"--parent-item-id ({parentItemId}) must not equal --item-id; a plan cannot be its own parent."); if (parentItemId == rootId) - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "invalid_argument", $"--parent-item-id ({parentItemId}) equals --root-id; omit --parent-item-id when the parent is the root plan."); @@ -245,7 +238,7 @@ private async Task MergePlanAdoBodyAsync( // ── 1b. Resolve local manifest path (Rev 4.2). ───────────────────── var resolvedPath = await ManifestPathHelper.ResolveAsync(statePaths, rootId, manifestPath, ct).ConfigureAwait(false); if (resolvedPath.Error is not null) - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, resolvedParent, organization, project, repository, slug, prUrl, prNumber, "internal_error", resolvedPath.Error, isRootPlan, itemKey, headBranch, baseBranch, manifestBranch); @@ -253,7 +246,7 @@ private async Task MergePlanAdoBodyAsync( if (ado is null) { - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, resolvedParent, organization, project, repository, slug, prUrl, prNumber, "ado_failed", "IAdoClient is not configured", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch); @@ -267,7 +260,7 @@ private async Task MergePlanAdoBodyAsync( } catch (Exception ex) { - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, resolvedParent, organization, project, repository, slug, prUrl, prNumber, "internal_error", ex.Message, isRootPlan, itemKey, headBranch, baseBranch, manifestBranch); @@ -299,7 +292,7 @@ private async Task MergePlanAdoBodyAsync( AcquireFailureReason.Stale => "lock_stale", _ => "lock_unreadable", }; - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, resolvedParent, organization, project, repository, slug, prUrl, prNumber, code, $"Could not acquire run lock at '{lockPath}' (reason: {acquireOutcome.Reason?.ToString().ToLowerInvariant() ?? "unknown"}).", @@ -338,7 +331,7 @@ private async Task MergePlanAdoBodyAsync( } - private async Task MergePlanAdoUnderLockAsync( + private async Task> MergePlanAdoUnderLockAsync( int rootId, int itemId, int parentItemId, @@ -362,7 +355,7 @@ private async Task MergePlanAdoUnderLockAsync( { var status = await git.GetStatusAsync(ct).ConfigureAwait(false); if (status.Count > 0) - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "worktree_dirty", $"Worktree is not clean ({status.Count} entries from `git status --porcelain`); commit, stash, or discard local changes before retrying.", @@ -371,7 +364,7 @@ private async Task MergePlanAdoUnderLockAsync( catch (OperationCanceledException) { throw; } catch (Exception ex) { - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "internal_error", $"Could not read worktree status: {ex.Message}", @@ -392,14 +385,14 @@ private async Task MergePlanAdoUnderLockAsync( catch (OperationCanceledException) { throw; } catch (InvalidOperationException ex) { - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "no_pat", ex.Message, isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, lockToken: lockToken); } catch (TimeoutException ex) { - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "ado_timeout", ex.Message, isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, lockToken: lockToken); @@ -409,28 +402,28 @@ private async Task MergePlanAdoUnderLockAsync( var code = ex.StatusCode is HttpStatusCode.Unauthorized or HttpStatusCode.Forbidden ? "no_pat" : "ado_failed"; - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, code, ex.Message, isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, lockToken: lockToken); } catch (Exception ex) { - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "ado_failed", $"ADO PR poll failed: {ex.Message}", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, lockToken: lockToken); } if (poll is null) - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "pr_not_found", $"PR #{prNumber} not found in {slug}.", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, lockToken: lockToken); if (!string.Equals(poll.HeadRefName, headBranch, StringComparison.Ordinal)) - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "pr_identity_mismatch", $"PR #{prNumber} head ref is '{poll.HeadRefName}' but the verb expected '{headBranch}'. Refusing to act on the wrong PR.", @@ -438,7 +431,7 @@ private async Task MergePlanAdoUnderLockAsync( prState: poll.State); if (!string.Equals(poll.BaseRefName, baseBranch, StringComparison.Ordinal)) - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "pr_identity_mismatch", $"PR #{prNumber} base ref is '{poll.BaseRefName}' but the verb expected '{baseBranch}'. Refusing to act on the wrong PR.", @@ -463,7 +456,7 @@ private async Task MergePlanAdoUnderLockAsync( } catch (Exception ex) { - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "internal_error", $"Could not parse manifest at {manifestPath} for staleness check: {ex.Message}", @@ -478,7 +471,7 @@ private async Task MergePlanAdoUnderLockAsync( var staleness = PlanGenerationStaleness.Check(snapshot, currentGens); if (staleness.IsEmpty) { - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "stale_generation", $"PR #{prNumber} body has no ancestor_plan_generations snapshot in front-matter; descendant plan PRs must carry a snapshot to be merged safely. Re-open the PR via `polyphony pr open-plan-ado` to embed the current snapshot.", @@ -497,7 +490,7 @@ private async Task MergePlanAdoUnderLockAsync( }) .ToList(); - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "stale_generation", $"PR #{prNumber} ancestor plan-generation snapshot is stale vs the current manifest on origin/{manifestBranch}. Stale entries: {PlanGenerationStaleness.FormatStaleEntries(staleness.StaleEntries)}. Re-open the PR with the current snapshot before merging.", @@ -517,7 +510,7 @@ private async Task MergePlanAdoUnderLockAsync( if (string.Equals(poll.State, "MERGED", StringComparison.OrdinalIgnoreCase)) { if (string.IsNullOrEmpty(poll.MergeCommit)) - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "missing_merge_commit", $"PR #{prNumber} reports state MERGED but ADO did not return a merge commit SHA.", @@ -541,7 +534,7 @@ private async Task MergePlanAdoUnderLockAsync( catch (OperationCanceledException) { throw; } catch (InvalidOperationException ex) { - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "no_pat", ex.Message, isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, lockToken: lockToken, @@ -549,7 +542,7 @@ private async Task MergePlanAdoUnderLockAsync( } catch (TimeoutException ex) { - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "ado_timeout", ex.Message, isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, lockToken: lockToken, @@ -560,7 +553,7 @@ private async Task MergePlanAdoUnderLockAsync( var code = ex.StatusCode is HttpStatusCode.Unauthorized or HttpStatusCode.Forbidden ? "no_pat" : "ado_complete_failed"; - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, code, ex.Message, isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, lockToken: lockToken, @@ -568,7 +561,7 @@ private async Task MergePlanAdoUnderLockAsync( } catch (Exception ex) { - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "ado_complete_failed", $"ADO complete-PR call failed: {ex.Message}", @@ -580,7 +573,7 @@ private async Task MergePlanAdoUnderLockAsync( { case "completed": if (string.IsNullOrEmpty(complete.MergeCommitSha)) - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "missing_merge_commit", "ADO complete-PR succeeded but did not return a merge commit SHA; cannot record the merge in the ledger.", @@ -590,21 +583,21 @@ private async Task MergePlanAdoUnderLockAsync( alreadyMerged = false; break; case "stale_head": - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "stale_head", $"ADO refused to complete PR #{prNumber}: source branch advanced past the polled head SHA '{poll.HeadRefOid}'. Re-poll and retry. Detail: {complete.ErrorBody}", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, lockToken: lockToken, prState: poll.State); case "not_found": - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "pr_not_found", $"PR #{prNumber} disappeared between poll and complete in {slug}.", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, lockToken: lockToken, prState: poll.State); case "completion_pending": - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "completion_pending", $"ADO accepted the complete-PR PATCH for PR #{prNumber} but the PR did not transition to status=completed within the poll budget. The merge may still land asynchronously, or the PR may be blocked by a policy. Inspect via `az repos pr show --id {prNumber}` and consider `az repos pr update --id {prNumber} --status completed` to land it manually. Detail: {complete.ErrorBody}", @@ -613,7 +606,7 @@ private async Task MergePlanAdoUnderLockAsync( case "not_mergeable": default: - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "ado_complete_failed", $"ADO refused to complete PR #{prNumber} (HTTP {complete.HttpStatus}, status={complete.Status}): {complete.ErrorBody}", @@ -623,7 +616,7 @@ private async Task MergePlanAdoUnderLockAsync( } else { - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "pr_state_invalid", $"PR #{prNumber} is in state '{poll.State}'; only OPEN or MERGED are actionable.", @@ -644,7 +637,7 @@ private async Task MergePlanAdoUnderLockAsync( } catch (Exception ex) { - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "manifest_read_failed", $"Could not load manifest at '{manifestPath}': {ex.Message}", @@ -654,7 +647,7 @@ private async Task MergePlanAdoUnderLockAsync( var ledger = ManifestPlanLedger.Apply(manifest, itemKey, prNumber, mergeCommit, DateTime.UtcNow); if (ledger.ConflictReason is not null) - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "ledger_conflict", ledger.ConflictReason, isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, lockToken: lockToken, @@ -673,7 +666,7 @@ private async Task MergePlanAdoUnderLockAsync( } catch (Exception ex) { - return EmitMergePlanAdoError( + return MergePlanAdoError( rootId, itemId, parentItemId, organization, project, repository, slug, prUrl, prNumber, "internal_error", $"Manifest save failed: {ex.Message}", @@ -685,42 +678,43 @@ private async Task MergePlanAdoUnderLockAsync( } // ── 9. Emit success. ─────────────────────────────────────────────── - EmitMergePlanAdo(new PrMergePlanAdoResult - { - RootId = rootId, - ItemId = itemId, - ParentItemId = parentItemId, - ItemKey = itemKey, - IsRootPlan = isRootPlan, - HeadBranch = headBranch, - BaseBranch = baseBranch, - ManifestBranch = manifestBranch, - Organization = organization, - Project = project, - Repository = repository, - RepoSlug = slug, - PrNumber = prNumber, - PrUrl = prUrl, - PrState = "MERGED", - Merged = true, - AlreadyMerged = alreadyMerged, - MergeCommit = mergeCommit, - ManifestRecorded = manifestRecorded, - ManifestPushed = manifestPushed, - PreviousGeneration = ledger.PreviousGeneration, - CurrentGeneration = ledger.CurrentGeneration, - LockToken = lockToken, - LockReleased = false, - ErrorCode = "", - }); - return ExitCodes.Success; + return new CommandExecution( + new PrMergePlanAdoResult + { + RootId = rootId, + ItemId = itemId, + ParentItemId = parentItemId, + ItemKey = itemKey, + IsRootPlan = isRootPlan, + HeadBranch = headBranch, + BaseBranch = baseBranch, + ManifestBranch = manifestBranch, + Organization = organization, + Project = project, + Repository = repository, + RepoSlug = slug, + PrNumber = prNumber, + PrUrl = prUrl, + PrState = "MERGED", + Merged = true, + AlreadyMerged = alreadyMerged, + MergeCommit = mergeCommit, + ManifestRecorded = manifestRecorded, + ManifestPushed = manifestPushed, + PreviousGeneration = ledger.PreviousGeneration, + CurrentGeneration = ledger.CurrentGeneration, + LockToken = lockToken, + LockReleased = false, + ErrorCode = "", + }, + ExitCodes.Success); } private static void EmitMergePlanAdo(PrMergePlanAdoResult result) => Console.WriteLine(JsonSerializer.Serialize( result, PolyphonyJsonContext.Default.PrMergePlanAdoResult)); - private static int EmitMergePlanAdoError( + private static CommandExecution MergePlanAdoError( int rootId, int itemId, int parentItemId, @@ -748,36 +742,37 @@ private static int EmitMergePlanAdoError( bool manifestPushed = false, IReadOnlyList? staleAncestors = null) { - EmitMergePlanAdo(new PrMergePlanAdoResult - { - RootId = rootId, - ItemId = itemId, - ParentItemId = parentItemId, - ItemKey = itemKey, - IsRootPlan = isRootPlan, - HeadBranch = headBranch, - BaseBranch = baseBranch, - ManifestBranch = manifestBranch, - Organization = organization ?? string.Empty, - Project = project ?? string.Empty, - Repository = repository ?? string.Empty, - RepoSlug = slug ?? string.Empty, - PrNumber = prNumber, - PrUrl = prUrl ?? string.Empty, - PrState = prState, - Merged = merged, - AlreadyMerged = alreadyMerged, - MergeCommit = mergeCommit, - ManifestRecorded = manifestRecorded, - ManifestPushed = manifestPushed, - PreviousGeneration = prevGen, - CurrentGeneration = currGen, - LockToken = lockToken, - LockReleased = false, - ErrorCode = errorCode, - Error = message, - StaleAncestors = staleAncestors, - }); - return ExitCodes.Success; // routing-style: workflow branches on ErrorCode, not exit code + return new CommandExecution( + new PrMergePlanAdoResult + { + RootId = rootId, + ItemId = itemId, + ParentItemId = parentItemId, + ItemKey = itemKey, + IsRootPlan = isRootPlan, + HeadBranch = headBranch, + BaseBranch = baseBranch, + ManifestBranch = manifestBranch, + Organization = organization ?? string.Empty, + Project = project ?? string.Empty, + Repository = repository ?? string.Empty, + RepoSlug = slug ?? string.Empty, + PrNumber = prNumber, + PrUrl = prUrl ?? string.Empty, + PrState = prState, + Merged = merged, + AlreadyMerged = alreadyMerged, + MergeCommit = mergeCommit, + ManifestRecorded = manifestRecorded, + ManifestPushed = manifestPushed, + PreviousGeneration = prevGen, + CurrentGeneration = currGen, + LockToken = lockToken, + LockReleased = false, + ErrorCode = errorCode, + Error = message, + StaleAncestors = staleAncestors, + }, + ExitCodes.Success); // routing-style: workflow branches on ErrorCode, not exit code } } diff --git a/src/Polyphony/Commands/PrCommands.MergePlanPr.cs b/src/Polyphony/Commands/PrCommands.MergePlanPr.cs index 68e0d23..70c0945 100644 --- a/src/Polyphony/Commands/PrCommands.MergePlanPr.cs +++ b/src/Polyphony/Commands/PrCommands.MergePlanPr.cs @@ -94,50 +94,36 @@ public async Task MergePlanPr( CreateJournalInvocation("pr_merge_plan_pr", PullRequestJournalTarget(prNumber), rootId, itemId), async innerCt => { - var sw = new StringWriter(); - var originalOut = Console.Out; - int exitCode; - try - { - Console.SetOut(sw); - exitCode = await MergePlanPrBodyAsync(rootId, itemId, prNumber, parentItemId, ancestorIds, manifestPath, admin, lockTtlHours, by, innerCt).ConfigureAwait(false); - } - finally + var execution = await MergePlanPrAsync( + rootId, itemId, prNumber, parentItemId, ancestorIds, manifestPath, admin, lockTtlHours, by, innerCt) + .ConfigureAwait(false); + EmitMergePlan(execution.Result); + var result = execution.Result; + payload = new PrMergePlanPrPayload { - Console.SetOut(originalOut); - } - var output = sw.ToString(); - Console.Write(output); - PrMergePlanPrResult? result = null; - try { result = JsonSerializer.Deserialize(output.Trim(), PolyphonyJsonContext.Default.PrMergePlanPrResult); } - catch (JsonException) { } - payload = result is null - ? new PrMergePlanPrPayload { RootId = rootId, ItemId = itemId, ParentItemId = parentItemId, PrNumber = prNumber, ItemKey = itemId == rootId ? "root" : (itemId > 0 ? itemId.ToString(CultureInfo.InvariantCulture) : ""), IsRootPlan = itemId == rootId, HeadBranch = "", BaseBranch = "", ManifestBranch = "", ResultAction = "error", Succeeded = false, WasMutated = false, AlreadyMerged = false, Error = output.Trim() } - : new PrMergePlanPrPayload - { - RootId = result.RootId, - ItemId = result.ItemId, - ParentItemId = result.ParentItemId, - PrNumber = result.PrNumber, - ItemKey = result.ItemKey, - IsRootPlan = result.IsRootPlan, - HeadBranch = result.HeadBranch, - BaseBranch = result.BaseBranch, - ManifestBranch = result.ManifestBranch, - RepoSlug = result.RepoSlug, - PrUrl = null, - LockToken = result.LockToken, - MergeCommit = result.MergeCommit, - ResultAction = result.ErrorCode?.Length > 0 ? "error" : (result.AlreadyMerged ? "already_merged" : "merged"), - Succeeded = string.IsNullOrEmpty(result.ErrorCode), - WasMutated = result.Merged && !result.AlreadyMerged, - AlreadyMerged = result.AlreadyMerged, - ManifestRecorded = result.ManifestRecorded, - ManifestPushed = result.ManifestPushed, - ErrorCode = result.ErrorCode?.Length > 0 ? result.ErrorCode : null, - Error = result.Error, - }; - return exitCode; + RootId = result.RootId, + ItemId = result.ItemId, + ParentItemId = result.ParentItemId, + PrNumber = result.PrNumber, + ItemKey = result.ItemKey, + IsRootPlan = result.IsRootPlan, + HeadBranch = result.HeadBranch, + BaseBranch = result.BaseBranch, + ManifestBranch = result.ManifestBranch, + RepoSlug = result.RepoSlug, + PrUrl = null, + LockToken = result.LockToken, + MergeCommit = result.MergeCommit, + ResultAction = result.ErrorCode?.Length > 0 ? "error" : (result.AlreadyMerged ? "already_merged" : "merged"), + Succeeded = string.IsNullOrEmpty(result.ErrorCode), + WasMutated = result.Merged && !result.AlreadyMerged, + AlreadyMerged = result.AlreadyMerged, + ManifestRecorded = result.ManifestRecorded, + ManifestPushed = result.ManifestPushed, + ErrorCode = result.ErrorCode?.Length > 0 ? result.ErrorCode : null, + Error = result.Error, + }; + return execution.ExitCode; }, outcomeSelector: exitCode => SelectJournalOutcome( exitCode, @@ -148,7 +134,14 @@ public async Task MergePlanPr( ct: ct).ConfigureAwait(false); } - private async Task MergePlanPrBodyAsync( + /// + /// Typed seam introduced by AB#3313. Computes the + /// envelope without any stdout side + /// effects. The public shell parses the CLI + /// envelope, calls this method, serializes the result, and feeds the + /// journal payload directly from the typed result (no JSON reparse). + /// + internal async Task> MergePlanPrAsync( int rootId, int itemId, int prNumber, @@ -163,15 +156,15 @@ private async Task MergePlanPrBodyAsync( // ── 1. Validate inputs + derive head/base. ────────────────────────── if (!Branching.RootId.TryParse(rootId, out var root)) - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "config_error", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "config_error", $"--root-id must be positive (got {rootId})"); if (!WorkItemId.TryParse(itemId, out var item)) - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "config_error", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "config_error", $"--item-id must be positive (got {itemId})"); if (prNumber <= 0) - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "config_error", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "config_error", $"--pr-number must be positive (got {prNumber})"); bool isRootPlan = itemId == rootId; @@ -183,7 +176,7 @@ private async Task MergePlanPrBodyAsync( if (isRootPlan) { if (parentItemId != 0) - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "config_error", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "config_error", $"--parent-item-id must be omitted when --item-id == --root-id (got {parentItemId}); the root plan has no parent."); itemKey = "root"; headBranch = BranchNameBuilder.RootPlan(root).Value; @@ -199,13 +192,13 @@ private async Task MergePlanPrBodyAsync( else { if (!WorkItemId.TryParse(parentItemId, out var parentItem)) - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "config_error", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "config_error", $"--parent-item-id must be positive (got {parentItemId})"); if (parentItemId == itemId) - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "config_error", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "config_error", $"--parent-item-id ({parentItemId}) must not equal --item-id; a plan cannot be its own parent."); if (parentItemId == rootId) - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "config_error", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "config_error", $"--parent-item-id ({parentItemId}) equals --root-id; omit --parent-item-id when the parent is the root plan."); resolvedParent = parentItemId; headBranch = BranchNameBuilder.DescendantPlan(root, item).Value; @@ -222,7 +215,7 @@ private async Task MergePlanPrBodyAsync( // Resolved here so we can fail-fast without holding a lock. var resolvedPath = await ManifestPathHelper.ResolveAsync(statePaths, rootId, manifestPath, ct).ConfigureAwait(false); if (resolvedPath.Error is not null) - return EmitMergePlanError(rootId, itemId, resolvedParent, prNumber, "internal_error", resolvedPath.Error, + return MergePlanError(rootId, itemId, resolvedParent, prNumber, "internal_error", resolvedPath.Error, isRootPlan, itemKey, headBranch, baseBranch, manifestBranch); var localManifestPath = resolvedPath.Path; @@ -235,11 +228,11 @@ private async Task MergePlanPrBodyAsync( catch (OperationCanceledException) { throw; } catch (Exception ex) { - return EmitMergePlanError(rootId, itemId, resolvedParent, prNumber, "repo_not_resolved", ex.Message, + return MergePlanError(rootId, itemId, resolvedParent, prNumber, "repo_not_resolved", ex.Message, isRootPlan, itemKey, headBranch, baseBranch, manifestBranch); } if (string.IsNullOrEmpty(slug)) - return EmitMergePlanError(rootId, itemId, resolvedParent, prNumber, "repo_not_resolved", + return MergePlanError(rootId, itemId, resolvedParent, prNumber, "repo_not_resolved", "Could not resolve repo slug from origin remote", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch); @@ -251,7 +244,7 @@ private async Task MergePlanPrBodyAsync( } catch (Exception ex) { - return EmitMergePlanError(rootId, itemId, resolvedParent, prNumber, "internal_error", ex.Message, + return MergePlanError(rootId, itemId, resolvedParent, prNumber, "internal_error", ex.Message, isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug); } @@ -281,7 +274,7 @@ private async Task MergePlanPrBodyAsync( AcquireFailureReason.Stale => "lock_stale", _ => "lock_unreadable", }; - return EmitMergePlanError(rootId, itemId, resolvedParent, prNumber, code, + return MergePlanError(rootId, itemId, resolvedParent, prNumber, code, $"Could not acquire run lock at '{lockPath}' (reason: {acquireOutcome.Reason?.ToString().ToLowerInvariant() ?? "unknown"}).", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug); } @@ -317,7 +310,7 @@ private async Task MergePlanPrBodyAsync( } - private async Task MergePlanPrUnderLockAsync( + private async Task> MergePlanPrUnderLockAsync( int rootId, int itemId, int parentItemId, @@ -339,14 +332,14 @@ private async Task MergePlanPrUnderLockAsync( { var status = await git.GetStatusAsync(ct).ConfigureAwait(false); if (status.Count > 0) - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "worktree_dirty", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "worktree_dirty", $"Worktree is not clean ({status.Count} entries from `git status --porcelain`); commit, stash, or discard local changes before retrying.", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug, lockToken: lockToken); } catch (OperationCanceledException) { throw; } catch (Exception ex) { - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "internal_error", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "internal_error", $"Could not read worktree status: {ex.Message}", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug, lockToken: lockToken); } @@ -366,23 +359,23 @@ private async Task MergePlanPrUnderLockAsync( catch (OperationCanceledException) { throw; } catch (Exception ex) { - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "internal_error", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "internal_error", $"gh pr view failed: {ex.Message}", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug, lockToken: lockToken); } if (poll is null) - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "pr_not_found", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "pr_not_found", $"PR #{prNumber} not found on {slug}.", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug, lockToken: lockToken); if (!string.Equals(poll.HeadRefName, headBranch, StringComparison.Ordinal)) - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "head_ref_mismatch", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "head_ref_mismatch", $"PR #{prNumber} head ref is '{poll.HeadRefName ?? ""}' but the verb expected '{headBranch}'. Refusing to act on the wrong PR.", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug, lockToken: lockToken, prState: poll.State); if (!string.Equals(poll.BaseRefName, baseBranch, StringComparison.Ordinal)) - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "base_ref_mismatch", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "base_ref_mismatch", $"PR #{prNumber} base ref is '{poll.BaseRefName ?? ""}' but the verb expected '{baseBranch}'. Refusing to act on the wrong PR.", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug, lockToken: lockToken, prState: poll.State); @@ -407,7 +400,7 @@ private async Task MergePlanPrUnderLockAsync( } catch (Exception ex) { - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "internal_error", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "internal_error", $"Could not parse manifest at {manifestPath} for staleness check: {ex.Message}", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug, lockToken: lockToken, prState: poll.State); @@ -424,7 +417,7 @@ private async Task MergePlanPrUnderLockAsync( // Root plans were already excluded above, so a descendant // without a snapshot indicates a hand-opened PR or one // pre-dating P3; we can't verify its safety. - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "stale_generation", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "stale_generation", $"PR #{prNumber} body has no ancestor_plan_generations snapshot in front-matter; descendant plan PRs must carry a snapshot to be merged safely. Re-open the PR via `polyphony pr open-plan-pr` to embed the current snapshot.", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug, lockToken: lockToken, prState: poll.State); @@ -441,7 +434,7 @@ private async Task MergePlanPrUnderLockAsync( }) .ToList(); - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "stale_generation", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "stale_generation", $"PR #{prNumber} ancestor plan-generation snapshot is stale vs the current manifest on origin/{manifestBranch}. Stale entries: {PlanGenerationStaleness.FormatStaleEntries(staleness.StaleEntries)}. Re-open the PR with the current snapshot before merging.", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug, lockToken: lockToken, prState: poll.State, staleAncestors: staleEntries); @@ -465,14 +458,14 @@ private async Task MergePlanPrUnderLockAsync( catch (OperationCanceledException) { throw; } catch (Exception ex) { - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "internal_error", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "internal_error", $"Could not fetch PR #{prNumber} changed files for diff validation: {ex.Message}", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug, lockToken: lockToken, prState: poll.State); } if (changedFiles is null) - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "pr_not_found", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "pr_not_found", $"PR #{prNumber} files endpoint returned no payload during diff validation.", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug, lockToken: lockToken, prState: poll.State); @@ -495,7 +488,7 @@ private async Task MergePlanPrUnderLockAsync( if (classification.Severity == ValidationSeverity.Blocking) { - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "validation_blocked", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "validation_blocked", $"PR #{prNumber} fails plan-diff validation ({classification.Code}): {classification.Message}", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug, lockToken: lockToken, prState: poll.State); @@ -512,7 +505,7 @@ private async Task MergePlanPrUnderLockAsync( if (string.Equals(poll.State, "MERGED", StringComparison.OrdinalIgnoreCase)) { if (string.IsNullOrEmpty(poll.MergeCommitSha)) - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "missing_merge_commit", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "missing_merge_commit", $"PR #{prNumber} reports state MERGED but the platform did not return a merge commit SHA.", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug, lockToken: lockToken, prState: poll.State); @@ -535,18 +528,18 @@ private async Task MergePlanPrUnderLockAsync( catch (OperationCanceledException) { throw; } catch (Exception ex) { - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "merge_failed", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "merge_failed", $"gh pr merge failed: {ex.Message}", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug, lockToken: lockToken, prState: poll.State); } if (!mergeResult.Succeeded) - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "merge_failed", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "merge_failed", $"gh pr merge did not succeed: {mergeResult.Detail}", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug, lockToken: lockToken, prState: poll.State); if (string.IsNullOrEmpty(mergeResult.MergeSha)) - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "missing_merge_commit", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "missing_merge_commit", "gh pr merge returned without a merge commit SHA; cannot record the merge in the ledger.", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug, lockToken: lockToken, prState: "MERGED"); @@ -555,7 +548,7 @@ private async Task MergePlanPrUnderLockAsync( } else { - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "pr_state_unmergeable", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "pr_state_unmergeable", $"PR #{prNumber} is in state '{poll.State}'; only OPEN or MERGED are actionable.", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug, lockToken: lockToken, prState: poll.State); @@ -574,7 +567,7 @@ private async Task MergePlanPrUnderLockAsync( } catch (Exception ex) { - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "internal_error", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "internal_error", $"Could not load manifest at '{manifestPath}': {ex.Message}", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug, lockToken: lockToken, prState: "MERGED", merged: true, alreadyMerged: alreadyMerged, mergeCommit: mergeCommit); @@ -582,7 +575,7 @@ private async Task MergePlanPrUnderLockAsync( var ledger = ManifestPlanLedger.Apply(manifest, itemKey, prNumber, mergeCommit, DateTime.UtcNow); if (ledger.ConflictReason is not null) - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "ledger_conflict", ledger.ConflictReason, + return MergePlanError(rootId, itemId, parentItemId, prNumber, "ledger_conflict", ledger.ConflictReason, isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug, lockToken: lockToken, prState: "MERGED", merged: true, alreadyMerged: alreadyMerged, mergeCommit: mergeCommit, prevGen: ledger.PreviousGeneration, currGen: ledger.CurrentGeneration); @@ -602,7 +595,7 @@ private async Task MergePlanPrUnderLockAsync( } catch (Exception ex) { - return EmitMergePlanError(rootId, itemId, parentItemId, prNumber, "internal_error", + return MergePlanError(rootId, itemId, parentItemId, prNumber, "internal_error", $"Manifest save failed: {ex.Message}", isRootPlan, itemKey, headBranch, baseBranch, manifestBranch, slug: slug, lockToken: lockToken, prState: "MERGED", merged: true, alreadyMerged: alreadyMerged, mergeCommit: mergeCommit, @@ -612,31 +605,32 @@ private async Task MergePlanPrUnderLockAsync( } // ── 10. Emit success. ────────────────────────────────────────────── - EmitMergePlan(new PrMergePlanPrResult - { - RootId = rootId, - ItemId = itemId, - ParentItemId = parentItemId, - ItemKey = itemKey, - IsRootPlan = isRootPlan, - HeadBranch = headBranch, - BaseBranch = baseBranch, - ManifestBranch = manifestBranch, - RepoSlug = slug, - PrNumber = prNumber, - PrState = "MERGED", - Merged = true, - AlreadyMerged = alreadyMerged, - MergeCommit = mergeCommit, - ManifestRecorded = manifestRecorded, - ManifestPushed = manifestPushed, - PreviousGeneration = ledger.PreviousGeneration, - CurrentGeneration = ledger.CurrentGeneration, - LockToken = lockToken, - LockReleased = false, // set by the caller after release; emitted as false here so workflows treat success+leaked-lock as a workflow-warning-not-failure case - ErrorCode = "", - }); - return ExitCodes.Success; + return new CommandExecution( + new PrMergePlanPrResult + { + RootId = rootId, + ItemId = itemId, + ParentItemId = parentItemId, + ItemKey = itemKey, + IsRootPlan = isRootPlan, + HeadBranch = headBranch, + BaseBranch = baseBranch, + ManifestBranch = manifestBranch, + RepoSlug = slug, + PrNumber = prNumber, + PrState = "MERGED", + Merged = true, + AlreadyMerged = alreadyMerged, + MergeCommit = mergeCommit, + ManifestRecorded = manifestRecorded, + ManifestPushed = manifestPushed, + PreviousGeneration = ledger.PreviousGeneration, + CurrentGeneration = ledger.CurrentGeneration, + LockToken = lockToken, + LockReleased = false, // set by the caller after release; emitted as false here so workflows treat success+leaked-lock as a workflow-warning-not-failure case + ErrorCode = "", + }, + ExitCodes.Success); } private async Task SafeResolveRepoRootAsync(CancellationToken ct) @@ -645,7 +639,7 @@ private async Task SafeResolveRepoRootAsync(CancellationToken ct) catch { return ""; } } - private static int EmitMergePlanError( + private static CommandExecution MergePlanError( int rootId, int itemId, int parentItemId, @@ -669,33 +663,34 @@ private static int EmitMergePlanError( bool manifestPushed = false, IReadOnlyList? staleAncestors = null) { - EmitMergePlan(new PrMergePlanPrResult - { - RootId = rootId, - ItemId = itemId, - ParentItemId = parentItemId, - ItemKey = itemKey, - IsRootPlan = isRootPlan, - HeadBranch = headBranch, - BaseBranch = baseBranch, - ManifestBranch = manifestBranch, - RepoSlug = slug, - PrNumber = prNumber, - PrState = prState, - Merged = merged, - AlreadyMerged = alreadyMerged, - MergeCommit = mergeCommit, - ManifestRecorded = manifestRecorded, - ManifestPushed = manifestPushed, - PreviousGeneration = prevGen, - CurrentGeneration = currGen, - LockToken = lockToken, - LockReleased = false, - ErrorCode = errorCode, - Error = message, - StaleAncestors = staleAncestors, - }); - return ExitCodes.Success; // routing-style: workflow branches on ErrorCode, not exit code + return new CommandExecution( + new PrMergePlanPrResult + { + RootId = rootId, + ItemId = itemId, + ParentItemId = parentItemId, + ItemKey = itemKey, + IsRootPlan = isRootPlan, + HeadBranch = headBranch, + BaseBranch = baseBranch, + ManifestBranch = manifestBranch, + RepoSlug = slug, + PrNumber = prNumber, + PrState = prState, + Merged = merged, + AlreadyMerged = alreadyMerged, + MergeCommit = mergeCommit, + ManifestRecorded = manifestRecorded, + ManifestPushed = manifestPushed, + PreviousGeneration = prevGen, + CurrentGeneration = currGen, + LockToken = lockToken, + LockReleased = false, + ErrorCode = errorCode, + Error = message, + StaleAncestors = staleAncestors, + }, + ExitCodes.Success); // routing-style: workflow branches on ErrorCode, not exit code } private static void EmitMergePlan(PrMergePlanPrResult result) diff --git a/src/Polyphony/Commands/PrCommands.OpenPlanAdo.cs b/src/Polyphony/Commands/PrCommands.OpenPlanAdo.cs index 53a8694..4f36167 100644 --- a/src/Polyphony/Commands/PrCommands.OpenPlanAdo.cs +++ b/src/Polyphony/Commands/PrCommands.OpenPlanAdo.cs @@ -108,49 +108,33 @@ public async Task OpenPlanAdo( CreateJournalInvocation("pr_open_plan_ado", BranchPairJournalTarget(jHead, jBase), rootId, itemId), async innerCt => { - var sw = new StringWriter(); - var originalOut = Console.Out; - int exitCode; - try - { - Console.SetOut(sw); - exitCode = await OpenPlanAdoBodyAsync(organization, project, repository, rootId, itemId, parentItemId, ancestorIds, manifestPath, title, body, innerCt).ConfigureAwait(false); - } - finally + var execution = await OpenPlanAdoAsync(organization, project, repository, rootId, itemId, parentItemId, ancestorIds, manifestPath, title, body, innerCt).ConfigureAwait(false); + EmitOpenPlanAdo(execution.Result); + var result = execution.Result; + payload = new PrOpenPlanAdoPayload { - Console.SetOut(originalOut); - } - var output = sw.ToString(); - Console.Write(output); - PrOpenPlanAdoResult? result = null; - try { result = JsonSerializer.Deserialize(output.Trim(), PolyphonyJsonContext.Default.PrOpenPlanAdoResult); } - catch (JsonException) { } - payload = result is null - ? new PrOpenPlanAdoPayload { RootId = rootId, ItemId = itemId, ParentItemId = parentItemId, ItemKey = itemId == rootId ? "root" : (itemId > 0 ? itemId.ToString(CultureInfo.InvariantCulture) : ""), IsRootPlan = itemId == rootId, Organization = organization, Project = project, Repository = repository, HeadBranch = jHead, BaseBranch = jBase, ResultAction = "error", Succeeded = false, WasMutated = false, Stale = false, Error = output.Trim() } - : new PrOpenPlanAdoPayload - { - RootId = result.RootId, - ItemId = result.ItemId, - ParentItemId = result.ParentItemId, - ItemKey = result.ItemKey, - IsRootPlan = result.IsRootPlan, - Organization = result.Organization, - Project = result.Project, - Repository = result.Repository, - HeadBranch = result.HeadBranch, - BaseBranch = result.BaseBranch, - RepoSlug = result.RepoSlug, - PrNumber = result.PrNumber, - PrUrl = result.PrUrl, - Title = result.Title, - ResultAction = result.ErrorCode?.Length > 0 ? "error" : (result.Stale ? "stale" : (result.Created ? "created" : "reused_existing_pr")), - Succeeded = string.IsNullOrEmpty(result.ErrorCode), - WasMutated = result.Created, - Stale = result.Stale, - ErrorCode = result.ErrorCode, - Error = result.Error, - }; - return exitCode; + RootId = result.RootId, + ItemId = result.ItemId, + ParentItemId = result.ParentItemId, + ItemKey = result.ItemKey, + IsRootPlan = result.IsRootPlan, + Organization = result.Organization, + Project = result.Project, + Repository = result.Repository, + HeadBranch = result.HeadBranch, + BaseBranch = result.BaseBranch, + RepoSlug = result.RepoSlug, + PrNumber = result.PrNumber, + PrUrl = result.PrUrl, + Title = result.Title, + ResultAction = result.ErrorCode?.Length > 0 ? "error" : (result.Stale ? "stale" : (result.Created ? "created" : "reused_existing_pr")), + Succeeded = string.IsNullOrEmpty(result.ErrorCode), + WasMutated = result.Created, + Stale = result.Stale, + ErrorCode = result.ErrorCode, + Error = result.Error, + }; + return execution.ExitCode; }, outcomeSelector: exitCode => SelectJournalOutcome( exitCode, @@ -161,7 +145,14 @@ public async Task OpenPlanAdo( ct: ct).ConfigureAwait(false); } - private async Task OpenPlanAdoBodyAsync( + /// + /// Typed seam introduced by AB#3313. Computes the + /// envelope without any stdout side + /// effects. The public shell parses the CLI + /// envelope, calls this method, serializes the result, and feeds the + /// journal payload directly from the typed result (no JSON reparse). + /// + internal async Task> OpenPlanAdoAsync( string organization, string project, string repository, @@ -181,21 +172,18 @@ private async Task OpenPlanAdoBodyAsync( || string.IsNullOrWhiteSpace(project) || string.IsNullOrWhiteSpace(repository)) { - EmitOpenPlanAdoError(rootId, itemId, parentItemId, organization, project, repository, slug, + return OpenPlanAdoError(rootId, itemId, parentItemId, organization, project, repository, slug, "invalid_argument", "organization, project, and repository are required"); - return ExitCodes.Success; } if (!Branching.RootId.TryParse(rootId, out var root)) { - EmitOpenPlanAdoError(rootId, itemId, parentItemId, organization, project, repository, slug, + return OpenPlanAdoError(rootId, itemId, parentItemId, organization, project, repository, slug, "invalid_argument", $"rootId must be positive (got {rootId})"); - return ExitCodes.Success; } if (!WorkItemId.TryParse(itemId, out var item)) { - EmitOpenPlanAdoError(rootId, itemId, parentItemId, organization, project, repository, slug, + return OpenPlanAdoError(rootId, itemId, parentItemId, organization, project, repository, slug, "invalid_argument", $"itemId must be positive (got {itemId})"); - return ExitCodes.Success; } bool isRootPlan = itemId == rootId; @@ -208,10 +196,9 @@ private async Task OpenPlanAdoBodyAsync( { if (parentItemId != 0) { - EmitOpenPlanAdoError(rootId, itemId, parentItemId, organization, project, repository, slug, + return OpenPlanAdoError(rootId, itemId, parentItemId, organization, project, repository, slug, "invalid_argument", $"--parent-item-id must not be provided when --item-id == --root-id (got {parentItemId}); the root plan has no parent."); - return ExitCodes.Success; } itemKey = "root"; headBranch = BranchNameBuilder.RootPlan(root).Value; @@ -228,23 +215,20 @@ private async Task OpenPlanAdoBodyAsync( { if (!WorkItemId.TryParse(parentItemId, out var parentItem)) { - EmitOpenPlanAdoError(rootId, itemId, parentItemId, organization, project, repository, slug, + return OpenPlanAdoError(rootId, itemId, parentItemId, organization, project, repository, slug, "invalid_argument", $"--parent-item-id must be positive (got {parentItemId})"); - return ExitCodes.Success; } if (parentItemId == itemId) { - EmitOpenPlanAdoError(rootId, itemId, parentItemId, organization, project, repository, slug, + return OpenPlanAdoError(rootId, itemId, parentItemId, organization, project, repository, slug, "invalid_argument", $"--parent-item-id ({parentItemId}) must not equal --item-id; a plan cannot be its own parent."); - return ExitCodes.Success; } if (parentItemId == rootId) { - EmitOpenPlanAdoError(rootId, itemId, parentItemId, organization, project, repository, slug, + return OpenPlanAdoError(rootId, itemId, parentItemId, organization, project, repository, slug, "invalid_argument", $"--parent-item-id ({parentItemId}) equals --root-id; omit --parent-item-id when the parent is the root plan."); - return ExitCodes.Success; } resolvedParent = parentItemId; headBranch = BranchNameBuilder.DescendantPlan(root, item).Value; @@ -255,18 +239,16 @@ private async Task OpenPlanAdoBodyAsync( if (!TryParseAncestorChain(ancestorIds, isRootPlan, itemKey, out var ancestorKeys, out var ancestorError)) { - EmitOpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, + return OpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, "invalid_argument", ancestorError, headBranch, baseBranch); - return ExitCodes.Success; } if (ado is null) { // Shouldn't happen in production (DI registers IAdoClient) but the // ctor allows null so unit tests can opt out of the ADO leg. - EmitOpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, + return OpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, "ado_failed", "IAdoClient is not configured", headBranch, baseBranch); - return ExitCodes.Success; } // ── 2. Read manifest + compute snapshot. ─────────────────────────── @@ -279,9 +261,8 @@ private async Task OpenPlanAdoBodyAsync( var resolvedPath = await ManifestPathHelper.ResolveAsync(statePaths, rootId, manifestPath, ct).ConfigureAwait(false); if (resolvedPath.Error is not null) { - EmitOpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, + return OpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, "manifest_read_failed", resolvedPath.Error, headBranch, baseBranch); - return ExitCodes.Success; } var localManifestPath = resolvedPath.Path; try @@ -291,18 +272,16 @@ private async Task OpenPlanAdoBodyAsync( } catch (FileNotFoundException) { - EmitOpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, + return OpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, "manifest_read_failed", $"manifest not found at {localManifestPath} — run `polyphony manifest init --root-id {rootId} ...` first", headBranch, baseBranch); - return ExitCodes.Success; } catch (OperationCanceledException) { throw; } catch (InvalidOperationException ex) { - EmitOpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, + return OpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, "manifest_invalid", $"manifest invalid: {ex.Message}", headBranch, baseBranch); - return ExitCodes.Success; } // ── 3. Build PR title + body. ────────────────────────────────────── @@ -323,11 +302,10 @@ private async Task OpenPlanAdoBodyAsync( if (activePrs is null) { - EmitOpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, + return OpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, "pr_not_found", $"Repository '{repository}' not found in {organization}/{project}.", headBranch, baseBranch); - return ExitCodes.Success; } var expectedSourceRef = "refs/heads/" + headBranch; @@ -370,7 +348,34 @@ private async Task OpenPlanAdoBodyAsync( if (SnapshotsEquivalent(existingMeta.AncestorPlanGenerations, snapshot)) { - EmitOpenPlanAdo(new PrOpenPlanAdoResult + return new CommandExecution( + new PrOpenPlanAdoResult + { + RootId = rootId, + ItemId = itemId, + ParentItemId = resolvedParent, + ItemKey = itemKey, + IsRootPlan = isRootPlan, + HeadBranch = headBranch, + BaseBranch = baseBranch, + Organization = organization, + Project = project, + Repository = repository, + RepoSlug = slug, + PrNumber = existing.PullRequestId, + PrUrl = BuildAdoPrUrl(organization, project, repository, existing.PullRequestId), + Title = prTitle, + Created = false, + Stale = false, + RequestsParentChange = existingMeta.RequestsParentChange, + AncestorPlanGenerations = existingMeta.AncestorPlanGenerations, + ErrorCode = "", + }, + ExitCodes.Success); + } + + return new CommandExecution( + new PrOpenPlanAdoResult { RootId = rootId, ItemId = itemId, @@ -387,38 +392,13 @@ private async Task OpenPlanAdoBodyAsync( PrUrl = BuildAdoPrUrl(organization, project, repository, existing.PullRequestId), Title = prTitle, Created = false, - Stale = false, + Stale = true, RequestsParentChange = existingMeta.RequestsParentChange, AncestorPlanGenerations = existingMeta.AncestorPlanGenerations, - ErrorCode = "", - }); - return ExitCodes.Success; - } - - EmitOpenPlanAdo(new PrOpenPlanAdoResult - { - RootId = rootId, - ItemId = itemId, - ParentItemId = resolvedParent, - ItemKey = itemKey, - IsRootPlan = isRootPlan, - HeadBranch = headBranch, - BaseBranch = baseBranch, - Organization = organization, - Project = project, - Repository = repository, - RepoSlug = slug, - PrNumber = existing.PullRequestId, - PrUrl = BuildAdoPrUrl(organization, project, repository, existing.PullRequestId), - Title = prTitle, - Created = false, - Stale = true, - RequestsParentChange = existingMeta.RequestsParentChange, - AncestorPlanGenerations = existingMeta.AncestorPlanGenerations, - ErrorCode = "stale_metadata", - Error = BuildStaleMessage(existingMeta.AncestorPlanGenerations, snapshot), - }); - return ExitCodes.Success; + ErrorCode = "stale_metadata", + Error = BuildStaleMessage(existingMeta.AncestorPlanGenerations, snapshot), + }, + ExitCodes.Success); } // ── 5. Create the PR. ───────────────────────────────────────── @@ -432,50 +412,48 @@ private async Task OpenPlanAdoBodyAsync( if (created is null) { - EmitOpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, + return OpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, "pr_not_found", $"Repository '{repository}' not found in {organization}/{project}.", headBranch, baseBranch); - return ExitCodes.Success; } - EmitOpenPlanAdo(new PrOpenPlanAdoResult - { - RootId = rootId, - ItemId = itemId, - ParentItemId = resolvedParent, - ItemKey = itemKey, - IsRootPlan = isRootPlan, - HeadBranch = headBranch, - BaseBranch = baseBranch, - Organization = organization, - Project = project, - Repository = repository, - RepoSlug = slug, - PrNumber = created.PullRequestId, - PrUrl = BuildAdoPrUrl(organization, project, repository, created.PullRequestId), - Title = prTitle, - Created = true, - Stale = false, - RequestsParentChange = false, - AncestorPlanGenerations = snapshot, - ErrorCode = "", - }); - return ExitCodes.Success; + return new CommandExecution( + new PrOpenPlanAdoResult + { + RootId = rootId, + ItemId = itemId, + ParentItemId = resolvedParent, + ItemKey = itemKey, + IsRootPlan = isRootPlan, + HeadBranch = headBranch, + BaseBranch = baseBranch, + Organization = organization, + Project = project, + Repository = repository, + RepoSlug = slug, + PrNumber = created.PullRequestId, + PrUrl = BuildAdoPrUrl(organization, project, repository, created.PullRequestId), + Title = prTitle, + Created = true, + Stale = false, + RequestsParentChange = false, + AncestorPlanGenerations = snapshot, + ErrorCode = "", + }, + ExitCodes.Success); } catch (OperationCanceledException) { throw; } catch (AdoAuthenticationException ex) { // Raised by IPolyphonyAuthProvider when no ADO credential chain succeeds (PAT env or AAD). - EmitOpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, + return OpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, "no_pat", ex.Message, headBranch, baseBranch); - return ExitCodes.Success; } catch (TimeoutException ex) { - EmitOpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, + return OpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, "ado_timeout", ex.Message, headBranch, baseBranch); - return ExitCodes.Success; } catch (HttpRequestException ex) { @@ -483,15 +461,13 @@ private async Task OpenPlanAdoBodyAsync( var code = ex.StatusCode is HttpStatusCode.Unauthorized or HttpStatusCode.Forbidden ? "no_pat" : "ado_failed"; - EmitOpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, + return OpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, code, ex.Message, headBranch, baseBranch); - return ExitCodes.Success; } catch (Exception ex) { - EmitOpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, + return OpenPlanAdoError(rootId, itemId, resolvedParent, organization, project, repository, slug, "ado_failed", ex.Message, headBranch, baseBranch); - return ExitCodes.Success; } } @@ -500,7 +476,7 @@ private static void EmitOpenPlanAdo(PrOpenPlanAdoResult result) => Console.WriteLine(JsonSerializer.Serialize( result, PolyphonyJsonContext.Default.PrOpenPlanAdoResult)); - private static void EmitOpenPlanAdoError( + private static CommandExecution OpenPlanAdoError( int rootId, int itemId, int parentItemId, @@ -516,28 +492,30 @@ private static void EmitOpenPlanAdoError( var itemKey = itemId == rootId ? "root" : (itemId > 0 ? itemId.ToString(CultureInfo.InvariantCulture) : ""); - EmitOpenPlanAdo(new PrOpenPlanAdoResult - { - RootId = rootId, - ItemId = itemId, - ParentItemId = parentItemId, - ItemKey = itemKey, - IsRootPlan = itemId == rootId && itemId > 0, - HeadBranch = headBranch, - BaseBranch = baseBranch, - Organization = organization ?? string.Empty, - Project = project ?? string.Empty, - Repository = repository ?? string.Empty, - RepoSlug = slug ?? string.Empty, - PrNumber = 0, - PrUrl = string.Empty, - Title = string.Empty, - Created = false, - Stale = false, - RequestsParentChange = false, - AncestorPlanGenerations = new Dictionary(StringComparer.Ordinal), - ErrorCode = errorCode, - Error = message, - }); + return new CommandExecution( + new PrOpenPlanAdoResult + { + RootId = rootId, + ItemId = itemId, + ParentItemId = parentItemId, + ItemKey = itemKey, + IsRootPlan = itemId == rootId && itemId > 0, + HeadBranch = headBranch, + BaseBranch = baseBranch, + Organization = organization ?? string.Empty, + Project = project ?? string.Empty, + Repository = repository ?? string.Empty, + RepoSlug = slug ?? string.Empty, + PrNumber = 0, + PrUrl = string.Empty, + Title = string.Empty, + Created = false, + Stale = false, + RequestsParentChange = false, + AncestorPlanGenerations = new Dictionary(StringComparer.Ordinal), + ErrorCode = errorCode, + Error = message, + }, + ExitCodes.Success); } } diff --git a/src/Polyphony/Commands/PrCommands.OpenPlanPr.cs b/src/Polyphony/Commands/PrCommands.OpenPlanPr.cs index 69261b2..9382c9b 100644 --- a/src/Polyphony/Commands/PrCommands.OpenPlanPr.cs +++ b/src/Polyphony/Commands/PrCommands.OpenPlanPr.cs @@ -84,61 +84,31 @@ public async Task OpenPlanPr( CreateJournalInvocation("pr_open_plan_pr", BranchPairJournalTarget(jHead, jBase), rootId, itemId), async innerCt => { - var sw = new StringWriter(); - var originalOut = Console.Out; - int exitCode; - try + var execution = await OpenPlanPrAsync(rootId, itemId, parentItemId, ancestorIds, manifestPath, title, body, innerCt).ConfigureAwait(false); + EmitPlanPr(execution.Result); + var result = execution.Result; + payload = new PrOpenPlanPrPayload { - Console.SetOut(sw); - exitCode = await OpenPlanPrBodyAsync(rootId, itemId, parentItemId, ancestorIds, manifestPath, title, body, innerCt).ConfigureAwait(false); - } - finally - { - Console.SetOut(originalOut); - } - var output = sw.ToString(); - Console.Write(output); - PrOpenPlanPrResult? result = null; - try { result = JsonSerializer.Deserialize(output.Trim(), PolyphonyJsonContext.Default.PrOpenPlanPrResult); } - catch (JsonException) { } - payload = result is null - ? new PrOpenPlanPrPayload - { - RootId = rootId, - ItemId = itemId, - ParentItemId = parentItemId, - ItemKey = itemId == rootId ? "root" : (itemId > 0 ? itemId.ToString(CultureInfo.InvariantCulture) : ""), - IsRootPlan = itemId == rootId, - HeadBranch = jHead, - BaseBranch = jBase, - ResultAction = "error", - Succeeded = false, - WasMutated = false, - Stale = false, - Error = output.Trim(), - } - : new PrOpenPlanPrPayload - { - RootId = result.RootId, - ItemId = result.ItemId, - ParentItemId = result.ParentItemId, - ItemKey = result.ItemKey, - IsRootPlan = result.IsRootPlan, - HeadBranch = result.HeadBranch, - BaseBranch = result.BaseBranch, - RepoSlug = result.RepoSlug, - PrNumber = result.PrNumber, - PrUrl = result.PrUrl, - Title = result.Title, - ResultAction = result.Error is not null ? "error" - : (result.Stale ? "stale" - : (result.Created ? "created" : "reused_existing_pr")), - Succeeded = result.Error is null && !result.Stale, - WasMutated = result.Created, - Stale = result.Stale, - Error = result.Error, - }; - return exitCode; + RootId = result.RootId, + ItemId = result.ItemId, + ParentItemId = result.ParentItemId, + ItemKey = result.ItemKey, + IsRootPlan = result.IsRootPlan, + HeadBranch = result.HeadBranch, + BaseBranch = result.BaseBranch, + RepoSlug = result.RepoSlug, + PrNumber = result.PrNumber, + PrUrl = result.PrUrl, + Title = result.Title, + ResultAction = result.Error is not null ? "error" + : (result.Stale ? "stale" + : (result.Created ? "created" : "reused_existing_pr")), + Succeeded = result.Error is null && !result.Stale, + WasMutated = result.Created, + Stale = result.Stale, + Error = result.Error, + }; + return execution.ExitCode; }, outcomeSelector: exitCode => SelectJournalOutcome( exitCode, @@ -149,7 +119,14 @@ public async Task OpenPlanPr( ct: ct).ConfigureAwait(false); } - private async Task OpenPlanPrBodyAsync( + /// + /// Typed seam introduced by AB#3313. Computes the + /// envelope without any stdout side + /// effects. The public shell parses the CLI + /// envelope, calls this method, serializes the result, and feeds the + /// journal payload directly from the typed result (no JSON reparse). + /// + internal async Task> OpenPlanPrAsync( int rootId, int itemId, int parentItemId, @@ -163,13 +140,11 @@ private async Task OpenPlanPrBodyAsync( // ── 1. Validate input + derive head/base + ancestor chain. ──────── if (!Branching.RootId.TryParse(rootId, out var root)) { - EmitPlanPrError(rootId, itemId, parentItemId, $"rootId must be positive (got {rootId})"); - return ExitCodes.ConfigError; + return new CommandExecution(BuildPlanPrErrorResult(rootId, itemId, parentItemId, $"rootId must be positive (got {rootId})"), ExitCodes.ConfigError); } if (!WorkItemId.TryParse(itemId, out var item)) { - EmitPlanPrError(rootId, itemId, parentItemId, $"itemId must be positive (got {itemId})"); - return ExitCodes.ConfigError; + return new CommandExecution(BuildPlanPrErrorResult(rootId, itemId, parentItemId, $"itemId must be positive (got {itemId})"), ExitCodes.ConfigError); } bool isRootPlan = itemId == rootId; @@ -182,9 +157,8 @@ private async Task OpenPlanPrBodyAsync( { if (parentItemId != 0) { - EmitPlanPrError(rootId, itemId, parentItemId, - $"--parent-item-id must not be provided when --item-id == --root-id (got {parentItemId}); the root plan has no parent."); - return ExitCodes.ConfigError; + return new CommandExecution(BuildPlanPrErrorResult(rootId, itemId, parentItemId, + $"--parent-item-id must not be provided when --item-id == --root-id (got {parentItemId}); the root plan has no parent."), ExitCodes.ConfigError); } itemKey = "root"; headBranch = BranchNameBuilder.RootPlan(root).Value; @@ -202,20 +176,17 @@ private async Task OpenPlanPrBodyAsync( { if (!WorkItemId.TryParse(parentItemId, out var parentItem)) { - EmitPlanPrError(rootId, itemId, parentItemId, $"--parent-item-id must be positive (got {parentItemId})"); - return ExitCodes.ConfigError; + return new CommandExecution(BuildPlanPrErrorResult(rootId, itemId, parentItemId, $"--parent-item-id must be positive (got {parentItemId})"), ExitCodes.ConfigError); } if (parentItemId == itemId) { - EmitPlanPrError(rootId, itemId, parentItemId, - $"--parent-item-id ({parentItemId}) must not equal --item-id; a plan cannot be its own parent."); - return ExitCodes.ConfigError; + return new CommandExecution(BuildPlanPrErrorResult(rootId, itemId, parentItemId, + $"--parent-item-id ({parentItemId}) must not equal --item-id; a plan cannot be its own parent."), ExitCodes.ConfigError); } if (parentItemId == rootId) { - EmitPlanPrError(rootId, itemId, parentItemId, - $"--parent-item-id ({parentItemId}) equals --root-id; omit --parent-item-id when the parent is the root plan."); - return ExitCodes.ConfigError; + return new CommandExecution(BuildPlanPrErrorResult(rootId, itemId, parentItemId, + $"--parent-item-id ({parentItemId}) equals --root-id; omit --parent-item-id when the parent is the root plan."), ExitCodes.ConfigError); } resolvedParent = parentItemId; headBranch = BranchNameBuilder.DescendantPlan(root, item).Value; @@ -226,8 +197,7 @@ private async Task OpenPlanPrBodyAsync( if (!TryParseAncestorChain(ancestorIds, isRootPlan, itemKey, out var ancestorKeys, out var ancestorError)) { - EmitPlanPrError(rootId, itemId, parentItemId, ancestorError, headBranch, baseBranch); - return ExitCodes.ConfigError; + return new CommandExecution(BuildPlanPrErrorResult(rootId, itemId, parentItemId, ancestorError, headBranch, baseBranch), ExitCodes.ConfigError); } // ── 2. Read manifest + compute snapshot. ─────────────────────────── @@ -242,8 +212,7 @@ private async Task OpenPlanPrBodyAsync( var resolvedPath = await ManifestPathHelper.ResolveAsync(statePaths, rootId, manifestPath, ct).ConfigureAwait(false); if (resolvedPath.Error is not null) { - EmitPlanPrError(rootId, itemId, parentItemId, resolvedPath.Error, headBranch, baseBranch); - return ExitCodes.CacheError; + return new CommandExecution(BuildPlanPrErrorResult(rootId, itemId, parentItemId, resolvedPath.Error, headBranch, baseBranch), ExitCodes.CacheError); } var localManifestPath = resolvedPath.Path; try @@ -253,15 +222,13 @@ private async Task OpenPlanPrBodyAsync( } catch (FileNotFoundException) { - EmitPlanPrError(rootId, itemId, parentItemId, + return new CommandExecution(BuildPlanPrErrorResult(rootId, itemId, parentItemId, $"manifest not found at {localManifestPath} — run `polyphony manifest init --root-id {rootId} ...` first", - headBranch, baseBranch); - return ExitCodes.CacheError; + headBranch, baseBranch), ExitCodes.CacheError); } catch (InvalidOperationException ex) { - EmitPlanPrError(rootId, itemId, parentItemId, $"manifest invalid: {ex.Message}", headBranch, baseBranch); - return ExitCodes.CacheError; + return new CommandExecution(BuildPlanPrErrorResult(rootId, itemId, parentItemId, $"manifest invalid: {ex.Message}", headBranch, baseBranch), ExitCodes.CacheError); } // ── 3. Confirm head + base exist on the remote. ──────────────────── @@ -270,27 +237,24 @@ private async Task OpenPlanPrBodyAsync( var headRefs = await git.LsRemoteHeadsAsync("origin", $"refs/heads/{headBranch}", ct).ConfigureAwait(false); if (headRefs.Count == 0) { - EmitPlanPrError(rootId, itemId, parentItemId, + return new CommandExecution(BuildPlanPrErrorResult(rootId, itemId, parentItemId, $"head branch '{headBranch}' does not exist on remote — run 'polyphony branch ensure-plan' and push first", - headBranch, baseBranch); - return ExitCodes.RoutingFailure; + headBranch, baseBranch), ExitCodes.RoutingFailure); } var baseRefs = await git.LsRemoteHeadsAsync("origin", $"refs/heads/{baseBranch}", ct).ConfigureAwait(false); if (baseRefs.Count == 0) { - EmitPlanPrError(rootId, itemId, parentItemId, + return new CommandExecution(BuildPlanPrErrorResult(rootId, itemId, parentItemId, $"base branch '{baseBranch}' does not exist on remote — ensure the parent plan branch is materialized first", - headBranch, baseBranch); - return ExitCodes.RoutingFailure; + headBranch, baseBranch), ExitCodes.RoutingFailure); } var slug = await TryResolveSlugAsync(ct).ConfigureAwait(false); if (string.IsNullOrEmpty(slug)) { - EmitPlanPrError(rootId, itemId, parentItemId, + return new CommandExecution(BuildPlanPrErrorResult(rootId, itemId, parentItemId, "Could not resolve repo slug from origin remote", - headBranch, baseBranch); - return ExitCodes.RoutingFailure; + headBranch, baseBranch), ExitCodes.RoutingFailure); } var prTitle = string.IsNullOrWhiteSpace(title) @@ -323,7 +287,30 @@ private async Task OpenPlanPrBodyAsync( if (SnapshotsEquivalent(existingMeta.AncestorPlanGenerations, snapshot)) { - EmitPlanPr(new PrOpenPlanPrResult + return new CommandExecution( + new PrOpenPlanPrResult + { + RootId = rootId, + ItemId = itemId, + ParentItemId = resolvedParent ?? 0, + ItemKey = itemKey, + IsRootPlan = isRootPlan, + HeadBranch = headBranch, + BaseBranch = baseBranch, + RepoSlug = slug, + PrNumber = found.Number, + PrUrl = found.Url ?? string.Empty, + Title = prTitle, + Created = false, + Stale = false, + RequestsParentChange = existingMeta.RequestsParentChange, + AncestorPlanGenerations = existingMeta.AncestorPlanGenerations, + }, + ExitCodes.Success); + } + + return new CommandExecution( + new PrOpenPlanPrResult { RootId = rootId, ItemId = itemId, @@ -337,14 +324,26 @@ private async Task OpenPlanPrBodyAsync( PrUrl = found.Url ?? string.Empty, Title = prTitle, Created = false, - Stale = false, + Stale = true, RequestsParentChange = existingMeta.RequestsParentChange, AncestorPlanGenerations = existingMeta.AncestorPlanGenerations, - }); - return ExitCodes.Success; - } + Error = BuildStaleMessage(existingMeta.AncestorPlanGenerations, snapshot), + }, + ExitCodes.RoutingFailure); + } - EmitPlanPr(new PrOpenPlanPrResult + // ── 5. Create the PR. ───────────────────────────────────────── + var url = await gh.CreatePullRequestAsync(slug, baseBranch, headBranch, prTitle, fullBody, ct).ConfigureAwait(false); + if (string.IsNullOrWhiteSpace(url)) + { + return new CommandExecution(BuildPlanPrErrorResult(rootId, itemId, parentItemId, + "gh pr create failed — no URL returned", + headBranch, baseBranch), ExitCodes.RoutingFailure); + } + + var trimmedUrl = url.Trim(); + return new CommandExecution( + new PrOpenPlanPrResult { RootId = rootId, ItemId = itemId, @@ -354,54 +353,20 @@ private async Task OpenPlanPrBodyAsync( HeadBranch = headBranch, BaseBranch = baseBranch, RepoSlug = slug, - PrNumber = found.Number, - PrUrl = found.Url ?? string.Empty, + PrNumber = ExtractPrNumber(trimmedUrl), + PrUrl = trimmedUrl, Title = prTitle, - Created = false, - Stale = true, - RequestsParentChange = existingMeta.RequestsParentChange, - AncestorPlanGenerations = existingMeta.AncestorPlanGenerations, - Error = BuildStaleMessage(existingMeta.AncestorPlanGenerations, snapshot), - }); - return ExitCodes.RoutingFailure; - } - - // ── 5. Create the PR. ───────────────────────────────────────── - var url = await gh.CreatePullRequestAsync(slug, baseBranch, headBranch, prTitle, fullBody, ct).ConfigureAwait(false); - if (string.IsNullOrWhiteSpace(url)) - { - EmitPlanPrError(rootId, itemId, parentItemId, - "gh pr create failed — no URL returned", - headBranch, baseBranch); - return ExitCodes.RoutingFailure; - } - - var trimmedUrl = url.Trim(); - EmitPlanPr(new PrOpenPlanPrResult - { - RootId = rootId, - ItemId = itemId, - ParentItemId = resolvedParent ?? 0, - ItemKey = itemKey, - IsRootPlan = isRootPlan, - HeadBranch = headBranch, - BaseBranch = baseBranch, - RepoSlug = slug, - PrNumber = ExtractPrNumber(trimmedUrl), - PrUrl = trimmedUrl, - Title = prTitle, - Created = true, - Stale = false, - RequestsParentChange = false, - AncestorPlanGenerations = snapshot, - }); - return ExitCodes.Success; + Created = true, + Stale = false, + RequestsParentChange = false, + AncestorPlanGenerations = snapshot, + }, + ExitCodes.Success); } catch (OperationCanceledException) { throw; } catch (Exception ex) { - EmitPlanPrError(rootId, itemId, parentItemId, ex.Message, headBranch, baseBranch); - return ExitCodes.RoutingFailure; + return new CommandExecution(BuildPlanPrErrorResult(rootId, itemId, parentItemId, ex.Message, headBranch, baseBranch), ExitCodes.RoutingFailure); } } @@ -591,7 +556,7 @@ private static void EmitPlanPr(PrOpenPlanPrResult result) => Console.WriteLine(JsonSerializer.Serialize( result, PolyphonyJsonContext.Default.PrOpenPlanPrResult)); - private static void EmitPlanPrError( + private static PrOpenPlanPrResult BuildPlanPrErrorResult( int rootId, int itemId, int parentItemId, @@ -602,7 +567,7 @@ private static void EmitPlanPrError( var itemKey = itemId == rootId ? "root" : itemId.ToString(CultureInfo.InvariantCulture); - EmitPlanPr(new PrOpenPlanPrResult + return new PrOpenPlanPrResult { RootId = rootId, ItemId = itemId, @@ -620,6 +585,6 @@ private static void EmitPlanPrError( RequestsParentChange = false, AncestorPlanGenerations = new Dictionary(StringComparer.Ordinal), Error = message, - }); + }; } } diff --git a/src/Polyphony/Commands/PrCommands.PollStatus.cs b/src/Polyphony/Commands/PrCommands.PollStatus.cs index 7a8f85a..620cd8f 100644 --- a/src/Polyphony/Commands/PrCommands.PollStatus.cs +++ b/src/Polyphony/Commands/PrCommands.PollStatus.cs @@ -38,10 +38,30 @@ public async Task PollStatus( ("--pr-url", string.IsNullOrEmpty(prUrl))) is { } halt) return halt; + var execution = await PollStatusAsync(prUrl, includeMetadata, ct).ConfigureAwait(false); + EmitPoll(execution.Result); + return execution.ExitCode; + } + + /// + /// Typed seam introduced by AB#3313. Computes the + /// envelope without any stdout side + /// effects. The public shell parses the CLI + /// envelope, calls this method, serializes the result, and translates + /// to a process exit code. Composable from in-process callers such as + /// the future PrLifecycle skeleton without round-tripping through + /// JSON. + /// + internal async Task> PollStatusAsync( + string prUrl, + bool includeMetadata, + CancellationToken ct) + { if (!TryParsePrUrl(prUrl, out var slug, out var prNumber)) { - EmitPollError(prUrl, $"could not parse pr url '{prUrl}' (expected https://github.com/owner/repo/pull/N)"); - return ExitCodes.Success; + return new CommandExecution( + BuildPollErrorResult(prUrl, $"could not parse pr url '{prUrl}' (expected https://github.com/owner/repo/pull/N)"), + ExitCodes.Success); } try @@ -49,8 +69,9 @@ public async Task PollStatus( var data = await gh.GetPullRequestPollDataAsync(slug, prNumber, ct).ConfigureAwait(false); if (data is null) { - EmitPollError(prUrl, $"PR #{prNumber} not found in {slug}", slug, prNumber); - return ExitCodes.Success; + return new CommandExecution( + BuildPollErrorResult(prUrl, $"PR #{prNumber} not found in {slug}", slug, prNumber), + ExitCodes.Success); } // Build review-thread snapshot. Threads are now the source @@ -128,19 +149,20 @@ public async Task PollStatus( Warnings = warnings, Metadata = metadata, }; - EmitPoll(result); - return ExitCodes.Success; + return new CommandExecution(result, ExitCodes.Success); } catch (OperationCanceledException) { throw; } catch (ExternalToolTimeoutException ex) { - EmitPollError(prUrl, ex.FormatErrorMessage("gh pr view"), slug, prNumber); - return ExitCodes.Success; + return new CommandExecution( + BuildPollErrorResult(prUrl, ex.FormatErrorMessage("gh pr view"), slug, prNumber), + ExitCodes.Success); } catch (Exception ex) { - EmitPollError(prUrl, ex.Message, slug, prNumber); - return ExitCodes.Success; + return new CommandExecution( + BuildPollErrorResult(prUrl, ex.Message, slug, prNumber), + ExitCodes.Success); } } @@ -252,13 +274,13 @@ private static void EmitPoll(PrPollStatusResult result) => Console.WriteLine(JsonSerializer.Serialize( result, PolyphonyJsonContext.Default.PrPollStatusResult)); - private static void EmitPollError( + private static PrPollStatusResult BuildPollErrorResult( string prUrl, string message, string repoSlug = "", int prNumber = 0) { - var result = new PrPollStatusResult + return new PrPollStatusResult { PrUrl = prUrl, PrNumber = prNumber, @@ -277,6 +299,5 @@ private static void EmitPollError( Policy = new PrPollPolicy { MergeAllowed = false, BlockingReasons = [message] }, Error = message, }; - EmitPoll(result); } } diff --git a/src/Polyphony/Commands/PrCommands.PollStatusAdo.cs b/src/Polyphony/Commands/PrCommands.PollStatusAdo.cs index a5b81e1..0959b96 100644 --- a/src/Polyphony/Commands/PrCommands.PollStatusAdo.cs +++ b/src/Polyphony/Commands/PrCommands.PollStatusAdo.cs @@ -58,41 +58,69 @@ public async Task PollStatusAdo( ("--pr-number", prNumber == RequiredInput.MissingInt)) is { } halt) return halt; + var execution = await PollStatusAdoAsync( + organization, project, repositoryId, prNumber, includeMetadata, allowAnyApprovalVote, ct) + .ConfigureAwait(false); + EmitPollStatusAdo(execution.Result); + return execution.ExitCode; + } + + /// + /// Typed seam introduced by AB#3313. Computes the + /// envelope without any stdout side + /// effects. The public shell parses the CLI + /// envelope, calls this method, serializes the result, and translates + /// to a process exit code. Composable from in-process callers such as + /// the future PrLifecycle skeleton without round-tripping through + /// JSON. + /// + internal async Task> PollStatusAdoAsync( + string organization, + string project, + string repositoryId, + int prNumber, + bool includeMetadata, + bool allowAnyApprovalVote, + CancellationToken ct) + { var prUrl = BuildAdoPrUrl(organization, project, repositoryId, prNumber); if (string.IsNullOrWhiteSpace(organization) || string.IsNullOrWhiteSpace(project) || string.IsNullOrWhiteSpace(repositoryId)) { - EmitPollStatusAdoError( - prUrl, - "organization, project, and repositoryId are required", - "invalid_argument", - slug: BuildAdoSlug(organization, project, repositoryId), - prNumber: prNumber); - return ExitCodes.Success; + return new CommandExecution( + BuildPollStatusAdoErrorResult( + prUrl, + "organization, project, and repositoryId are required", + "invalid_argument", + slug: BuildAdoSlug(organization, project, repositoryId), + prNumber: prNumber), + ExitCodes.Success); } if (prNumber <= 0) { - EmitPollStatusAdoError( - prUrl, - $"prNumber must be a positive integer (got {prNumber})", - "invalid_argument", - slug: BuildAdoSlug(organization, project, repositoryId), - prNumber: prNumber); - return ExitCodes.Success; + return new CommandExecution( + BuildPollStatusAdoErrorResult( + prUrl, + $"prNumber must be a positive integer (got {prNumber})", + "invalid_argument", + slug: BuildAdoSlug(organization, project, repositoryId), + prNumber: prNumber), + ExitCodes.Success); } if (ado is null) { // Shouldn't happen in production (DI registers IAdoClient) but the // ctor allows null so unit tests can opt out of the ADO leg. - EmitPollStatusAdoError( - prUrl, - "IAdoClient is not configured", - "ado_failed", - slug: BuildAdoSlug(organization, project, repositoryId), - prNumber: prNumber); - return ExitCodes.Success; + return new CommandExecution( + BuildPollStatusAdoErrorResult( + prUrl, + "IAdoClient is not configured", + "ado_failed", + slug: BuildAdoSlug(organization, project, repositoryId), + prNumber: prNumber), + ExitCodes.Success); } var slug = BuildAdoSlug(organization, project, repositoryId); @@ -103,13 +131,14 @@ public async Task PollStatusAdo( organization, project, repositoryId, prNumber, allowAnyApprovalVote, ct).ConfigureAwait(false); if (data is null) { - EmitPollStatusAdoError( - prUrl, - $"PR #{prNumber} not found in {slug}", - "pr_not_found", - slug: slug, - prNumber: prNumber); - return ExitCodes.Success; + return new CommandExecution( + BuildPollStatusAdoErrorResult( + prUrl, + $"PR #{prNumber} not found in {slug}", + "pr_not_found", + slug: slug, + prNumber: prNumber), + ExitCodes.Success); } // Fetch ADO threads alongside the PR detail. Threads drive the @@ -207,20 +236,21 @@ public async Task PollStatusAdo( Warnings = warnings, Metadata = metadata, }; - EmitPollStatusAdo(result); - return ExitCodes.Success; + return new CommandExecution(result, ExitCodes.Success); } catch (OperationCanceledException) { throw; } catch (AdoAuthenticationException ex) { // Raised by IPolyphonyAuthProvider when no ADO credential chain succeeds (PAT env or AAD). - EmitPollStatusAdoError(prUrl, ex.Message, "no_pat", slug, prNumber); - return ExitCodes.Success; + return new CommandExecution( + BuildPollStatusAdoErrorResult(prUrl, ex.Message, "no_pat", slug, prNumber), + ExitCodes.Success); } catch (TimeoutException ex) { - EmitPollStatusAdoError(prUrl, ex.Message, "ado_timeout", slug, prNumber); - return ExitCodes.Success; + return new CommandExecution( + BuildPollStatusAdoErrorResult(prUrl, ex.Message, "ado_timeout", slug, prNumber), + ExitCodes.Success); } catch (HttpRequestException ex) { @@ -228,13 +258,15 @@ public async Task PollStatusAdo( var code = ex.StatusCode is HttpStatusCode.Unauthorized or HttpStatusCode.Forbidden ? "no_pat" : "ado_failed"; - EmitPollStatusAdoError(prUrl, ex.Message, code, slug, prNumber); - return ExitCodes.Success; + return new CommandExecution( + BuildPollStatusAdoErrorResult(prUrl, ex.Message, code, slug, prNumber), + ExitCodes.Success); } catch (Exception ex) { - EmitPollStatusAdoError(prUrl, ex.Message, "ado_failed", slug, prNumber); - return ExitCodes.Success; + return new CommandExecution( + BuildPollStatusAdoErrorResult(prUrl, ex.Message, "ado_failed", slug, prNumber), + ExitCodes.Success); } } @@ -332,14 +364,14 @@ private static void EmitPollStatusAdo(PrPollStatusResult result) => Console.WriteLine(JsonSerializer.Serialize( result, PolyphonyJsonContext.Default.PrPollStatusResult)); - private static void EmitPollStatusAdoError( + private static PrPollStatusResult BuildPollStatusAdoErrorResult( string prUrl, string message, string errorCode, string slug, int prNumber) { - var result = new PrPollStatusResult + return new PrPollStatusResult { PrUrl = prUrl, PrNumber = prNumber, @@ -359,6 +391,5 @@ private static void EmitPollStatusAdoError( Error = message, ErrorCode = errorCode, }; - EmitPollStatusAdo(result); } }