Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
6b68529
Add structured freeform context payload + ExecutingTask/ready recovery
m-nash Apr 30, 2026
a765a5d
Distinguish sampling-classified custom instruction from sampling-unav…
m-nash May 5, 2026
b8bef58
Attach comment context for waiting-for-reply elicitations too
m-nash May 5, 2026
126fed6
Don't leak synthetic 'ready_unresolved' event name into user prompt
m-nash May 5, 2026
0cd0b53
Add 'Treat as comment replied' option to comment-flow recovery prompt
m-nash May 5, 2026
6bb39be
Don't assume comment_addressed for ambiguous push-detected recovery
m-nash May 5, 2026
5a8a976
Preserve waiting-comment context in ExecutingTask recovery prompt
m-nash May 5, 2026
97d597e
Handle 'skip' in all comment-flow sub-states in recovery prompt
m-nash May 5, 2026
42aad23
Fix InvalidOperationException leak in ClassifyFreeformAsync (PR #51 r…
m-nash May 6, 2026
51fd161
Route freeform Path B in waiting-comment context (PR #51 review)
m-nash May 6, 2026
decce7b
Extend (ready) recovery to ApplyingFix state (PR #51 review)
m-nash May 6, 2026
ba7a0c6
Preserve recovery snapshot in EmitComposeReplyAction (PR #51 review)
m-nash May 6, 2026
baaa2a9
Fix duplicate <summary> tag in TryClassifyFreeformViaSamplingAsync (P…
m-nash May 6, 2026
9649219
Hoist "stop" choice above per-flow routing (PR #51 review)
m-nash May 6, 2026
3240197
Make treat_as_replied_externally flow-aware (PR #51 review)
m-nash May 6, 2026
021e99a
Guard ProcessTaskComplete against waiting-thread stale indexing (PR #…
m-nash May 6, 2026
cede694
Add misattributes and rerequest to cspell dictionary
m-nash May 6, 2026
10991be
Rename 'Treat as comment replied' to 'Treat as replied externally' (P…
m-nash May 6, 2026
4de9393
Distinguish HEAD-refresh failure from no-push in recover_from_ready (…
m-nash May 6, 2026
9274c74
Use actual state in BuildRecoverFromReadyAction debug log (PR #51 rev…
m-nash May 6, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions PrCopilot/src/PrCopilot/StateMachine/MonitorState.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,15 @@ public class MonitorState
/// <summary>Reply text composed by the agent, to be posted by the server via the REST API.</summary>
public string? PendingReplyText { get; set; }

/// <summary>
/// HEAD SHA snapshotted when the state most recently entered <see cref="MonitorStateId.ExecutingTask"/>.
/// Used by the recovery path for <c>(ExecutingTask, "ready")</c> to detect whether the agent
/// pushed during the task — if HEAD has advanced, "ready" is reinterpreted as a completion event
/// (<c>comment_addressed</c> in comment flows, <c>push_completed</c> in CI flows).
/// Set via <see cref="EnterExecutingTask"/>; null/empty means no snapshot was captured.
/// </summary>
public string? HeadShaAtTaskStart { get; set; }

/// <summary>Transient: completion event set by sampling handler for MonitorFlowTools to feed back to state machine.</summary>
public string? SamplingCompletionEvent { get; set; }
/// <summary>Transient: completion event set by EmitComposeReplyAction for the sampling compose_reply handler.</summary>
Expand Down Expand Up @@ -130,4 +139,20 @@ public void ClearPendingCommentState()
/// Set when ReviewerReplied terminal state is detected.
/// </summary>
public CommentInfo? RepliedComment { get; set; }

/// <summary>
/// Transition to <see cref="MonitorStateId.ExecutingTask"/> and snapshot the current
/// <see cref="HeadSha"/> as <see cref="HeadShaAtTaskStart"/>. The snapshot lets the
/// recovery path detect post-push resumes (where the agent pushed during the task and
/// then re-entered via the post-push <c>pr_monitor_start</c> hook with event=ready
/// instead of calling the documented completion event).
///
/// Use this instead of assigning <c>CurrentState = MonitorStateId.ExecutingTask</c>
/// directly so the snapshot is never forgotten at a new task-entry site.
/// </summary>
public void EnterExecutingTask()
{
CurrentState = MonitorStateId.ExecutingTask;
HeadShaAtTaskStart = HeadSha;
}
}
125 changes: 118 additions & 7 deletions PrCopilot/src/PrCopilot/StateMachine/MonitorTransitions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ public static MonitorAction BuildTerminalAction(MonitorState state, TerminalStat
["I'll handle the comments myself"] = "handle_myself",
["I'll handle them myself"] = "handle_myself",
["Skip this comment"] = "skip",
["Treat as comment addressed"] = "treat_as_addressed",
["Done — resume monitoring"] = "done",
["Address next comment"] = "continue",
["I'll handle the rest myself"] = "done",
Expand All @@ -126,6 +127,7 @@ public static MonitorAction BuildTerminalAction(MonitorState state, TerminalStat
["Re-run failed jobs"] = "rerun",
["Apply the recommendation"] = "apply_fix",
["Run a new build"] = "run_new",
["Treat as push completed"] = "treat_as_pushed",

// Waiting-for-reply comment choices (ProcessWaitingCommentChoice)
["Resolve this thread"] = "resolve",
Expand Down Expand Up @@ -182,6 +184,14 @@ public static MonitorAction ProcessEvent(MonitorState state, string eventType, s
// LLM finished executing a generic task
(MonitorStateId.ExecutingTask, "task_complete") => ProcessTaskComplete(state),

// Recovery: agent skipped the documented completion event and re-entered with
// event=ready (commonly because the user's post-push custom-instruction hook
// fires pr_monitor_start + ready instead of the Path B comment_addressed/
// push_completed events). Returns an auto_execute that fetches HEAD; if HEAD
// advanced during the task, the wrapper re-dispatches as the right completion
// event, otherwise falls through to flow-aware recovery prompt.
(MonitorStateId.ExecutingTask, "ready") => BuildRecoverFromReadyAction(state),
Comment thread
m-nash marked this conversation as resolved.

// Recovery: agent sent task_complete from AwaitingUser (skipped a tool call)
(MonitorStateId.AwaitingUser, "task_complete") => RecoverFromUnexpectedTaskComplete(state),

Expand Down Expand Up @@ -331,8 +341,20 @@ private static MonitorAction RecoverFromUnexpectedTaskComplete(MonitorState stat
private static MonitorAction RecoverFromUnexpectedState(MonitorState state, string eventType)
{
var priorState = state.CurrentState;
var priorCommentFlow = state.CommentFlow;
var priorCiFailureFlow = state.CiFailureFlow;
DebugLogger.Log("StateMachine", $"RECOVERY: Unexpected state {priorState}/{eventType}. Transitioning to AwaitingUser so next user_chose can recover.");
state.CurrentState = MonitorStateId.AwaitingUser;

// Preserve flow context when recovering from ExecutingTask — the user is mid-flow
// and may want to resume by treating the task as completed (e.g., comment addressed).
// Wiping the flow state would force them to start the comment loop from scratch and
// lose their place. For other prior states, keep the original aggressive cleanup.
if (priorState == MonitorStateId.ExecutingTask)
{
return BuildExecutingTaskRecoveryPrompt(state, eventType, priorCommentFlow, priorCiFailureFlow);
}

state.CommentFlow = CommentFlowState.None;
state.CiFailureFlow = CiFailureFlowState.None;
state.ActiveWaitingComment = null;
Expand All @@ -344,6 +366,83 @@ private static MonitorAction RecoverFromUnexpectedState(MonitorState state, stri
};
}

/// <summary>
/// Recovery prompt for unexpected events arriving while in <see cref="MonitorStateId.ExecutingTask"/>.
/// Offers flow-aware choices so the user can mark the task done in-flow rather than
/// being forced to "Resume monitoring" (which would wipe the flow state and restart polling).
/// Flow state (CommentFlow / CiFailureFlow / current comment index) is preserved so the
/// follow-up <c>user_chose</c> can dispatch into the correct flow handler.
/// </summary>
private static MonitorAction BuildExecutingTaskRecoveryPrompt(
MonitorState state,
string eventType,
CommentFlowState priorCommentFlow,
CiFailureFlowState priorCiFailureFlow)
{
if (priorCommentFlow != CommentFlowState.None)
{
return new MonitorAction
{
Action = "ask_user",
Question = $"Unexpected event '{eventType}' while addressing a comment. " +
"If you finished the work, pick how to mark it; otherwise resume or stop.",
Choices =
[
"Treat as comment addressed",
"Skip this comment",
Comment thread
m-nash marked this conversation as resolved.
"Resume monitoring",
"Stop monitoring"
Comment thread
m-nash marked this conversation as resolved.
]
Comment thread
m-nash marked this conversation as resolved.
Comment thread
m-nash marked this conversation as resolved.
};
}

if (priorCiFailureFlow != CiFailureFlowState.None)
{
return new MonitorAction
{
Action = "ask_user",
Question = $"Unexpected event '{eventType}' while investigating a CI failure. " +
"If you finished the work, pick how to mark it; otherwise resume or stop.",
Choices =
[
"Treat as push completed",
"Resume monitoring",
"Stop monitoring"
]
};
}

// No active flow — same as the generic recovery
state.ActiveWaitingComment = null;
return new MonitorAction
{
Action = "ask_user",
Question = $"Unexpected state: ExecutingTask/{eventType}. What would you like to do?",
Choices = ["Resume monitoring", "Stop monitoring"]
};
}

/// <summary>
/// Returns an <c>auto_execute</c> action that the MCP wrapper handles by fetching the
/// latest HEAD SHA from GitHub. If HEAD advanced since <see cref="MonitorState.HeadShaAtTaskStart"/>,
/// the wrapper re-dispatches the event as <c>comment_addressed</c> (comment flow) or
/// <c>push_completed</c> (CI flow) — recovering automatically from the post-push hook
/// that called <c>ready</c> instead of the documented completion event. If HEAD did
/// not advance, the wrapper falls back to <see cref="BuildExecutingTaskRecoveryPrompt"/>.
/// </summary>
private static MonitorAction BuildRecoverFromReadyAction(MonitorState state)
{
DebugLogger.Log("StateMachine", $"ExecutingTask/ready: dispatching auto_execute recover_from_ready_in_executing_task (snapshot HEAD={ShortSha(state.HeadShaAtTaskStart)})");
Comment thread
m-nash marked this conversation as resolved.
Outdated
return new MonitorAction
{
Action = "auto_execute",
Task = "recover_from_ready_in_executing_task"
};
}

private static string ShortSha(string? sha) =>
string.IsNullOrEmpty(sha) ? "(none)" : sha.Length <= 7 ? sha : sha[..7];

private static MonitorAction TransitionToPolling(MonitorState state)
{
state.CurrentState = MonitorStateId.Polling;
Expand Down Expand Up @@ -435,6 +534,12 @@ private static MonitorAction BuildCommentAction(MonitorState state, string times

private static MonitorAction ProcessCommentChoice(MonitorState state, string? choice)
{
// Recovery shortcut available from any comment-flow sub-state — the (ExecutingTask, "ready")
// recovery path offers this when HEAD did NOT advance during the task. Routes through
// ProcessCommentAddressed which gracefully composes a reply if PendingReplyText is empty.
if (choice == "treat_as_addressed")
return ProcessCommentAddressed(state, null);

return (state.CommentFlow, choice) switch
{
(CommentFlowState.MultiCommentPrompt, "address_all") => BeginAddressAll(state),
Expand Down Expand Up @@ -514,7 +619,7 @@ private static MonitorAction BeginExplainAll(MonitorState state)
private static MonitorAction EmitExplainForCurrentComment(MonitorState state)
{
var c = state.UnresolvedComments[state.CurrentCommentIndex];
state.CurrentState = MonitorStateId.ExecutingTask;
state.EnterExecutingTask();
state.PendingExplainResult = true;
state.LastRecommendation = null;
return new MonitorAction
Expand Down Expand Up @@ -573,7 +678,7 @@ private static MonitorAction HandlePickedComment(MonitorState state, string? cho

private static MonitorAction BeginAddressCurrentComment(MonitorState state)
{
state.CurrentState = MonitorStateId.ExecutingTask;
state.EnterExecutingTask();
return EmitAddressCommentAction(state);
}

Expand All @@ -583,7 +688,7 @@ private static MonitorAction BeginApplyRecommendation(MonitorState state)
return TransitionToPolling(state);

var c = state.UnresolvedComments[state.CurrentCommentIndex];
state.CurrentState = MonitorStateId.ExecutingTask;
state.EnterExecutingTask();
return new MonitorAction
{
Action = "execute",
Expand All @@ -604,7 +709,7 @@ private static MonitorAction BeginApplyRecommendation(MonitorState state)
private static MonitorAction BeginExplainComment(MonitorState state, bool isReplyEvent = false)
{
var c = state.UnresolvedComments[state.CurrentCommentIndex];
state.CurrentState = MonitorStateId.ExecutingTask;
state.EnterExecutingTask();
state.PendingExplainResult = true;
state.LastRecommendation = null;
var replyContext = isReplyEvent && !string.IsNullOrEmpty(c.LastReplyAuthor)
Expand Down Expand Up @@ -633,7 +738,7 @@ private static MonitorAction EmitAddressCommentAction(MonitorState state)
}

var c = state.UnresolvedComments[state.CurrentCommentIndex];
state.CurrentState = MonitorStateId.ExecutingTask;
state.EnterExecutingTask();
return new MonitorAction
{
Action = "execute",
Expand Down Expand Up @@ -853,7 +958,7 @@ private static MonitorAction BuildPostReplyAction(MonitorState state, CommentInf
/// </summary>
private static MonitorAction EmitComposeReplyAction(MonitorState state, CommentInfo c, string completionEvent)
{
state.CurrentState = MonitorStateId.ExecutingTask;
state.EnterExecutingTask();
Comment thread
m-nash marked this conversation as resolved.
Outdated
state.PendingCompletionEvent = completionEvent;
return new MonitorAction
{
Expand Down Expand Up @@ -946,6 +1051,12 @@ private static MonitorAction BuildCiFailureAction(MonitorState state)

private static MonitorAction ProcessCiFailureChoice(MonitorState state, string? choice)
{
// Recovery shortcut from any CI-failure sub-state — the (ExecutingTask, "ready")
// recovery path offers this when HEAD did NOT advance during the task. Surfaces the
// documented push_completed behavior: clear failure state and resume polling.
if (choice == "treat_as_pushed")
return TransitionToPolling(state);

return (state.CiFailureFlow, choice) switch
{
(CiFailureFlowState.InvestigationResults, "apply_fix") => BeginApplyFix(state),
Expand Down Expand Up @@ -1060,7 +1171,7 @@ private static MonitorAction BuildRerunAction(MonitorState state)
// Azure DevOps has no API for rerun-failed-only; only the web UI supports it.
state.PendingRerunWhenChecksComplete = false;
var buildUrl = state.FailedChecks.FirstOrDefault()?.Url;
state.CurrentState = MonitorStateId.ExecutingTask;
state.EnterExecutingTask();
state.CommentFlow = CommentFlowState.None;
state.CiFailureFlow = CiFailureFlowState.None;
return new MonitorAction
Expand Down
15 changes: 15 additions & 0 deletions PrCopilot/src/PrCopilot/Tools/CiFailureContext.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Licensed under the MIT License.

using System.Text.Json.Serialization;

namespace PrCopilot.Tools;

/// <summary>
/// CI failure context, attached to <see cref="FreeformInterpretContext"/> when the
/// user is replying about a failed CI check.
/// </summary>
internal sealed class CiFailureContext
{
[JsonPropertyName("failedChecks")]
public List<FailedCheckContext> FailedChecks { get; set; } = [];
}
28 changes: 28 additions & 0 deletions PrCopilot/src/PrCopilot/Tools/CommentContext.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Licensed under the MIT License.

using System.Text.Json.Serialization;

namespace PrCopilot.Tools;

/// <summary>
/// The active comment thread context, attached to <see cref="FreeformInterpretContext"/>
/// when the user is replying about a code review comment.
/// </summary>
internal sealed class CommentContext
{
[JsonPropertyName("author")]
public string Author { get; set; } = "";

[JsonPropertyName("filePath")]
public string FilePath { get; set; } = "";

[JsonPropertyName("line")]
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public int? Line { get; set; }

[JsonPropertyName("body")]
public string Body { get; set; } = "";

[JsonPropertyName("url")]
public string Url { get; set; } = "";
}
21 changes: 21 additions & 0 deletions PrCopilot/src/PrCopilot/Tools/ElicitationChoiceContext.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Licensed under the MIT License.

using System.Text.Json.Serialization;

namespace PrCopilot.Tools;

/// <summary>
/// A single choice from an elicitation prompt. <see cref="Display"/> is what the
/// user saw; <see cref="Value"/> is the internal mapped value the agent should
/// pass back as <c>choice</c> for a Path A clean choice match.
/// </summary>
internal sealed class ElicitationChoiceContext
{
/// <summary>Human-readable choice label as shown to the user.</summary>
[JsonPropertyName("display")]
public string Display { get; set; } = "";

/// <summary>Internal mapped value (what would be passed back as <c>choice</c>).</summary>
[JsonPropertyName("value")]
public string Value { get; set; } = "";
}
19 changes: 19 additions & 0 deletions PrCopilot/src/PrCopilot/Tools/ElicitationContext.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Licensed under the MIT License.

using System.Text.Json.Serialization;

namespace PrCopilot.Tools;

/// <summary>
/// The elicitation prompt that was shown to the user, captured for the
/// <see cref="FreeformInterpretContext"/> payload so the agent knows what
/// question the user was responding to.
/// </summary>
internal sealed class ElicitationContext
{
[JsonPropertyName("question")]
public string Question { get; set; } = "";

[JsonPropertyName("choices")]
public List<ElicitationChoiceContext> Choices { get; set; } = [];
}
20 changes: 20 additions & 0 deletions PrCopilot/src/PrCopilot/Tools/FailedCheckContext.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Licensed under the MIT License.

using System.Text.Json.Serialization;

namespace PrCopilot.Tools;

/// <summary>
/// A single failed CI check entry in <see cref="CiFailureContext.FailedChecks"/>.
/// </summary>
internal sealed class FailedCheckContext
{
[JsonPropertyName("name")]
public string Name { get; set; } = "";

[JsonPropertyName("conclusion")]
public string Conclusion { get; set; } = "";

[JsonPropertyName("url")]
public string Url { get; set; } = "";
}
Loading
Loading