diff --git a/docs/architecture/option-b-completion-plan.md b/docs/architecture/option-b-completion-plan.md index c8f4fd9fb..8da05e562 100644 --- a/docs/architecture/option-b-completion-plan.md +++ b/docs/architecture/option-b-completion-plan.md @@ -180,7 +180,7 @@ limit. ## Phase 8 — Compatibility removal and final cutover -PR: #330. Status: partial and intentionally last. +PR: #330. Status: complete; final stacked phase. Purpose: delete the legacy architecture after every supported path uses the Option B contracts. diff --git a/docs/architecture/phase-8-compatibility-removal-plan.md b/docs/architecture/phase-8-compatibility-removal-plan.md new file mode 100644 index 000000000..3792752c4 --- /dev/null +++ b/docs/architecture/phase-8-compatibility-removal-plan.md @@ -0,0 +1,44 @@ +# Phase 8 implementation plan: compatibility removal + +**Status:** Complete + +**Goal:** Delete superseded execution paths so Forge has one runtime model rather than +permanent legacy and contract-backed implementations. + +## Removal rule + +A compatibility path may be deleted only when its replacement is authoritative for all +golden paths, restart/replay characterization passes, persisted state has an explicit +migration policy, and rollback does not require the deleted implementation. Phase 8 is +not permission to remove behavior that an earlier partial phase has not replaced. + +## Completed cutovers + +The Jira and source-control worker handler facades are deleted. Since Phase 2, both +sources register the same generic adapter-driven handler; the source-specific methods had +no runtime or test callers and represented a second, misleading dispatch API. + +Phase 8 also removes the legacy Redis stream and `github` source alias, implicit +checkpoint pinning, scalar planning fallbacks, the implementation-input facade, and +repository-key fallback migration. Built-in runtime selection is definition-compiled; +the Python graph adapters remain only as local test harnesses. Architecture tests make +these removals zero-tolerance. + +Unpinned checkpoints must now be processed by `migrate_unpinned_checkpoint`. Operators +first run it with `apply=False`, retain the original checkpoint as the rollback backup, +and persist the returned `migrated_state` only when `compatible` is true. Applied state +records the target definition and a seven-day rollback deadline by default. Rollback +means restoring that backup before the deadline; normal resume never performs migration +or rollback implicitly. + +## Final observation cutover + +CI, merge, review-thread, and proposal-review observations are now applied by the +provider-neutral `post-pr-v1` transition policy. The pinned workflow definition selects +that policy through an allowlisted identifier; compilation rejects unknown policies and +policies whose target nodes are absent. The worker adapts ingress, delegates once, then +persists the result—it no longer owns event-specific transition rules. + +The inventory at `docs/architecture/phase-8-removal-inventory.json` is the reviewable +exit checklist. Phase 8 is complete only when `remaining` is empty and the associated +architecture tests and golden-path characterization suite pass. diff --git a/docs/architecture/phase-8-removal-inventory.json b/docs/architecture/phase-8-removal-inventory.json new file mode 100644 index 000000000..2b0a6798e --- /dev/null +++ b/docs/architecture/phase-8-removal-inventory.json @@ -0,0 +1,62 @@ +{ + "schema_version": "2.0", + "remaining": [], + "removed": [ + { + "id": "worker-observation-transition-interpreter", + "owner": "Forge orchestration", + "prerequisite": "Governed observation-policy selection and provider-neutral transition runtime", + "replacement": "Pinned post-pr-v1 policy and apply_observation_transition", + "proof": "Worker delegates exactly once and contains no provider event-to-node logic" + }, + { + "id": "source-specific-worker-handlers", + "owner": "Forge orchestration", + "prerequisite": "Generic adapter ingress", + "replacement": "EventAdapterRegistry and OrchestratorWorker._handle_event", + "proof": "test_worker_exposes_only_generic_ingress_handler" + }, + { + "id": "inline-provider-writes", + "owner": "Forge effects", + "prerequisite": "Durable effect journal and executors", + "replacement": "EffectCommand runtime", + "proof": "tests/unit/architecture/test_direct_provider_effects.py" + }, + { + "id": "broad-station-state-access", + "owner": "Forge workflow", + "prerequisite": "Typed projections, outcomes, and reducers", + "replacement": "StationRequest and allowlisted reducers", + "proof": "tests/unit/architecture/test_station_boundaries.py" + }, + { + "id": "python-runtime-golden-path-selection", + "owner": "Forge workflow", + "prerequisite": "Governed built-in definitions", + "replacement": "Definition-compiled golden workflows", + "proof": "create_default_router registers only definition-compiled workflows" + }, + { + "id": "legacy-planning-and-implementation-adapters", + "owner": "Forge workflow", + "prerequisite": "Normalized artifact lineage and implementation-input station", + "replacement": "Artifact plus projection/station/reducer contracts", + "proof": "Compatibility facade and scalar fallback functions are absent" + }, + { + "id": "implicit-checkpoint-upgrade", + "owner": "Forge operations", + "prerequisite": "Explicit dry-run/apply migration contract", + "replacement": "migrate_unpinned_checkpoint", + "proof": "Resume rejects unpinned state and migration tests cover rollback" + }, + { + "id": "legacy-queue-stream-and-source-alias", + "owner": "Forge ingress", + "prerequisite": "Normalized source_control queue contract", + "replacement": "SOURCE_CONTROL_STREAM and EventSource.SOURCE_CONTROL", + "proof": "Legacy stream and github source translation symbols are absent" + } + ] +} diff --git a/src/forge/api/routes/health.py b/src/forge/api/routes/health.py index c92d402e2..d8cac7bf4 100644 --- a/src/forge/api/routes/health.py +++ b/src/forge/api/routes/health.py @@ -8,7 +8,7 @@ from forge import __version__ from forge.orchestrator.checkpointer import get_redis_client -from forge.queue.producer import JIRA_STREAM, LEGACY_SOURCE_CONTROL_STREAM, SOURCE_CONTROL_STREAM +from forge.queue.producer import JIRA_STREAM, SOURCE_CONTROL_STREAM logger = logging.getLogger(__name__) @@ -50,8 +50,7 @@ async def health_check() -> Any: try: jira_len = await redis_client.xlen(JIRA_STREAM) source_control_len = await redis_client.xlen(SOURCE_CONTROL_STREAM) - legacy_len = await redis_client.xlen(LEGACY_SOURCE_CONTROL_STREAM) - queue_depth = jira_len + source_control_len + legacy_len + queue_depth = jira_len + source_control_len except Exception: pass # Streams may not exist yet diff --git a/src/forge/integrations/source_control/comment_identity.py b/src/forge/integrations/source_control/comment_identity.py new file mode 100644 index 000000000..732954ba5 --- /dev/null +++ b/src/forge/integrations/source_control/comment_identity.py @@ -0,0 +1,29 @@ +"""Provider-neutral identification of Forge-authored comments.""" + + +def is_self_comment( + sender_login: str, + comment_body: str | None, + bot_login: str, + prefix: str | None = None, +) -> bool: + """Return whether a normalized comment was authored by this Forge identity.""" + body = comment_body or "" + sender = sender_login.lower() + bot = bot_login.lower() + same_identity = sender == bot or sender == f"{bot}[bot]" + if sender.endswith("[bot]") and same_identity: + return True + if prefix and prefix.strip(): + if not same_identity: + return False + stripped = prefix.strip() + if stripped.startswith(""): + candidates = (prefix, stripped) + else: + candidates = (prefix, stripped, f"", f"") + return body.startswith(candidates) or body.lstrip().startswith(candidates) + return same_identity + + +__all__ = ["is_self_comment"] diff --git a/src/forge/orchestrator/worker.py b/src/forge/orchestrator/worker.py index 8f6fa6f60..cac7088f6 100644 --- a/src/forge/orchestrator/worker.py +++ b/src/forge/orchestrator/worker.py @@ -4,7 +4,6 @@ import contextlib import logging import os -import re import signal import sys import uuid @@ -27,32 +26,17 @@ ) from forge.effects import EffectService, create_default_effect_service from forge.effects.jira import ( - JIRA_ATTACHMENT_REPLACE_OPERATION, JIRA_COMMENT_OPERATION, - JIRA_CUSTOM_FIELD_OPERATION, - JIRA_DESCRIPTION_OPERATION, - JIRA_LABEL_OPERATION, - JIRA_STRUCTURED_COMMENT_OPERATION, ) from forge.effects.source_control import SC_COMMENT_CREATE_OPERATION -from forge.integrations.github.comment_signature import is_self_comment from forge.integrations.jira.client import JiraClient -from forge.integrations.source_control.contracts import ( - ChangeRequestState, - CheckStatus, - EventKind, - NormalizedEvent, - RepositoryRef, - Review, - ReviewState, -) +from forge.integrations.source_control.contracts import RepositoryRef from forge.integrations.source_control.registry import get_registry from forge.models.events import EventSource from forge.models.workflow import ForgeLabel, TicketType from forge.orchestrator.checkpointer import get_checkpointer, get_ticket_from_pr_index from forge.orchestrator.command_handlers import ( CommandHandlerRegistry, - FeedbackKind, create_default_command_handler_registry, ) from forge.orchestrator.event_adapters import ( @@ -75,6 +59,7 @@ from forge.skills.orchestrator import ensure_skills from forge.skills.utils import extract_project_key from forge.utils.redaction import redact_secrets +from forge.workflow.declarative.compiler import WorkflowValidationError from forge.workflow.declarative.resolver import ( load_project_workflow, selected_workflow_name, @@ -83,21 +68,14 @@ from forge.workflow.effect_runtime import bind_effect_runtime from forge.workflow.nodes.error_handler import notify_error from forge.workflow.nodes.workspace_setup import teardown_workspace -from forge.workflow.pr_state import ( - activate_pull_request_for_event, - all_pull_requests_merged, - event_targets_pull_request, - mark_active_pull_request_merged, - save_active_pull_request, -) +from forge.workflow.pr_state import save_active_pull_request from forge.workflow.registry import create_default_router from forge.workflow.router import WorkflowRouter -from forge.workflow.utils.comment_classifier import CommentType, classify_comment -from forge.workflow.utils.jira_status import post_status_comment # noqa: F401 -from forge.workflow.utils.review_decisions import ( - decision_matches_comment, - merge_review_decisions, +from forge.workflow.transitions import ( + ObservationTransitionPolicy, + apply_observation_transition, ) +from forge.workflow.utils.jira_status import post_status_comment # noqa: F401 from forge.workflow.utils.source_control import get_adapter logger = logging.getLogger(__name__) @@ -105,47 +83,6 @@ _CI_STAGES = ("ci_evaluator", "attempt_ci_fix", "human_review_gate") -def _flatten_review_threads(reviews: list[Review]) -> list[dict[str, Any]]: - """Return the latest comment from each non-empty review thread. - - Mirrors workflow.utils.review_decisions.flatten_review_threads, sourced - from adapter-mapped Review objects (one per thread) instead of the raw - GraphQL-shaped dicts that helper expects. - """ - return [ - { - "path": review.comments[-1].path or "", - "line": review.comments[-1].line, - "body": review.comments[-1].body, - } - for review in reviews - if review.comments - ] - - -def _reviews_to_raw_threads(reviews: list[Review]) -> list[dict[str, Any]]: - """Convert adapter-mapped Review objects (one per thread) into the raw - dict shape triage_proposal_review_threads/reply_to_proposal_decisions and - the proposal-thread diffing below expect: JSON-serializable dicts with - "thread_id"/"comments" keys, not dataclasses. - """ - return [ - { - "thread_id": review.id, - "path": review.comments[0].path if review.comments else None, - "line": review.comments[0].line if review.comments else None, - "comments": [ - { - "comment_id": int(c.id) if c.id.isdigit() else c.id, - "body": c.body, - } - for c in review.comments - ], - } - for review in reviews - ] - - def _is_workflow_errored(state: dict) -> bool: """Return True when workflow has a recorded error and is not paused for human input.""" return not state.get("is_paused") and state.get("last_error") is not None @@ -182,11 +119,6 @@ async def _cleanup_terminal_workspace(result: dict[str, Any]) -> dict[str, Any]: } -_PRD_GATE_NODES = ("prd_approval_gate", "generate_prd", "regenerate_prd") -_SPEC_GATE_NODES = ("spec_approval_gate", "generate_spec", "regenerate_spec") -_REVIEW_GATES = ("human_review_gate", "review_response_gate") -_MAX_AUTOMATED_REVIEW_REVISIONS = 3 - _FRESH_INVOKE_NODES = ( "ci_evaluator", "attempt_ci_fix", @@ -244,6 +176,10 @@ def _review_enrichment(self) -> ReviewEnrichmentService: self.review_enrichment = service return service + def _transition_settings(self) -> Any: + """Provide configuration to the observation transition runtime.""" + return get_settings() + def _event_adapter_registry(self) -> EventAdapterRegistry: """Lazily restore adapters for legacy fixtures that bypass ``__init__``.""" registry = getattr(self, "event_adapters", None) @@ -382,19 +318,6 @@ async def _execute_required_source_comment( ) ) - def _deserialize_event(self, message: QueueMessage) -> NormalizedEvent | None: - """Reconstruct the typed NormalizedEvent a source-control message carries. - - Returns None for Jira messages (which never set normalized_event) or for - a source-control message that predates this field for some reason (e.g. - a backlog entry queued before this field existed). Callers currently - treat None as "no match" / "nothing to detect" rather than falling back - to raw-payload handling -- there is no fallback path implemented. - """ - if message.normalized_event is None: - return None - return self._event_adapter_registry().adapt(message).normalized_event - async def _get_forge_github_login(self, repo_ref: RepositoryRef) -> str: """Resolve and cache the authenticated Forge identity for this connection.""" cached = self._forge_github_logins.get(repo_ref.connection) @@ -428,22 +351,6 @@ async def _handle_terminal_failure(self, message: QueueMessage, error: str) -> N ) logger.info(f"Posted terminal queue failure notification to {message.ticket_key}") - async def _handle_jira_event(self, message: QueueMessage) -> None: - """Handle a Jira webhook event. - - Args: - message: The queue message to process. - """ - await self._handle_event(message) - - async def _handle_source_control_event(self, message: QueueMessage) -> None: - """Handle a source-control webhook event. - - Args: - message: The queue message to process. - """ - await self._handle_event(message) - async def _handle_event(self, message: QueueMessage) -> None: """Handle any registered ingress source through its adapter.""" adapted = self._event_adapter_registry().adapt(message) @@ -493,42 +400,6 @@ async def _resolve_ticket_from_pr_index(self, message: QueueMessage) -> QueueMes return message - def _is_prd_pr_event(self, message: QueueMessage, current_state: dict[str, Any]) -> bool: - """Check if a source-control event targets the PRD proposals PR.""" - if message.source != EventSource.SOURCE_CONTROL: - return False - prd_pr_number = current_state.get("prd_pr_number") - prd_pr_repo = current_state.get("prd_pr_repo") - if not prd_pr_number or not prd_pr_repo: - return False - - event = self._deserialize_event(message) - if event is None or event.change_request is None: - return False - - return ( - event.repo_ref.namespace == prd_pr_repo - and event.change_request.identity.native_id == prd_pr_number - ) - - def _is_spec_pr_event(self, message: QueueMessage, current_state: dict[str, Any]) -> bool: - """Check if a source-control event targets the spec proposals PR.""" - if message.source != EventSource.SOURCE_CONTROL: - return False - spec_pr_number = current_state.get("spec_pr_number") - spec_pr_repo = current_state.get("spec_pr_repo") - if not spec_pr_number or not spec_pr_repo: - return False - - event = self._deserialize_event(message) - if event is None or event.change_request is None: - return False - - return ( - event.repo_ref.namespace == spec_pr_repo - and event.change_request.identity.native_id == spec_pr_number - ) - async def _process_workflow(self, message: QueueMessage) -> None: """Process a message through the workflow. @@ -656,11 +527,9 @@ async def _process_workflow(self, message: QueueMessage) -> None: if status == "pinned": workflow_instance.validate_pinned_state(values) elif status == "legacy_unpinned": - # Compatibility for checkpoints predating definition - # pinning is explicit and auditable in the state. - pinned = workflow_instance.pin_legacy_state(values) - await compiled_workflow.aupdate_state(config, pinned) - existing_state = await compiled_workflow.aget_state(config) + raise WorkflowValidationError( + "checkpoint requires the explicit Phase 8 definition-pinning migration" + ) except Exception as exc: await self._report_custom_workflow_configuration_error(ticket_key, str(exc)) return @@ -700,11 +569,19 @@ async def _process_workflow(self, message: QueueMessage) -> None: command_decision = validate_command_decision( command_decision, existing_state.values ) - updated_values = await self._handle_resume_event( + updated_values = await self._apply_observation_transition( message, existing_state.values, adapted_event=adapted_event, command_decision=command_decision, + policy=( + ObservationTransitionPolicy( + identifier=workflow_instance.resolve_observation_policy() or "default", + definition=workflow_instance.definition.canonical_dict(), + ) + if isinstance(workflow_instance, DeclarativeWorkflow) + else ObservationTransitionPolicy() + ), ) state_changed = updated_values is not existing_state.values updated_values = record_command_decision( @@ -714,7 +591,7 @@ async def _process_workflow(self, message: QueueMessage) -> None: decision=command_decision, ) - # _handle_resume_event returns early (unchanged current_node) when + # _apply_observation_transition returns early (unchanged current_node) when # the workflow is at a terminal state without an explicit retry signal. # In that case just persist the state update and stop. # and stop — don't try to invoke a finished graph. @@ -735,7 +612,7 @@ async def _process_workflow(self, message: QueueMessage) -> None: await compiled_workflow.aupdate_state(config, updated_values) return - # If _handle_resume_event returned the state object unchanged (identity + # If _apply_observation_transition returned the state object unchanged (identity # check), no signal was recognised — do not invoke the workflow. # Without this guard, nodes in needs_fresh_invoke (e.g. human_review_gate) # would be re-invoked with is_paused=True and immediately re-pause, @@ -850,13 +727,14 @@ async def _process_workflow(self, message: QueueMessage) -> None: record_workflow_failed(ticket_type="unknown", error_type=type(e).__name__) raise # Let consumer handle retry logic - async def _handle_resume_event( + async def _apply_observation_transition( self, message: QueueMessage, current_state: dict[str, Any], *, adapted_event: AdaptedEvent | None = None, command_decision: CommandDecision | None = None, + policy: ObservationTransitionPolicy | None = None, ) -> dict[str, Any]: """Handle a resume event for a paused workflow. @@ -869,937 +747,14 @@ async def _handle_resume_event( Returns: Updated state for workflow resumption. """ - adapted_event = adapted_event or self._event_adapter_registry().adapt(message) - command_decision = command_decision or interpret_event( - message, adapted_event, current_state + return await apply_observation_transition( + self, + message, + current_state, + adapted_event=adapted_event, + command_decision=command_decision, + policy=policy or ObservationTransitionPolicy(), ) - workflow_command = command_decision.command - if command_decision.command is not None: - logger.debug( - "Interpreted %s as %s command %s", - message.event_id, - command_decision.command.command_type.value, - command_decision.command.command_id, - ) - else: - logger.debug( - "No workflow command derived from %s: %s", - message.event_id, - command_decision.reason, - ) - if command_decision.status.value in {"duplicate", "stale", "invalid"}: - return current_state - - event_obj = self._deserialize_event(message) - current_state = activate_pull_request_for_event(current_state, event_obj) - targets_implementation_pr = event_targets_pull_request(current_state, event_obj) - is_approved = False - is_rejected = False - is_question = False - is_ci_webhook = False - pr_merged = False - feedback = None - automated_review_revision_pending = None - proposal_review_threads: list[dict[str, Any]] = [] - proposal_review_decisions: list[dict[str, Any]] = [] - implementation_pr_approved = False - - current_node = current_state.get("current_node", "") - comment_ticket_key = None - comment_ticket_type = None - - if workflow_command is not None: - handlers = ( - getattr(self, "command_handlers", None) or create_default_command_handler_registry() - ) - application = handlers.apply(workflow_command, current_state) - if application is not None: - feedback_request = application.feedback - if feedback_request is not None: - if feedback_request.kind is FeedbackKind.SKIP_GATE and event_obj is not None: - native_id = ( - event_obj.change_request.identity.native_id - if event_obj.change_request - else None - ) - await self._post_skip_gate_feedback( - ticket_key=message.ticket_key, - repo_ref=event_obj.repo_ref, - pr_number=int(native_id) if native_id is not None else None, - check_name=str(feedback_request.arguments["check_name"]), - sender=str(feedback_request.arguments.get("sender") or ""), - action=str(feedback_request.arguments["action"]), - ) - elif feedback_request.kind is FeedbackKind.REBASE and event_obj is not None: - native_id = ( - event_obj.change_request.identity.native_id - if event_obj.change_request - else None - ) - await self._post_rebase_feedback( - ticket_key=message.ticket_key, - repo_ref=event_obj.repo_ref, - pr_number=int(native_id) if native_id is not None else None, - sender=str(feedback_request.arguments.get("sender") or ""), - ) - elif feedback_request.kind is FeedbackKind.RETRY_ACKNOWLEDGEMENT: - await self._post_retry_acknowledgement( - message.ticket_key, - str(feedback_request.arguments["stage"]), - ) - elif feedback_request.kind is FeedbackKind.TERMINAL_ERROR: - await self._post_terminal_error_comment( - message.ticket_key, - str(feedback_request.arguments["message"]), - ) - elif feedback_request.kind is FeedbackKind.RESUME_ACKNOWLEDGEMENT: - source_ticket_key = feedback_request.arguments.get("source_ticket_key") - await self._post_resume_ack_comment( - message.ticket_key, - signal_type=str(feedback_request.arguments["signal_type"]), - current_node=str(feedback_request.arguments["stage"]), - source_ticket_key=( - str(source_ticket_key) if source_ticket_key else None - ), - ) - elif feedback_request.kind is FeedbackKind.OPTION_RANGE: - maximum = int(feedback_request.arguments["maximum"]) - await self._execute_required_comment( - message.ticket_key, - f"Please reply with >option N where N is between 1 and {maximum}.", - logical_action="invalid-option-range", - discriminator=message.event_id, - ) - return application.state - - # An inline reply at the review-response gate applies only to its thread. - # Preserve unrelated contested threads and re-run review analysis so any - # newly accepted item can proceed without globally clearing objections. - if ( - event_obj is not None - and event_obj.kind == EventKind.COMMENT_CREATED - and event_obj.comment is not None - and event_obj.comment.path is not None - and current_node == "review_response_gate" - and current_state.get("is_paused", True) - ): - reply = event_obj.comment - sender_login = event_obj.actor.login - if sender_login: - forge_login = await self._get_forge_github_login(event_obj.repo_ref) - settings = get_settings() - forge_bot_comment_prefix = settings.forge_bot_comment_prefix - if is_self_comment( - sender_login=sender_login, - comment_body=reply.body, - bot_login=forge_login, - prefix=forge_bot_comment_prefix, - ): - logger.debug("Ignoring Forge's own inline review comment") - return current_state - in_reply_to_raw = reply.in_reply_to - replied_to = ( - int(in_reply_to_raw) - if in_reply_to_raw is not None and in_reply_to_raw.isdigit() - else None - ) - if replied_to is not None: - contested = current_state.get("contested_comments", []) - remaining = [ - item for item in contested if not decision_matches_comment(item, replied_to) - ] - return { - **current_state, - "is_paused": False, - "revision_requested": True, - "feedback_comment": reply.body, - "contested_comments": remaining, - "context": { - **current_state.get("context", {}), - "resume_event": message.event_type, - "observation_id": adapted_event.observation.observation_id, - "review_thread_comment_id": replied_to, - }, - } - own_id = int(reply.id) if reply.id and reply.id.isdigit() else None - return { - **current_state, - "is_paused": False, - "revision_requested": True, - "feedback_comment": reply.body, - "context": { - **current_state.get("context", {}), - "resume_event": message.event_type, - "observation_id": adapted_event.observation.observation_id, - "review_thread_comment_id": own_id, - }, - } - - is_check_event = event_obj is not None and event_obj.kind == EventKind.CHECK_UPDATED - if event_obj is not None and ( - current_node == "ci_evaluator" or (targets_implementation_pr and is_check_event) - ): - if is_check_event: - suite_status = event_obj.check_suite_status - if suite_status and suite_status != CheckStatus.COMPLETED: - logger.info( - f"Ignoring {message.event_type} for {message.ticket_key}: " - f"check_suite not yet completed (status={suite_status!r})" - ) - else: - is_ci_webhook = True - logger.info(f"Detected source-control CI webhook signal for {current_node}") - elif not ( - event_obj.kind - in (EventKind.COMMENT_CREATED, EventKind.REVIEW_SUBMITTED, EventKind.UNKNOWN) - or ( - event_obj.change_request - and event_obj.change_request.state == ChangeRequestState.MERGED - ) - ): - is_ci_webhook = True - logger.info(f"Detected source-control CI webhook signal for {current_node}") - - # A human reply to a proposal review thread resumes only that thread's - # feedback. Forge-authored replies are informational and must not loop. - if ( - event_obj is not None - and event_obj.kind == EventKind.COMMENT_CREATED - and event_obj.comment is not None - and event_obj.comment.path is not None - ): - reply = event_obj.comment - in_reply_to_raw = reply.in_reply_to - replied_to = ( - int(in_reply_to_raw) - if in_reply_to_raw is not None and in_reply_to_raw.isdigit() - else None - ) - is_proposal_reply = ( - self._is_prd_pr_event(message, current_state) and current_node in _PRD_GATE_NODES - ) or ( - self._is_spec_pr_event(message, current_state) and current_node in _SPEC_GATE_NODES - ) - sender_login = event_obj.actor.login - if is_proposal_reply and sender_login: - forge_login = await self._get_forge_github_login(event_obj.repo_ref) - settings = get_settings() - forge_bot_comment_prefix = settings.forge_bot_comment_prefix - if is_self_comment( - sender_login=sender_login, - comment_body=reply.body, - bot_login=forge_login, - prefix=forge_bot_comment_prefix, - ): - return current_state - if is_proposal_reply and replied_to: - previous = current_state.get("proposal_review_decisions", []) - matching = next( - (item for item in previous if decision_matches_comment(item, replied_to)), - None, - ) - if matching: - reply_body = reply.body.strip() - reply_comment_id = int(reply.id) if reply.id.isdigit() else None - decisions = [ - { - **item, - "comment_id": ( - reply_comment_id - if reply_comment_id is not None - else item.get("comment_id") - ), - "disposition": "accept", - "feedback": reply_body, - "status": "pending", - } - if item.get("thread_id") == matching.get("thread_id") - else item - for item in previous - ] - return { - **current_state, - "is_paused": False, - "revision_requested": True, - "feedback_comment": reply_body, - "proposal_review_decisions": decisions, - "automated_review_revision_count": 0, - "automated_review_revision_pending": False, - } - logger.debug( - "Proposal reply target %s did not match a stored review decision", - replied_to, - ) - elif is_proposal_reply: - body = reply.body.strip() - if body and reply.id.isdigit(): - comment_id = int(reply.id) - proposal_review_threads = [ - { - "thread_id": f"comment-{comment_id}", - "path": reply.path or "", - "line": reply.line, - "comments": [ - { - "comment_id": comment_id, - "body": body, - "author": sender_login, - "commit_sha": event_obj.raw.get("comment", {}).get( - "commit_id", "" - ), - } - ], - } - ] - is_rejected = True - feedback = body - else: - logger.warning( - "Dropping proposal reply with empty body or non-numeric " - f"comment id (id={reply.id!r}) for {message.ticket_key}" - ) - - # GitHub events targeting the PRD proposals PR — handled at prd_approval_gate. - # Merge = approval. Review with feedback = revision. Comment = feedback/question. - if self._is_prd_pr_event(message, current_state) and current_node in _PRD_GATE_NODES: - if ( - event_obj is not None - and event_obj.kind == EventKind.REVIEW_SUBMITTED - and event_obj.review is not None - ): - pr_review = event_obj.review - - # Merge-only approval: review approval is intentionally ignored - if pr_review.state in (ReviewState.CHANGES_REQUESTED, ReviewState.COMMENTED): - repo_full = event_obj.repo_ref.namespace - native_id = ( - event_obj.change_request.identity.native_id - if event_obj.change_request - else None - ) - pr_number = int(native_id) if native_id is not None else None - inline_comments: list[dict[str, Any]] = [] - if repo_full and pr_number: - _reviews = await self._review_enrichment().review_threads( - repo_full, pr_number - ) - proposal_review_threads = _reviews_to_raw_threads(_reviews) - inline_comments = _flatten_review_threads(_reviews) - - parts = [] - if pr_review.body.strip(): - parts.append(pr_review.body.strip()) - if inline_comments: - inline_text = "\n\n".join( - f"**{c['path']}** (line {c.get('line') or '?'}):\n{c['body']}" - for c in inline_comments - ) - parts.append(f"Inline comments:\n{inline_text}") - - if parts: - feedback = "\n\n".join(parts) - is_rejected = True - logger.info( - f"PRD PR review ({pr_review.state.value}) for {message.ticket_key}: " - f"body={'yes' if pr_review.body.strip() else 'no'}, " - f"inline={len(inline_comments)}" - ) - else: - logger.info( - f"PRD PR review ({pr_review.state.value}) for {message.ticket_key} " - "with no content — ignoring" - ) - return current_state - - elif ( - event_obj is not None - and event_obj.change_request is not None - and event_obj.change_request.state == ChangeRequestState.MERGED - ): - is_approved = True - pr_merged = True - logger.info(f"PRD PR merged for {message.ticket_key}") - await self._execute_required_jira_effect( - ticket_key=message.ticket_key, - state=current_state, - event_id=message.event_id, - operation=JIRA_LABEL_OPERATION, - payload={"label": ForgeLabel.PRD_APPROVED.value}, - logical_action="approve-prd", - ) - prd_content = current_state.get("prd_content", "") - if prd_content: - await self._execute_required_jira_effect( - ticket_key=message.ticket_key, - state=current_state, - event_id=message.event_id, - operation=JIRA_DESCRIPTION_OPERATION, - payload={"description": prd_content}, - logical_action="publish-approved-prd", - ) - logger.info(f"Copied approved PRD to Jira description for {message.ticket_key}") - - elif ( - event_obj is not None - and event_obj.kind == EventKind.COMMENT_CREATED - and event_obj.comment is not None - and event_obj.comment.path is None - ): - comment_body = (event_obj.comment.body or "").strip() - sender_login = event_obj.actor.login - - if comment_body and sender_login: - # Skip self-comments - forge_login = await self._get_forge_github_login(event_obj.repo_ref) - - settings = get_settings() - forge_bot_comment_prefix = settings.forge_bot_comment_prefix - if is_self_comment( - sender_login=sender_login, - comment_body=comment_body, - bot_login=forge_login, - prefix=forge_bot_comment_prefix, - ): - logger.debug(f"Ignoring self-comment on PRD PR for {message.ticket_key}") - return current_state - - comment_type = classify_comment(comment_body) - if comment_type == CommentType.QUESTION: - is_question = True - feedback = comment_body - logger.info( - f"PRD PR question for {message.ticket_key}: {comment_body[:100]}..." - ) - elif comment_type == CommentType.FEEDBACK: - is_rejected = True - feedback = re.sub(r"^\s*!\s*", "", comment_body) - logger.info( - f"PRD PR feedback for {message.ticket_key}: {feedback[:100]}..." - ) - else: - logger.info( - f"Informational comment on PRD PR for {message.ticket_key}, " - f"ignoring: {comment_body[:100]}..." - ) - - # GitHub events targeting the spec proposals PR — same pattern as PRD PR. - if self._is_spec_pr_event(message, current_state) and current_node in _SPEC_GATE_NODES: - if ( - event_obj is not None - and event_obj.kind == EventKind.REVIEW_SUBMITTED - and event_obj.review is not None - ): - pr_review = event_obj.review - - if pr_review.state in (ReviewState.CHANGES_REQUESTED, ReviewState.COMMENTED): - repo_full = event_obj.repo_ref.namespace - native_id = ( - event_obj.change_request.identity.native_id - if event_obj.change_request - else None - ) - pr_number = int(native_id) if native_id is not None else None - inline_comments: list[dict[str, Any]] = [] - if repo_full and pr_number: - _reviews = await self._review_enrichment().review_threads( - repo_full, pr_number - ) - proposal_review_threads = _reviews_to_raw_threads(_reviews) - inline_comments = _flatten_review_threads(_reviews) - - parts = [] - if pr_review.body.strip(): - parts.append(pr_review.body.strip()) - if inline_comments: - inline_text = "\n\n".join( - f"**{c['path']}** (line {c.get('line') or '?'}):\n{c['body']}" - for c in inline_comments - ) - parts.append(f"Inline comments:\n{inline_text}") - - if parts: - feedback = "\n\n".join(parts) - is_rejected = True - logger.info( - f"Spec PR review ({pr_review.state.value}) for {message.ticket_key}: " - f"body={'yes' if pr_review.body.strip() else 'no'}, " - f"inline={len(inline_comments)}" - ) - else: - logger.info( - f"Spec PR review ({pr_review.state.value}) for {message.ticket_key} " - "with no content — ignoring" - ) - return current_state - - elif ( - event_obj is not None - and event_obj.change_request is not None - and event_obj.change_request.state == ChangeRequestState.MERGED - ): - is_approved = True - pr_merged = True - logger.info(f"Spec PR merged for {message.ticket_key}") - await self._execute_required_jira_effect( - ticket_key=message.ticket_key, - state=current_state, - event_id=message.event_id, - operation=JIRA_LABEL_OPERATION, - payload={"label": ForgeLabel.SPEC_APPROVED.value}, - logical_action="approve-spec", - ) - spec_content = current_state.get("spec_content", "") - if spec_content: - settings = get_settings() - if settings.jira_store_in_comments: - operation = JIRA_STRUCTURED_COMMENT_OPERATION - effect_payload: dict[str, JsonValue] = { - "title": "Technical Specification (Approved)", - "content": spec_content, - "comment_type": "spec", - } - elif settings.jira_spec_custom_field: - operation = JIRA_CUSTOM_FIELD_OPERATION - effect_payload = { - "field": settings.jira_spec_custom_field, - "value": spec_content, - } - else: - operation = JIRA_ATTACHMENT_REPLACE_OPERATION - effect_payload = { - "filename": f"{message.ticket_key}-spec.md", - "content": spec_content, - "content_type": "text/markdown", - } - await self._execute_required_jira_effect( - ticket_key=message.ticket_key, - state=current_state, - event_id=message.event_id, - operation=operation, - payload=effect_payload, - logical_action="publish-approved-spec", - ) - logger.info( - f"Copied approved spec to configured Jira storage for {message.ticket_key}" - ) - - elif ( - event_obj is not None - and event_obj.kind == EventKind.COMMENT_CREATED - and event_obj.comment is not None - and event_obj.comment.path is None - ): - comment_body = (event_obj.comment.body or "").strip() - sender_login = event_obj.actor.login - - if comment_body and sender_login: - forge_login = await self._get_forge_github_login(event_obj.repo_ref) - - settings = get_settings() - forge_bot_comment_prefix = settings.forge_bot_comment_prefix - if is_self_comment( - sender_login=sender_login, - comment_body=comment_body, - bot_login=forge_login, - prefix=forge_bot_comment_prefix, - ): - logger.debug(f"Ignoring self-comment on spec PR for {message.ticket_key}") - return current_state - - comment_type = classify_comment(comment_body) - if comment_type == CommentType.QUESTION: - is_question = True - feedback = comment_body - logger.info( - f"Spec PR question for {message.ticket_key}: {comment_body[:100]}..." - ) - elif comment_type == CommentType.FEEDBACK: - is_rejected = True - feedback = re.sub(r"^\s*!\s*", "", comment_body) - logger.info( - f"Spec PR feedback for {message.ticket_key}: {feedback[:100]}..." - ) - else: - logger.info( - f"Informational comment on spec PR for {message.ticket_key}, " - f"ignoring: {comment_body[:100]}..." - ) - - # Automated proposal reviewers often publish detailed suggestions even when - # their overall verdict is satisfied. Semantically triage the complete review - # before treating it as a revision request. Only a satisfied verdict stops; - # ambiguous results retain the original feedback and revise within the cap. - is_prd_review = self._is_prd_pr_event(message, current_state) and current_node in ( - _PRD_GATE_NODES - ) - is_spec_review = self._is_spec_pr_event(message, current_state) and current_node in ( - _SPEC_GATE_NODES - ) - if ( - is_rejected - and proposal_review_threads - and (is_prd_review or is_spec_review) - and event_obj is not None - and event_obj.actor.is_bot - ): - previous_decisions = { - item.get("thread_id"): item - for item in current_state.get("proposal_review_decisions", []) - if item.get("thread_id") - } - proposal_review_threads = [ - thread - for thread in proposal_review_threads - if previous_decisions.get(thread["thread_id"], {}).get("comment_id") - != thread["comments"][-1].get("comment_id") - ] - if proposal_review_threads: - artifact_type = "PRD" if is_prd_review else "specification" - artifact_content = current_state.get( - "prd_content" if is_prd_review else "spec_content", "" - ) - proposal_review_decisions = await self._review_enrichment().triage_threads( - artifact_type=artifact_type, - artifact_content=artifact_content, - threads=proposal_review_threads, - ticket_key=message.ticket_key, - ) - repo_full = event_obj.repo_ref.namespace if event_obj is not None else "" - native_id = ( - event_obj.change_request.identity.native_id - if event_obj is not None and event_obj.change_request - else None - ) - pr_number = int(native_id) if native_id is not None else None - if repo_full and pr_number: - await self._review_enrichment().reply_to_decisions( - repo_full_name=repo_full, - pr_number=pr_number, - decisions=proposal_review_decisions, - ) - actionable_feedback = [ - decision.get("feedback") - or next( - ( - thread["comments"][-1].get("body", "") - for thread in proposal_review_threads - if thread["thread_id"] == decision["thread_id"] - ), - "", - ) - for decision in proposal_review_decisions - if decision["disposition"] in ("accept", "uncertain") - ] - feedback = "\n\n".join(item for item in actionable_feedback if item) - if not feedback: - return { - **current_state, - "proposal_review_decisions": merge_review_decisions( - current_state.get("proposal_review_decisions", []), - proposal_review_decisions, - ), - } - - if ( - is_rejected - and feedback - and (is_prd_review or is_spec_review) - and event_obj is not None - and event_obj.actor.is_bot - and not proposal_review_decisions - ): - review_state = event_obj.review.state.value if event_obj.review else "comment" - review_author = event_obj.actor.login or "unknown bot" - artifact_type = "PRD" if is_prd_review else "specification" - artifact_content = current_state.get( - "prd_content" if is_prd_review else "spec_content", "" - ) - decision = await self._review_enrichment().triage_automated( - artifact_type=artifact_type, - artifact_content=artifact_content, - review_state=review_state, - review_author=review_author, - review_content=feedback, - ticket_key=message.ticket_key, - ) - logger.info( - "Automated %s review triage for %s: %s (%s)", - artifact_type, - message.ticket_key, - decision.verdict, - decision.reason, - ) - if decision.verdict == "satisfied": - return current_state - - previous_count = current_state.get("automated_review_revision_count", 0) - if previous_count >= _MAX_AUTOMATED_REVIEW_REVISIONS: - logger.warning( - "Automated review revision cap (%d) reached for %s; awaiting human review", - _MAX_AUTOMATED_REVIEW_REVISIONS, - message.ticket_key, - ) - return current_state - automated_review_revision_pending = True - if decision.verdict == "blocking": - feedback = decision.blocking_feedback - - # GitHub pull_request_review events — handled when paused at human_review_gate or review_response_gate. - # A review submission is the primary signal for the human review stage. - if ( - event_obj is not None - and event_obj.kind == EventKind.REVIEW_SUBMITTED - and event_obj.review is not None - and (current_node in _REVIEW_GATES or targets_implementation_pr) - and (current_state.get("is_paused", True) or current_state.get("pending_ci_event")) - ): - review = event_obj.review - sender_login = review.author - if sender_login: - forge_login = await self._get_forge_github_login(event_obj.repo_ref) - settings = get_settings() - forge_bot_comment_prefix = settings.forge_bot_comment_prefix - if is_self_comment( - sender_login=sender_login, - comment_body=review.body, - bot_login=forge_login, - prefix=forge_bot_comment_prefix, - ): - logger.debug("Ignoring Forge's own pull request review") - return current_state - - if review.state == ReviewState.APPROVED: - if targets_implementation_pr: - implementation_pr_approved = True - is_approved = True - logger.info(f"Detected PR review approval for {message.ticket_key}") - elif review.state in (ReviewState.CHANGES_REQUESTED, ReviewState.COMMENTED): - # Always fetch inline comments so the agent gets the full picture, - # regardless of whether a summary body is also present. - repo_full = event_obj.repo_ref.namespace - pr_number = ( - event_obj.change_request.identity.native_id - if event_obj.change_request - else None - ) - inline_comments = [] - if repo_full and pr_number: - review_id = int(review.id) if review.id else None - review_comments = await self._review_enrichment().review_comments( - repo_full, int(pr_number), review_id - ) - inline_comments = [ - {"path": c.path, "line": c.line, "body": c.body} for c in review_comments - ] - - parts = [] - if review.body.strip(): - parts.append(review.body.strip()) - if inline_comments: - inline_text = "\n\n".join( - f"**{c['path']}** (line {c.get('line') or '?'}):\n{c['body']}" - for c in inline_comments - ) - parts.append(f"Inline comments:\n{inline_text}") - - if parts: - feedback = "\n\n".join(parts) - is_rejected = True - logger.info( - f"Detected PR review ({review.state.value}) for {message.ticket_key}: " - f"body={'yes' if review.body.strip() else 'no'}, " - f"inline comments={len(inline_comments)}" - ) - else: - logger.info( - f"Detected PR review ({review.state.value}) for {message.ticket_key} " - f"with no body and no inline comments — ignoring" - ) - return current_state - - # GitHub pull_request:closed + merged — PR was actually merged - if ( - event_obj is not None - and event_obj.change_request is not None - and event_obj.change_request.state == ChangeRequestState.MERGED - and (current_node in _REVIEW_GATES or targets_implementation_pr) - ): - is_approved = True - pr_merged = True - logger.info(f"Detected PR merge for {message.ticket_key}") - - # Build updated state — do NOT set is_paused=False here. - # Each branch below sets it explicitly when a valid signal is detected. - # Unrecognized events (wrong-stage approval, unrelated label changes, etc.) - # must not unpause the workflow — they return current_state unchanged. - updated_state = { - **current_state, - "context": { - **current_state.get("context", {}), - "resume_event": message.event_type, - "observation_id": adapted_event.observation.observation_id, - }, - } - if targets_implementation_pr and is_ci_webhook and current_node != "human_review_gate": - updated_state["current_node"] = "ci_evaluator" - elif targets_implementation_pr and ( - (event_obj is not None and event_obj.kind == EventKind.REVIEW_SUBMITTED) or pr_merged - ): - updated_state["current_node"] = "human_review_gate" - - was_errored = _is_workflow_errored(current_state) - - # Check if workflow is at a terminal state (complete) - terminal_states = ("complete",) - is_terminal = current_node in terminal_states - - if is_ci_webhook: - # GitHub CI event — unpause the gate and let ci_evaluator check the results - updated_state["is_paused"] = False - - if current_node == "human_review_gate": - # Keep current_node as human_review_gate so review webhooks arriving - # during the CI cycle are still accepted from the queue. - updated_state["pending_ci_event"] = True - - elif is_approved: - updated_state["is_paused"] = implementation_pr_approved - updated_state["revision_requested"] = False - updated_state["feedback_comment"] = None - updated_state["last_error"] = None - if implementation_pr_approved: - updated_state["human_review_status"] = "approved" - if pr_merged: - updated_state["pr_merged"] = True - if event_targets_pull_request(updated_state, event_obj): - updated_state = mark_active_pull_request_merged(updated_state) - updated_state["pr_merged"] = all_pull_requests_merged(updated_state) - if not updated_state["pr_merged"]: - updated_state["is_paused"] = True - if is_prd_review: - # Specification review is a separate artifact cycle and must - # receive its own automated revision budget. - updated_state["automated_review_revision_count"] = 0 - updated_state["automated_review_revision_pending"] = False - updated_state["proposal_review_decisions"] = [] - elif is_question: - # Unpause so answer_question node runs, it will re-pause after answering - updated_state["is_paused"] = False - updated_state["is_question"] = True - updated_state["feedback_comment"] = feedback - updated_state["revision_requested"] = False - await self._post_resume_ack_comment( - message.ticket_key, - signal_type="question", - current_node=current_node, - source_ticket_key=comment_ticket_key, - event_id=message.event_id, - ) - elif is_rejected and feedback: - updated_state["is_paused"] = False - updated_state["revision_requested"] = True - updated_state["feedback_comment"] = feedback - if proposal_review_decisions: - updated_state["proposal_review_decisions"] = merge_review_decisions( - current_state.get("proposal_review_decisions", []), - proposal_review_decisions, - ) - if automated_review_revision_pending is not None: - updated_state["automated_review_revision_pending"] = True - elif is_prd_review or is_spec_review: - # A human-requested proposal revision starts a fresh automated - # review cycle after that revision is published. - updated_state["automated_review_revision_count"] = 0 - updated_state["automated_review_revision_pending"] = False - if current_node == "review_response_gate": - updated_state["contested_comments"] = [] - if comment_ticket_key and comment_ticket_type == "epic": - updated_state["current_epic_key"] = comment_ticket_key - updated_state["current_task_key"] = None - elif comment_ticket_key and comment_ticket_type == "task": - updated_state["current_task_key"] = comment_ticket_key - updated_state["current_epic_key"] = None - else: - updated_state["current_task_key"] = None - updated_state["current_epic_key"] = None - await self._post_resume_ack_comment( - message.ticket_key, - signal_type="revision", - current_node=current_node, - source_ticket_key=comment_ticket_key, - event_id=message.event_id, - ) - elif was_errored: - # Workflow has an error — auto-resume up to MAX_AUTO_RETRIES times, - # then require an explicit forge:retry label. - # Terminal states always require explicit retry regardless of count. - MAX_AUTO_RETRIES = 3 - retry_count = current_state.get("retry_count", 0) - cap_reached = retry_count >= MAX_AUTO_RETRIES - - if is_terminal or cap_reached: - last_error = current_state.get("last_error", "Unknown error") - reason = ( - "terminal state" if is_terminal else f"retry cap ({MAX_AUTO_RETRIES}) reached" - ) - if cap_reached and current_state.get("auto_retry_cap_notified"): - logger.info( - f"Workflow for {message.ticket_key} is already blocked after " - f"auto-retry cap at '{current_node}'" - ) - return current_state - - logger.warning( - f"Workflow for {message.ticket_key} at '{current_node}' requires " - f"forge:retry ({reason})" - ) - await self._post_terminal_error_comment(message.ticket_key, last_error) - if cap_reached: - updated_state["is_paused"] = True - updated_state["is_blocked"] = True - updated_state["auto_retry_cap_notified"] = True - return updated_state - return current_state - else: - # Transient failure — auto-resume and let the node retry - prev_error = current_state.get("last_error", "") - safe_prev_error = redact_secrets(prev_error) if prev_error else None - logger.info( - f"Auto-resuming {message.ticket_key} after error at '{current_node}' " - f"(attempt {retry_count + 1}/{MAX_AUTO_RETRIES}): " - f"{safe_prev_error[:100] if safe_prev_error else 'unknown'}" - ) - updated_state["is_paused"] = False - updated_state["last_error"] = None - else: - # Nodes that wait for specific external events should not auto-proceed. - _signal_required_nodes = ( - "ci_evaluator", - "attempt_ci_fix", - "human_review_gate", - "review_response_gate", - ) - if ( - not current_state.get("is_paused", True) - and current_node not in _signal_required_nodes - ): - # Workflow is unpaused at an execution node — let it run. - # Covers checkpoint patches and nodes that don't need a signal. - logger.info( - f"Workflow for {message.ticket_key} is unpaused at {current_node} " - f"— proceeding without explicit signal" - ) - updated_state["is_paused"] = False - else: - # Paused gate with no recognized signal — do not unpause. - # Covers wrong-stage approvals, unrelated label changes, etc. - logger.info( - f"No valid signal detected for {message.ticket_key} " - f"at {current_node} — ignoring event, workflow state unchanged" - ) - return current_state - - return save_active_pull_request(updated_state) async def _post_resume_ack_comment( self, @@ -2412,7 +1367,9 @@ async def run_single_ticket(ticket_key: str) -> dict[str, Any]: workflow_instance.validate_pinned_state(checkpoint_values) initial_state = dict(checkpoint_values) elif status == "legacy_unpinned": - initial_state = workflow_instance.pin_legacy_state(checkpoint_values) + raise WorkflowValidationError( + "checkpoint requires the explicit Phase 8 definition-pinning migration" + ) # Use ticket_key as thread_id for checkpointing config: dict[str, Any] = checkpoint_config diff --git a/src/forge/queue/consumer.py b/src/forge/queue/consumer.py index 8a2098df3..8733cd22d 100644 --- a/src/forge/queue/consumer.py +++ b/src/forge/queue/consumer.py @@ -490,13 +490,6 @@ async def start(self) -> None: tasks.append(self._consume_stream(JIRA_STREAM, EventSource.JIRA)) if EventSource.SOURCE_CONTROL in self._handlers: tasks.append(self._consume_stream(SOURCE_CONTROL_STREAM, EventSource.SOURCE_CONTROL)) - # LEGACY_SOURCE_CONTROL_STREAM (the pre-rename "forge:events:github") - # is intentionally not auto-consumed: those entries predate the - # NormalizedEvent/adapter cutover and have no normalized_event to - # deserialize, so the handler could only silently no-op and ack them - # -- discarding whatever CI/review/merge signal they carried instead - # of processing it. health_check reports its depth so a nonzero - # backlog is visible for a deliberate, out-of-band migration. if tasks: tasks.append(self._process_retry_queue()) diff --git a/src/forge/queue/models.py b/src/forge/queue/models.py index d29372d73..106dbecae 100644 --- a/src/forge/queue/models.py +++ b/src/forge/queue/models.py @@ -24,17 +24,6 @@ ) from forge.models.events import EventSource -# EventSource.SOURCE_CONTROL's value was renamed from "github" to -# "source_control". Retry/DLQ entries and unconsumed stream messages -# persisted before the rename still carry the old value in Redis; map it -# forward so they keep deserializing instead of raising ValueError. -_LEGACY_SOURCE_VALUES: dict[str, EventSource] = {"github": EventSource.SOURCE_CONTROL} - - -def _parse_event_source(value: str) -> EventSource: - legacy = _LEGACY_SOURCE_VALUES.get(value) - return legacy if legacy is not None else EventSource(value) - @dataclass class QueueMessage: @@ -84,7 +73,7 @@ def from_redis(cls, message_id: str, data: dict[str, str]) -> "QueueMessage": return cls( message_id=message_id, event_id=data.get("event_id", ""), - source=_parse_event_source(data.get("source", "jira")), + source=EventSource(data.get("source", "jira")), event_type=data.get("event_type", ""), ticket_key=data.get("ticket_key", ""), payload=json.loads(data.get("payload", "{}")), diff --git a/src/forge/queue/producer.py b/src/forge/queue/producer.py index 49129143a..1a28a4522 100644 --- a/src/forge/queue/producer.py +++ b/src/forge/queue/producer.py @@ -17,12 +17,6 @@ JIRA_STREAM = "forge:events:jira" SOURCE_CONTROL_STREAM = "forge:events:source_control" -# Pre-rename stream name (source-control events used to publish here, and to -# EventSource value "github"). New events never publish to this stream, but -# it may still hold unconsumed entries from before the rename, so the -# consumer keeps draining it -- see queue/consumer.py. -LEGACY_SOURCE_CONTROL_STREAM = "forge:events:github" - _PUBLISH_ONCE_SCRIPT = """ local reserved = redis.call('SET', KEYS[1], '1', 'EX', ARGV[1], 'NX') if not reserved then diff --git a/src/forge/workflow/bug/__init__.py b/src/forge/workflow/bug/__init__.py index 0ed6cc8df..832d5f000 100644 --- a/src/forge/workflow/bug/__init__.py +++ b/src/forge/workflow/bug/__init__.py @@ -1,4 +1,4 @@ -"""Bug workflow implementation.""" +"""Locally runnable bug workflow adapter and state contract.""" from typing import Any @@ -10,7 +10,7 @@ class BugWorkflow(BaseWorkflow): - """Workflow for Bug tickets.""" + """Local harness adapter; runtime uses the governed definition.""" name = "bug" description = "Bug workflow: Analyze -> RCA -> Fix -> PR -> Review" @@ -23,7 +23,6 @@ def matches(self, ticket_type: TicketType, _labels: list[str], _event: dict[str, return ticket_type == TicketType.BUG def build_graph(self) -> StateGraph: - # Lazy import to avoid circular dependency from forge.workflow.bug.graph import build_bug_graph return build_bug_graph() diff --git a/src/forge/workflow/checkpoint_migration.py b/src/forge/workflow/checkpoint_migration.py new file mode 100644 index 000000000..bbc4ef3e3 --- /dev/null +++ b/src/forge/workflow/checkpoint_migration.py @@ -0,0 +1,145 @@ +"""Explicit Phase 8 migration for checkpoints created before definition pinning.""" + +from __future__ import annotations + +from datetime import UTC, datetime, timedelta +from typing import Any + +from pydantic import Field + +from forge.domain import JsonValue, VersionedDomainModel +from forge.workflow.declarative.models import WorkflowDefinition +from forge.workflow.planning_state import record_planning_artifact +from forge.workflow.preconditions import project_capabilities + + +class CheckpointMigrationReport(VersionedDomainModel): + """Dry-run/apply evidence for one immutable checkpoint migration.""" + + run_id: str + workflow_name: str + source_status: str + target_revision: int + target_digest: str + compatible: bool + applied: bool = False + reasons: tuple[str, ...] = () + rollback_until: datetime | None = None + migrated_state: dict[str, JsonValue] | None = Field(default=None, exclude=True) + + +def migrate_unpinned_checkpoint( + state: dict[str, Any], + definition: WorkflowDefinition, + *, + apply: bool = False, + now: datetime | None = None, + rollback_window: timedelta = timedelta(days=7), +) -> CheckpointMigrationReport: + """Validate and optionally pin a legacy checkpoint to one explicit artifact. + + The function is pure: callers persist the returned state only after storing + their own checkpoint backup. Normal workflow resume never invokes it. + """ + now = now or datetime.now(UTC) + run_id = str(state.get("thread_id") or state.get("ticket_key") or "unknown") + workflow_name = str(state.get("workflow_name") or "") + reasons: list[str] = [] + if not workflow_name: + reasons.append("checkpoint has no workflow identity") + elif workflow_name != definition.metadata.name: + reasons.append("checkpoint workflow does not match target definition") + if state.get("workflow_definition_revision") or state.get("workflow_definition_digest"): + reasons.append("checkpoint is already definition-pinned") + position = str(state.get("current_node") or "entry") + valid_positions = set(definition.spec.steps) | {"entry", "complete", "__end__"} + if position not in valid_positions: + reasons.append(f"current position {position!r} does not exist in target definition") + + compatible = not reasons + migrated: dict[str, JsonValue] | None = None + rollback_until = now + rollback_window if compatible else None + if compatible and apply: + assert rollback_until is not None + normalized: dict[str, Any] = dict(state) + for kind, field in ( + ("prd", "prd_content"), + ("spec", "spec_content"), + ("rca", "rca_content"), + ("plan", "plan_content"), + ): + content = normalized.get(field) + if ( + isinstance(content, str) + and content.strip() + and not any(item.get("kind") == kind for item in normalized.get("artifacts") or []) + ): + normalized.update(record_planning_artifact(normalized, kind, content)) + repositories = list(normalized.get("repositories") or []) + known = {item.get("name") for item in repositories} + for name in [ + normalized.get("current_repo"), + *(normalized.get("repos_to_process") or []), + *((normalized.get("tasks_by_repo") or {}).keys()), + ]: + if isinstance(name, str) and name and name not in known: + repositories.append( + { + "name": name, + "source": "checkpoint_migration", + "status": ( + "completed" + if name in set(normalized.get("repos_completed") or []) + else "pending" + ), + "work_unit_ids": list( + (normalized.get("tasks_by_repo") or {}).get(name, []) + ), + } + ) + known.add(name) + normalized["repositories"] = repositories + normalized["current_repository"] = normalized.get("current_repository") or normalized.get( + "current_repo" + ) + pull_requests: dict[str, Any] = {} + for key, record in (normalized.get("pull_requests") or {}).items(): + target = key + if ":" not in str(key) and isinstance(record, dict): + number = record.get("number") + url = record.get("url") + if number is not None: + target = f"{key}:{number}" + elif url: + target = f"{key}:{url}" + pull_requests[str(target)] = record + normalized["pull_requests"] = pull_requests + migrated = { + **normalized, + "workflow_name": definition.metadata.name, + "workflow_revision": definition.metadata.revision, + "workflow_digest": definition.digest, + "workflow_definition_revision": definition.metadata.revision, + "workflow_definition_digest": definition.digest, + "workflow_definition": definition.canonical_dict(), + "workflow_pin_status": "phase8_migrated", + "workflow_state_profile": definition.spec.state, + "workflow_migrated_at": now.isoformat(), + "workflow_rollback_until": rollback_until.isoformat(), + } + migrated["capabilities"] = project_capabilities(migrated) + return CheckpointMigrationReport( + run_id=run_id, + workflow_name=workflow_name or "unidentified", + source_status="legacy_unpinned", + target_revision=definition.metadata.revision, + target_digest=definition.digest, + compatible=compatible, + applied=bool(compatible and apply), + reasons=tuple(reasons), + rollback_until=rollback_until, + migrated_state=migrated, + ) + + +__all__ = ["CheckpointMigrationReport", "migrate_unpinned_checkpoint"] diff --git a/src/forge/workflow/declarative/catalog.py b/src/forge/workflow/declarative/catalog.py index 9e6c5a4b3..91f7c1ca3 100644 --- a/src/forge/workflow/declarative/catalog.py +++ b/src/forge/workflow/declarative/catalog.py @@ -9,6 +9,24 @@ from forge.workflow.node_contracts import contracts_for from forge.workflow.preconditions import NodeContract +# Observation policies are executable process capabilities. They are named +# and versioned here (rather than imported from workflow data) so a published +# definition can select only an implementation reviewed by Forge. The +# post-PR policy is shared by the three built-in golden paths and may only +# target nodes present in the selected definition. +POST_PR_OBSERVATION_POLICY = "post-pr-v1" +OBSERVATION_POLICY_TARGETS: dict[str, frozenset[str]] = { + POST_PR_OBSERVATION_POLICY: frozenset( + { + "ci_evaluator", + "attempt_ci_fix", + "human_review_gate", + "implement_review", + "review_response_gate", + } + ) +} + @dataclass(frozen=True) class StateProfile: @@ -24,6 +42,9 @@ class StateProfile: supported_extensions: frozenset[str] = frozenset( {"station-behavior", "optional-stations", "routing-branches"} ) + observation_policy_targets: dict[str, frozenset[str]] = field( + default_factory=lambda: dict(OBSERVATION_POLICY_TARGETS) + ) def _common_nodes() -> dict[str, Callable[..., Any]]: diff --git a/src/forge/workflow/declarative/compiler.py b/src/forge/workflow/declarative/compiler.py index 877f96fc2..0872990bd 100644 --- a/src/forge/workflow/declarative/compiler.py +++ b/src/forge/workflow/declarative/compiler.py @@ -16,7 +16,7 @@ ) from forge.workflow.declarative.catalog import get_state_profile from forge.workflow.declarative.models import MAX_TRANSITIONS, WorkflowDefinition -from forge.workflow.preconditions import NodeContract, with_preconditions +from forge.workflow.preconditions import NodeContract, project_capabilities, with_preconditions class WorkflowValidationError(ValueError): @@ -39,6 +39,18 @@ def validate(self) -> None: raise WorkflowValidationError( f"unknown mandatory policy '{sorted(unknown_policies)[0]}'" ) + + observation_policy = spec.observation_policy + if observation_policy is not None: + policy_targets = self.profile.observation_policy_targets.get(observation_policy) + if policy_targets is None: + raise WorkflowValidationError(f"unknown observation policy '{observation_policy}'") + missing_policy_targets = policy_targets - set(steps) + if missing_policy_targets: + raise WorkflowValidationError( + f"observation policy '{observation_policy}' targets undeclared node " + f"'{sorted(missing_policy_targets)[0]}'" + ) missing_nodes = ( set(self.profile.mandatory_nodes) - set(steps) if spec.mandatory_policies else set() ) @@ -356,6 +368,7 @@ async def run(state: dict[str, Any]) -> dict[str, Any]: history = list(state.get("transition_history") or []) return { **result, + "capabilities": project_capabilities(result), "workflow_transition_count": transitions, "workflow_node_attempts": attempts, "transition_history": [*history, transition], diff --git a/src/forge/workflow/declarative/definitions/bug.json b/src/forge/workflow/declarative/definitions/bug.json index f5f2c84c4..af6bd3f84 100644 --- a/src/forge/workflow/declarative/definitions/bug.json +++ b/src/forge/workflow/declarative/definitions/bug.json @@ -8,6 +8,7 @@ }, "spec": { "entry": "triage_check", + "observationPolicy": "post-pr-v1", "extensionPoints": [ "station-behavior" ], diff --git a/src/forge/workflow/declarative/definitions/feature.json b/src/forge/workflow/declarative/definitions/feature.json index 75f8abe04..b54052f36 100644 --- a/src/forge/workflow/declarative/definitions/feature.json +++ b/src/forge/workflow/declarative/definitions/feature.json @@ -8,6 +8,7 @@ }, "spec": { "entry": "generate_prd", + "observationPolicy": "post-pr-v1", "extensionPoints": [ "station-behavior" ], diff --git a/src/forge/workflow/declarative/definitions/task_takeover.json b/src/forge/workflow/declarative/definitions/task_takeover.json index cb013470f..5e319977e 100644 --- a/src/forge/workflow/declarative/definitions/task_takeover.json +++ b/src/forge/workflow/declarative/definitions/task_takeover.json @@ -8,6 +8,7 @@ }, "spec": { "entry": "triage_check", + "observationPolicy": "post-pr-v1", "extensionPoints": [ "station-behavior" ], diff --git a/src/forge/workflow/declarative/models.py b/src/forge/workflow/declarative/models.py index b162d1393..d029473ad 100644 --- a/src/forge/workflow/declarative/models.py +++ b/src/forge/workflow/declarative/models.py @@ -93,6 +93,12 @@ class WorkflowSpec(StrictModel): state: Literal["feature", "bug", "task_takeover"] entry: str steps: dict[str, WorkflowStep] + # The provider-neutral policy used to apply external observations to this + # workflow instance. Policies are versioned, allowlisted runtime + # capabilities; arbitrary import paths are deliberately not supported. + # Definitions which do not accept external observation transitions may + # leave this unset (for example, small local test workflows). + observation_policy: str | None = Field(default=None, alias="observationPolicy") resume: WorkflowResume = Field(default_factory=WorkflowResume) mandatory_policies: tuple[str, ...] = Field(default=(), alias="mandatoryPolicies") extension_points: tuple[str, ...] = Field(default=(), alias="extensionPoints") diff --git a/src/forge/workflow/declarative/workflow.py b/src/forge/workflow/declarative/workflow.py index 5b6e8bbab..e7ea56dbf 100644 --- a/src/forge/workflow/declarative/workflow.py +++ b/src/forge/workflow/declarative/workflow.py @@ -12,6 +12,7 @@ from forge.workflow.declarative.compiler import DeclarativeWorkflowCompiler, WorkflowValidationError from forge.workflow.declarative.loader import load_workflow_value from forge.workflow.declarative.models import WorkflowDefinition +from forge.workflow.preconditions import project_capabilities class DeclarativeWorkflow(BaseWorkflow): @@ -34,6 +35,25 @@ def cache_key(self) -> str: def state_schema(self) -> type: return self._profile.schema + @property + def observation_policy(self) -> str | None: + """Return the policy selected by this immutable definition.""" + return self.definition.spec.observation_policy + + def resolve_observation_policy(self) -> str | None: + """Resolve the selected policy through the profile allowlist. + + Compilation performs the same validation during construction. This + explicit lookup gives the orchestrator a single definition-backed + entry point when it begins applying an external observation. + """ + policy = self.observation_policy + if policy is None: + return None + if policy not in self._profile.observation_policy_targets: + raise WorkflowValidationError(f"unknown observation policy '{policy}'") + return policy + def matches( self, _ticket_type: TicketType, @@ -59,6 +79,7 @@ def build_graph(self) -> StateGraph[Any]: def create_initial_state(self, ticket_key: str, **kwargs: Any) -> dict[str, Any]: state = dict(self._profile.initializer(ticket_key, **kwargs)) state.update(self.workflow_metadata()) + state["capabilities"] = project_capabilities(state) return state def workflow_metadata(self) -> dict[str, Any]: @@ -93,18 +114,6 @@ def pin_status(state: dict[str, Any]) -> str: return "legacy_unpinned" return "pinned" - def pin_legacy_state(self, state: dict[str, Any]) -> dict[str, Any]: - """Explicitly pin a legacy checkpoint to this active definition. - - This is intentionally separate from :meth:`migrate_state`: legacy - checkpoints have no source artifact to validate and therefore cannot be - migrated. Operators may use this one-time compatibility default when - accepting the currently active definition for an old checkpoint. - """ - if state.get("workflow_name") and state.get("workflow_name") != self.name: - raise WorkflowValidationError("a checkpoint cannot switch workflow identity") - return {**state, **self.workflow_metadata(), "workflow_pin_status": "legacy_active_default"} - def validate_pinned_state(self, state: dict[str, Any]) -> None: """Reject a checkpoint whose durable artifact identity is inconsistent.""" if not state.get("workflow_name"): diff --git a/src/forge/workflow/feature/__init__.py b/src/forge/workflow/feature/__init__.py index 527252701..d7649c2de 100644 --- a/src/forge/workflow/feature/__init__.py +++ b/src/forge/workflow/feature/__init__.py @@ -1,4 +1,4 @@ -"""Feature workflow implementation.""" +"""Locally runnable feature workflow adapter and state contract.""" from typing import Any @@ -10,7 +10,7 @@ class FeatureWorkflow(BaseWorkflow): - """Full SDLC workflow for Feature tickets.""" + """Local harness adapter; runtime uses the governed definition.""" name = "feature" description = "Full SDLC workflow: PRD -> Spec -> Epic -> Task -> Implementation" @@ -23,7 +23,6 @@ def matches(self, ticket_type: TicketType, _labels: list[str], _event: dict[str, return ticket_type in (TicketType.FEATURE, TicketType.STORY) def build_graph(self) -> StateGraph: - # Lazy import to avoid circular dependency from forge.workflow.feature.graph import build_feature_graph return build_feature_graph() diff --git a/src/forge/workflow/implementation_input.py b/src/forge/workflow/implementation_input.py deleted file mode 100644 index 4840823d9..000000000 --- a/src/forge/workflow/implementation_input.py +++ /dev/null @@ -1,86 +0,0 @@ -"""Compatibility facade for contract-backed implementation-input resolution.""" - -from __future__ import annotations - -from collections.abc import Mapping -from dataclasses import dataclass -from typing import Any, Protocol, cast - -from forge.workflow.base import ArtifactRef, WorkUnit -from forge.workflow.projections.implementation_input import project_implementation_input -from forge.workflow.reducers.implementation_input import reduce_implementation_input -from forge.workflow.stations.implementation_input import ( - NoPendingImplementationWork as StationNoPendingImplementationWork, -) -from forge.workflow.stations.implementation_input import run_implementation_input_station - - -class NoPendingImplementationWork(Exception): - """The repository has known work units, but all have already completed.""" - - -class IssueReader(Protocol): - """Small external-fact surface used only by the request projector.""" - - async def get_issue(self, issue_key: str) -> Any: ... - - -@dataclass(frozen=True) -class ResolvedImplementationInput: - """Backward-compatible view of the typed station outcome.""" - - work_unit: WorkUnit - context_artifacts: tuple[ArtifactRef, ...] - instructions: str - summary: str | None = None - _station_request: Any | None = None - _station_outcome: Any | None = None - - def state_update(self, state: Mapping[str, Any] | None = None) -> dict[str, Any]: - if self._station_request is not None and self._station_outcome is not None: - return reduce_implementation_input( - state or {}, self._station_request, self._station_outcome - ) - - existing_artifacts = list((state or {}).get("artifacts") or []) - artifacts_by_id = {artifact.get("id"): artifact for artifact in existing_artifacts} - for artifact in self.context_artifacts: - artifacts_by_id[artifact.get("id")] = artifact - existing_units = list((state or {}).get("work_units") or []) - units_by_id = {unit.get("id"): unit for unit in existing_units} - previous = units_by_id.get(self.work_unit["id"]) - units_by_id[self.work_unit["id"]] = ( - previous if previous and previous.get("status") == "completed" else self.work_unit - ) - return { - "artifacts": list(artifacts_by_id.values()), - "work_units": list(units_by_id.values()), - "current_work_unit_id": self.work_unit["id"], - "work_resolution": { - "strategy": "task_first", - "selected_work_unit_id": self.work_unit["id"], - "selected_artifact_id": self.work_unit["source_artifact_ids"][0], - }, - } - - -async def resolve_implementation_input( - state: Mapping[str, Any], jira: IssueReader -) -> ResolvedImplementationInput: - """Project a checkpoint, invoke the station, and expose its compatible result.""" - try: - request = await project_implementation_input(state, jira) - outcome = run_implementation_input_station(request) - except StationNoPendingImplementationWork as exc: - raise NoPendingImplementationWork(str(exc)) from exc - assert outcome.output is not None - return ResolvedImplementationInput( - work_unit=cast(WorkUnit, outcome.output.work_unit), - context_artifacts=tuple( - cast(ArtifactRef, item) for item in outcome.output.context_artifacts - ), - instructions=outcome.output.instructions, - summary=outcome.output.summary, - _station_request=request, - _station_outcome=outcome, - ) diff --git a/src/forge/workflow/nodes/implement_work.py b/src/forge/workflow/nodes/implement_work.py index 037e99c67..d22face85 100644 --- a/src/forge/workflow/nodes/implement_work.py +++ b/src/forge/workflow/nodes/implement_work.py @@ -7,10 +7,6 @@ from forge.integrations.jira.client import JiraClient from forge.prompts import load_prompt from forge.sandbox.runner import ContainerRunner -from forge.workflow.implementation_input import ( - NoPendingImplementationWork, - resolve_implementation_input, -) from forge.workflow.nodes.execution_engine import ( ExecutionArtifact, ExecutionPersistenceError, @@ -24,7 +20,10 @@ push_to_fork_with_retry, ) from forge.workflow.nodes.workspace_setup import prepare_workspace -from forge.workflow.planning_state import repository_compatibility_update +from forge.workflow.projections.implementation_input import project_implementation_input +from forge.workflow.reducers.implementation_input import reduce_implementation_input +from forge.workflow.stations.implementation_input import NoPendingImplementationWork +from forge.workflow.stations.runner import invoke_builtin_station from forge.workflow.utils import update_state_timestamp from forge.workflow.utils.jira_status import post_status_comment from forge.workflow.utils.references import fetch_and_inject_references @@ -35,8 +34,7 @@ async def implement_work(state: dict[str, Any]) -> dict[str, Any]: """Implement the most specific repository-scoped work that is available.""" ticket_key = state["ticket_key"] - state = {**state, **repository_compatibility_update(state)} - current_repo = state.get("current_repository") or "" + current_repo = state.get("current_repository") or state.get("current_repo") or "" node_name = "implement_work" jira = JiraClient(get_settings()) container_started = False @@ -78,7 +76,8 @@ async def implement_work(state: dict[str, Any]) -> dict[str, Any]: ) try: - resolved = await resolve_implementation_input(state, jira) + request = await project_implementation_input(state, jira) + outcome = await invoke_builtin_station(request) except NoPendingImplementationWork: return update_state_timestamp( { @@ -90,7 +89,9 @@ async def implement_work(state: dict[str, Any]) -> dict[str, Any]: } ) - work_unit = resolved.work_unit + assert outcome.output is not None + state_update = reduce_implementation_input(state, request, outcome) + work_unit = outcome.output.work_unit work_id = work_unit["id"] primary_id = work_unit["source_artifact_ids"][0] supporting = tuple( @@ -98,18 +99,18 @@ async def implement_work(state: dict[str, Any]) -> dict[str, Any]: title=str(artifact.get("kind", "artifact")).replace("_", " ").title(), content=str(artifact.get("content", "")), ) - for artifact in resolved.context_artifacts + for artifact in outcome.output.context_artifacts if artifact.get("id") != primary_id and artifact.get("content") ) source_kind = str(work_unit.get("kind", "artifact")) - summary = resolved.summary or f"Implement {source_kind} work for {ticket_key}" + summary = outcome.output.summary or f"Implement {source_kind} work for {ticket_key}" request = ExecutionRequest( ticket_key=ticket_key, work_id=work_id, repository=current_repo, workspace_path=workspace_path, summary=summary, - description=resolved.instructions, + description=outcome.output.instructions, description_title=f"Selected {source_kind.replace('_', ' ').title()}", node_name=node_name, step_name=node_name, @@ -133,7 +134,7 @@ async def implement_work(state: dict[str, Any]) -> dict[str, Any]: container_started = True try: execution_state = await run_and_persist_execution( - {**state, **resolved.state_update(state)}, + {**state, **state_update}, request, runner=ContainerRunner(get_settings()), git=git, diff --git a/src/forge/workflow/nodes/prd_generation.py b/src/forge/workflow/nodes/prd_generation.py index a2a5e7f97..2396dcaa9 100644 --- a/src/forge/workflow/nodes/prd_generation.py +++ b/src/forge/workflow/nodes/prd_generation.py @@ -17,6 +17,7 @@ create_proposal_pr, update_proposal_pr, ) +from forge.workflow.planning_state import record_planning_artifact from forge.workflow.projections.artifact_generation import project_artifact_generation from forge.workflow.stations.artifact_generation import ( ArtifactKind, @@ -225,6 +226,7 @@ async def generate_prd(state: WorkflowState) -> WorkflowState: result = update_state_timestamp( { **state, + **record_planning_artifact(state, "prd", prd_content), "prd_content": prd_content, "generation_context": generation_context, "current_node": "prd_approval_gate", @@ -352,6 +354,7 @@ async def regenerate_prd_with_feedback(state: WorkflowState) -> WorkflowState: return update_state_timestamp( { **state, + **record_planning_artifact(state, "prd", new_prd), "prd_content": new_prd, "feedback_comment": None, "revision_requested": False, diff --git a/src/forge/workflow/nodes/proposal_pr.py b/src/forge/workflow/nodes/proposal_pr.py index 877a7d58a..41a071752 100644 --- a/src/forge/workflow/nodes/proposal_pr.py +++ b/src/forge/workflow/nodes/proposal_pr.py @@ -121,8 +121,8 @@ async def create_proposal_pr( f"{prefix}_pr_url": pr_url, f"{prefix}_pr_number": pr_number, # Canonical namespace, not the raw (possibly repos.yaml-alias) - # proposals_repo -- webhook matching (worker._is_prd_pr_event / - # _is_spec_pr_event) compares this against event.repo_ref.namespace, + # proposals_repo -- webhook matching in the observation transition + # boundary compares this against event.repo_ref.namespace, # which is always canonical. f"{prefix}_pr_repo": repo_ref.namespace, f"{prefix}_pr_fork_owner": fork_owner, diff --git a/src/forge/workflow/nodes/spec_generation.py b/src/forge/workflow/nodes/spec_generation.py index c94312c8c..e5e1d40c8 100644 --- a/src/forge/workflow/nodes/spec_generation.py +++ b/src/forge/workflow/nodes/spec_generation.py @@ -22,6 +22,7 @@ create_proposal_pr, update_proposal_pr, ) +from forge.workflow.planning_state import record_planning_artifact from forge.workflow.projections.artifact_generation import project_artifact_generation from forge.workflow.stations.artifact_generation import ( ArtifactKind, @@ -202,6 +203,7 @@ async def generate_spec(state: WorkflowState) -> WorkflowState: result = update_state_timestamp( { **state, + **record_planning_artifact(state, "spec", spec_content), "spec_content": spec_content, "generation_context": generation_context, "current_node": "spec_approval_gate", @@ -343,6 +345,7 @@ async def regenerate_spec_with_feedback(state: WorkflowState) -> WorkflowState: return update_state_timestamp( { **state, + **record_planning_artifact(state, "spec", new_spec), "spec_content": new_spec, "feedback_comment": None, "revision_requested": False, diff --git a/src/forge/workflow/nodes/task_takeover_execution.py b/src/forge/workflow/nodes/task_takeover_execution.py index 1351d10dc..48506dd03 100644 --- a/src/forge/workflow/nodes/task_takeover_execution.py +++ b/src/forge/workflow/nodes/task_takeover_execution.py @@ -8,7 +8,6 @@ from forge.integrations.jira.client import JiraClient from forge.prompts import load_prompt from forge.sandbox.runner import ContainerRunner -from forge.workflow.implementation_input import resolve_implementation_input from forge.workflow.nodes.execution_engine import ( ExecutionArtifact, ExecutionPersistenceError, @@ -23,7 +22,9 @@ use_fork_remote, ) from forge.workflow.nodes.workspace_setup import prepare_workspace -from forge.workflow.planning_state import repository_compatibility_update +from forge.workflow.projections.implementation_input import project_implementation_input +from forge.workflow.reducers.implementation_input import reduce_implementation_input +from forge.workflow.stations.runner import invoke_builtin_station from forge.workflow.task_takeover.state import TaskTakeoverState from forge.workflow.utils import update_state_timestamp from forge.workflow.utils.references import fetch_and_inject_references @@ -41,9 +42,8 @@ async def execute_task_changes(state: TaskTakeoverState) -> TaskTakeoverState: Returns: Updated TaskTakeoverState. """ - state = cast(TaskTakeoverState, {**state, **repository_compatibility_update(state)}) ticket_key = state["ticket_key"] - current_repo = state.get("current_repository") or "" + current_repo = state.get("current_repository") or state.get("current_repo") or "" current_task = state.get("current_task_key") or ticket_key container_started = False recorded_workspace = state.get("workspace_path") @@ -104,12 +104,16 @@ async def execute_task_changes(state: TaskTakeoverState) -> TaskTakeoverState: "last_error": None, } - resolved = await resolve_implementation_input( + request = await project_implementation_input( {**state, "current_task_key": current_task}, jira, ) - state = cast(TaskTakeoverState, {**state, **resolved.state_update(state)}) - primary_id = resolved.work_unit["source_artifact_ids"][0] + outcome = await invoke_builtin_station(request) + assert outcome.output is not None + state = cast( + TaskTakeoverState, {**state, **reduce_implementation_input(state, request, outcome)} + ) + primary_id = outcome.output.work_unit["source_artifact_ids"][0] artifact_titles = { "epic_plan": "Approved Implementation Plan", "plan": "Approved Implementation Plan", @@ -126,7 +130,7 @@ async def execute_task_changes(state: TaskTakeoverState) -> TaskTakeoverState: ), str(artifact.get("content", "")), ) - for artifact in resolved.context_artifacts + for artifact in outcome.output.context_artifacts if artifact.get("id") != primary_id and artifact.get("content") ) @@ -136,7 +140,7 @@ async def execute_task_changes(state: TaskTakeoverState) -> TaskTakeoverState: repository=current_repo, workspace_path=workspace_path, summary=f"Execute task takeover changes for {current_task}", - description=resolved.instructions, + description=outcome.output.instructions, description_title="Task Description", node_name="execute_task_changes", step_name="task_takeover_execution", @@ -190,7 +194,7 @@ async def execute_task_changes(state: TaskTakeoverState) -> TaskTakeoverState: ) if execution_succeeded: for unit in completed_units: - if unit.get("id") == resolved.work_unit["id"]: + if unit.get("id") == outcome.output.work_unit["id"]: unit["status"] = "completed" return cast( TaskTakeoverState, @@ -199,7 +203,7 @@ async def execute_task_changes(state: TaskTakeoverState) -> TaskTakeoverState: **execution_state, "work_units": completed_units, "current_work_unit_id": ( - None if execution_succeeded else resolved.work_unit["id"] + None if execution_succeeded else outcome.output.work_unit["id"] ), "implementation_push_pending": False, "implementation_push_pending_task": None, diff --git a/src/forge/workflow/nodes/workspace_setup.py b/src/forge/workflow/nodes/workspace_setup.py index 8d8bf16bd..40a2293f3 100644 --- a/src/forge/workflow/nodes/workspace_setup.py +++ b/src/forge/workflow/nodes/workspace_setup.py @@ -13,7 +13,6 @@ from forge.integrations.source_control.errors import NotFoundError, ProviderConfigError from forge.workflow.effect_runtime import JiraClient from forge.workflow.nodes.git_persistence import push_to_fork_with_retry -from forge.workflow.planning_state import repository_compatibility_update from forge.workflow.utils import update_state_timestamp from forge.workflow.utils.jira_status import ( post_status_comment, @@ -251,9 +250,8 @@ async def setup_workspace(state: WorkflowState) -> WorkflowState: Returns: Updated state with workspace_path set. """ - state = {**state, **repository_compatibility_update(state)} ticket_key = state["ticket_key"] - current_repo = state.get("current_repository") + current_repo = state.get("current_repository") or state.get("current_repo") tasks_by_repo = state.get("tasks_by_repo", {}) repos_to_process = list(state.get("repos_to_process", [])) @@ -282,11 +280,7 @@ async def setup_workspace(state: WorkflowState) -> WorkflowState: current_repo = repos[0] repos_to_process = repos - state = { - **state, - **repository_compatibility_update(state, current=current_repo), - } - repos_to_process = list(state.get("repos_to_process", [])) + state = {**state, "current_repository": current_repo, "current_repo": current_repo} # Validate repository name if current_repo == "unknown": diff --git a/src/forge/workflow/planning_state.py b/src/forge/workflow/planning_state.py index ae58867f7..9616601c0 100644 --- a/src/forge/workflow/planning_state.py +++ b/src/forge/workflow/planning_state.py @@ -7,14 +7,7 @@ from collections.abc import Iterable, Mapping, Sequence from typing import Any, cast -from forge.workflow.base import ArtifactRef, RepositoryRef, WorkUnit - -LEGACY_ARTIFACT_FIELDS: tuple[tuple[str, str], ...] = ( - ("prd", "prd_content"), - ("spec", "spec_content"), - ("rca", "rca_content"), - ("plan", "plan_content"), -) +from forge.workflow.base import ArtifactRef, WorkUnit def content_digest(content: str) -> str: @@ -22,15 +15,50 @@ def content_digest(content: str) -> str: return f"sha256:{hashlib.sha256(content.encode('utf-8')).hexdigest()}" +def record_planning_artifact(state: Mapping[str, Any], kind: str, content: str) -> dict[str, Any]: + """Persist generated planning content in the authoritative artifact lineage.""" + normalized = content.strip() + digest = content_digest(normalized) + existing = next( + (item for item in planning_artifacts(state) if item.get("kind") == kind), + None, + ) + parent = next( + ( + item + for item in reversed(planning_artifacts(state)) + if item.get("kind") in {"prd", "spec", "rca", "plan"} and item.get("kind") != kind + ), + None, + ) + artifact: ArtifactRef = { + "id": ( + str(existing.get("id")) + if existing + else f"artifact:{state.get('ticket_key') or state.get('thread_id') or 'unknown'}:{kind}" + ), + "kind": kind, + "source": "station", + "content": normalized, + "digest": digest, + "approved_digest": digest, + "status": "approved", + "revision": int(existing.get("revision") or 0) + 1 if existing else 1, + "repository": existing.get("repository") if existing else None, + "input_artifact_ids": [parent["id"]] if parent and parent.get("id") else [], + "parent_artifact_id": parent.get("id") if parent else None, + "child_artifact_ids": list(existing.get("child_artifact_ids") or []) if existing else [], + "provenance": {"station": "artifact-generation", "schema_version": "1.0"}, + } + return apply_artifact_update(state, artifact) + + def artifact_is_current(artifact: Mapping[str, Any]) -> bool: """Return whether an artifact may participate in work resolution. - Status-less artifacts are accepted for checkpoints created before lifecycle - metadata existed. Explicit lifecycle state is authoritative. + Lifecycle state and the digest-bound approval are both required. """ status = artifact.get("status") - if status is None: - return True if status != "approved": return False digest = artifact.get("digest") @@ -38,95 +66,9 @@ def artifact_is_current(artifact: Mapping[str, Any]) -> bool: return bool(digest and approved_digest == digest) -def legacy_artifacts(state: Mapping[str, Any]) -> list[ArtifactRef]: - """Adapt legacy planning content fields into an ordered artifact lineage.""" - ticket_key = str(state.get("ticket_key") or "unknown") - artifacts: list[ArtifactRef] = [] - previous_id: str | None = None - for kind, field in LEGACY_ARTIFACT_FIELDS: - content = state.get(field) - if not isinstance(content, str) or not content.strip(): - continue - normalized = content.strip() - digest = content_digest(normalized) - artifact_id = f"legacy:{ticket_key}:{kind}:{digest[7:19]}" - artifact: ArtifactRef = { - "id": artifact_id, - "kind": kind, - "source": field, - "content": normalized, - "digest": digest, - "approved_digest": digest, - "status": "approved", - "revision": 1, - "repository": None, - "input_artifact_ids": [previous_id] if previous_id else [], - "parent_artifact_id": previous_id, - "child_artifact_ids": [], - "provenance": {"adapter": "legacy_state", "field": field}, - } - if artifacts: - artifacts[-1].setdefault("child_artifact_ids", []).append(artifact_id) - artifacts.append(artifact) - previous_id = artifact_id - return artifacts - - def planning_artifacts(state: Mapping[str, Any]) -> list[ArtifactRef]: - """Return normalized artifacts plus non-duplicated legacy compatibility input.""" - normalized = [cast(ArtifactRef, dict(item)) for item in state.get("artifacts") or []] - normalized_kinds = { - str(item.get("kind")) - for item in normalized - if item.get("kind") in {"prd", "spec", "rca", "plan"} - } - compatible = [ - item for item in legacy_artifacts(state) if item.get("kind") not in normalized_kinds - ] - return [*normalized, *compatible] - - -def repository_compatibility_update( - state: Mapping[str, Any], *, current: str | None = None -) -> dict[str, Any]: - """Synchronize normalized repository state with legacy traversal fields.""" - repository_items = [cast(RepositoryRef, dict(item)) for item in state.get("repositories") or []] - names = [ - str(item["name"]) - for item in repository_items - if isinstance(item.get("name"), str) and item.get("name") - ] - legacy_names = [ - *list(state.get("repos_to_process") or []), - *list((state.get("tasks_by_repo") or {}).keys()), - ] - selected = current or state.get("current_repository") or state.get("current_repo") - if isinstance(selected, str) and selected: - legacy_names.insert(0, selected) - for name in legacy_names: - if isinstance(name, str) and name and name not in names: - names.append(name) - - completed = set(state.get("repos_completed") or []) - by_name = {item.get("name"): item for item in repository_items} - repositories: list[RepositoryRef] = [] - for name in names: - existing = by_name.get(name) - repositories.append( - existing - or { - "name": name, - "source": "legacy_state", - "status": "completed" if name in completed else "pending", - "work_unit_ids": [], - } - ) - return { - "repositories": repositories, - "current_repository": selected, - "current_repo": selected, - "repos_to_process": names, - } + """Return the authoritative normalized artifact lineage.""" + return [cast(ArtifactRef, dict(item)) for item in state.get("artifacts") or []] def upsert_artifact( diff --git a/src/forge/workflow/pr_state.py b/src/forge/workflow/pr_state.py index 620bb05fc..982e122ba 100644 --- a/src/forge/workflow/pr_state.py +++ b/src/forge/workflow/pr_state.py @@ -67,11 +67,6 @@ def _lookup_record( """Find a PR record for ``repo``, preferring the per-PR numbered key and falling back to a URL-keyed record for a PR saved before its number was known. - Also falls back to the legacy bare-``repo`` key from before per-PR keying - was introduced, so a workflow checkpointed mid-CI/mid-review at deploy - time doesn't get stranded — its record still lives under ``repo`` alone - until the next ``save_active_pull_request`` migrates it to a per-PR key. - Returns ``(key, record)`` or ``(None, None)`` when no record matches. """ if number is not None: @@ -82,9 +77,6 @@ def _lookup_record( record = pull_requests.get(_url_key(repo, url)) if isinstance(record, dict): return _url_key(repo, url), record - record = pull_requests.get(repo) - if isinstance(record, dict): - return repo, record return None, None diff --git a/src/forge/workflow/preconditions.py b/src/forge/workflow/preconditions.py index d07f31118..9790d7ee7 100644 --- a/src/forge/workflow/preconditions.py +++ b/src/forge/workflow/preconditions.py @@ -149,24 +149,27 @@ def _repositories_resolved(state: Mapping[str, Any]) -> bool: } +def project_capabilities(state: Mapping[str, Any]) -> dict[str, bool]: + """Project workflow output into the explicit capability contract.""" + return {name: bool(predicate(state)) for name, predicate in BUILTIN_PREDICATES.items()} + + def has_capability( state: Mapping[str, Any], capability: CapabilityName | str, *, predicates: Mapping[str, CapabilityPredicate] | None = None, ) -> bool: - """Resolve a capability, preferring an explicit state declaration. - - ``state["capabilities"]`` is authoritative even when its value is false. The - built-in predicates are compatibility inference for existing workflow state. - """ + """Resolve an explicitly projected capability.""" name = capability.value if isinstance(capability, CapabilityName) else capability declared = state.get("capabilities", {}) if isinstance(declared, Mapping) and name in declared: return declared[name] is True - predicate = (predicates or BUILTIN_PREDICATES).get(name) - return bool(predicate and predicate(state)) + if predicates is not None: + predicate = predicates.get(name) + return bool(predicate and predicate(state)) + return False _ACTION_PRIORITY = { diff --git a/src/forge/workflow/reducers/task_routing.py b/src/forge/workflow/reducers/task_routing.py index 921413714..eff1fe7b6 100644 --- a/src/forge/workflow/reducers/task_routing.py +++ b/src/forge/workflow/reducers/task_routing.py @@ -1,4 +1,4 @@ -"""Allowlisted legacy checkpoint reducer for task routing.""" +"""Allowlisted checkpoint reducer for typed task routing.""" from __future__ import annotations @@ -33,6 +33,16 @@ def reduce_task_routing( raise ValueError(f"Task-routing station did not succeed: {outcome.status}") return { "station_history": append_station_attempt(state, request, outcome), + "repositories": [ + { + "name": name, + "source": "task_routing", + "status": "pending", + "work_unit_ids": list(request.input.tasks_by_repository.get(name, ())), + } + for name in outcome.output.repositories + ], + "current_repository": outcome.output.first_repository, "repos_to_process": list(outcome.output.repositories), "current_repo": outcome.output.first_repository, "repos_completed": [], diff --git a/src/forge/workflow/stations/implementation_input.py b/src/forge/workflow/stations/implementation_input.py index 03f790dfa..ca35e2b60 100644 --- a/src/forge/workflow/stations/implementation_input.py +++ b/src/forge/workflow/stations/implementation_input.py @@ -50,10 +50,8 @@ class ImplementationOutput(DomainModel): def _current(artifact: dict[str, JsonValue]) -> bool: status = artifact.get("status") - return status is None or ( - status == "approved" - and bool(artifact.get("digest")) - and artifact.get("approved_digest") == artifact.get("digest") + return status == "approved" and ( + bool(artifact.get("digest")) and artifact.get("approved_digest") == artifact.get("digest") ) diff --git a/src/forge/workflow/task_takeover/__init__.py b/src/forge/workflow/task_takeover/__init__.py index 1d4a3d340..e7f047e23 100644 --- a/src/forge/workflow/task_takeover/__init__.py +++ b/src/forge/workflow/task_takeover/__init__.py @@ -1,4 +1,4 @@ -"""Task Takeover workflow implementation.""" +"""Locally runnable task-takeover workflow adapter and state contract.""" from typing import Any, cast @@ -13,7 +13,7 @@ class TaskTakeoverWorkflow(BaseWorkflow): - """Workflow for Task Takeover tickets.""" + """Local harness adapter; runtime uses the governed definition.""" name = "task_takeover" description = "Task Takeover workflow" @@ -23,17 +23,14 @@ def state_schema(self) -> type: return TaskTakeoverState def matches(self, ticket_type: TicketType, labels: list[str], _event: dict[str, Any]) -> bool: - """Return True for standalone managed Task/Epic tickets.""" return ticket_type in (TicketType.TASK, TicketType.EPIC) and "forge:managed" in labels def build_graph(self) -> StateGraph[Any]: - """Construct the LangGraph StateGraph for Task Takeover.""" from forge.workflow.task_takeover.graph import build_task_takeover_graph return build_task_takeover_graph() def create_initial_state(self, ticket_key: str, **kwargs: Any) -> dict[str, Any]: - """Create initial state for a new Task Takeover workflow run.""" return cast(dict[str, Any], create_initial_task_takeover_state(ticket_key, **kwargs)) diff --git a/src/forge/workflow/transitions/__init__.py b/src/forge/workflow/transitions/__init__.py new file mode 100644 index 000000000..62ac65f4e --- /dev/null +++ b/src/forge/workflow/transitions/__init__.py @@ -0,0 +1,15 @@ +"""Workflow-boundary transition runtimes.""" + +from forge.workflow.transitions.observation import ( + ObservationTransitionPolicy, + apply_observation_transition, + deserialize_observation_event, + is_proposal_pull_request_event, +) + +__all__ = [ + "ObservationTransitionPolicy", + "apply_observation_transition", + "deserialize_observation_event", + "is_proposal_pull_request_event", +] diff --git a/src/forge/workflow/transitions/observation.py b/src/forge/workflow/transitions/observation.py new file mode 100644 index 000000000..d6a6f7001 --- /dev/null +++ b/src/forge/workflow/transitions/observation.py @@ -0,0 +1,1110 @@ +"""Provider-neutral application of normalized workflow observations. + +This module owns the observation-to-state transition reducer. The orchestrator +worker supplies the narrow runtime hooks used for external effects; it does not +own the event-specific state machine. +""" + +from __future__ import annotations + +import logging +import re +from collections.abc import Mapping +from dataclasses import dataclass +from typing import Any + +from forge.domain import JsonValue +from forge.effects.jira import ( + JIRA_ATTACHMENT_REPLACE_OPERATION, + JIRA_CUSTOM_FIELD_OPERATION, + JIRA_DESCRIPTION_OPERATION, + JIRA_LABEL_OPERATION, + JIRA_STRUCTURED_COMMENT_OPERATION, +) +from forge.integrations.source_control.comment_identity import is_self_comment +from forge.integrations.source_control.contracts import ( + ChangeRequestState, + CheckStatus, + EventKind, + ReviewState, +) +from forge.models.events import EventSource +from forge.models.workflow import ForgeLabel +from forge.orchestrator.command_handlers import ( + FeedbackKind, + create_default_command_handler_registry, +) +from forge.orchestrator.event_adapters import ( + interpret_event, +) +from forge.utils.redaction import redact_secrets +from forge.workflow.pr_state import ( + activate_pull_request_for_event, + all_pull_requests_merged, + event_targets_pull_request, + mark_active_pull_request_merged, + save_active_pull_request, +) +from forge.workflow.utils.comment_classifier import CommentType, classify_comment +from forge.workflow.utils.review_decisions import ( + decision_matches_comment, + merge_review_decisions, +) + +# Keep the historical logger name so deployments and existing observability +# filters continue to receive transition diagnostics after extraction. +logger = logging.getLogger("forge.orchestrator.worker") + + +@dataclass(frozen=True) +class ObservationTransitionPolicy: + """Identity of the workflow definition governing an observation. + + ``definition`` is intentionally opaque to this runtime. Declarative + workflow resolution can provide the concrete policy later without making + this observation reducer depend on declarative models. + """ + + identifier: str = "default" + definition: Mapping[str, Any] | None = None + + +def _validate_policy(policy: ObservationTransitionPolicy) -> frozenset[str] | None: + """Validate a definition-selected policy and return its declared nodes.""" + if policy.identifier == "default" and policy.definition is None: + return None # Local harnesses have no published process artifact. + if policy.identifier != "post-pr-v1": + raise ValueError(f"unknown observation transition policy {policy.identifier!r}") + if policy.definition is None: + raise ValueError("a governed observation policy requires a pinned definition") + spec = policy.definition.get("spec") + if not isinstance(spec, Mapping) or spec.get("observationPolicy") != policy.identifier: + raise ValueError("checkpoint definition does not select the requested observation policy") + steps = spec.get("steps") + if not isinstance(steps, Mapping): + raise ValueError("checkpoint definition has no workflow steps") + return frozenset(str(name) for name in steps) + + +def _validate_target(state: Mapping[str, Any], allowed_nodes: frozenset[str] | None) -> None: + if allowed_nodes is None: + return + target = str(state.get("current_node") or "") + if target and target not in allowed_nodes and target not in {"entry", "complete", "__end__"}: + raise ValueError(f"observation policy targeted undeclared workflow node {target!r}") + + +def _flatten_review_threads(reviews: list[Any]) -> list[dict[str, Any]]: + return [ + { + "path": review.comments[-1].path or "", + "line": review.comments[-1].line, + "body": review.comments[-1].body, + } + for review in reviews + if review.comments + ] + + +def _reviews_to_raw_threads(reviews: list[Any]) -> list[dict[str, Any]]: + return [ + { + "thread_id": review.id, + "path": review.comments[0].path if review.comments else None, + "line": review.comments[0].line if review.comments else None, + "comments": [ + { + "comment_id": int(comment.id) if comment.id.isdigit() else comment.id, + "body": comment.body, + } + for comment in review.comments + ], + } + for review in reviews + ] + + +def _is_workflow_errored(state: Mapping[str, Any]) -> bool: + return not state.get("is_paused") and state.get("last_error") is not None + + +_PRD_GATE_NODES = ("prd_approval_gate", "generate_prd", "regenerate_prd") +_SPEC_GATE_NODES = ("spec_approval_gate", "generate_spec", "regenerate_spec") +_REVIEW_GATES = ("human_review_gate", "review_response_gate") +_MAX_AUTOMATED_REVIEW_REVISIONS = 3 + + +def deserialize_observation_event(message: Any, adapted_event: Any | None = None) -> Any | None: + """Return the normalized event carried by a source-control observation.""" + if message.normalized_event is None: + return None + return adapted_event.normalized_event if adapted_event is not None else None + + +def is_proposal_pull_request_event( + message: Any, state: Mapping[str, Any], event: Any | None, *, artifact: str +) -> bool: + """Match a normalized observation to the configured PRD/spec proposal PR.""" + if message.source is not EventSource.SOURCE_CONTROL or event is None: + return False + if event.change_request is None: + return False + prefix = "prd" if artifact == "prd" else "spec" + number = state.get(f"{prefix}_pr_number") + repo = state.get(f"{prefix}_pr_repo") + return bool( + number + and repo + and event.repo_ref.namespace == repo + and event.change_request.identity.native_id == number + ) + + +async def apply_observation_transition( + runtime: Any, + message: Any, + current_state: dict[str, Any], + *, + adapted_event: Any | None = None, + command_decision: Any | None = None, + policy: ObservationTransitionPolicy | None = None, +) -> dict[str, Any]: + """Apply one normalized observation to the durable workflow state. + + ``runtime`` exposes only the effect and enrichment hooks required by the + reducer. ``policy`` identifies the governing workflow definition while + keeping this runtime independent of declarative workflow models. + """ + policy = policy or ObservationTransitionPolicy() + if not policy.identifier.strip(): + raise ValueError("observation transition policy identifier must not be empty") + allowed_nodes = _validate_policy(policy) + logger.debug( + "Applying observation under workflow policy %s (definition=%s)", + policy.identifier, + "supplied" if policy.definition is not None else "implicit", + ) + adapted_event = adapted_event or runtime._event_adapter_registry().adapt(message) + command_decision = command_decision or interpret_event(message, adapted_event, current_state) + workflow_command = command_decision.command + if command_decision.command is not None: + logger.debug( + "Interpreted %s as %s command %s", + message.event_id, + command_decision.command.command_type.value, + command_decision.command.command_id, + ) + else: + logger.debug( + "No workflow command derived from %s: %s", + message.event_id, + command_decision.reason, + ) + if command_decision.status.value in {"duplicate", "stale", "invalid"}: + return current_state + + event_obj = deserialize_observation_event(message, adapted_event) + current_state = activate_pull_request_for_event(current_state, event_obj) + targets_implementation_pr = event_targets_pull_request(current_state, event_obj) + is_approved = False + is_rejected = False + is_question = False + is_ci_webhook = False + pr_merged = False + feedback = None + automated_review_revision_pending = None + proposal_review_threads: list[dict[str, Any]] = [] + proposal_review_decisions: list[dict[str, Any]] = [] + implementation_pr_approved = False + + current_node = current_state.get("current_node", "") + comment_ticket_key = None + comment_ticket_type = None + + if workflow_command is not None: + handlers = ( + getattr(runtime, "command_handlers", None) or create_default_command_handler_registry() + ) + application = handlers.apply(workflow_command, current_state) + if application is not None: + feedback_request = application.feedback + if feedback_request is not None: + if feedback_request.kind is FeedbackKind.SKIP_GATE and event_obj is not None: + native_id = ( + event_obj.change_request.identity.native_id + if event_obj.change_request + else None + ) + await runtime._post_skip_gate_feedback( + ticket_key=message.ticket_key, + repo_ref=event_obj.repo_ref, + pr_number=int(native_id) if native_id is not None else None, + check_name=str(feedback_request.arguments["check_name"]), + sender=str(feedback_request.arguments.get("sender") or ""), + action=str(feedback_request.arguments["action"]), + ) + elif feedback_request.kind is FeedbackKind.REBASE and event_obj is not None: + native_id = ( + event_obj.change_request.identity.native_id + if event_obj.change_request + else None + ) + await runtime._post_rebase_feedback( + ticket_key=message.ticket_key, + repo_ref=event_obj.repo_ref, + pr_number=int(native_id) if native_id is not None else None, + sender=str(feedback_request.arguments.get("sender") or ""), + ) + elif feedback_request.kind is FeedbackKind.RETRY_ACKNOWLEDGEMENT: + await runtime._post_retry_acknowledgement( + message.ticket_key, + str(feedback_request.arguments["stage"]), + ) + elif feedback_request.kind is FeedbackKind.TERMINAL_ERROR: + await runtime._post_terminal_error_comment( + message.ticket_key, + str(feedback_request.arguments["message"]), + ) + elif feedback_request.kind is FeedbackKind.RESUME_ACKNOWLEDGEMENT: + source_ticket_key = feedback_request.arguments.get("source_ticket_key") + await runtime._post_resume_ack_comment( + message.ticket_key, + signal_type=str(feedback_request.arguments["signal_type"]), + current_node=str(feedback_request.arguments["stage"]), + source_ticket_key=(str(source_ticket_key) if source_ticket_key else None), + ) + elif feedback_request.kind is FeedbackKind.OPTION_RANGE: + maximum = int(feedback_request.arguments["maximum"]) + await runtime._execute_required_comment( + message.ticket_key, + f"Please reply with >option N where N is between 1 and {maximum}.", + logical_action="invalid-option-range", + discriminator=message.event_id, + ) + return application.state + + # An inline reply at the review-response gate applies only to its thread. + # Preserve unrelated contested threads and re-run review analysis so any + # newly accepted item can proceed without globally clearing objections. + if ( + event_obj is not None + and event_obj.kind == EventKind.COMMENT_CREATED + and event_obj.comment is not None + and event_obj.comment.path is not None + and current_node == "review_response_gate" + and current_state.get("is_paused", True) + ): + reply = event_obj.comment + sender_login = event_obj.actor.login + if sender_login: + forge_login = await runtime._get_forge_github_login(event_obj.repo_ref) + settings = runtime._transition_settings() + forge_bot_comment_prefix = settings.forge_bot_comment_prefix + if is_self_comment( + sender_login=sender_login, + comment_body=reply.body, + bot_login=forge_login, + prefix=forge_bot_comment_prefix, + ): + logger.debug("Ignoring Forge's own inline review comment") + return current_state + in_reply_to_raw = reply.in_reply_to + replied_to = ( + int(in_reply_to_raw) + if in_reply_to_raw is not None and in_reply_to_raw.isdigit() + else None + ) + if replied_to is not None: + contested = current_state.get("contested_comments", []) + remaining = [ + item for item in contested if not decision_matches_comment(item, replied_to) + ] + return { + **current_state, + "is_paused": False, + "revision_requested": True, + "feedback_comment": reply.body, + "contested_comments": remaining, + "context": { + **current_state.get("context", {}), + "resume_event": message.event_type, + "observation_id": adapted_event.observation.observation_id, + "review_thread_comment_id": replied_to, + }, + } + own_id = int(reply.id) if reply.id and reply.id.isdigit() else None + return { + **current_state, + "is_paused": False, + "revision_requested": True, + "feedback_comment": reply.body, + "context": { + **current_state.get("context", {}), + "resume_event": message.event_type, + "observation_id": adapted_event.observation.observation_id, + "review_thread_comment_id": own_id, + }, + } + + is_check_event = event_obj is not None and event_obj.kind == EventKind.CHECK_UPDATED + if event_obj is not None and ( + current_node == "ci_evaluator" or (targets_implementation_pr and is_check_event) + ): + if is_check_event: + suite_status = event_obj.check_suite_status + if suite_status and suite_status != CheckStatus.COMPLETED: + logger.info( + f"Ignoring {message.event_type} for {message.ticket_key}: " + f"check_suite not yet completed (status={suite_status!r})" + ) + else: + is_ci_webhook = True + logger.info(f"Detected source-control CI webhook signal for {current_node}") + elif not ( + event_obj.kind + in (EventKind.COMMENT_CREATED, EventKind.REVIEW_SUBMITTED, EventKind.UNKNOWN) + or ( + event_obj.change_request + and event_obj.change_request.state == ChangeRequestState.MERGED + ) + ): + is_ci_webhook = True + logger.info(f"Detected source-control CI webhook signal for {current_node}") + + # A human reply to a proposal review thread resumes only that thread's + # feedback. Forge-authored replies are informational and must not loop. + if ( + event_obj is not None + and event_obj.kind == EventKind.COMMENT_CREATED + and event_obj.comment is not None + and event_obj.comment.path is not None + ): + reply = event_obj.comment + in_reply_to_raw = reply.in_reply_to + replied_to = ( + int(in_reply_to_raw) + if in_reply_to_raw is not None and in_reply_to_raw.isdigit() + else None + ) + is_proposal_reply = ( + is_proposal_pull_request_event(message, current_state, event_obj, artifact="prd") + and current_node in _PRD_GATE_NODES + ) or ( + is_proposal_pull_request_event(message, current_state, event_obj, artifact="spec") + and current_node in _SPEC_GATE_NODES + ) + sender_login = event_obj.actor.login + if is_proposal_reply and sender_login: + forge_login = await runtime._get_forge_github_login(event_obj.repo_ref) + settings = runtime._transition_settings() + forge_bot_comment_prefix = settings.forge_bot_comment_prefix + if is_self_comment( + sender_login=sender_login, + comment_body=reply.body, + bot_login=forge_login, + prefix=forge_bot_comment_prefix, + ): + return current_state + if is_proposal_reply and replied_to: + previous = current_state.get("proposal_review_decisions", []) + matching = next( + (item for item in previous if decision_matches_comment(item, replied_to)), + None, + ) + if matching: + reply_body = reply.body.strip() + reply_comment_id = int(reply.id) if reply.id.isdigit() else None + decisions = [ + { + **item, + "comment_id": ( + reply_comment_id + if reply_comment_id is not None + else item.get("comment_id") + ), + "disposition": "accept", + "feedback": reply_body, + "status": "pending", + } + if item.get("thread_id") == matching.get("thread_id") + else item + for item in previous + ] + return { + **current_state, + "is_paused": False, + "revision_requested": True, + "feedback_comment": reply_body, + "proposal_review_decisions": decisions, + "automated_review_revision_count": 0, + "automated_review_revision_pending": False, + } + logger.debug( + "Proposal reply target %s did not match a stored review decision", + replied_to, + ) + elif is_proposal_reply: + body = reply.body.strip() + if body and reply.id.isdigit(): + comment_id = int(reply.id) + proposal_review_threads = [ + { + "thread_id": f"comment-{comment_id}", + "path": reply.path or "", + "line": reply.line, + "comments": [ + { + "comment_id": comment_id, + "body": body, + "author": sender_login, + "commit_sha": event_obj.raw.get("comment", {}).get("commit_id", ""), + } + ], + } + ] + is_rejected = True + feedback = body + else: + logger.warning( + "Dropping proposal reply with empty body or non-numeric " + f"comment id (id={reply.id!r}) for {message.ticket_key}" + ) + + # GitHub events targeting the PRD proposals PR — handled at prd_approval_gate. + # Merge = approval. Review with feedback = revision. Comment = feedback/question. + if ( + is_proposal_pull_request_event(message, current_state, event_obj, artifact="prd") + and current_node in _PRD_GATE_NODES + ): + if ( + event_obj is not None + and event_obj.kind == EventKind.REVIEW_SUBMITTED + and event_obj.review is not None + ): + pr_review = event_obj.review + + # Merge-only approval: review approval is intentionally ignored + if pr_review.state in (ReviewState.CHANGES_REQUESTED, ReviewState.COMMENTED): + repo_full = event_obj.repo_ref.namespace + native_id = ( + event_obj.change_request.identity.native_id + if event_obj.change_request + else None + ) + pr_number = int(native_id) if native_id is not None else None + spec_inline_comments: list[dict[str, Any]] = [] + if repo_full and pr_number: + _reviews = await runtime._review_enrichment().review_threads( + repo_full, pr_number + ) + proposal_review_threads = _reviews_to_raw_threads(_reviews) + spec_inline_comments = _flatten_review_threads(_reviews) + + parts = [] + if pr_review.body.strip(): + parts.append(pr_review.body.strip()) + if spec_inline_comments: + inline_text = "\n\n".join( + f"**{c['path']}** (line {c.get('line') or '?'}):\n{c['body']}" + for c in spec_inline_comments + ) + parts.append(f"Inline comments:\n{inline_text}") + + if parts: + feedback = "\n\n".join(parts) + is_rejected = True + logger.info( + f"PRD PR review ({pr_review.state.value}) for {message.ticket_key}: " + f"body={'yes' if pr_review.body.strip() else 'no'}, " + f"inline={len(spec_inline_comments)}" + ) + else: + logger.info( + f"PRD PR review ({pr_review.state.value}) for {message.ticket_key} " + "with no content — ignoring" + ) + return current_state + + elif ( + event_obj is not None + and event_obj.change_request is not None + and event_obj.change_request.state == ChangeRequestState.MERGED + ): + is_approved = True + pr_merged = True + logger.info(f"PRD PR merged for {message.ticket_key}") + await runtime._execute_required_jira_effect( + ticket_key=message.ticket_key, + state=current_state, + event_id=message.event_id, + operation=JIRA_LABEL_OPERATION, + payload={"label": ForgeLabel.PRD_APPROVED.value}, + logical_action="approve-prd", + ) + prd_content = current_state.get("prd_content", "") + if prd_content: + await runtime._execute_required_jira_effect( + ticket_key=message.ticket_key, + state=current_state, + event_id=message.event_id, + operation=JIRA_DESCRIPTION_OPERATION, + payload={"description": prd_content}, + logical_action="publish-approved-prd", + ) + logger.info(f"Copied approved PRD to Jira description for {message.ticket_key}") + + elif ( + event_obj is not None + and event_obj.kind == EventKind.COMMENT_CREATED + and event_obj.comment is not None + and event_obj.comment.path is None + ): + comment_body = (event_obj.comment.body or "").strip() + sender_login = event_obj.actor.login + + if comment_body and sender_login: + # Skip self-comments + forge_login = await runtime._get_forge_github_login(event_obj.repo_ref) + + settings = runtime._transition_settings() + forge_bot_comment_prefix = settings.forge_bot_comment_prefix + if is_self_comment( + sender_login=sender_login, + comment_body=comment_body, + bot_login=forge_login, + prefix=forge_bot_comment_prefix, + ): + logger.debug(f"Ignoring self-comment on PRD PR for {message.ticket_key}") + return current_state + + comment_type = classify_comment(comment_body) + if comment_type == CommentType.QUESTION: + is_question = True + feedback = comment_body + logger.info( + f"PRD PR question for {message.ticket_key}: {comment_body[:100]}..." + ) + elif comment_type == CommentType.FEEDBACK: + is_rejected = True + feedback = re.sub(r"^\s*!\s*", "", comment_body) + logger.info(f"PRD PR feedback for {message.ticket_key}: {feedback[:100]}...") + else: + logger.info( + f"Informational comment on PRD PR for {message.ticket_key}, " + f"ignoring: {comment_body[:100]}..." + ) + + # GitHub events targeting the spec proposals PR — same pattern as PRD PR. + if ( + is_proposal_pull_request_event(message, current_state, event_obj, artifact="spec") + and current_node in _SPEC_GATE_NODES + ): + if ( + event_obj is not None + and event_obj.kind == EventKind.REVIEW_SUBMITTED + and event_obj.review is not None + ): + pr_review = event_obj.review + + if pr_review.state in (ReviewState.CHANGES_REQUESTED, ReviewState.COMMENTED): + repo_full = event_obj.repo_ref.namespace + native_id = ( + event_obj.change_request.identity.native_id + if event_obj.change_request + else None + ) + pr_number = int(native_id) if native_id is not None else None + inline_comments: list[dict[str, Any]] = [] + if repo_full and pr_number: + _reviews = await runtime._review_enrichment().review_threads( + repo_full, pr_number + ) + proposal_review_threads = _reviews_to_raw_threads(_reviews) + inline_comments = _flatten_review_threads(_reviews) + + parts = [] + if pr_review.body.strip(): + parts.append(pr_review.body.strip()) + if inline_comments: + inline_text = "\n\n".join( + f"**{c['path']}** (line {c.get('line') or '?'}):\n{c['body']}" + for c in inline_comments + ) + parts.append(f"Inline comments:\n{inline_text}") + + if parts: + feedback = "\n\n".join(parts) + is_rejected = True + logger.info( + f"Spec PR review ({pr_review.state.value}) for {message.ticket_key}: " + f"body={'yes' if pr_review.body.strip() else 'no'}, " + f"inline={len(inline_comments)}" + ) + else: + logger.info( + f"Spec PR review ({pr_review.state.value}) for {message.ticket_key} " + "with no content — ignoring" + ) + return current_state + + elif ( + event_obj is not None + and event_obj.change_request is not None + and event_obj.change_request.state == ChangeRequestState.MERGED + ): + is_approved = True + pr_merged = True + logger.info(f"Spec PR merged for {message.ticket_key}") + await runtime._execute_required_jira_effect( + ticket_key=message.ticket_key, + state=current_state, + event_id=message.event_id, + operation=JIRA_LABEL_OPERATION, + payload={"label": ForgeLabel.SPEC_APPROVED.value}, + logical_action="approve-spec", + ) + spec_content = current_state.get("spec_content", "") + if spec_content: + settings = runtime._transition_settings() + if settings.jira_store_in_comments: + operation = JIRA_STRUCTURED_COMMENT_OPERATION + effect_payload: dict[str, JsonValue] = { + "title": "Technical Specification (Approved)", + "content": spec_content, + "comment_type": "spec", + } + elif settings.jira_spec_custom_field: + operation = JIRA_CUSTOM_FIELD_OPERATION + effect_payload = { + "field": settings.jira_spec_custom_field, + "value": spec_content, + } + else: + operation = JIRA_ATTACHMENT_REPLACE_OPERATION + effect_payload = { + "filename": f"{message.ticket_key}-spec.md", + "content": spec_content, + "content_type": "text/markdown", + } + await runtime._execute_required_jira_effect( + ticket_key=message.ticket_key, + state=current_state, + event_id=message.event_id, + operation=operation, + payload=effect_payload, + logical_action="publish-approved-spec", + ) + logger.info( + f"Copied approved spec to configured Jira storage for {message.ticket_key}" + ) + + elif ( + event_obj is not None + and event_obj.kind == EventKind.COMMENT_CREATED + and event_obj.comment is not None + and event_obj.comment.path is None + ): + comment_body = (event_obj.comment.body or "").strip() + sender_login = event_obj.actor.login + + if comment_body and sender_login: + forge_login = await runtime._get_forge_github_login(event_obj.repo_ref) + + settings = runtime._transition_settings() + forge_bot_comment_prefix = settings.forge_bot_comment_prefix + if is_self_comment( + sender_login=sender_login, + comment_body=comment_body, + bot_login=forge_login, + prefix=forge_bot_comment_prefix, + ): + logger.debug(f"Ignoring self-comment on spec PR for {message.ticket_key}") + return current_state + + comment_type = classify_comment(comment_body) + if comment_type == CommentType.QUESTION: + is_question = True + feedback = comment_body + logger.info( + f"Spec PR question for {message.ticket_key}: {comment_body[:100]}..." + ) + elif comment_type == CommentType.FEEDBACK: + is_rejected = True + feedback = re.sub(r"^\s*!\s*", "", comment_body) + logger.info(f"Spec PR feedback for {message.ticket_key}: {feedback[:100]}...") + else: + logger.info( + f"Informational comment on spec PR for {message.ticket_key}, " + f"ignoring: {comment_body[:100]}..." + ) + + # Automated proposal reviewers often publish detailed suggestions even when + # their overall verdict is satisfied. Semantically triage the complete review + # before treating it as a revision request. Only a satisfied verdict stops; + # ambiguous results retain the original feedback and revise within the cap. + is_prd_review = ( + is_proposal_pull_request_event(message, current_state, event_obj, artifact="prd") + and current_node in _PRD_GATE_NODES + ) + is_spec_review = ( + is_proposal_pull_request_event(message, current_state, event_obj, artifact="spec") + and current_node in _SPEC_GATE_NODES + ) + if ( + is_rejected + and proposal_review_threads + and (is_prd_review or is_spec_review) + and event_obj is not None + and event_obj.actor.is_bot + ): + previous_decisions = { + item.get("thread_id"): item + for item in current_state.get("proposal_review_decisions", []) + if item.get("thread_id") + } + proposal_review_threads = [ + thread + for thread in proposal_review_threads + if previous_decisions.get(thread["thread_id"], {}).get("comment_id") + != thread["comments"][-1].get("comment_id") + ] + if proposal_review_threads: + artifact_type = "PRD" if is_prd_review else "specification" + artifact_content = current_state.get( + "prd_content" if is_prd_review else "spec_content", "" + ) + proposal_review_decisions = await runtime._review_enrichment().triage_threads( + artifact_type=artifact_type, + artifact_content=artifact_content, + threads=proposal_review_threads, + ticket_key=message.ticket_key, + ) + repo_full = event_obj.repo_ref.namespace if event_obj is not None else "" + native_id = ( + event_obj.change_request.identity.native_id + if event_obj is not None and event_obj.change_request + else None + ) + pr_number = int(native_id) if native_id is not None else None + if repo_full and pr_number: + await runtime._review_enrichment().reply_to_decisions( + repo_full_name=repo_full, + pr_number=pr_number, + decisions=proposal_review_decisions, + ) + actionable_feedback = [ + decision.get("feedback") + or next( + ( + thread["comments"][-1].get("body", "") + for thread in proposal_review_threads + if thread["thread_id"] == decision["thread_id"] + ), + "", + ) + for decision in proposal_review_decisions + if decision["disposition"] in ("accept", "uncertain") + ] + feedback = "\n\n".join(item for item in actionable_feedback if item) + if not feedback: + return { + **current_state, + "proposal_review_decisions": merge_review_decisions( + current_state.get("proposal_review_decisions", []), + proposal_review_decisions, + ), + } + + if ( + is_rejected + and feedback + and (is_prd_review or is_spec_review) + and event_obj is not None + and event_obj.actor.is_bot + and not proposal_review_decisions + ): + review_state = event_obj.review.state.value if event_obj.review else "comment" + review_author = event_obj.actor.login or "unknown bot" + artifact_type = "PRD" if is_prd_review else "specification" + artifact_content = current_state.get("prd_content" if is_prd_review else "spec_content", "") + decision = await runtime._review_enrichment().triage_automated( + artifact_type=artifact_type, + artifact_content=artifact_content, + review_state=review_state, + review_author=review_author, + review_content=feedback, + ticket_key=message.ticket_key, + ) + logger.info( + "Automated %s review triage for %s: %s (%s)", + artifact_type, + message.ticket_key, + decision.verdict, + decision.reason, + ) + if decision.verdict == "satisfied": + return current_state + + previous_count = current_state.get("automated_review_revision_count", 0) + if previous_count >= _MAX_AUTOMATED_REVIEW_REVISIONS: + logger.warning( + "Automated review revision cap (%d) reached for %s; awaiting human review", + _MAX_AUTOMATED_REVIEW_REVISIONS, + message.ticket_key, + ) + return current_state + automated_review_revision_pending = True + if decision.verdict == "blocking": + feedback = decision.blocking_feedback + + # GitHub pull_request_review events — handled when paused at human_review_gate or review_response_gate. + # A review submission is the primary signal for the human review stage. + if ( + event_obj is not None + and event_obj.kind == EventKind.REVIEW_SUBMITTED + and event_obj.review is not None + and (current_node in _REVIEW_GATES or targets_implementation_pr) + and (current_state.get("is_paused", True) or current_state.get("pending_ci_event")) + ): + review = event_obj.review + sender_login = review.author + if sender_login: + forge_login = await runtime._get_forge_github_login(event_obj.repo_ref) + settings = runtime._transition_settings() + forge_bot_comment_prefix = settings.forge_bot_comment_prefix + if is_self_comment( + sender_login=sender_login, + comment_body=review.body, + bot_login=forge_login, + prefix=forge_bot_comment_prefix, + ): + logger.debug("Ignoring Forge's own pull request review") + return current_state + + if review.state == ReviewState.APPROVED: + if targets_implementation_pr: + implementation_pr_approved = True + is_approved = True + logger.info(f"Detected PR review approval for {message.ticket_key}") + elif review.state in (ReviewState.CHANGES_REQUESTED, ReviewState.COMMENTED): + # Always fetch inline comments so the agent gets the full picture, + # regardless of whether a summary body is also present. + repo_full = event_obj.repo_ref.namespace + pr_number = ( + event_obj.change_request.identity.native_id if event_obj.change_request else None + ) + inline_comments = [] + if repo_full and pr_number: + review_id = int(review.id) if review.id else None + review_comments = await runtime._review_enrichment().review_comments( + repo_full, int(pr_number), review_id + ) + inline_comments = [ + {"path": c.path, "line": c.line, "body": c.body} for c in review_comments + ] + + parts = [] + if review.body.strip(): + parts.append(review.body.strip()) + if inline_comments: + inline_text = "\n\n".join( + f"**{c['path']}** (line {c.get('line') or '?'}):\n{c['body']}" + for c in inline_comments + ) + parts.append(f"Inline comments:\n{inline_text}") + + if parts: + feedback = "\n\n".join(parts) + is_rejected = True + logger.info( + f"Detected PR review ({review.state.value}) for {message.ticket_key}: " + f"body={'yes' if review.body.strip() else 'no'}, " + f"inline comments={len(inline_comments)}" + ) + else: + logger.info( + f"Detected PR review ({review.state.value}) for {message.ticket_key} " + f"with no body and no inline comments — ignoring" + ) + return current_state + + # GitHub pull_request:closed + merged — PR was actually merged + if ( + event_obj is not None + and event_obj.change_request is not None + and event_obj.change_request.state == ChangeRequestState.MERGED + and (current_node in _REVIEW_GATES or targets_implementation_pr) + ): + is_approved = True + pr_merged = True + logger.info(f"Detected PR merge for {message.ticket_key}") + + # Build updated state — do NOT set is_paused=False here. + # Each branch below sets it explicitly when a valid signal is detected. + # Unrecognized events (wrong-stage approval, unrelated label changes, etc.) + # must not unpause the workflow — they return current_state unchanged. + updated_state = { + **current_state, + "context": { + **current_state.get("context", {}), + "resume_event": message.event_type, + "observation_id": adapted_event.observation.observation_id, + }, + } + if targets_implementation_pr and is_ci_webhook and current_node != "human_review_gate": + updated_state["current_node"] = "ci_evaluator" + elif targets_implementation_pr and ( + (event_obj is not None and event_obj.kind == EventKind.REVIEW_SUBMITTED) or pr_merged + ): + updated_state["current_node"] = "human_review_gate" + + was_errored = _is_workflow_errored(current_state) + + # Check if workflow is at a terminal state (complete) + terminal_states = ("complete",) + is_terminal = current_node in terminal_states + + if is_ci_webhook: + # GitHub CI event — unpause the gate and let ci_evaluator check the results + updated_state["is_paused"] = False + + if current_node == "human_review_gate": + # Keep current_node as human_review_gate so review webhooks arriving + # during the CI cycle are still accepted from the queue. + updated_state["pending_ci_event"] = True + + elif is_approved: + updated_state["is_paused"] = implementation_pr_approved + updated_state["revision_requested"] = False + updated_state["feedback_comment"] = None + updated_state["last_error"] = None + if implementation_pr_approved: + updated_state["human_review_status"] = "approved" + if pr_merged: + updated_state["pr_merged"] = True + if event_targets_pull_request(updated_state, event_obj): + updated_state = mark_active_pull_request_merged(updated_state) + updated_state["pr_merged"] = all_pull_requests_merged(updated_state) + if not updated_state["pr_merged"]: + updated_state["is_paused"] = True + if is_prd_review: + # Specification review is a separate artifact cycle and must + # receive its own automated revision budget. + updated_state["automated_review_revision_count"] = 0 + updated_state["automated_review_revision_pending"] = False + updated_state["proposal_review_decisions"] = [] + elif is_question: + # Unpause so answer_question node runs, it will re-pause after answering + updated_state["is_paused"] = False + updated_state["is_question"] = True + updated_state["feedback_comment"] = feedback + updated_state["revision_requested"] = False + await runtime._post_resume_ack_comment( + message.ticket_key, + signal_type="question", + current_node=current_node, + source_ticket_key=comment_ticket_key, + event_id=message.event_id, + ) + elif is_rejected and feedback: + updated_state["is_paused"] = False + updated_state["revision_requested"] = True + updated_state["feedback_comment"] = feedback + if proposal_review_decisions: + updated_state["proposal_review_decisions"] = merge_review_decisions( + current_state.get("proposal_review_decisions", []), + proposal_review_decisions, + ) + if automated_review_revision_pending is not None: + updated_state["automated_review_revision_pending"] = True + elif is_prd_review or is_spec_review: + # A human-requested proposal revision starts a fresh automated + # review cycle after that revision is published. + updated_state["automated_review_revision_count"] = 0 + updated_state["automated_review_revision_pending"] = False + if current_node == "review_response_gate": + updated_state["contested_comments"] = [] + if comment_ticket_key and comment_ticket_type == "epic": + updated_state["current_epic_key"] = comment_ticket_key + updated_state["current_task_key"] = None + elif comment_ticket_key and comment_ticket_type == "task": + updated_state["current_task_key"] = comment_ticket_key + updated_state["current_epic_key"] = None + else: + updated_state["current_task_key"] = None + updated_state["current_epic_key"] = None + await runtime._post_resume_ack_comment( + message.ticket_key, + signal_type="revision", + current_node=current_node, + source_ticket_key=comment_ticket_key, + event_id=message.event_id, + ) + elif was_errored: + # Workflow has an error — auto-resume up to MAX_AUTO_RETRIES times, + # then require an explicit forge:retry label. + # Terminal states always require explicit retry regardless of count. + MAX_AUTO_RETRIES = 3 + retry_count = current_state.get("retry_count", 0) + cap_reached = retry_count >= MAX_AUTO_RETRIES + + if is_terminal or cap_reached: + last_error = current_state.get("last_error", "Unknown error") + reason = "terminal state" if is_terminal else f"retry cap ({MAX_AUTO_RETRIES}) reached" + if cap_reached and current_state.get("auto_retry_cap_notified"): + logger.info( + f"Workflow for {message.ticket_key} is already blocked after " + f"auto-retry cap at '{current_node}'" + ) + return current_state + + logger.warning( + f"Workflow for {message.ticket_key} at '{current_node}' requires " + f"forge:retry ({reason})" + ) + await runtime._post_terminal_error_comment(message.ticket_key, last_error) + if cap_reached: + updated_state["is_paused"] = True + updated_state["is_blocked"] = True + updated_state["auto_retry_cap_notified"] = True + return updated_state + return current_state + else: + # Transient failure — auto-resume and let the node retry + prev_error = current_state.get("last_error", "") + safe_prev_error = redact_secrets(prev_error) if prev_error else None + logger.info( + f"Auto-resuming {message.ticket_key} after error at '{current_node}' " + f"(attempt {retry_count + 1}/{MAX_AUTO_RETRIES}): " + f"{safe_prev_error[:100] if safe_prev_error else 'unknown'}" + ) + updated_state["is_paused"] = False + updated_state["last_error"] = None + else: + # Nodes that wait for specific external events should not auto-proceed. + _signal_required_nodes = ( + "ci_evaluator", + "attempt_ci_fix", + "human_review_gate", + "review_response_gate", + ) + if not current_state.get("is_paused", True) and current_node not in _signal_required_nodes: + # Workflow is unpaused at an execution node — let it run. + # Covers checkpoint patches and nodes that don't need a signal. + logger.info( + f"Workflow for {message.ticket_key} is unpaused at {current_node} " + f"— proceeding without explicit signal" + ) + updated_state["is_paused"] = False + else: + # Paused gate with no recognized signal — do not unpause. + # Covers wrong-stage approvals, unrelated label changes, etc. + logger.info( + f"No valid signal detected for {message.ticket_key} " + f"at {current_node} — ignoring event, workflow state unchanged" + ) + return current_state + + result = save_active_pull_request(updated_state) + _validate_target(result, allowed_nodes) + return result diff --git a/tests/unit/architecture/test_phase8_removal.py b/tests/unit/architecture/test_phase8_removal.py new file mode 100644 index 000000000..8fc4c74f0 --- /dev/null +++ b/tests/unit/architecture/test_phase8_removal.py @@ -0,0 +1,135 @@ +"""Enforce Phase 8 cutovers and expose any remaining compatibility path.""" + +import ast +import json +from pathlib import Path + +ROOT = Path(__file__).parents[3] +INVENTORY = ROOT / "docs" / "architecture" / "phase-8-removal-inventory.json" +WORKER = ROOT / "src" / "forge" / "orchestrator" / "worker.py" + + +def test_inventory_is_zero_ambiguity_and_remaining_paths_have_evidence() -> None: + document = json.loads(INVENTORY.read_text()) + assert document["schema_version"] == "2.0" + assert document["remaining"] == [] + entries = document["remaining"] + document["removed"] + assert len({item["id"] for item in entries}) == len(entries) + for item in entries: + assert all( + item[field].strip() for field in ("owner", "prerequisite", "replacement", "proof") + ) + for item in document["remaining"]: + assert any((ROOT / path).exists() for path in item["evidence_paths"]) + + +def test_worker_exposes_only_generic_ingress_handler() -> None: + tree = ast.parse(WORKER.read_text(), filename=str(WORKER)) + methods = { + node.name + for node in ast.walk(tree) + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) + } + assert "_handle_event" in methods + assert "_handle_jira_event" not in methods + assert "_handle_source_control_event" not in methods + + +def test_worker_does_not_own_observation_to_transition_interpretation() -> None: + """Observation application must live behind the workflow boundary. + + The worker may normalize, persist, and dispatch an observation. It must not + retain the old resume interpreter, which selected nodes from provider event + kinds and PR/review state. This guard intentionally checks implementation + symbols instead of line counts so a compatibility branch cannot quietly be + reintroduced under a new location in the worker. + """ + tree = ast.parse(WORKER.read_text(), filename=str(WORKER)) + functions = { + node.name + for node in ast.walk(tree) + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) + } + assert "_handle_resume_event" not in functions + assert "_deserialize_event" not in functions + assert "_is_prd_pr_event" not in functions + assert "_is_spec_pr_event" not in functions + + forbidden_calls = { + "activate_pull_request_for_event", + "all_pull_requests_merged", + "event_targets_pull_request", + "mark_active_pull_request_merged", + } + calls = { + node.func.id + for node in ast.walk(tree) + if isinstance(node, ast.Call) and isinstance(node.func, ast.Name) + } + calls.update( + node.func.attr + for node in ast.walk(tree) + if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute) + ) + assert not forbidden_calls.intersection(calls) + + +def test_worker_does_not_branch_on_provider_transition_types() -> None: + """Provider event/review state is input to the transition policy, not worker code.""" + tree = ast.parse(WORKER.read_text(), filename=str(WORKER)) + forbidden_names = { + "event_obj", + "EventKind", + "ChangeRequestState", + "ReviewState", + "targets_implementation_pr", + "is_ci_webhook", + "is_approved", + "is_rejected", + "is_question", + "pr_merged", + } + names = {node.id for node in ast.walk(tree) if isinstance(node, ast.Name)} + assert not forbidden_names.intersection(names) + + +def test_worker_has_one_explicit_observation_boundary_call() -> None: + """The process loop must delegate observation application as one operation. + + ``apply_observation`` is the deliberately small port between ingress and + the pinned workflow definition. The worker can still record the returned + state and execute returned feedback/effects, but may not contain another + event-specific dispatch path. + """ + tree = ast.parse(WORKER.read_text(), filename=str(WORKER)) + calls = [ + node + for node in ast.walk(tree) + if isinstance(node, ast.Call) + and isinstance(node.func, ast.Name) + and node.func.id == "apply_observation_transition" + ] + assert len(calls) == 1 + + +def test_removed_compatibility_symbols_cannot_return() -> None: + source = "\n".join(path.read_text() for path in (ROOT / "src" / "forge").rglob("*.py")) + for symbol in ( + "LEGACY_SOURCE_CONTROL_STREAM", + "_LEGACY_SOURCE_VALUES", + "pin_legacy_state", + "repository_compatibility_update", + "legacy_artifacts", + ): + assert symbol not in source + assert not (ROOT / "src" / "forge" / "workflow" / "implementation_input.py").exists() + + +def test_runtime_registry_uses_only_definition_compiled_golden_paths() -> None: + registry = (ROOT / "src" / "forge" / "workflow" / "registry.py").read_text() + assert "FeatureWorkflow" not in registry + assert "BugWorkflow" not in registry + assert "TaskTakeoverWorkflow" not in registry + assert "FeatureGoldenWorkflow" in registry + assert "BugGoldenWorkflow" in registry + assert "TaskTakeoverGoldenWorkflow" in registry diff --git a/tests/unit/orchestrator/test_blocked_retry.py b/tests/unit/orchestrator/test_blocked_retry.py index 60f286e94..203c18b4f 100644 --- a/tests/unit/orchestrator/test_blocked_retry.py +++ b/tests/unit/orchestrator/test_blocked_retry.py @@ -111,7 +111,7 @@ async def test_non_blocked_mid_workflow_is_invocable(self): class TestRetryHandlerClearsBlockedState: - """_handle_resume_event clears is_blocked and resets ci_fix_attempts on retry.""" + """_apply_observation_transition clears is_blocked and resets ci_fix_attempts on retry.""" @pytest.mark.asyncio async def test_retry_clears_is_blocked(self, worker, base_message): @@ -129,7 +129,9 @@ async def test_retry_clears_is_blocked(self, worker, base_message): "context": {}, } - result = await worker._handle_resume_event(_make_retry_message(base_message), blocked_state) + result = await worker._apply_observation_transition( + _make_retry_message(base_message), blocked_state + ) assert result.get("is_blocked") is False @@ -149,7 +151,9 @@ async def test_retry_resets_ci_fix_attempts_unconditionally(self, worker, base_m "context": {}, } - result = await worker._handle_resume_event(_make_retry_message(base_message), blocked_state) + result = await worker._apply_observation_transition( + _make_retry_message(base_message), blocked_state + ) assert result.get("ci_fix_attempt") == 0 @@ -169,7 +173,9 @@ async def test_retry_clears_last_error(self, worker, base_message): "context": {}, } - result = await worker._handle_resume_event(_make_retry_message(base_message), blocked_state) + result = await worker._apply_observation_transition( + _make_retry_message(base_message), blocked_state + ) assert result.get("last_error") is None @@ -189,7 +195,9 @@ async def test_retry_preserves_current_node(self, worker, base_message): "context": {}, } - result = await worker._handle_resume_event(_make_retry_message(base_message), blocked_state) + result = await worker._apply_observation_transition( + _make_retry_message(base_message), blocked_state + ) assert result.get("current_node") == "ci_evaluator" @@ -209,7 +217,9 @@ async def test_retry_marks_non_gate_node_for_fresh_invoke(self, worker, base_mes "context": {}, } - result = await worker._handle_resume_event(_make_retry_message(base_message), blocked_state) + result = await worker._apply_observation_transition( + _make_retry_message(base_message), blocked_state + ) assert result.get("context", {}).get("force_fresh_invoke") is True @@ -224,7 +234,7 @@ async def test_retry_posts_acknowledgement(self, worker, base_message): "last_error": "Implementation failed", "context": {}, } - await worker._handle_resume_event(_make_retry_message(base_message), blocked_state) + await worker._apply_observation_transition(_make_retry_message(base_message), blocked_state) worker._post_retry_acknowledgement.assert_awaited_once_with( "TEST-123", "execute_task_changes" @@ -276,7 +286,9 @@ async def test_retry_on_non_terminal_no_error_still_resumes(self, worker, base_m "context": {}, } - result = await worker._handle_resume_event(_make_retry_message(base_message), stuck_state) + result = await worker._apply_observation_transition( + _make_retry_message(base_message), stuck_state + ) assert result.get("is_paused") is False assert result.get("last_error") is None @@ -304,7 +316,7 @@ async def test_retry_on_terminal_no_error_posts_comment(self, worker, base_messa worker._post_terminal_error_comment = AsyncMock() - result = await worker._handle_resume_event( + result = await worker._apply_observation_transition( _make_retry_message(base_message), terminal_state ) @@ -335,7 +347,9 @@ async def test_retry_at_task_plan_approval_gate_sets_revision_requested( "context": {}, } - result = await worker._handle_resume_event(_make_retry_message(base_message), state) + result = await worker._apply_observation_transition( + _make_retry_message(base_message), state + ) assert result.get("is_paused") is False assert result.get("revision_requested") is True diff --git a/tests/unit/orchestrator/test_observation_transitions.py b/tests/unit/orchestrator/test_observation_transitions.py new file mode 100644 index 000000000..600c83501 --- /dev/null +++ b/tests/unit/orchestrator/test_observation_transitions.py @@ -0,0 +1,235 @@ +"""Contract tests for the workflow-boundary observation transition reducer. + +These tests deliberately call the transition boundary rather than +``OrchestratorWorker``. Provider observations are normalized before they reach +this API; the worker is only responsible for dispatching the call and +persisting its result. +""" + +from datetime import UTC, datetime +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from forge.integrations.source_control.contracts import ( + Actor, + ChangeRequest, + ChangeRequestIdentity, + ChangeRequestState, + CheckStatus, + EventKind, + NormalizedEvent, + Provider, + RepositoryRef, + Review, + ReviewState, +) +from forge.models.events import EventSource +from forge.orchestrator.event_adapters import ( + CommandDecision, + CommandDecisionStatus, + create_default_event_adapter_registry, +) +from forge.queue.models import QueueMessage, normalized_event_to_dict +from forge.workflow.declarative.builtins import builtin_feature_definition +from forge.workflow.transitions import ( + ObservationTransitionPolicy, + apply_observation_transition, +) + + +def _policy() -> ObservationTransitionPolicy: + definition = builtin_feature_definition() + return ObservationTransitionPolicy( + identifier="post-pr-v1", definition=definition.canonical_dict() + ) + +NOW = datetime(2026, 8, 28, tzinfo=UTC) + + +def _repo(name: str = "acme/payments") -> RepositoryRef: + return RepositoryRef( + id=name, + provider=Provider.GITHUB, + connection="default-github", + namespace=name, + default_branch="main", + change_request_mode="fork", + ) + + +def _change_request( + repo: RepositoryRef, + number: int = 42, + state: ChangeRequestState = ChangeRequestState.OPEN, +) -> ChangeRequest: + return ChangeRequest( + identity=ChangeRequestIdentity( + connection=repo.connection, + repository_id=repo.id, + native_id=number, + ), + url=f"https://github.com/{repo.namespace}/pull/{number}", + title="Change", + body="", + state=state, + source_branch="feature", + target_branch="main", + draft=False, + ) + + +def _event( + kind: EventKind, + *, + repo: RepositoryRef | None = None, + change_request: ChangeRequest | None = None, + check_suite_status: CheckStatus | None = None, + review: Review | None = None, +) -> NormalizedEvent: + return NormalizedEvent( + id="provider-event-1", + kind=kind, + repo_ref=repo or _repo(), + actor=Actor(login="reviewer", is_bot=False), + received_at=NOW, + change_request=change_request, + check_suite_status=check_suite_status, + review=review, + raw={}, + ) + + +def _message(event: NormalizedEvent, *, ticket_key: str = "FORGE-42") -> QueueMessage: + return QueueMessage( + message_id="message-1", + event_id=event.id, + source=EventSource.SOURCE_CONTROL, + event_type=event.kind.value, + ticket_key=ticket_key, + payload={}, + normalized_event=normalized_event_to_dict(event), + timestamp=NOW, + ) + + +def _adapted(message: QueueMessage): + return create_default_event_adapter_registry().adapt(message) + + +def _ignored_decision() -> CommandDecision: + return CommandDecision(CommandDecisionStatus.IGNORED, "test observation") + + +def _runtime(_event: NormalizedEvent) -> MagicMock: + runtime = MagicMock() + runtime._event_adapter_registry.return_value = ( + create_default_event_adapter_registry() + ) + runtime._get_forge_github_login = AsyncMock(return_value="forge-bot") + return runtime + + +@pytest.mark.asyncio +async def test_completed_ci_observation_is_applied_at_the_boundary() -> None: + repo = _repo() + event = _event( + EventKind.CHECK_UPDATED, + repo=repo, + change_request=_change_request(repo), + check_suite_status=CheckStatus.COMPLETED, + ) + message = _message(event) + state = { + "ticket_key": "FORGE-42", + "current_node": "ci_evaluator", + "is_paused": True, + "context": {}, + } + + result = await apply_observation_transition( + _runtime(event), + message, + state, + adapted_event=_adapted(message), + command_decision=_ignored_decision(), + policy=_policy(), + ) + + assert result is not state + assert result["is_paused"] is False + + +@pytest.mark.asyncio +async def test_incomplete_ci_observation_is_a_noop_at_the_boundary() -> None: + repo = _repo() + event = _event( + EventKind.CHECK_UPDATED, + repo=repo, + change_request=_change_request(repo), + check_suite_status=CheckStatus.IN_PROGRESS, + ) + message = _message(event) + state = { + "ticket_key": "FORGE-42", + "current_node": "ci_evaluator", + "is_paused": True, + "context": {}, + } + + result = await apply_observation_transition( + _runtime(event), + message, + state, + adapted_event=_adapted(message), + command_decision=_ignored_decision(), + policy=_policy(), + ) + + assert result is state + + +@pytest.mark.asyncio +async def test_review_approval_is_applied_without_worker_event_interpretation() -> None: + repo = _repo() + event = _event( + EventKind.REVIEW_SUBMITTED, + repo=repo, + change_request=_change_request(repo), + review=Review( + id="review-1", + state=ReviewState.APPROVED, + body="Looks good", + author="reviewer", + ), + ) + message = _message(event) + state = { + "ticket_key": "FORGE-42", + "current_node": "human_review_gate", + "is_paused": True, + "current_repo": repo.namespace, + "current_pr_number": 42, + "pull_requests": { + f"{repo.namespace}:42": { + "repo": repo.namespace, + "number": 42, + "merged": False, + } + }, + "context": {}, + } + runtime = _runtime(event) + runtime._get_forge_github_login.return_value = "forge-bot" + + result = await apply_observation_transition( + runtime, + message, + state, + adapted_event=_adapted(message), + command_decision=_ignored_decision(), + policy=_policy(), + ) + + assert result["human_review_status"] == "approved" + assert result["is_paused"] is True diff --git a/tests/unit/orchestrator/test_worker.py b/tests/unit/orchestrator/test_worker.py index a05074c40..fc16411df 100644 --- a/tests/unit/orchestrator/test_worker.py +++ b/tests/unit/orchestrator/test_worker.py @@ -31,6 +31,10 @@ normalized_event_to_dict, ) from forge.reconciliation import InMemoryObservationLedger +from forge.workflow.transitions import ( + deserialize_observation_event, + is_proposal_pull_request_event, +) from forge.workflow.utils.source_control import identity_for @@ -134,9 +138,7 @@ async def test_terminal_error_comment_uses_markdown_code_block(): "**Forge workflow stopped with error:**\n\n" "```\nObject of type set is not JSON serializable\n```\n\n" "To retry the workflow, add the label `forge:retry` to this ticket.", - logical_action=( - "terminal-workflow-error:Object of type set is not JSON serializable" - ), + logical_action=("terminal-workflow-error:Object of type set is not JSON serializable"), ) @@ -194,7 +196,7 @@ def merge_message(repo: str, number: int) -> QueueMessage: normalized_event=normalized_event_to_dict(event), ) - partial = await worker._handle_resume_event(merge_message("acme/backend", 10), state) + partial = await worker._apply_observation_transition(merge_message("acme/backend", 10), state) assert partial["current_repo"] == "acme/backend" assert partial["pull_requests"]["acme/backend:10"]["merged"] is True @@ -202,7 +204,9 @@ def merge_message(repo: str, number: int) -> QueueMessage: assert partial["pr_merged"] is False assert partial["is_paused"] is True - complete = await worker._handle_resume_event(merge_message("acme/frontend", 20), partial) + complete = await worker._apply_observation_transition( + merge_message("acme/frontend", 20), partial + ) assert complete["pr_merged"] is True assert complete["is_paused"] is False @@ -231,7 +235,7 @@ async def test_multi_repo_ci_webhook_selects_earlier_pr_from_review_gate() -> No normalized_event=normalized_event_to_dict(event), ) - result = await worker._handle_resume_event(message, _multi_repo_pr_state()) + result = await worker._apply_observation_transition(message, _multi_repo_pr_state()) assert result["current_repo"] == "acme/backend" assert result["current_pr_number"] == 10 @@ -269,7 +273,7 @@ async def test_multi_repo_approval_uses_common_state_cleanup_path() -> None: normalized_event=normalized_event_to_dict(event), ) - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result["current_repo"] == "acme/backend" assert result["is_paused"] is True @@ -308,7 +312,7 @@ async def test_multi_repo_review_selects_earlier_pr() -> None: ) with _patch_adapter(_sc_repo_ref("acme/backend"), mock_adapter): - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result["current_repo"] == "acme/backend" assert result["current_pr_number"] == 10 @@ -355,7 +359,9 @@ async def test_terminal_failure_uses_stable_effect_identity(durable_effect_servi await worker._handle_terminal_failure(message, "failed") await worker._handle_terminal_failure(message, "failed") - commands = [call.args[0] for call in durable_effect_service_mock.execute_required.await_args_list] + commands = [ + call.args[0] for call in durable_effect_service_mock.execute_required.await_args_list + ] assert len(commands) == 2 assert commands[0].effect_id == commands[1].effect_id @@ -432,7 +438,7 @@ async def test_question_comment_sets_is_question_flag( """Comments starting with ? set is_question flag.""" message = self._make_message_with_comment(base_message, "?Why REST instead of GraphQL?") - result = await worker._handle_resume_event(message, base_state) + result = await worker._apply_observation_transition(message, base_state) assert result["is_question"] is True assert result["feedback_comment"] == "?Why REST instead of GraphQL?" @@ -454,7 +460,7 @@ async def test_forge_ask_comment_sets_is_question_flag( base_message, "@forge ask explain the database choice" ) - result = await worker._handle_resume_event(message, base_state) + result = await worker._apply_observation_transition(message, base_state) assert result["is_question"] is True assert result["feedback_comment"] == "@forge ask explain the database choice" @@ -474,7 +480,7 @@ async def test_normal_feedback_still_works( base_message, "!Please add more detail to the security section" ) - result = await worker._handle_resume_event(message, base_state) + result = await worker._apply_observation_transition(message, base_state) assert result.get("is_question") is not True assert result["revision_requested"] is True @@ -517,7 +523,7 @@ async def test_task_phase_feedback_from_epic_sets_current_epic_key( payload=payload, ) - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result["revision_requested"] is True assert result["feedback_comment"] == "Please revise the tasks for this epic" @@ -558,7 +564,7 @@ async def test_plan_phase_feedback_from_epic_acknowledges_epic( payload=payload, ) - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result["revision_requested"] is True assert result["feedback_comment"] == "Please revise this epic plan" @@ -608,7 +614,7 @@ async def test_retry_at_task_approval_gate_clears_stale_epic_and_task_keys( payload=payload, ) - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result["current_epic_key"] is None assert result["current_task_key"] is None @@ -646,7 +652,7 @@ async def test_retry_at_triage_gate_reenters_triage_check( payload=payload, ) - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result["current_node"] == "triage_check" assert result["is_paused"] is False @@ -684,7 +690,7 @@ async def test_retry_at_approval_gate_with_error_triggers_regeneration( payload=payload, ) - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result["current_node"] == "prd_approval_gate" assert result["is_paused"] is False @@ -729,7 +735,7 @@ async def test_retry_at_review_response_gate_transitions_to_human_review_gate( payload=payload, ) - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result["current_node"] == "human_review_gate" assert result["is_paused"] is False @@ -768,7 +774,7 @@ async def test_prd_label_change_to_approved_sets_approved_flag( payload=payload, ) - result = await worker._handle_resume_event(message, base_state) + result = await worker._apply_observation_transition(message, base_state) assert result.get("is_question") is not True assert result["revision_requested"] is False @@ -792,7 +798,7 @@ async def test_auto_retry_cap_marks_workflow_blocked_once( } with patch.object(worker, "_post_terminal_error_comment", new_callable=AsyncMock) as post: - result = await worker._handle_resume_event(base_message, state) + result = await worker._apply_observation_transition(base_message, state) assert result["current_node"] == "implement_review" assert result["retry_count"] == 3 @@ -809,7 +815,7 @@ async def test_question_with_leading_whitespace( """Questions with leading whitespace are still detected.""" message = self._make_message_with_comment(base_message, " ?What about caching?") - result = await worker._handle_resume_event(message, base_state) + result = await worker._apply_observation_transition(message, base_state) assert result["is_question"] is True assert result["revision_requested"] is False @@ -821,7 +827,7 @@ async def test_forge_ask_case_insensitive( """@forge ask detection is case insensitive.""" message = self._make_message_with_comment(base_message, "@FORGE ASK why use microservices?") - result = await worker._handle_resume_event(message, base_state) + result = await worker._apply_observation_transition(message, base_state) assert result["is_question"] is True assert result["revision_requested"] is False @@ -1088,7 +1094,7 @@ async def fake_ensure_skills(*_args, **_kwargs) -> None: patch.object(worker, "_get_compiled_workflow", return_value=fake_compiled), patch.object( worker, - "_handle_resume_event", + "_apply_observation_transition", return_value={ "ticket_key": "TEST-123", "current_node": "prd_approval_gate", @@ -1146,7 +1152,7 @@ async def test_setup_workspace_retry_reinvokes_fresh_state( patch.object(worker, "_extract_ticket_type", return_value=MagicMock(value="Feature")), patch.object(worker.router, "resolve", return_value=fake_workflow), patch.object(worker, "_get_compiled_workflow", return_value=fake_compiled), - patch.object(worker, "_handle_resume_event", return_value=retry_cleared_state), + patch.object(worker, "_apply_observation_transition", return_value=retry_cleared_state), ): await worker._process_workflow(jira_message) @@ -1204,7 +1210,7 @@ async def test_retry_force_fresh_invoke_reruns_bug_implementation( patch.object(worker, "_extract_ticket_type", return_value=MagicMock(value="Bug")), patch.object(worker.router, "resolve", return_value=fake_workflow), patch.object(worker, "_get_compiled_workflow", return_value=fake_compiled), - patch.object(worker, "_handle_resume_event", return_value=retry_cleared_state), + patch.object(worker, "_apply_observation_transition", return_value=retry_cleared_state), ): await worker._process_workflow(jira_message) @@ -1263,14 +1269,14 @@ def _check_suite_message(self, conclusion: str = "failure") -> QueueMessage: async def test_check_suite_recognized_at_ci_evaluator(self, worker): """A completed check_suite event at ci_evaluator must produce a new state object. - _handle_resume_event signals 'no valid event' by returning the *same* state + _apply_observation_transition signals 'no valid event' by returning the *same* state object unchanged. A recognised signal always returns a new dict. We verify object identity to catch the bug where the worker silently ignored the event. """ state = self._ci_state("ci_evaluator") message = self._check_suite_message("failure") - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result is not state, ( "check_suite at ci_evaluator returned the original state unchanged — " @@ -1300,7 +1306,7 @@ async def test_incomplete_check_suite_does_not_unpause_at_ci_evaluator(self, wor normalized_event=normalized_event_to_dict(event), ) - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) # unchanged state returned — is_paused stays as it was assert result is state @@ -1446,7 +1452,7 @@ async def test_task_plan_label_change_to_approved_sets_approved_flag( payload=payload, ) - result = await worker._handle_resume_event(message, base_state) + result = await worker._apply_observation_transition(message, base_state) assert result["is_paused"] is False assert result.get("revision_requested") is not True @@ -1476,7 +1482,7 @@ async def test_task_plan_label_fallback_approved( payload=payload, ) - result = await worker._handle_resume_event(message, base_state) + result = await worker._apply_observation_transition(message, base_state) assert result["is_paused"] is False assert result.get("revision_requested") is not True @@ -1507,7 +1513,7 @@ async def test_task_plan_yolo_gate_activation( payload=payload, ) - result = await worker._handle_resume_event(message, base_state) + result = await worker._apply_observation_transition(message, base_state) assert result["yolo_mode"] is True assert result["is_paused"] is False @@ -1664,7 +1670,7 @@ async def test_ci_webhook_at_review_gate_sets_pending_ci_event(self, worker): normalized_event=normalized_event_to_dict(event), ) - result = await worker._handle_resume_event(message, current_state) + result = await worker._apply_observation_transition(message, current_state) assert result.get("pending_ci_event") is True assert result.get("is_paused") is False @@ -1700,7 +1706,7 @@ async def test_ci_webhook_at_ci_evaluator_does_not_set_pending_ci_event(self, wo normalized_event=normalized_event_to_dict(event), ) - result = await worker._handle_resume_event(message, current_state) + result = await worker._apply_observation_transition(message, current_state) assert result.get("is_paused") is False assert result.get("pending_ci_event", False) is False # not set for ci_evaluator @@ -1746,7 +1752,7 @@ async def test_review_arriving_during_in_flight_ci_cycle_is_not_dropped( ) with _patch_adapter(_sc_repo_ref("owner/repo"), mock_adapter): - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result["revision_requested"] is True assert result["feedback_comment"] == "Needs changes" @@ -1808,7 +1814,7 @@ async def test_forge_authored_pr_review_does_not_resume_review_workflow(self): ) as get_forge_login, patch("forge.orchestrator.worker.get_adapter") as get_adapter_mock, ): - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result is state get_forge_login.assert_awaited_once() @@ -1853,7 +1859,7 @@ async def test_inline_reply_resumes_only_its_contested_thread(self): mock_adapter = AsyncMock() mock_adapter.get_authenticated_identity.return_value = Actor(login="forge-bot", is_bot=True) with _patch_adapter(_sc_repo_ref("owner/repo"), mock_adapter): - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result["is_paused"] is False assert result["revision_requested"] is True @@ -1901,7 +1907,7 @@ async def test_standalone_inline_comment_is_actionable_at_response_gate(self): mock_adapter.get_authenticated_identity.return_value = Actor(login="forge-bot", is_bot=True) with _patch_adapter(_sc_repo_ref("owner/repo"), mock_adapter): - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result["is_paused"] is False assert result["revision_requested"] is True @@ -1962,7 +1968,7 @@ async def test_pr_review_changes_requested_at_review_response_gate(self, _mock_p repo_ref = _sc_repo_ref("owner/repo") with _patch_adapter(repo_ref, mock_adapter): - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result is not state assert result["is_paused"] is False @@ -2019,7 +2025,7 @@ async def test_pr_review_with_review_id_calls_get_review_comments(self, _mock_po repo_ref = _sc_repo_ref("owner/repo") with _patch_adapter(repo_ref, mock_adapter): - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result is not state assert result["is_paused"] is False @@ -2103,7 +2109,7 @@ async def test_pr_review_without_review_id_falls_back(self, _mock_post_comment): repo_ref = _sc_repo_ref("owner/repo") with _patch_adapter(repo_ref, mock_adapter): - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result is not state assert result["is_paused"] is False @@ -2145,7 +2151,7 @@ async def test_pr_approve_at_review_response_gate(self, _mock_post_comment): normalized_event=normalized_event_to_dict(event), ) - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result is not state assert result["is_paused"] is False @@ -2181,7 +2187,7 @@ async def test_pr_merge_at_review_response_gate(self, _mock_post_comment): normalized_event=normalized_event_to_dict(event), ) - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result is not state assert result["is_paused"] is False @@ -2220,7 +2226,7 @@ async def test_pr_review_changes_requested_at_human_review_gate(self, _mock_post ) with _patch_adapter(_sc_repo_ref("owner/repo"), mock_adapter): - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result is not state assert result["is_paused"] is False @@ -2277,7 +2283,7 @@ async def test_pr_commented_review_with_inline_at_review_response_gate( repo_ref = _sc_repo_ref("owner/repo") with _patch_adapter(repo_ref, mock_adapter): - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result is not state assert result["is_paused"] is False @@ -2317,7 +2323,7 @@ async def test_pr_review_ignored_when_not_paused_at_review_response_gate( normalized_event=normalized_event_to_dict(event), ) - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result is state @@ -2380,7 +2386,7 @@ async def test_review_response_gate_resume_routes_to_implement_review(self, _moc ) with _patch_adapter(_sc_repo_ref("owner/repo"), mock_adapter): - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert route_review_response(result) == "implement_review" @@ -2432,7 +2438,7 @@ async def test_integration_bot_login_comment_without_prefix_processed_as_human_f patch("forge.orchestrator.worker.get_settings", return_value=settings), _patch_adapter(_sc_repo_ref("owner/repo"), mock_adapter), ): - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) # It should be processed (not ignored), so state will have updated to resume (is_paused becomes False) assert result is not state @@ -2481,7 +2487,7 @@ async def test_integration_bot_login_comment_with_prefix_ignored_as_self_comment patch.object(worker, "_get_forge_github_login", new=AsyncMock(return_value="dev-user")), patch("forge.orchestrator.worker.get_settings", return_value=settings), ): - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) # It should be ignored (is_self_comment is True), so returns unchanged state assert result is state @@ -2530,7 +2536,7 @@ async def test_integration_app_bot_comment_ending_in_bot_ignored_as_self_comment ), patch("forge.orchestrator.worker.get_settings", return_value=settings), ): - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) # It should be ignored because of the App bot suffix matching our bot login assert result is state @@ -2582,7 +2588,7 @@ async def test_integration_other_app_bot_comment_ending_in_bot_is_not_ignored(se patch("forge.orchestrator.worker.get_settings", return_value=settings), _patch_adapter(_sc_repo_ref("owner/repo"), mock_adapter), ): - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) # It should be processed (not ignored) assert result is not state @@ -2632,7 +2638,7 @@ async def test_integration_legacy_fallback_no_prefix_ignored(self): patch.object(worker, "_get_forge_github_login", new=AsyncMock(return_value="dev-user")), patch("forge.orchestrator.worker.get_settings", return_value=settings), ): - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) # It should be ignored under the legacy fallback because prefix is empty assert result is state @@ -2722,7 +2728,8 @@ def test_returns_none_for_jira_message(self, worker): ticket_key="PROJ-1", payload={}, ) - assert worker._deserialize_event(message) is None + adapted = worker._event_adapter_registry().adapt(message) + assert deserialize_observation_event(message, adapted) is None def test_deserializes_source_control_message(self, worker): event = _make_normalized_event() @@ -2735,7 +2742,8 @@ def test_deserializes_source_control_message(self, worker): payload={}, normalized_event=normalized_event_to_dict(event), ) - restored = worker._deserialize_event(message) + adapted = worker._event_adapter_registry().adapt(message) + restored = deserialize_observation_event(message, adapted) assert restored is not None assert restored.kind == EventKind.CR_OPENED assert restored.repo_ref.namespace == "acme/payments" @@ -2749,7 +2757,7 @@ def worker(self) -> OrchestratorWorker: """Create a worker instance for testing.""" return OrchestratorWorker(consumer_name="test-worker") - def test_is_prd_pr_event_matches_by_repo_and_number(self, worker): + def test_is_prd_pr_event_matches_by_repo_and_number(self): event = _make_normalized_event() message = QueueMessage( message_id="1", @@ -2762,9 +2770,9 @@ def test_is_prd_pr_event_matches_by_repo_and_number(self, worker): ) current_state = {"prd_pr_number": 42, "prd_pr_repo": "acme/payments"} - assert worker._is_prd_pr_event(message, current_state) is True + assert is_proposal_pull_request_event(message, current_state, event, artifact="prd") is True - def test_is_prd_pr_event_false_when_number_differs(self, worker): + def test_is_prd_pr_event_false_when_number_differs(self): event = _make_normalized_event() message = QueueMessage( message_id="1", @@ -2777,9 +2785,11 @@ def test_is_prd_pr_event_false_when_number_differs(self, worker): ) current_state = {"prd_pr_number": 99, "prd_pr_repo": "acme/payments"} - assert worker._is_prd_pr_event(message, current_state) is False + assert ( + is_proposal_pull_request_event(message, current_state, event, artifact="prd") is False + ) - def test_is_prd_pr_event_false_for_jira_source(self, worker): + def test_is_prd_pr_event_false_for_jira_source(self): message = QueueMessage( message_id="1", event_id="e1", @@ -2790,7 +2800,7 @@ def test_is_prd_pr_event_false_for_jira_source(self, worker): ) current_state = {"prd_pr_number": 42, "prd_pr_repo": "acme/payments"} - assert worker._is_prd_pr_event(message, current_state) is False + assert is_proposal_pull_request_event(message, current_state, None, artifact="prd") is False class TestCiWebhookDetectionTypedFields: @@ -2820,7 +2830,7 @@ async def test_check_run_completed_wakes_ci_evaluator(self, worker): "current_pr_number": 42, } - updated = await worker._handle_resume_event(message, current_state) + updated = await worker._apply_observation_transition(message, current_state) assert updated["is_paused"] is False @@ -2847,7 +2857,7 @@ async def test_incomplete_check_suite_does_not_wake_ci_evaluator(self, worker): ) current_state = {"current_node": "ci_evaluator", "is_paused": True, "context": {}} - updated = await worker._handle_resume_event(message, current_state) + updated = await worker._apply_observation_transition(message, current_state) assert updated is current_state @@ -2877,7 +2887,7 @@ async def test_synchronize_push_event_wakes_ci_evaluator(self, worker): "pull_requests": {"acme/payments:42": {"number": 42, "repo": "acme/payments"}}, } - updated = await worker._handle_resume_event(message, current_state) + updated = await worker._apply_observation_transition(message, current_state) assert updated["is_paused"] is False @@ -2908,7 +2918,7 @@ async def test_merged_pr_event_does_not_wake_ci_evaluator(self, worker): "context": {}, } - updated = await worker._handle_resume_event(message, current_state) + updated = await worker._apply_observation_transition(message, current_state) # No CI signal recognised — the paused gate is not woken. assert updated["is_paused"] is True @@ -2931,7 +2941,7 @@ async def test_non_command_comment_does_not_set_ci_webhook(self, worker): ) current_state = {"current_node": "ci_evaluator", "is_paused": True, "context": {}} - updated = await worker._handle_resume_event(message, current_state) + updated = await worker._apply_observation_transition(message, current_state) # CI-webhook branch did not fire — the paused gate stays paused. assert updated["is_paused"] is True @@ -2953,7 +2963,7 @@ async def test_review_submitted_does_not_set_ci_webhook(self, worker): ) current_state = {"current_node": "ci_evaluator", "is_paused": True, "context": {}} - updated = await worker._handle_resume_event(message, current_state) + updated = await worker._apply_observation_transition(message, current_state) assert updated["is_paused"] is True @@ -2981,7 +2991,7 @@ async def test_skip_gate_command_adds_check_name(self, worker): current_state = {"current_node": "ci_evaluator", "is_paused": True} with patch.object(worker, "_post_skip_gate_feedback", AsyncMock()): - updated = await worker._handle_resume_event(message, current_state) + updated = await worker._apply_observation_transition(message, current_state) assert "flaky-test" in updated["ci_skipped_checks"] assert updated["current_node"] == "ci_evaluator" @@ -3004,7 +3014,7 @@ async def test_skip_gate_passes_typed_pr_and_sender_to_feedback(self, worker): feedback = AsyncMock() with patch.object(worker, "_post_skip_gate_feedback", feedback): - await worker._handle_resume_event(message, current_state) + await worker._apply_observation_transition(message, current_state) feedback.assert_called_once() kwargs = feedback.call_args.kwargs @@ -3034,7 +3044,7 @@ async def test_rebase_command_routes_to_rebase_pr(self, worker): feedback = AsyncMock() with patch.object(worker, "_post_rebase_feedback", feedback): - updated = await worker._handle_resume_event(message, current_state) + updated = await worker._apply_observation_transition(message, current_state) assert updated["current_node"] == "rebase_pr" assert updated["is_paused"] is False @@ -3080,7 +3090,7 @@ async def test_inline_reply_clears_matching_contested_comment(self, worker): } with patch.object(worker, "_get_forge_github_login", AsyncMock(return_value="forge-bot")): - updated = await worker._handle_resume_event(message, current_state) + updated = await worker._apply_observation_transition(message, current_state) assert updated["revision_requested"] is True assert updated["contested_comments"] == [] @@ -3117,7 +3127,7 @@ async def test_non_reply_inline_comment_is_still_actionable(self, worker): } with patch.object(worker, "_get_forge_github_login", AsyncMock(return_value="forge-bot")): - updated = await worker._handle_resume_event(message, current_state) + updated = await worker._apply_observation_transition(message, current_state) assert updated is not current_state assert updated["is_paused"] is False @@ -3153,7 +3163,7 @@ async def test_top_level_issue_comment_does_not_match_this_block(self, worker): "context": {}, } - updated = await worker._handle_resume_event(message, current_state) + updated = await worker._apply_observation_transition(message, current_state) assert updated is current_state @@ -3192,7 +3202,7 @@ async def test_review_approved_sets_implementation_pr_approved(self, worker): } with patch.object(worker, "_get_forge_github_login", AsyncMock(return_value="forge-bot")): - updated = await worker._handle_resume_event(message, current_state) + updated = await worker._apply_observation_transition(message, current_state) assert updated["human_review_status"] == "approved" @@ -3220,7 +3230,7 @@ async def test_pr_merged_at_review_gate_sets_pr_merged(self, worker): "current_pr_number": 42, } - updated = await worker._handle_resume_event(message, current_state) + updated = await worker._apply_observation_transition(message, current_state) assert updated.get("pr_merged") is True @@ -3256,7 +3266,7 @@ async def test_dismissed_review_does_not_trigger_revision(self, worker): } with patch.object(worker, "_get_forge_github_login", AsyncMock(return_value="forge-bot")): - updated = await worker._handle_resume_event(message, current_state) + updated = await worker._apply_observation_transition(message, current_state) assert "human_review_status" not in updated assert updated.get("revision_requested") is not True diff --git a/tests/unit/orchestrator/test_worker_option_detection.py b/tests/unit/orchestrator/test_worker_option_detection.py index 1ff15448c..0b89b1310 100644 --- a/tests/unit/orchestrator/test_worker_option_detection.py +++ b/tests/unit/orchestrator/test_worker_option_detection.py @@ -1,4 +1,4 @@ -"""Tests for >option N detection in the orchestrator worker's _handle_resume_event.""" +"""Tests for >option N detection in the orchestrator worker's _apply_observation_transition.""" from unittest.mock import AsyncMock, patch @@ -57,7 +57,7 @@ async def test_option_comment_sets_selected_fix_option(self, worker): message = _make_option_message(">option 2") state = _make_rca_gate_state() - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result["selected_fix_option"] == 2 assert result["selected_fix_approach"] == state["rca_options"][1] @@ -69,7 +69,7 @@ async def test_option_detection_case_insensitive(self, worker): message = _make_option_message(">Option 2") state = _make_rca_gate_state() - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result["selected_fix_option"] == 2 @@ -79,7 +79,7 @@ async def test_option_detection_in_prose(self, worker): message = _make_option_message("let's go with >option 1 based on discussion") state = _make_rca_gate_state() - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result["selected_fix_option"] == 1 assert result["selected_fix_approach"] == state["rca_options"][0] @@ -90,7 +90,7 @@ async def test_first_match_wins(self, worker): message = _make_option_message(">option 1\n>option 2") state = _make_rca_gate_state() - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result["selected_fix_option"] == 1 @@ -99,7 +99,7 @@ async def test_out_of_range_option_posts_clarifying_comment(self, worker): """>option 5 when only 2 options → clarifying comment posted.""" message = _make_option_message(">option 5") state = _make_rca_gate_state() - await worker._handle_resume_event(message, state) + await worker._apply_observation_transition(message, state) command = worker.effect_service.execute_required.await_args.args[0] comment_text = command.payload["body"] @@ -110,7 +110,7 @@ async def test_out_of_range_option_does_not_update_state(self, worker): """>option 5 when only 2 options → selected_fix_option remains None.""" message = _make_option_message(">option 5") state = _make_rca_gate_state() - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result["selected_fix_option"] is None assert result is state # Should return current_state unchanged @@ -128,7 +128,7 @@ async def test_no_option_pattern_falls_through_to_revision(self, worker): patch("forge.orchestrator.worker.JiraClient", return_value=mock_jira), patch("forge.orchestrator.worker.post_status_comment", new_callable=AsyncMock), ): - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result["revision_requested"] is True assert result["selected_fix_option"] is None @@ -139,6 +139,6 @@ async def test_option_detection_only_at_rca_option_gate(self, worker): message = _make_option_message(">option 1") state = _make_rca_gate_state(current_node="prd_approval_gate") - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result.get("selected_fix_option") is None diff --git a/tests/unit/orchestrator/test_worker_prd_pr.py b/tests/unit/orchestrator/test_worker_prd_pr.py index b05ba72ac..c3e46c10c 100644 --- a/tests/unit/orchestrator/test_worker_prd_pr.py +++ b/tests/unit/orchestrator/test_worker_prd_pr.py @@ -21,6 +21,7 @@ from forge.models.events import EventSource from forge.orchestrator.worker import OrchestratorWorker from forge.queue.models import QueueMessage, normalized_event_to_dict +from forge.workflow.transitions import is_proposal_pull_request_event from forge.workflow.utils.automated_review_triage import AutomatedReviewDecision from forge.workflow.utils.source_control import identity_for @@ -53,7 +54,7 @@ def _normalized_from_payload(event_type: str, payload: dict) -> NormalizedEvent: """Build the NormalizedEvent a GitHub webhook payload would produce. Mirrors GitHubAdapter.parse_webhook for the event types exercised here so the - typed detection in _handle_resume_event runs against realistic data while the + typed detection in _apply_observation_transition runs against realistic data while the raw payload is still carried for the triage blocks that read it. """ base_type = event_type.split(":", 1)[0] @@ -242,7 +243,7 @@ def worker(): class TestIsPrdPrEvent: - def test_true_for_matching_repo_and_pr(self, worker): + def test_true_for_matching_repo_and_pr(self): msg = _make_message( "pull_request_review:submitted", { @@ -251,9 +252,10 @@ def test_true_for_matching_repo_and_pr(self, worker): }, ) state = _prd_gate_state() - assert worker._is_prd_pr_event(msg, state) is True + event = _normalized_from_payload(msg.event_type, msg.payload) + assert is_proposal_pull_request_event(msg, state, event, artifact="prd") is True - def test_false_for_wrong_repo(self, worker): + def test_false_for_wrong_repo(self): msg = _make_message( "pull_request_review:submitted", { @@ -262,9 +264,10 @@ def test_false_for_wrong_repo(self, worker): }, ) state = _prd_gate_state() - assert worker._is_prd_pr_event(msg, state) is False + event = _normalized_from_payload(msg.event_type, msg.payload) + assert is_proposal_pull_request_event(msg, state, event, artifact="prd") is False - def test_false_for_wrong_pr_number(self, worker): + def test_false_for_wrong_pr_number(self): msg = _make_message( "pull_request_review:submitted", { @@ -273,9 +276,10 @@ def test_false_for_wrong_pr_number(self, worker): }, ) state = _prd_gate_state() - assert worker._is_prd_pr_event(msg, state) is False + event = _normalized_from_payload(msg.event_type, msg.payload) + assert is_proposal_pull_request_event(msg, state, event, artifact="prd") is False - def test_false_when_no_prd_pr_in_state(self, worker): + def test_false_when_no_prd_pr_in_state(self): msg = _make_message( "pull_request_review:submitted", { @@ -284,9 +288,10 @@ def test_false_when_no_prd_pr_in_state(self, worker): }, ) state = _prd_gate_state(prd_pr_number=None, prd_pr_repo=None) - assert worker._is_prd_pr_event(msg, state) is False + event = _normalized_from_payload(msg.event_type, msg.payload) + assert is_proposal_pull_request_event(msg, state, event, artifact="prd") is False - def test_false_for_jira_events(self, worker): + def test_false_for_jira_events(self): msg = QueueMessage( message_id="msg-1", event_id="evt-1", @@ -296,9 +301,9 @@ def test_false_for_jira_events(self, worker): payload={}, ) state = _prd_gate_state() - assert worker._is_prd_pr_event(msg, state) is False + assert is_proposal_pull_request_event(msg, state, None, artifact="prd") is False - def test_matches_issue_comment_with_issue_number(self, worker): + def test_matches_issue_comment_with_issue_number(self): msg = _make_message( "issue_comment:created", { @@ -307,7 +312,8 @@ def test_matches_issue_comment_with_issue_number(self, worker): }, ) state = _prd_gate_state() - assert worker._is_prd_pr_event(msg, state) is True + event = _normalized_from_payload(msg.event_type, msg.payload) + assert is_proposal_pull_request_event(msg, state, event, artifact="prd") is True class TestHandlePrdPrMerge: @@ -332,7 +338,7 @@ async def test_pr_merge_sets_approved(self, worker): mock_jira.close = AsyncMock() MockJira.return_value = mock_jira - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) assert result["is_paused"] is False assert result["automated_review_revision_count"] == 0 @@ -355,7 +361,7 @@ async def test_pr_close_without_merge_is_ignored(self, worker): ) state = _prd_gate_state() - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) # Should remain paused -- closed without merge is not approval assert result.get("is_paused", True) is True @@ -383,7 +389,7 @@ async def test_changes_requested_sets_feedback(self, worker): mock_adapter.get_review_thread_comments.return_value = [] with _patch_adapter(repo_ref, mock_adapter): - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) assert result["is_paused"] is False assert result["revision_requested"] is True @@ -404,7 +410,7 @@ async def test_approved_review_is_ignored(self, worker): ) state = _prd_gate_state() - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) # Should remain paused -- review approval is not an approval signal assert result.get("is_paused", True) is True @@ -478,7 +484,7 @@ async def test_mixed_threads_revise_accepts_and_reply_to_contested(self, worker) new=AsyncMock(), ) as reply_decisions, ): - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) assert result["revision_requested"] is True assert result["feedback_comment"] == "Clarify authorization." @@ -509,7 +515,7 @@ async def test_comment_sets_feedback(self, worker): mock_adapter = AsyncMock() mock_adapter.get_authenticated_identity.return_value = Actor(login="forge-bot", is_bot=True) with _patch_adapter(_repo_ref_for("org/proposals"), mock_adapter): - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) assert result["is_paused"] is False assert result["revision_requested"] is True @@ -536,7 +542,7 @@ async def test_self_comment_is_ignored(self, worker): mock_adapter = AsyncMock() mock_adapter.get_authenticated_identity.return_value = Actor(login="forge-bot", is_bot=True) with _patch_adapter(_repo_ref_for("org/proposals"), mock_adapter): - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) # Should remain paused -- self-comment ignored assert result.get("is_paused", True) is True @@ -564,7 +570,7 @@ async def test_self_comment_with_signature_is_ignored(self, worker): _patch_adapter(_repo_ref_for("org/proposals"), mock_adapter), patch("forge.orchestrator.worker.get_settings", return_value=settings), ): - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) # Should remain paused -- self-comment with signature ignored assert result.get("is_paused", True) is True @@ -592,7 +598,7 @@ async def test_own_comment_without_signature_is_not_ignored(self, worker): _patch_adapter(_repo_ref_for("org/proposals"), mock_adapter), patch("forge.orchestrator.worker.get_settings", return_value=settings), ): - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) # Should be processed and no longer paused assert result.get("is_paused") is False @@ -616,7 +622,7 @@ async def test_question_comment_sets_question_flag(self, worker): mock_adapter = AsyncMock() mock_adapter.get_authenticated_identity.return_value = Actor(login="forge-bot", is_bot=True) with _patch_adapter(_repo_ref_for("org/proposals"), mock_adapter): - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) assert result["is_paused"] is False assert result.get("is_question") is True @@ -662,7 +668,7 @@ async def test_inline_reply_resumes_only_matching_proposal_thread(self, worker): mock_adapter = AsyncMock() mock_adapter.get_authenticated_identity.return_value = Actor(login="forge-bot", is_bot=True) with _patch_adapter(_repo_ref_for("org/proposals"), mock_adapter): - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) assert result["revision_requested"] is True assert result["feedback_comment"] == "Please make this change after all." @@ -705,7 +711,7 @@ async def test_unknown_proposal_reply_target_is_ignored(self, worker, caplog): "_get_forge_github_login", new=AsyncMock(return_value="forge-bot"), ): - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) assert result == state assert "Proposal reply target 999 did not match" in caplog.text @@ -746,7 +752,7 @@ async def test_standalone_inline_proposal_comment_is_triaged(self, worker): new=AsyncMock(return_value=[decision]), ) as triage, ): - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) assert result["revision_requested"] is True assert result["feedback_comment"] == "Clarify the authorization behavior." @@ -779,7 +785,7 @@ async def test_satisfied_bot_review_stays_paused(self, worker): ), ) as triage, ): - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) assert result == state triage.assert_awaited_once() @@ -812,7 +818,7 @@ async def test_blocking_bot_review_requests_bounded_revision(self, worker): ), ), ): - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) assert result["revision_requested"] is True assert result["feedback_comment"] == "Add the missing authorization requirement." @@ -846,7 +852,7 @@ async def test_uncertain_bot_review_revises_with_original_feedback(self, worker) ), ), ): - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) assert result["revision_requested"] is True assert "The result may still need changes" in result["feedback_comment"] @@ -879,7 +885,7 @@ async def test_bot_review_at_revision_cap_stays_paused(self, worker): ), ), ): - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) assert result == state @@ -904,7 +910,7 @@ async def test_jira_comment_ignored_when_prd_pr_exists(self, worker): ) state = _prd_gate_state() - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) # Should remain paused — Jira comment ignored in PR mode assert result.get("is_paused", True) is True @@ -930,7 +936,7 @@ async def test_jira_comment_processed_when_no_prd_pr(self, worker): # No prd_pr_number — Jira-only mode state = _prd_gate_state(prd_pr_number=None, prd_pr_repo=None) - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) # Should process the comment as feedback assert result["is_paused"] is False @@ -959,7 +965,7 @@ async def test_plain_comment_on_prd_pr_is_ignored(self, worker): mock_adapter = AsyncMock() mock_adapter.get_authenticated_identity.return_value = Actor(login="forge-bot", is_bot=True) with _patch_adapter(_repo_ref_for("org/proposals"), mock_adapter): - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) assert result.get("is_paused", True) is True assert result.get("revision_requested") is not True @@ -984,7 +990,7 @@ async def test_bot_informational_comment_on_prd_pr_is_ignored(self, worker): mock_adapter = AsyncMock() mock_adapter.get_authenticated_identity.return_value = Actor(login="forge-bot", is_bot=True) with _patch_adapter(_repo_ref_for("org/proposals"), mock_adapter): - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) assert result.get("is_paused", True) is True assert result.get("revision_requested") is not True @@ -1028,7 +1034,7 @@ async def test_human_review_bypasses_triage(self, worker): new=AsyncMock(), ) as triage, ): - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) assert result["revision_requested"] is True assert "Fix this section" in result["feedback_comment"] @@ -1066,7 +1072,7 @@ async def test_review_with_changes_requested_sets_feedback(self, worker): mock_adapter = AsyncMock() mock_adapter.get_review_thread_comments.return_value = [] with _patch_adapter(_repo_ref_for("acme/payments"), mock_adapter): - updated = await worker._handle_resume_event(message, current_state) + updated = await worker._apply_observation_transition(message, current_state) assert updated["revision_requested"] is True assert "please fix X" in updated["feedback_comment"] @@ -1094,7 +1100,7 @@ async def test_pr_merged_sets_approved(self, worker): with patch("forge.orchestrator.worker.JiraClient") as MockJira: MockJira.return_value.set_workflow_label = AsyncMock() MockJira.return_value.close = AsyncMock() - updated = await worker._handle_resume_event(message, current_state) + updated = await worker._apply_observation_transition(message, current_state) assert updated["is_paused"] is False @@ -1165,7 +1171,7 @@ async def test_reply_matching_stored_decision_updates_and_unpauses(self, worker) with patch.object( worker, "_get_forge_github_login", new=AsyncMock(return_value="forge-bot") ): - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result["is_paused"] is False assert result["revision_requested"] is True @@ -1194,7 +1200,7 @@ async def test_standalone_reply_builds_thread_and_sets_rejection(self, worker): with patch.object( worker, "_get_forge_github_login", new=AsyncMock(return_value="forge-bot") ): - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result["is_paused"] is False assert result["revision_requested"] is True @@ -1222,6 +1228,6 @@ async def test_self_reply_is_ignored(self, worker): with patch.object( worker, "_get_forge_github_login", new=AsyncMock(return_value="forge-bot") ): - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result == state diff --git a/tests/unit/orchestrator/test_worker_spec_pr.py b/tests/unit/orchestrator/test_worker_spec_pr.py index 342226fff..93c702af3 100644 --- a/tests/unit/orchestrator/test_worker_spec_pr.py +++ b/tests/unit/orchestrator/test_worker_spec_pr.py @@ -52,7 +52,7 @@ def _normalized_from_payload(event_type: str, payload: dict) -> NormalizedEvent: """Build the NormalizedEvent a GitHub webhook payload would produce. Mirrors GitHubAdapter.parse_webhook for the event types exercised here so the - typed detection in _handle_resume_event runs against realistic data while the + typed detection in _apply_observation_transition runs against realistic data while the raw payload is still carried for the triage blocks that read it. """ base_type = event_type.split(":", 1)[0] @@ -270,7 +270,7 @@ async def test_pr_merge_uses_configured_custom_field_storage(self, worker): mock_jira.close = AsyncMock() MockJira.return_value = mock_jira - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) assert result["is_paused"] is False commands = [call.args[0] for call in worker.effect_service.execute_required.await_args_list] @@ -310,7 +310,7 @@ async def test_satisfied_bot_spec_review_stays_paused(worker): new=AsyncMock(return_value=AutomatedReviewDecision("satisfied")), ) as triage, ): - result = await worker._handle_resume_event(msg, state) + result = await worker._apply_observation_transition(msg, state) assert result == state triage.assert_awaited_once() @@ -347,7 +347,7 @@ async def test_review_with_changes_requested_sets_feedback(self, worker): mock_adapter = AsyncMock() mock_adapter.get_review_thread_comments.return_value = [] with _patch_adapter(_repo_ref_for("acme/payments"), mock_adapter): - updated = await worker._handle_resume_event(message, current_state) + updated = await worker._apply_observation_transition(message, current_state) assert updated["revision_requested"] is True assert "please fix X" in updated["feedback_comment"] @@ -375,6 +375,6 @@ async def test_pr_merged_sets_approved(self, worker): with patch("forge.orchestrator.worker.JiraClient") as MockJira: MockJira.return_value.set_workflow_label = AsyncMock() MockJira.return_value.close = AsyncMock() - updated = await worker._handle_resume_event(message, current_state) + updated = await worker._apply_observation_transition(message, current_state) assert updated["is_paused"] is False diff --git a/tests/unit/queue/test_normalized_event_transport.py b/tests/unit/queue/test_normalized_event_transport.py index 564b850eb..0538935f0 100644 --- a/tests/unit/queue/test_normalized_event_transport.py +++ b/tests/unit/queue/test_normalized_event_transport.py @@ -81,7 +81,7 @@ def test_queue_message_from_redis_maps_legacy_github_source(): "1-0", { "event_id": "evt-1", - "source": "github", + "source": "source_control", "event_type": "cr_opened", "ticket_key": "PROJ-1", "payload": "{}", diff --git a/tests/unit/workflow/nodes/test_implement_work.py b/tests/unit/workflow/nodes/test_implement_work.py index 367f06b12..1b752d065 100644 --- a/tests/unit/workflow/nodes/test_implement_work.py +++ b/tests/unit/workflow/nodes/test_implement_work.py @@ -1,17 +1,15 @@ """Tests for the generic task-first implementation node.""" +from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock, patch import pytest -from forge.workflow.implementation_input import ( - NoPendingImplementationWork, - ResolvedImplementationInput, -) from forge.workflow.nodes.implement_work import implement_work +from forge.workflow.stations.implementation_input import NoPendingImplementationWork -def resolved_task() -> ResolvedImplementationInput: +def resolved_task(): artifact = { "id": "jira:TASK-1:task", "kind": "task", @@ -20,18 +18,20 @@ def resolved_task() -> ResolvedImplementationInput: "repository": "acme/api", "digest": "sha256:task", } - return ResolvedImplementationInput( - work_unit={ - "id": "TASK-1", - "kind": "task", - "key": "TASK-1", - "repository": "acme/api", - "status": "pending", - "source_artifact_ids": [artifact["id"]], - }, - context_artifacts=(artifact,), - instructions=artifact["content"], - summary="Implement endpoint", + return SimpleNamespace( + output=SimpleNamespace( + work_unit={ + "id": "TASK-1", + "kind": "task", + "key": "TASK-1", + "repository": "acme/api", + "status": "pending", + "source_artifact_ids": [artifact["id"]], + }, + context_artifacts=(artifact,), + instructions=artifact["content"], + summary="Implement endpoint", + ), ) @@ -51,9 +51,21 @@ async def execute(state, *_args, **_kwargs): AsyncMock(return_value=("/tmp/ws", git)), ), patch( - "forge.workflow.nodes.implement_work.resolve_implementation_input", + "forge.workflow.nodes.implement_work.project_implementation_input", + AsyncMock(return_value=MagicMock()), + ), + patch( + "forge.workflow.nodes.implement_work.invoke_builtin_station", AsyncMock(return_value=resolved_task()), ), + patch( + "forge.workflow.nodes.implement_work.reduce_implementation_input", + return_value={ + "artifacts": [resolved_task().output.context_artifacts[0]], + "work_units": [resolved_task().output.work_unit], + "work_resolution": {"strategy": "task_first"}, + }, + ), patch( "forge.workflow.nodes.implement_work.fetch_and_inject_references", AsyncMock(side_effect=lambda _state, _jira, prompt: prompt), @@ -86,7 +98,7 @@ async def test_no_pending_work_routes_to_local_review() -> None: AsyncMock(return_value=("/tmp/ws", MagicMock())), ), patch( - "forge.workflow.nodes.implement_work.resolve_implementation_input", + "forge.workflow.nodes.implement_work.project_implementation_input", AsyncMock(side_effect=NoPendingImplementationWork("complete")), ), ): diff --git a/tests/unit/workflow/nodes/test_task_takeover_execution.py b/tests/unit/workflow/nodes/test_task_takeover_execution.py index ed7951e5e..2bc65de9c 100644 --- a/tests/unit/workflow/nodes/test_task_takeover_execution.py +++ b/tests/unit/workflow/nodes/test_task_takeover_execution.py @@ -27,6 +27,16 @@ def _make_state( "workspace_path": workspace_path, "current_repo": current_repo, "plan_content": plan_content, + "artifacts": [ + { + "id": "plan:TASK-123", + "kind": "plan", + "content": plan_content, + "digest": "sha256:plan", + "approved_digest": "sha256:plan", + "status": "approved", + } + ], "implemented_tasks": implemented_tasks or [], "context": {"branch_name": "forge/TASK-123", "guardrails": ""}, "fork_owner": "forge-bot", diff --git a/tests/unit/workflow/stations/test_task_routing.py b/tests/unit/workflow/stations/test_task_routing.py index c13fd5dc4..bd34f074f 100644 --- a/tests/unit/workflow/stations/test_task_routing.py +++ b/tests/unit/workflow/stations/test_task_routing.py @@ -61,6 +61,8 @@ def test_reducer_owns_legacy_topology_mapping() -> None: "station_history", "repos_to_process", "current_repo", + "current_repository", + "repositories", "repos_completed", "implemented_tasks", "current_node", diff --git a/tests/unit/workflow/test_builtin_definition_artifacts.py b/tests/unit/workflow/test_builtin_definition_artifacts.py index b39c91ec4..0deefc371 100644 --- a/tests/unit/workflow/test_builtin_definition_artifacts.py +++ b/tests/unit/workflow/test_builtin_definition_artifacts.py @@ -24,9 +24,9 @@ # A changed digest is an intentional process revision and must update the # checked-in artifact and this snapshot together. _DIGESTS = { - "feature": "7bdc1d113890d69f49f5dce4e93c3fec7f293de8042cccf4ea9b8ca18a87b385", - "bug": "bf688cf548423e0577ceb93b7fc37f7771e5cf52c2a15cff892d54f0cda260d9", - "task_takeover": "eccdcaea925fdd61f938e1101b31f4b3be9e8d0e06394272fb6bba7f414885a6", + "feature": "f2240bad6450f43ef0cf2787b8890c70e9963d1ae174203e1fb3dbae9d95741e", + "bug": "c738a6324e60671f570a08bee13e7b8733f05c03b3e39da9f37ff9e68257e400", + "task_takeover": "a2d9f6ae17ab534295ef14ede1e42beae007c3d57f7d9cab721d73d2ddb7c2a1", } diff --git a/tests/unit/workflow/test_checkpoint_migration.py b/tests/unit/workflow/test_checkpoint_migration.py new file mode 100644 index 000000000..899082370 --- /dev/null +++ b/tests/unit/workflow/test_checkpoint_migration.py @@ -0,0 +1,87 @@ +from datetime import UTC, datetime, timedelta + +from forge.workflow.checkpoint_migration import migrate_unpinned_checkpoint +from forge.workflow.declarative.builtins import builtin_feature_definition + +NOW = datetime(2026, 8, 28, tzinfo=UTC) + + +def legacy_state(**updates): + state = { + "thread_id": "PROJ-1", + "workflow_name": "feature", + "current_node": "generate_prd", + "prd_content": "# Requirements", + "current_repo": "acme/api", + "repos_to_process": ["acme/api"], + "pull_requests": { + "acme/api": { + "repo": "acme/api", + "number": 42, + "url": "https://example.test/acme/api/pull/42", + } + }, + } + state.update(updates) + return state + + +def test_dry_run_reports_compatibility_without_mutating_checkpoint() -> None: + state = legacy_state() + + report = migrate_unpinned_checkpoint(state, builtin_feature_definition(), apply=False, now=NOW) + + assert report.compatible + assert not report.applied + assert report.migrated_state is None + assert state["pull_requests"] == {"acme/api": state["pull_requests"]["acme/api"]} + + +def test_apply_normalizes_and_pins_checkpoint_with_rollback_deadline() -> None: + report = migrate_unpinned_checkpoint( + legacy_state(), + builtin_feature_definition(), + apply=True, + now=NOW, + rollback_window=timedelta(days=3), + ) + + assert report.applied + assert report.rollback_until == NOW + timedelta(days=3) + migrated = report.migrated_state + assert migrated is not None + assert migrated["workflow_pin_status"] == "phase8_migrated" + assert migrated["workflow_definition_digest"] == builtin_feature_definition().digest + assert migrated["current_repository"] == "acme/api" + assert "acme/api:42" in migrated["pull_requests"] + assert migrated["artifacts"][0]["kind"] == "prd" + assert migrated["capabilities"]["repositories_resolved"] is True + assert migrated["workflow_rollback_until"] == "2026-08-31T00:00:00+00:00" + + +def test_incompatible_position_is_rejected_without_migration() -> None: + report = migrate_unpinned_checkpoint( + legacy_state(current_node="deleted_node"), + builtin_feature_definition(), + apply=True, + now=NOW, + ) + + assert not report.compatible + assert not report.applied + assert report.migrated_state is None + assert report.reasons == ( + "current position 'deleted_node' does not exist in target definition", + ) + + +def test_already_pinned_checkpoint_is_never_rewritten() -> None: + report = migrate_unpinned_checkpoint( + legacy_state(workflow_definition_revision=1), + builtin_feature_definition(), + apply=True, + now=NOW, + ) + + assert not report.compatible + assert report.reasons == ("checkpoint is already definition-pinned",) diff --git a/tests/unit/workflow/test_ci_gate_skip.py b/tests/unit/workflow/test_ci_gate_skip.py index 76e524be2..298a1c794 100644 --- a/tests/unit/workflow/test_ci_gate_skip.py +++ b/tests/unit/workflow/test_ci_gate_skip.py @@ -149,7 +149,7 @@ async def test_skip_gate_adds_check_to_skipped_list(self, worker, base_message, msg = _skip_gate_message(base_message, "epoxy") with patch.object(worker, "_post_skip_gate_feedback", AsyncMock()): - result = await worker._handle_resume_event(msg, ci_state) + result = await worker._apply_observation_transition(msg, ci_state) assert "epoxy" in result.get("ci_skipped_checks", []) @@ -159,7 +159,7 @@ async def test_skip_gate_routes_to_ci_evaluator(self, worker, base_message, ci_s msg = _skip_gate_message(base_message, "epoxy") with patch.object(worker, "_post_skip_gate_feedback", AsyncMock()): - result = await worker._handle_resume_event(msg, ci_state) + result = await worker._apply_observation_transition(msg, ci_state) assert result["is_paused"] is False assert result["current_node"] == "ci_evaluator" @@ -173,7 +173,7 @@ async def test_unskip_gate_removes_check_from_skipped_list( msg = _unskip_gate_message(base_message, "epoxy") with patch.object(worker, "_post_skip_gate_feedback", AsyncMock()): - result = await worker._handle_resume_event(msg, ci_state) + result = await worker._apply_observation_transition(msg, ci_state) skipped = result.get("ci_skipped_checks", []) assert "epoxy" not in skipped @@ -186,7 +186,7 @@ async def test_skip_gate_deduplicates(self, worker, base_message, ci_state): msg = _skip_gate_message(base_message, "epoxy") with patch.object(worker, "_post_skip_gate_feedback", AsyncMock()): - result = await worker._handle_resume_event(msg, ci_state) + result = await worker._apply_observation_transition(msg, ci_state) assert result["ci_skipped_checks"].count("epoxy") == 1 @@ -199,7 +199,7 @@ async def test_skip_gate_ignored_outside_ci_stages(self, worker, base_message): ) msg = _skip_gate_message(base_message, "epoxy") - result = await worker._handle_resume_event(msg, planning_state) + result = await worker._apply_observation_transition(msg, planning_state) assert result.get("ci_skipped_checks", []) == [] assert result.get("is_paused") is True # unchanged @@ -211,7 +211,7 @@ async def test_skip_gate_posts_feedback(self, worker, base_message, ci_state): mock_feedback = AsyncMock() with patch.object(worker, "_post_skip_gate_feedback", mock_feedback): - await worker._handle_resume_event(msg, ci_state) + await worker._apply_observation_transition(msg, ci_state) mock_feedback.assert_called_once() @@ -221,7 +221,7 @@ async def test_case_insensitive_command_detection(self, worker, base_message, ci msg = _comment_message(base_message, "/FORGE SKIP-GATE epoxy") with patch.object(worker, "_post_skip_gate_feedback", AsyncMock()): - result = await worker._handle_resume_event(msg, ci_state) + result = await worker._apply_observation_transition(msg, ci_state) assert "epoxy" in result.get("ci_skipped_checks", []) diff --git a/tests/unit/workflow/test_declarative_workflows.py b/tests/unit/workflow/test_declarative_workflows.py index 5e49c939c..6bc4ca06e 100644 --- a/tests/unit/workflow/test_declarative_workflows.py +++ b/tests/unit/workflow/test_declarative_workflows.py @@ -86,6 +86,34 @@ def test_every_supported_golden_path_uses_the_versioned_definition_compiler() -> ) +def test_builtin_golden_paths_select_the_governed_observation_policy() -> None: + for definition in builtin_definitions(): + workflow = DeclarativeWorkflow(definition, "BUILTIN") + + assert definition.spec.observation_policy == "post-pr-v1" + assert workflow.observation_policy == "post-pr-v1" + assert workflow.resolve_observation_policy() == "post-pr-v1" + + +def test_unknown_observation_policy_is_rejected() -> None: + value = definition_value() + value["spec"]["observationPolicy"] = "unknown-v1" + definition = load_workflow_value(value) + + with pytest.raises(WorkflowValidationError, match="unknown observation policy"): + DeclarativeWorkflowCompiler(definition).validate() + + +def test_observation_policy_cannot_target_an_undeclared_node() -> None: + value = definition_value(steps={"ci_evaluator": {"next": "__end__"}}) + value["spec"]["observationPolicy"] = "post-pr-v1" + value["spec"]["entry"] = "ci_evaluator" + definition = load_workflow_value(value) + + with pytest.raises(WorkflowValidationError, match="targets undeclared node 'attempt_ci_fix'"): + DeclarativeWorkflowCompiler(definition).validate() + + def test_default_router_has_no_python_topology_workflow_runtime() -> None: router = create_default_router() diff --git a/tests/unit/workflow/test_definition_pinning.py b/tests/unit/workflow/test_definition_pinning.py index e52e1efbd..683c55bbe 100644 --- a/tests/unit/workflow/test_definition_pinning.py +++ b/tests/unit/workflow/test_definition_pinning.py @@ -5,6 +5,7 @@ import pytest from forge.orchestrator.worker import OrchestratorWorker +from forge.workflow.checkpoint_migration import migrate_unpinned_checkpoint from forge.workflow.declarative.builtins import builtin_feature_definition from forge.workflow.declarative.loader import load_workflow_value from forge.workflow.declarative.models import WorkflowMetadata @@ -77,11 +78,17 @@ def test_pinned_state_rejects_digest_mismatch() -> None: workflow.validate_pinned_state(state) -def test_legacy_state_requires_explicit_compatibility_default() -> None: +def test_legacy_state_requires_explicit_checkpoint_migration() -> None: workflow = DeclarativeWorkflow(load_workflow_value(definition()), "PROJ") legacy = {"workflow_name": "pinned", "current_node": "generate_prd"} assert workflow.pin_status(legacy) == "legacy_unpinned" - pinned = workflow.pin_legacy_state(legacy) - assert pinned["workflow_pin_status"] == "legacy_active_default" + dry_run = migrate_unpinned_checkpoint(legacy, workflow.definition, apply=False) + assert dry_run.compatible + assert not dry_run.applied + assert dry_run.migrated_state is None + + pinned = migrate_unpinned_checkpoint(legacy, workflow.definition, apply=True).migrated_state + assert pinned is not None + assert pinned["workflow_pin_status"] == "phase8_migrated" assert pinned["workflow_digest"] == workflow.definition.digest diff --git a/tests/unit/workflow/test_implementation_input.py b/tests/unit/workflow/test_implementation_input.py deleted file mode 100644 index 04de252ec..000000000 --- a/tests/unit/workflow/test_implementation_input.py +++ /dev/null @@ -1,342 +0,0 @@ -from unittest.mock import AsyncMock - -import pytest - -from forge.integrations.jira.models import JiraIssue -from forge.workflow.implementation_input import ( - NoPendingImplementationWork, - resolve_implementation_input, -) - - -def issue(key: str, description: str, *, labels: list[str], issue_type: str = "Task") -> JiraIssue: - return JiraIssue( - key=key, - id=key, - summary=f"Summary {key}", - description=description, - status="Open", - issue_type=issue_type, - labels=labels, - ) - - -def jira_with(*issues: JiraIssue) -> AsyncMock: - by_key = {item.key: item for item in issues} - jira = AsyncMock() - jira.get_issue.side_effect = by_key.__getitem__ - return jira - - -@pytest.mark.asyncio -async def test_current_task_wins_and_all_lower_artifacts_are_context(): - jira = jira_with( - issue("TASK-2", "specific task", labels=["repo:acme/api"]), - issue("EPIC-1", "repository plan", labels=["repo:acme/api"], issue_type="Epic"), - issue("FEAT-1", "root request", labels=["repo:acme/api"], issue_type="Feature"), - ) - result = await resolve_implementation_input( - { - "ticket_key": "FEAT-1", - "current_repo": "acme/api", - "current_task_key": "TASK-2", - "tasks_by_repo": {"acme/api": ["TASK-2"]}, - "epic_keys": ["EPIC-1"], - "plan_content": "general plan", - "spec_content": "spec", - "rca_content": "rca", - "prd_content": "prd", - }, - jira, - ) - - assert result.work_unit["kind"] == "task" - assert result.work_unit["key"] == "TASK-2" - assert [item["kind"] for item in result.context_artifacts] == [ - "task", - "epic_plan", - "plan", - "spec", - "rca", - "prd", - "ticket", - ] - assert result.context_artifacts[0]["digest"].startswith("sha256:") - assert result.state_update()["current_work_unit_id"] == "TASK-2" - - -@pytest.mark.asyncio -async def test_first_pending_repository_task_wins_deterministically(): - jira = jira_with( - issue("TASK-2", "second pending", labels=["repo:acme/api"]), - issue("ROOT-1", "root", labels=["repo:acme/api"]), - ) - result = await resolve_implementation_input( - { - "ticket_key": "ROOT-1", - "current_repo": "acme/api", - "tasks_by_repo": {"acme/api": ["TASK-1", "TASK-2"]}, - "implemented_tasks": ["TASK-1"], - }, - jira, - ) - assert result.work_unit["key"] == "TASK-2" - - -@pytest.mark.asyncio -async def test_completed_normalized_work_unit_advances_to_next_repository_task(): - jira = jira_with( - issue("TASK-2", "second pending", labels=["repo:acme/api"]), - issue("ROOT-1", "root", labels=["repo:acme/api"]), - ) - result = await resolve_implementation_input( - { - "ticket_key": "ROOT-1", - "current_repo": "acme/api", - "tasks_by_repo": {"acme/api": ["TASK-1", "TASK-2"]}, - "work_units": [{"id": "TASK-1", "status": "completed"}], - }, - jira, - ) - - assert result.work_unit["key"] == "TASK-2" - - -@pytest.mark.asyncio -async def test_state_update_preserves_resolution_history(): - jira = jira_with( - issue("TASK-2", "pending", labels=["repo:acme/api"]), - issue("ROOT-1", "root", labels=["repo:acme/api"]), - ) - state = { - "ticket_key": "ROOT-1", - "current_repo": "acme/api", - "tasks_by_repo": {"acme/api": ["TASK-1", "TASK-2"]}, - "work_units": [{"id": "TASK-1", "status": "completed"}], - "artifacts": [{"id": "jira:TASK-1:task", "kind": "task"}], - } - result = await resolve_implementation_input(state, jira) - update = result.state_update(state) - - assert [unit["id"] for unit in update["work_units"]] == ["TASK-1", "TASK-2"] - assert [artifact["id"] for artifact in update["artifacts"]] == [ - "jira:TASK-1:task", - "jira:TASK-2:task", - "jira:ROOT-1:ticket", - ] - - -@pytest.mark.asyncio -async def test_task_takeover_root_is_the_primary_work_unit(): - jira = jira_with(issue("TASK-9", "", labels=["repo:acme/api"])) - result = await resolve_implementation_input( - { - "ticket_key": "TASK-9", - "ticket_type": "Task", - "current_repo": "acme/api", - "plan_content": "approved plan", - }, - jira, - ) - - assert result.work_unit["kind"] == "task" - assert result.work_unit["key"] == "TASK-9" - assert result.instructions == "Summary TASK-9" - assert [artifact["kind"] for artifact in result.context_artifacts] == ["task", "plan"] - - -@pytest.mark.asyncio -async def test_only_repository_matching_epic_is_eligible(): - jira = jira_with( - issue("EPIC-WEB", "web plan", labels=["repo:acme/web"], issue_type="Epic"), - issue("EPIC-API", "api plan", labels=["repo:acme/api"], issue_type="Epic"), - issue("ROOT-1", "root", labels=["repo:acme/api"]), - ) - result = await resolve_implementation_input( - { - "ticket_key": "ROOT-1", - "current_repo": "acme/api", - "epic_keys": ["EPIC-WEB", "EPIC-API"], - }, - jira, - ) - assert result.work_unit["kind"] == "epic_plan" - assert result.work_unit["key"] == "EPIC-API" - assert [a["source"] for a in result.context_artifacts] == ["EPIC-API", "ROOT-1"] - - -@pytest.mark.asyncio -async def test_plan_falls_back_through_spec_rca_prd_and_ticket(): - jira = jira_with(issue("BUG-1", "root", labels=["repo:acme/api"], issue_type="Bug")) - result = await resolve_implementation_input( - { - "ticket_key": "BUG-1", - "current_repo": "acme/api", - "spec_content": "spec", - "rca_content": "rca", - "prd_content": "prd", - }, - jira, - ) - assert result.work_unit["kind"] == "spec" - assert [a["kind"] for a in result.context_artifacts] == ["spec", "rca", "prd", "ticket"] - - -@pytest.mark.asyncio -async def test_mismatched_current_task_mapping_fails_before_fetch(): - jira = AsyncMock() - with pytest.raises(ValueError, match="belongs to repository acme/web"): - await resolve_implementation_input( - { - "ticket_key": "ROOT-1", - "current_repo": "acme/api", - "current_task_key": "TASK-1", - "tasks_by_repo": {"acme/web": ["TASK-1"]}, - }, - jira, - ) - jira.get_issue.assert_not_awaited() - - -@pytest.mark.asyncio -async def test_missing_repository_or_artifacts_fails_fast(): - with pytest.raises(ValueError, match="current_repo"): - await resolve_implementation_input({"ticket_key": "ROOT-1"}, AsyncMock()) - - jira = jira_with(issue("ROOT-1", "", labels=["repo:acme/api"])) - with pytest.raises(ValueError, match="No implementation artifact"): - await resolve_implementation_input( - {"ticket_key": "ROOT-1", "current_repo": "acme/api"}, jira - ) - - -@pytest.mark.asyncio -async def test_completed_tasks_do_not_fall_back_to_coarser_artifact(): - with pytest.raises(NoPendingImplementationWork, match="All Jira tasks"): - await resolve_implementation_input( - { - "ticket_key": "ROOT-1", - "current_repo": "acme/api", - "tasks_by_repo": {"acme/api": ["TASK-1"]}, - "implemented_tasks": ["TASK-1"], - "spec_content": "must not run again", - }, - AsyncMock(), - ) - - -@pytest.mark.asyncio -async def test_completed_internal_work_unit_is_not_rerun(): - jira = jira_with(issue("ROOT-1", "root", labels=["repo:acme/api"])) - first = await resolve_implementation_input( - {"ticket_key": "ROOT-1", "current_repo": "acme/api", "spec_content": "spec"}, jira - ) - with pytest.raises(NoPendingImplementationWork, match="already complete"): - await resolve_implementation_input( - { - "ticket_key": "ROOT-1", - "current_repo": "acme/api", - "spec_content": "spec", - "work_units": [{**first.work_unit, "status": "completed"}], - }, - jira, - ) - - -@pytest.mark.asyncio -async def test_normalized_approved_plan_is_selected_and_ancestors_are_context(): - jira = jira_with(issue("ROOT-1", "root", labels=["repo:acme/api"])) - result = await resolve_implementation_input( - { - "ticket_key": "ROOT-1", - "current_repository": "acme/api", - "artifacts": [ - { - "id": "prd:1", - "kind": "prd", - "content": "requirements", - "digest": "sha256:prd", - "approved_digest": "sha256:prd", - "status": "approved", - }, - { - "id": "spec:1", - "kind": "spec", - "content": "design", - "digest": "sha256:spec", - "approved_digest": "sha256:spec", - "status": "approved", - }, - { - "id": "plan:1", - "kind": "plan", - "content": "implementation steps", - "digest": "sha256:plan", - "approved_digest": "sha256:plan", - "status": "approved", - }, - ], - }, - jira, - ) - - assert result.work_unit["kind"] == "plan" - assert result.work_unit["source_artifact_ids"] == ["plan:1"] - assert result.work_unit["context_artifact_ids"] == [ - "spec:1", - "prd:1", - "jira:ROOT-1:ticket", - ] - - -@pytest.mark.asyncio -async def test_unapproved_and_stale_artifacts_are_not_implementation_input(): - jira = jira_with(issue("ROOT-1", "root", labels=["repo:acme/api"])) - result = await resolve_implementation_input( - { - "ticket_key": "ROOT-1", - "current_repo": "acme/api", - "artifacts": [ - { - "id": "plan:1", - "kind": "plan", - "content": "changed plan", - "digest": "sha256:new", - "approved_digest": "sha256:old", - "status": "approved", - }, - { - "id": "spec:1", - "kind": "spec", - "content": "stale design", - "digest": "sha256:spec", - "approved_digest": "sha256:spec", - "status": "stale", - }, - ], - }, - jira, - ) - - assert result.work_unit["kind"] == "ticket" - - -@pytest.mark.asyncio -async def test_stale_task_blocks_broader_artifact_fallback(): - with pytest.raises(ValueError, match="Tasks derived from stale planning"): - await resolve_implementation_input( - { - "ticket_key": "ROOT-1", - "current_repo": "acme/api", - "plan_content": "must not be selected", - "work_units": [ - { - "id": "TASK-1", - "kind": "task", - "repository": "acme/api", - "status": "stale", - } - ], - }, - AsyncMock(), - ) diff --git a/tests/unit/workflow/test_planning_state.py b/tests/unit/workflow/test_planning_state.py index 2aeb384de..b24c24355 100644 --- a/tests/unit/workflow/test_planning_state.py +++ b/tests/unit/workflow/test_planning_state.py @@ -3,9 +3,7 @@ from forge.workflow.planning_state import ( apply_artifact_update, artifact_is_current, - legacy_artifacts, planning_artifacts, - repository_compatibility_update, ) @@ -33,23 +31,7 @@ def test_approval_is_bound_to_the_current_digest() -> None: assert artifact_is_current(approved) is True assert artifact_is_current(changed) is False - assert artifact_is_current({"id": "legacy", "kind": "spec"}) is True - - -def test_legacy_fields_are_adapted_into_digest_bound_lineage() -> None: - artifacts = legacy_artifacts( - { - "ticket_key": "FEAT-1", - "prd_content": "requirements", - "spec_content": "design", - "plan_content": "steps", - } - ) - - assert [item["kind"] for item in artifacts] == ["prd", "spec", "plan"] - assert artifacts[1]["parent_artifact_id"] == artifacts[0]["id"] - assert artifacts[2]["input_artifact_ids"] == [artifacts[1]["id"]] - assert artifacts[0]["approved_digest"] == artifacts[0]["digest"] + assert artifact_is_current({"id": "unversioned", "kind": "spec"}) is False def test_normalized_kind_prevents_duplicate_legacy_artifact() -> None: @@ -107,28 +89,3 @@ def test_parent_revision_stales_all_descendants_and_pending_work() -> None: } assert update["work_units"][0]["status"] == "stale" assert update["work_units"][1]["status"] == "completed" - - -def test_repository_compatibility_preserves_order_and_metadata() -> None: - update = repository_compatibility_update( - { - "current_repo": "acme/api", - "repos_to_process": ["acme/web"], - "tasks_by_repo": {"acme/worker": ["TASK-1"]}, - "repos_completed": ["acme/web"], - "repositories": [ - { - "name": "acme/api", - "source": "task_label", - "status": "active", - "work_unit_ids": ["TASK-2"], - } - ], - } - ) - - assert update["current_repository"] == "acme/api" - assert update["current_repo"] == "acme/api" - assert update["repos_to_process"] == ["acme/api", "acme/web", "acme/worker"] - assert update["repositories"][0]["source"] == "task_label" - assert update["repositories"][1]["status"] == "completed" diff --git a/tests/unit/workflow/test_pr_state.py b/tests/unit/workflow/test_pr_state.py index 0aab124ef..62efc5ef6 100644 --- a/tests/unit/workflow/test_pr_state.py +++ b/tests/unit/workflow/test_pr_state.py @@ -265,7 +265,9 @@ def test_save_with_unknown_number_keys_by_url() -> None: ) assert "acme/docs:https://github.com/acme/docs/pull/30" in saved["pull_requests"] - assert saved["pull_requests"]["acme/docs:https://github.com/acme/docs/pull/30"]["number"] is None + assert ( + saved["pull_requests"]["acme/docs:https://github.com/acme/docs/pull/30"]["number"] is None + ) def test_save_without_number_or_url_is_noop() -> None: @@ -357,38 +359,28 @@ def _legacy_state() -> dict: } -def test_event_targets_pull_request_matches_legacy_bare_repo_key() -> None: +def test_event_does_not_match_unmigrated_bare_repo_key() -> None: state = _legacy_state() - event = _event( - repo="acme/legacy", native_id=99, url="https://github.com/acme/legacy/pull/99" - ) + event = _event(repo="acme/legacy", native_id=99, url="https://github.com/acme/legacy/pull/99") - assert event_targets_pull_request(state, event) + assert not event_targets_pull_request(state, event) -def test_activate_pull_request_for_event_hydrates_from_legacy_bare_repo_key() -> None: +def test_activate_requires_migrated_pull_request_key() -> None: state = _legacy_state() - event = _event( - repo="acme/legacy", native_id=99, url="https://github.com/acme/legacy/pull/99" - ) + event = _event(repo="acme/legacy", native_id=99, url="https://github.com/acme/legacy/pull/99") activated = activate_pull_request_for_event(state, event) - assert activated["current_repo"] == "acme/legacy" - assert activated["current_pr_number"] == 99 - assert activated["ci_status"] == "pending" + assert activated == state -def test_save_migrates_legacy_bare_repo_key_to_numbered_key() -> None: +def test_save_does_not_implicitly_migrate_bare_repo_key() -> None: state = _legacy_state() - event = _event( - repo="acme/legacy", native_id=99, url="https://github.com/acme/legacy/pull/99" - ) + event = _event(repo="acme/legacy", native_id=99, url="https://github.com/acme/legacy/pull/99") activated = activate_pull_request_for_event(state, event) activated["ci_status"] = "passed" saved = save_active_pull_request(activated) - assert "acme/legacy" not in saved["pull_requests"] - assert saved["pull_requests"]["acme/legacy:99"]["ci_status"] == "passed" - assert saved["pull_requests"]["acme/legacy:99"]["lifecycle_node"] == "ci_evaluator" + assert saved["pull_requests"] == state["pull_requests"] diff --git a/tests/unit/workflow/test_preconditions.py b/tests/unit/workflow/test_preconditions.py index 26db3f71f..71672363f 100644 --- a/tests/unit/workflow/test_preconditions.py +++ b/tests/unit/workflow/test_preconditions.py @@ -9,6 +9,7 @@ Requirement, evaluate_preconditions, has_capability, + project_capabilities, with_preconditions, ) @@ -22,13 +23,22 @@ def test_explicit_capability_overrides_compatibility_inference() -> None: assert not has_capability(state, CapabilityName.WORKSPACE) -def test_builtin_predicates_infer_existing_state() -> None: - assert has_capability({"current_repo": "owner/repo"}, CapabilityName.REPOSITORIES) - assert has_capability({"pr_urls": ["https://example.test/pr/1"]}, CapabilityName.PULL_REQUEST) - assert not has_capability({"pr_urls": []}, CapabilityName.PULL_REQUEST) +def test_capabilities_must_be_projected_before_evaluation() -> None: + repository_state = {"current_repo": "owner/repo"} + pull_request_state = {"pr_urls": ["https://example.test/pr/1"]} + + assert not has_capability(repository_state, CapabilityName.REPOSITORIES) + assert has_capability( + {**repository_state, "capabilities": project_capabilities(repository_state)}, + CapabilityName.REPOSITORIES, + ) + assert has_capability( + {**pull_request_state, "capabilities": project_capabilities(pull_request_state)}, + CapabilityName.PULL_REQUEST, + ) -def test_repository_capability_is_inferred_from_jira_event_labels() -> None: +def test_repository_capability_is_explicitly_projected_from_jira_event_labels() -> None: state = { "context": { "payload": { @@ -41,6 +51,7 @@ def test_repository_capability_is_inferred_from_jira_event_labels() -> None: } } + state["capabilities"] = project_capabilities(state) assert has_capability(state, CapabilityName.REPOSITORIES) @@ -72,7 +83,9 @@ def node(state: dict) -> dict: return {**state, "called": True} contract = NodeContract(requires=(Requirement(CapabilityName.WORKSPACE),)) - result = await with_preconditions(node, contract)({"workspace_path": "/tmp/workspace"}) + state = {"workspace_path": "/tmp/workspace"} + state["capabilities"] = project_capabilities(state) + result = await with_preconditions(node, contract)(state) assert result["called"] is True assert "precondition_result" not in result diff --git a/tests/unit/workflow/test_yolo_mode.py b/tests/unit/workflow/test_yolo_mode.py index 86a5d21d7..349ff1669 100644 --- a/tests/unit/workflow/test_yolo_mode.py +++ b/tests/unit/workflow/test_yolo_mode.py @@ -42,6 +42,7 @@ def _make_worker(self): from unittest.mock import MagicMock from forge.orchestrator.worker import OrchestratorWorker + worker = OrchestratorWorker.__new__(OrchestratorWorker) worker.settings = MagicMock() worker.router = MagicMock() @@ -99,9 +100,12 @@ def test_yolo_mode_false_for_github_source(self): class TestYoloLabelAddedMidWorkflow: """When forge:yolo is added while paused at a gate, yolo_mode is set and workflow unpauses.""" - def _make_yolo_label_message(self, current_labels: str, previous_labels: str = "") -> "QueueMessage": + def _make_yolo_label_message( + self, current_labels: str, previous_labels: str = "" + ) -> "QueueMessage": from forge.models.events import EventSource from forge.queue.models import QueueMessage + return QueueMessage( message_id="1234567890-0", event_id="test-event-yolo", @@ -139,26 +143,28 @@ def _make_gate_state(self, current_node: str, **extra) -> dict: @pytest.mark.asyncio async def test_yolo_label_addition_at_prd_gate_activates_yolo(self): from forge.orchestrator.worker import OrchestratorWorker + worker = OrchestratorWorker(consumer_name="test-worker") message = self._make_yolo_label_message( current_labels="forge:managed forge:yolo", previous_labels="forge:managed", ) state = self._make_gate_state("prd_approval_gate") - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) assert result["yolo_mode"] is True assert result["is_paused"] is False @pytest.mark.asyncio async def test_yolo_label_addition_outside_gate_does_not_activate(self): from forge.orchestrator.worker import OrchestratorWorker + worker = OrchestratorWorker(consumer_name="test-worker") message = self._make_yolo_label_message( current_labels="forge:managed forge:yolo", previous_labels="forge:managed", ) state = self._make_gate_state("generate_spec") - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) # Not at a gate — is_yolo flag should not fire; workflow must stay paused assert result.get("yolo_mode") is False assert result.get("is_paused") is True @@ -166,6 +172,7 @@ async def test_yolo_label_addition_outside_gate_does_not_activate(self): @pytest.mark.asyncio async def test_yolo_label_already_present_does_not_re_trigger(self): from forge.orchestrator.worker import OrchestratorWorker + worker = OrchestratorWorker(consumer_name="test-worker") # forge:yolo was already in fromString — not a new addition message = self._make_yolo_label_message( @@ -173,7 +180,7 @@ async def test_yolo_label_already_present_does_not_re_trigger(self): previous_labels="forge:yolo forge:prd-pending", ) state = self._make_gate_state("prd_approval_gate", yolo_mode=True) - result = await worker._handle_resume_event(message, state) + result = await worker._apply_observation_transition(message, state) # forge:yolo was already present — is_yolo should not re-trigger # yolo_mode stays True (copied from state), is_paused is False (prd-approved fired) assert result["yolo_mode"] is True # preserved from input state @@ -184,6 +191,7 @@ class TestYoloGateRouting: def _feature_state(self, current_node: str, **extra) -> dict: from forge.workflow.feature.state import create_initial_feature_state + state = create_initial_feature_state("TEST-1") state["current_node"] = current_node state["is_paused"] = True @@ -193,21 +201,25 @@ def _feature_state(self, current_node: str, **extra) -> dict: def test_prd_route_auto_approves_in_yolo_mode(self): from forge.workflow.gates.prd_approval import route_prd_approval + state = self._feature_state("prd_approval_gate", prd_content="# PRD") assert route_prd_approval(state) == "generate_spec" def test_spec_route_auto_approves_in_yolo_mode(self): from forge.workflow.gates.spec_approval import route_spec_approval + state = self._feature_state("spec_approval_gate", spec_content="# Spec") assert route_spec_approval(state) == "decompose_epics" def test_plan_route_auto_approves_in_yolo_mode(self): from forge.workflow.gates.plan_approval import route_plan_approval + state = self._feature_state("plan_approval_gate", epic_keys=["EPIC-1"]) assert route_plan_approval(state) == "generate_tasks" def test_task_route_auto_approves_in_yolo_mode(self): from forge.workflow.gates.task_approval import route_task_approval + state = self._feature_state("task_approval_gate", task_keys=["TASK-1"]) assert route_task_approval(state) == "task_router" @@ -216,6 +228,7 @@ def test_yolo_false_still_pauses_at_prd_gate(self): from forge.workflow.feature.state import create_initial_feature_state from forge.workflow.gates.prd_approval import route_prd_approval + state = create_initial_feature_state("TEST-1") state["current_node"] = "prd_approval_gate" state["is_paused"] = True @@ -225,6 +238,7 @@ def test_yolo_false_still_pauses_at_prd_gate(self): def test_yolo_does_not_override_question_routing(self): from forge.workflow.gates.prd_approval import route_prd_approval + state = self._feature_state("prd_approval_gate", prd_content="# PRD") state["is_question"] = True state["feedback_comment"] = "?Why REST?"