diff --git a/.env.example b/.env.example index 7a5243459..25c672d45 100644 --- a/.env.example +++ b/.env.example @@ -270,6 +270,10 @@ WORKER_METRICS_ENABLED=true # API server uses port 8000, worker uses 8001 for metrics WORKER_METRICS_PORT=8001 +# Enables authenticated durable-effect inspection and replay endpoints. +# Leave unset to keep /api/v1/effects disabled. +# EFFECT_OPERATOR_TOKEN=replace-with-a-long-random-secret + # ============================================================================= # Application Configuration # ============================================================================= diff --git a/docs/architecture/internals.md b/docs/architecture/internals.md index f349d88df..cae45a502 100644 --- a/docs/architecture/internals.md +++ b/docs/architecture/internals.md @@ -18,7 +18,10 @@ Gateway and Worker communicate only through Redis and can be deployed on separat **Idempotency:** A `DeduplicationService` exists but is not yet wired into the webhook routes. Branch creation and label operations are naturally idempotent; Jira comment posting is not. -**Consistency caveat:** Checkpoint writes and external side effects (Jira comments, GitHub PRs) are not transactional. A crash between a side effect and its checkpoint write can cause duplicate actions on retry. +**Consistency boundary:** Workflow mutations are persisted as stable effect intents before +provider execution. Provider-specific recovery evidence and idempotent ref updates close +the crash window between provider success and checkpoint acknowledgement. Workflow state +and effect state remain separate durable records, joined by the workflow run identity. ## Failure and Recovery diff --git a/docs/architecture/option-b-completion-plan.md b/docs/architecture/option-b-completion-plan.md index 826c9dc64..0aa650c5e 100644 --- a/docs/architecture/option-b-completion-plan.md +++ b/docs/architecture/option-b-completion-plan.md @@ -54,7 +54,7 @@ Exit gate: all new control-plane boundaries can be expressed with Phase 1 contra ## Phase 2 — Event interpretation and authoritative commands -PR: #325. Status: partial. +PR: #325. Status: complete. Purpose: make provider events inputs to pure interpretation, not direct selectors of graph nodes. @@ -71,7 +71,7 @@ Exit gate: no provider event directly chooses a workflow node; every event produ ## Phase 3 — Durable effects -PR: #326. Status: partial. +PR: #326. Status: complete. Purpose: make every external mutation recoverable, idempotent, and observable across crashes. diff --git a/docs/architecture/phase-3-durable-effects-plan.md b/docs/architecture/phase-3-durable-effects-plan.md new file mode 100644 index 000000000..6cc1b3ffc --- /dev/null +++ b/docs/architecture/phase-3-durable-effects-plan.md @@ -0,0 +1,60 @@ +# Phase 3 implementation plan: durable external effects + +**Status:** Complete + +**Depends on:** Phase 1 effect contracts and Phase 2 event/command boundary + +**Goal:** Persist external intent before calling a provider, execute it through narrow +provider adapters, and record a durable result so recovery never requires rerunning an +agent station merely to repeat an external write. + +## Delivery slices + +1. **Journal and leasing.** Store `EffectCommand` records by stable idempotency key, + index them by workflow run, atomically claim due work, recover expired leases and + retain terminal results. +2. **Executor runtime.** Resolve provider-neutral operations through a registry, apply + bounded exponential retry, and turn exceptions into structured `EffectResult` + records. +3. **First end-to-end effect.** Route Jira resume acknowledgements through the journal. + Embed the idempotency key in the provider object so a crash after the provider write + but before the result write is recoverable without duplication. +4. **Effect migration.** Convert remaining direct Jira and source-control writes by + operation family, preserving their existing behavior and preconditions. +5. **Operational surface.** Add pending/retry/terminal metrics, administrative replay, + retention policy and workflow-level effect history to the operator API. + +## Correctness rules + +- A command is durable before an executor is called. +- One logical write has one stable idempotency key across retries and duplicate events. +- Provider calls either support native idempotency or leave searchable recovery evidence. +- Executors do not advance workflow position; reducers consume successful results. +- A retry resumes the effect, not the station that produced it. +- Terminal and precondition failures remain inspectable and are never silently replayed. + +## Completion evidence + +- Workflow Jira and source-control writes pass through provider-neutral durable ports; + architecture tests reject imports or registry access that bypass those ports. +- Repository ref pushes are journalled using the intended commit SHA. Recovery treats an + already-pushed ref as success and prevents an older pending effect from overwriting a + newer local commit. +- The production worker binds one Redis-backed effect service and pinned workflow + identity around every graph invocation. Local station execution uses the same ports + with an isolated in-memory conformance journal. +- Jira comments, labels, descriptions, fields, attachments, transitions, issue creation, + links, archival, error notices, source-control branches/files/change requests/comments, + and repository pushes have idempotent executors and stable identities. +- Required mutations fail closed before the workflow invocation can commit its next + checkpoint. Attempt history, retry state, replay count, provider references, and + terminal failures remain durable and inspectable. +- `GET /api/v1/effects/{idempotency_key}`, `GET + /api/v1/effects/workflow/{run_id}`, and `POST + /api/v1/effects/{idempotency_key}/replay` provide the operational surface. They are + disabled unless `EFFECT_OPERATOR_TOKEN` is configured and require its bearer token. +- Prometheus reports effect attempts, results by status, and operator replays. Terminal + retention is exposed by the journal/service API and does not delete pending work. +- Crash-window tests cover expired leases, duplicate submission, provider-success + recovery, stale push supersession, retry history, explicit replay, and terminal + retention. diff --git a/src/forge/api/routes/__init__.py b/src/forge/api/routes/__init__.py index 91f9d4588..2f606fa9b 100644 --- a/src/forge/api/routes/__init__.py +++ b/src/forge/api/routes/__init__.py @@ -7,7 +7,9 @@ __all__ = [ "github_router", + "effects_router", "health_router", "jira_router", "metrics_router", ] +from forge.api.routes.effects import router as effects_router diff --git a/src/forge/api/routes/effects.py b/src/forge/api/routes/effects.py new file mode 100644 index 000000000..61eab64da --- /dev/null +++ b/src/forge/api/routes/effects.py @@ -0,0 +1,68 @@ +"""Authenticated operational API for durable external effects.""" + +import secrets +from collections.abc import Sequence +from typing import Annotated + +from fastapi import APIRouter, Depends, Header, HTTPException, status + +from forge.config import get_settings +from forge.effects import EffectRecord, EffectService, create_default_effect_service + +router = APIRouter(prefix="/api/v1/effects", tags=["effects"]) + + +def get_effect_service() -> EffectService: + return create_default_effect_service() + + +def authorize_operator(authorization: str | None) -> None: + configured = get_settings().effect_operator_token + if configured is None: + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail="Effect operator API is disabled", + ) + scheme, _, supplied = (authorization or "").partition(" ") + if scheme.lower() != "bearer" or not secrets.compare_digest( + supplied, configured.get_secret_value() + ): + raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Unauthorized") + + +OperatorAuth = Annotated[str | None, Header(alias="Authorization")] +EffectServiceDep = Annotated[EffectService, Depends(get_effect_service)] + + +@router.get("/workflow/{run_id}", response_model=list[EffectRecord]) +async def list_workflow_effects( + run_id: str, service: EffectServiceDep, authorization: OperatorAuth = None +) -> Sequence[EffectRecord]: + authorize_operator(authorization) + return await service.journal.list_for_workflow(run_id) + + +@router.get("/{idempotency_key}", response_model=EffectRecord) +async def get_effect( + idempotency_key: str, service: EffectServiceDep, authorization: OperatorAuth = None +) -> EffectRecord: + authorize_operator(authorization) + record = await service.journal.get(idempotency_key) + if record is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Effect not found") + return record + + +@router.post("/{idempotency_key}/replay", response_model=EffectRecord) +async def replay_effect( + idempotency_key: str, service: EffectServiceDep, authorization: OperatorAuth = None +) -> EffectRecord: + authorize_operator(authorization) + try: + return await service.replay(idempotency_key) + except KeyError as exc: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="Effect not found" + ) from exc + except ValueError as exc: + raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(exc)) from exc diff --git a/src/forge/api/routes/metrics.py b/src/forge/api/routes/metrics.py index 742df1a34..24087eef7 100644 --- a/src/forge/api/routes/metrics.py +++ b/src/forge/api/routes/metrics.py @@ -148,6 +148,24 @@ buckets=[1, 5, 10, 30, 60, 120, 300, 600], # Same as AGENT_DURATION ) +EFFECT_ATTEMPTS = Counter( + "forge_effect_attempts_total", + "Durable external effect attempts", + ["operation"], +) + +EFFECT_RESULTS = Counter( + "forge_effect_results_total", + "Durable external effect results", + ["operation", "status"], +) + +EFFECT_REPLAYS = Counter( + "forge_effect_replays_total", + "Operator-requested durable effect replays", + ["operation"], +) + @router.get("/metrics") async def metrics() -> Response: @@ -228,6 +246,18 @@ def record_revision_requested(stage: str) -> None: REVISIONS_REQUESTED.labels(stage=stage).inc() +def record_effect_attempt(operation: str) -> None: + EFFECT_ATTEMPTS.labels(operation=operation).inc() + + +def record_effect_result(operation: str, status: str) -> None: + EFFECT_RESULTS.labels(operation=operation, status=status).inc() + + +def record_effect_replay(operation: str) -> None: + EFFECT_REPLAYS.labels(operation=operation).inc() + + def record_proposal_review_decision(artifact_type: str, disposition: str) -> None: """Record one semantic proposal-review thread decision.""" PROPOSAL_REVIEW_DECISIONS.labels( diff --git a/src/forge/config.py b/src/forge/config.py index 9c8d2895d..33519cc38 100644 --- a/src/forge/config.py +++ b/src/forge/config.py @@ -602,6 +602,13 @@ def ignored_ci_checks(self) -> list[str]: default=True, description="Enable Prometheus metrics endpoint in worker", ) + effect_operator_token: SecretStr | None = Field( + default=None, + description=( + "Bearer token for durable-effect inspection and replay endpoints. " + "The endpoints remain disabled when unset." + ), + ) # OpenTelemetry Configuration otlp_endpoint: str = Field( diff --git a/src/forge/effects/__init__.py b/src/forge/effects/__init__.py new file mode 100644 index 000000000..47ec29221 --- /dev/null +++ b/src/forge/effects/__init__.py @@ -0,0 +1,20 @@ +"""Durable execution boundary for external side effects.""" + +from forge.effects.defaults import create_default_effect_service +from forge.effects.executors import EffectExecutor, EffectExecutorRegistry +from forge.effects.journal import EffectJournal, InMemoryEffectJournal, RedisEffectJournal +from forge.effects.models import EffectRecord, EffectRecordStatus +from forge.effects.service import EffectService, RequiredEffectError + +__all__ = [ + "EffectExecutor", + "EffectExecutorRegistry", + "EffectJournal", + "EffectRecord", + "EffectRecordStatus", + "EffectService", + "RequiredEffectError", + "InMemoryEffectJournal", + "RedisEffectJournal", + "create_default_effect_service", +] diff --git a/src/forge/effects/defaults.py b/src/forge/effects/defaults.py new file mode 100644 index 000000000..01d05d060 --- /dev/null +++ b/src/forge/effects/defaults.py @@ -0,0 +1,16 @@ +"""Default durable effect runtime wiring.""" + +from forge.effects.executors import EffectExecutorRegistry +from forge.effects.jira import register_jira_executors +from forge.effects.journal import RedisEffectJournal +from forge.effects.repository import register_repository_executors +from forge.effects.service import EffectService +from forge.effects.source_control import register_source_control_executors + + +def create_default_effect_service() -> EffectService: + registry = EffectExecutorRegistry() + register_jira_executors(registry) + register_source_control_executors(registry) + register_repository_executors(registry) + return EffectService(RedisEffectJournal(), registry) diff --git a/src/forge/effects/executors.py b/src/forge/effects/executors.py new file mode 100644 index 000000000..7b7bf78c2 --- /dev/null +++ b/src/forge/effects/executors.py @@ -0,0 +1,29 @@ +"""Provider executor contracts and operation registry.""" + +from __future__ import annotations + +from typing import Protocol + +from forge.domain import EffectCommand, EffectResult + + +class EffectExecutor(Protocol): + operation: str + + async def execute(self, command: EffectCommand) -> EffectResult: ... + + +class EffectExecutorRegistry: + def __init__(self) -> None: + self._executors: dict[str, EffectExecutor] = {} + + def register(self, executor: EffectExecutor) -> None: + if executor.operation in self._executors: + raise ValueError(f"Executor already registered for {executor.operation}") + self._executors[executor.operation] = executor + + def resolve(self, operation: str) -> EffectExecutor: + try: + return self._executors[operation] + except KeyError as exc: + raise ValueError(f"No effect executor registered for {operation}") from exc diff --git a/src/forge/effects/jira.py b/src/forge/effects/jira.py new file mode 100644 index 000000000..ba47635dd --- /dev/null +++ b/src/forge/effects/jira.py @@ -0,0 +1,307 @@ +"""Jira effect executors.""" + +from __future__ import annotations + +from collections.abc import Callable +from datetime import UTC, datetime +from typing import Any + +from forge.domain import EffectCommand, EffectResult, EffectResultStatus +from forge.effects.executors import EffectExecutorRegistry +from forge.integrations.jira.client import JiraClient + +JIRA_COMMENT_OPERATION = "jira.comment.create" +JIRA_LABEL_OPERATION = "jira.label.set" +JIRA_DESCRIPTION_OPERATION = "jira.description.update" +JIRA_CUSTOM_FIELD_OPERATION = "jira.custom_field.update" +JIRA_ATTACHMENT_REPLACE_OPERATION = "jira.attachment.replace" +JIRA_ATTACHMENT_ADD_OPERATION = "jira.attachment.add" +JIRA_ATTACHMENT_DELETE_BY_NAME_OPERATION = "jira.attachment.delete_by_name" +JIRA_STRUCTURED_COMMENT_OPERATION = "jira.structured_comment.create" +JIRA_TRANSITION_OPERATION = "jira.issue.transition" +JIRA_LABELS_ADD_OPERATION = "jira.labels.add" +JIRA_LABELS_REMOVE_OPERATION = "jira.labels.remove" +JIRA_ARCHIVE_OPERATION = "jira.issue.archive" +JIRA_PROJECT_PROPERTY_SET_OPERATION = "jira.project_property.set" +JIRA_PROJECT_PROPERTY_DELETE_OPERATION = "jira.project_property.delete" +JIRA_TASK_CREATE_OPERATION = "jira.task.create" +JIRA_EPIC_CREATE_OPERATION = "jira.epic.create" +JIRA_ISSUE_LINK_CREATE_OPERATION = "jira.issue_link.create" +JIRA_REMOTE_LINK_CREATE_OPERATION = "jira.remote_link.create" +JIRA_ERROR_COMMENT_OPERATION = "jira.error_comment.create" +JIRA_MODEL_POLICY_ERROR_COMMENT_OPERATION = "jira.model_policy_error_comment.create" + + +class JiraCommentExecutor: + operation = JIRA_COMMENT_OPERATION + + def __init__(self, client_factory: Callable[[], JiraClient] = JiraClient) -> None: + self._client_factory = client_factory + + async def execute(self, command: EffectCommand) -> EffectResult: + issue_key = command.target.external_id + body = str(command.payload["body"]) + marker = f"forge-effect:{command.idempotency_key}" + rendered = f"{body}\n\n{{{marker}}}" + jira = self._client_factory() + try: + comments = await jira.get_comments(issue_key) + existing = next((comment for comment in comments if marker in comment.body), None) + if existing is None: + created = await jira.add_comment(issue_key, rendered) + provider_reference = str(created.id) + else: + provider_reference = str(existing.id) + return EffectResult( + effect_id=command.effect_id, + idempotency_key=command.idempotency_key, + status=EffectResultStatus.SUCCEEDED, + completed_at=datetime.now(UTC), + provider_reference=provider_reference, + ) + finally: + await jira.close() + + +class JiraMutationExecutor: + """Execute naturally idempotent Jira mutations from durable intent.""" + + def __init__( + self, + operation: str, + client_factory: Callable[[], JiraClient] = JiraClient, + ) -> None: + self.operation = operation + self._client_factory = client_factory + + async def execute(self, command: EffectCommand) -> EffectResult: + issue_key = command.target.external_id + jira = self._client_factory() + provider_reference: str | None = issue_key + output: dict[str, Any] = {} + try: + if self.operation == JIRA_LABEL_OPERATION: + await jira.set_workflow_label(issue_key, str(command.payload["label"])) + elif self.operation == JIRA_DESCRIPTION_OPERATION: + await jira.update_description(issue_key, str(command.payload["description"])) + elif self.operation == JIRA_CUSTOM_FIELD_OPERATION: + await jira.update_custom_field( + issue_key, + str(command.payload["field"]), + str(command.payload["value"]), + ) + elif self.operation == JIRA_ATTACHMENT_REPLACE_OPERATION: + filename = str(command.payload["filename"]) + await jira.delete_attachments_by_name(issue_key, filename) + replacement = await jira.add_attachment( + issue_key, + filename=filename, + content=str(command.payload["content"]), + content_type=str(command.payload.get("content_type", "text/plain")), + ) + provider_reference = _provider_id(replacement, filename) + elif self.operation == JIRA_ATTACHMENT_ADD_OPERATION: + attachment = await jira.add_attachment( + issue_key, + filename=str(command.payload["filename"]), + content=str(command.payload["content"]), + content_type=str(command.payload.get("content_type", "text/markdown")), + ) + provider_reference = _provider_id(attachment, str(command.payload["filename"])) + output = dict(attachment) if isinstance(attachment, dict) else {} + elif self.operation == JIRA_ATTACHMENT_DELETE_BY_NAME_OPERATION: + deleted = await jira.delete_attachments_by_name( + issue_key, str(command.payload["filename"]) + ) + output = {"deleted": deleted} + elif self.operation == JIRA_STRUCTURED_COMMENT_OPERATION: + marker = f"forge-effect:{command.idempotency_key}" + comments = await jira.get_comments(issue_key) + existing = next((comment for comment in comments if marker in comment.body), None) + if existing is None: + structured_comment = await jira.add_structured_comment( + issue_key, + str(command.payload["title"]), + f"{command.payload['content']}\n\n{{{marker}}}", + comment_type=str(command.payload["comment_type"]), + ) + provider_reference = str(structured_comment.id) + else: + provider_reference = str(existing.id) + elif self.operation == JIRA_TRANSITION_OPERATION: + transition = str(command.payload["transition"]) + issue = await jira.get_issue(issue_key) + if issue.status.lower() != transition.lower(): + await jira.transition_issue(issue_key, transition) + elif self.operation == JIRA_LABELS_ADD_OPERATION: + requested = _string_list(command.payload["labels"]) + current = set(await jira.get_labels(issue_key)) + missing = [label for label in requested if label not in current] + if missing: + await jira.add_labels(issue_key, missing) + elif self.operation == JIRA_LABELS_REMOVE_OPERATION: + requested = _string_list(command.payload["labels"]) + current = set(await jira.get_labels(issue_key)) + present = [label for label in requested if label in current] + if present: + await jira.remove_labels(issue_key, present) + elif self.operation == JIRA_ARCHIVE_OPERATION: + await jira.archive_issue( + issue_key, archive_subtasks=bool(command.payload.get("archive_subtasks", True)) + ) + elif self.operation == JIRA_PROJECT_PROPERTY_SET_OPERATION: + await jira.set_project_property( + issue_key, + str(command.payload["property_key"]), + command.payload["value"], + ) + elif self.operation == JIRA_PROJECT_PROPERTY_DELETE_OPERATION: + await jira.delete_project_property(issue_key, str(command.payload["property_key"])) + elif self.operation in {JIRA_TASK_CREATE_OPERATION, JIRA_EPIC_CREATE_OPERATION}: + marker = _creation_marker(command.idempotency_key) + existing_issues = await jira.search_issues( + f'project = "{command.payload["project_key"]}" AND labels = "{marker}"', + fields=["summary", "labels"], + max_results=2, + ) + if len(existing_issues) > 1: + raise RuntimeError(f"Creation marker {marker} resolves to multiple Jira issues") + if existing_issues: + provider_reference = existing_issues[0].key + else: + labels = _string_list(command.payload.get("labels", [])) + labels.append(marker) + if self.operation == JIRA_TASK_CREATE_OPERATION: + provider_reference = await jira.create_task( + str(command.payload["project_key"]), + str(command.payload["summary"]), + str(command.payload["description"]), + parent_key=_optional_string(command.payload.get("parent_key")), + labels=labels, + ) + else: + provider_reference = await jira.create_epic( + str(command.payload["project_key"]), + str(command.payload["summary"]), + str(command.payload["description"]), + str(command.payload["parent_key"]), + labels=labels, + ) + elif self.operation == JIRA_ISSUE_LINK_CREATE_OPERATION: + inward_key = str(command.payload["inward_key"]) + outward_key = str(command.payload["outward_key"]) + link_type = str(command.payload["link_type"]) + links = await jira.get_issue_links(inward_key) + exists = any( + str(link.get("type", "")).lower() == link_type.lower() + and { + str(link.get("inward_key") or ""), + str(link.get("outward_key") or ""), + } + == {inward_key, outward_key} + for link in links + ) + if not exists: + await jira.create_issue_link(link_type, inward_key, outward_key) + provider_reference = f"{inward_key}:{link_type}:{outward_key}" + elif self.operation == JIRA_REMOTE_LINK_CREATE_OPERATION: + url = str(command.payload["url"]) + title = str(command.payload["title"]) + remote_links = await jira.get_remote_links(issue_key) + if not any(link.get("url") == url for link in remote_links): + await jira.create_remote_link(issue_key, url, title) + provider_reference = url + elif self.operation == JIRA_ERROR_COMMENT_OPERATION: + marker = f"forge-effect:{command.idempotency_key}" + comments = await jira.get_comments(issue_key) + existing = next((comment for comment in comments if marker in comment.body), None) + if existing is None: + error_comment = await jira.add_error_comment( + issue_key, + f"{command.payload['error_message']}\n\n{{{marker}}}", + str(command.payload["node_name"]), + mention_account_ids=[ + *_string_list(command.payload.get("mention_account_ids", [])) + ], + ) + provider_reference = str(error_comment.id) + else: + provider_reference = str(existing.id) + elif self.operation == JIRA_MODEL_POLICY_ERROR_COMMENT_OPERATION: + marker = f"forge-effect:{command.idempotency_key}" + comments = await jira.get_comments(issue_key) + existing = next((comment for comment in comments if marker in comment.body), None) + if existing is None: + policy_comment = await jira.add_model_policy_error_comment( + issue_key, + str(command.payload["node_name"]), + f"{command.payload['problem']}\n\n{{{marker}}}", + str(command.payload["available_connections"]), + str(command.payload["fix_command"]), + mention_account_ids=[ + *_string_list(command.payload.get("mention_account_ids", [])) + ], + ) + provider_reference = str(policy_comment.id) + else: + provider_reference = str(existing.id) + else: # pragma: no cover - registry construction prevents this + raise ValueError(f"Unsupported Jira effect operation: {self.operation}") + return EffectResult( + effect_id=command.effect_id, + idempotency_key=command.idempotency_key, + status=EffectResultStatus.SUCCEEDED, + completed_at=datetime.now(UTC), + provider_reference=provider_reference, + output=output, + ) + finally: + await jira.close() + + +def register_jira_executors(registry: EffectExecutorRegistry) -> None: + registry.register(JiraCommentExecutor()) + for operation in ( + JIRA_LABEL_OPERATION, + JIRA_DESCRIPTION_OPERATION, + JIRA_CUSTOM_FIELD_OPERATION, + JIRA_ATTACHMENT_REPLACE_OPERATION, + JIRA_ATTACHMENT_ADD_OPERATION, + JIRA_ATTACHMENT_DELETE_BY_NAME_OPERATION, + JIRA_STRUCTURED_COMMENT_OPERATION, + JIRA_TRANSITION_OPERATION, + JIRA_LABELS_ADD_OPERATION, + JIRA_LABELS_REMOVE_OPERATION, + JIRA_ARCHIVE_OPERATION, + JIRA_PROJECT_PROPERTY_SET_OPERATION, + JIRA_PROJECT_PROPERTY_DELETE_OPERATION, + JIRA_TASK_CREATE_OPERATION, + JIRA_EPIC_CREATE_OPERATION, + JIRA_ISSUE_LINK_CREATE_OPERATION, + JIRA_REMOTE_LINK_CREATE_OPERATION, + JIRA_ERROR_COMMENT_OPERATION, + JIRA_MODEL_POLICY_ERROR_COMMENT_OPERATION, + ): + registry.register(JiraMutationExecutor(operation)) + + +def _creation_marker(idempotency_key: str) -> str: + """Return a Jira-label-safe recovery marker for create crash windows.""" + digest = idempotency_key.rsplit(":", 1)[-1] + return f"forge-effect-{digest[:40]}" + + +def _optional_string(value: object) -> str | None: + return str(value) if value is not None else None + + +def _string_list(value: object) -> list[str]: + if not isinstance(value, list): + raise ValueError("Expected a list") + return [str(item) for item in value] + + +def _provider_id(value: object, fallback: str) -> str: + if isinstance(value, dict): + return str(value.get("id") or fallback) + return str(getattr(value, "id", fallback)) diff --git a/src/forge/effects/journal.py b/src/forge/effects/journal.py new file mode 100644 index 000000000..821386997 --- /dev/null +++ b/src/forge/effects/journal.py @@ -0,0 +1,435 @@ +"""Durable effect journal implementations.""" + +from __future__ import annotations + +import asyncio +from collections.abc import Sequence +from datetime import UTC, datetime, timedelta +from typing import Any, Protocol + +from forge.domain import EffectCommand, EffectResult, EffectResultStatus +from forge.effects.models import EffectRecord, EffectRecordStatus +from forge.orchestrator.checkpointer import get_redis_client + +_RECORD_PREFIX = "forge:effects:record:" +_DUE_KEY = "forge:effects:due" +_WORKFLOW_PREFIX = "forge:effects:workflow:" + +_SUBMIT_SCRIPT = """ +if redis.call('EXISTS', KEYS[1]) == 1 then + return 0 +end +redis.call('SET', KEYS[1], ARGV[1]) +redis.call('ZADD', KEYS[2], ARGV[2], ARGV[3]) +redis.call('SADD', KEYS[3], ARGV[3]) +return 1 +""" + +_CLAIM_SCRIPT = """ +local members = redis.call('ZRANGEBYSCORE', KEYS[1], '-inf', ARGV[1], 'LIMIT', 0, ARGV[3]) +for _, member in ipairs(members) do + redis.call('ZADD', KEYS[1], ARGV[2], member) +end +return members +""" + +_CLAIM_ONE_SCRIPT = """ +local score = redis.call('ZSCORE', KEYS[1], ARGV[1]) +if not score or tonumber(score) > tonumber(ARGV[2]) then + return nil +end +redis.call('ZADD', KEYS[1], ARGV[3], ARGV[1]) +return ARGV[1] +""" + + +class EffectJournal(Protocol): + async def submit(self, command: EffectCommand) -> EffectRecord: ... + + async def get(self, idempotency_key: str) -> EffectRecord | None: ... + + async def list_for_workflow(self, run_id: str) -> Sequence[EffectRecord]: ... + + async def claim_due(self, limit: int = 10) -> Sequence[EffectRecord]: ... + + async def claim(self, idempotency_key: str) -> EffectRecord | None: ... + + async def complete(self, result: EffectResult) -> EffectRecord: ... + + async def retry(self, result: EffectResult, delay: timedelta) -> EffectRecord: ... + + async def replay(self, idempotency_key: str) -> EffectRecord: ... + + async def purge_terminal_before(self, cutoff: datetime) -> int: ... + + +def _now() -> datetime: + return datetime.now(UTC) + + +def _pending(command: EffectCommand, now: datetime) -> EffectRecord: + return EffectRecord( + command=command, + status=EffectRecordStatus.PENDING, + attempt=0, + created_at=now, + updated_at=now, + next_attempt_at=now, + ) + + +class InMemoryEffectJournal: + """Deterministic journal for local execution and contract tests.""" + + def __init__(self, *, lease: timedelta = timedelta(minutes=5)) -> None: + self._records: dict[str, EffectRecord] = {} + self._lock = asyncio.Lock() + self._lease = lease + + async def submit(self, command: EffectCommand) -> EffectRecord: + async with self._lock: + existing = self._records.get(command.idempotency_key) + if existing: + return existing + record = _pending(command, _now()) + self._records[command.idempotency_key] = record + return record + + async def get(self, idempotency_key: str) -> EffectRecord | None: + return self._records.get(idempotency_key) + + async def list_for_workflow(self, run_id: str) -> Sequence[EffectRecord]: + return [ + record for record in self._records.values() if record.command.workflow.run_id == run_id + ] + + async def claim_due(self, limit: int = 10) -> Sequence[EffectRecord]: + now = _now() + async with self._lock: + due = [ + record + for record in self._records.values() + if record.status + in { + EffectRecordStatus.PENDING, + EffectRecordStatus.RETRYABLE_FAILURE, + EffectRecordStatus.RUNNING, + } + and record.next_attempt_at <= now + and (record.lease_until is None or record.lease_until <= now) + ][:limit] + claimed = [] + for record in due: + updated = record.model_copy( + update={ + "status": EffectRecordStatus.RUNNING, + "attempt": record.attempt + 1, + "updated_at": now, + "lease_until": now + self._lease, + } + ) + self._records[record.command.idempotency_key] = updated + claimed.append(updated) + return claimed + + async def claim(self, idempotency_key: str) -> EffectRecord | None: + now = _now() + async with self._lock: + record = self._records.get(idempotency_key) + if ( + record is None + or record.status + not in { + EffectRecordStatus.PENDING, + EffectRecordStatus.RETRYABLE_FAILURE, + EffectRecordStatus.RUNNING, + } + or record.next_attempt_at > now + or (record.lease_until is not None and record.lease_until > now) + ): + return None + updated = record.model_copy( + update={ + "status": EffectRecordStatus.RUNNING, + "attempt": record.attempt + 1, + "updated_at": now, + "lease_until": now + self._lease, + } + ) + self._records[idempotency_key] = updated + return updated + + async def complete(self, result: EffectResult) -> EffectRecord: + return await self._store_result(result, delay=None) + + async def retry(self, result: EffectResult, delay: timedelta) -> EffectRecord: + return await self._store_result(result, delay=delay) + + async def _store_result(self, result: EffectResult, delay: timedelta | None) -> EffectRecord: + async with self._lock: + record = self._records[result.idempotency_key] + status = EffectRecordStatus(result.status.value) + updated = record.model_copy( + update={ + "status": status, + "updated_at": result.completed_at, + "next_attempt_at": result.completed_at + (delay or timedelta()), + "lease_until": None, + "result": result, + "attempt_history": [*record.attempt_history, result], + } + ) + self._records[result.idempotency_key] = updated + return updated + + async def replay(self, idempotency_key: str) -> EffectRecord: + async with self._lock: + record = self._records[idempotency_key] + if record.status not in { + EffectRecordStatus.PRECONDITION_FAILED, + EffectRecordStatus.TERMINAL_FAILURE, + }: + raise ValueError(f"Effect {idempotency_key} is not terminal") + now = _now() + updated = record.model_copy( + update={ + "status": EffectRecordStatus.PENDING, + "updated_at": now, + "next_attempt_at": now, + "lease_until": None, + "result": None, + "replay_count": record.replay_count + 1, + } + ) + self._records[idempotency_key] = updated + return updated + + async def purge_terminal_before(self, cutoff: datetime) -> int: + async with self._lock: + keys = [ + key + for key, record in self._records.items() + if record.status + in { + EffectRecordStatus.SUCCEEDED, + EffectRecordStatus.PRECONDITION_FAILED, + EffectRecordStatus.TERMINAL_FAILURE, + } + and record.updated_at < cutoff + ] + for key in keys: + del self._records[key] + return len(keys) + + +class RedisEffectJournal: + """Redis-backed journal with atomic submission and exclusive leases.""" + + def __init__( + self, + redis_client: Any = None, + *, + lease: timedelta = timedelta(minutes=5), + ) -> None: + self._redis = redis_client + self._lease = lease + + async def _client(self) -> Any: + if self._redis is None: + self._redis = await get_redis_client() + return self._redis + + async def submit(self, command: EffectCommand) -> EffectRecord: + redis = await self._client() + now = _now() + record = _pending(command, now) + key = f"{_RECORD_PREFIX}{command.idempotency_key}" + await redis.eval( + _SUBMIT_SCRIPT, + 3, + key, + _DUE_KEY, + f"{_WORKFLOW_PREFIX}{command.workflow.run_id}", + record.model_dump_json(), + now.timestamp(), + command.idempotency_key, + ) + stored = await self.get(command.idempotency_key) + assert stored is not None + return stored + + async def get(self, idempotency_key: str) -> EffectRecord | None: + redis = await self._client() + value = await redis.get(f"{_RECORD_PREFIX}{idempotency_key}") + return EffectRecord.model_validate_json(value) if value else None + + async def list_for_workflow(self, run_id: str) -> Sequence[EffectRecord]: + redis = await self._client() + members = await redis.smembers(f"{_WORKFLOW_PREFIX}{run_id}") + records = [] + for raw in members: + key = raw.decode() if isinstance(raw, bytes) else raw + record = await self.get(key) + if record is not None: + records.append(record) + return records + + async def claim_due(self, limit: int = 10) -> Sequence[EffectRecord]: + redis = await self._client() + now = _now() + lease_until = now + self._lease + members = await redis.eval( + _CLAIM_SCRIPT, + 1, + _DUE_KEY, + now.timestamp(), + lease_until.timestamp(), + limit, + ) + claimed = [] + for raw in members: + idempotency_key = raw.decode() if isinstance(raw, bytes) else raw + record = await self.get(idempotency_key) + if record is None: + await redis.zrem(_DUE_KEY, idempotency_key) + continue + if record.status not in { + EffectRecordStatus.PENDING, + EffectRecordStatus.RETRYABLE_FAILURE, + EffectRecordStatus.RUNNING, + }: + await redis.zrem(_DUE_KEY, idempotency_key) + continue + updated = record.model_copy( + update={ + "status": EffectRecordStatus.RUNNING, + "attempt": record.attempt + 1, + "updated_at": now, + "lease_until": lease_until, + "next_attempt_at": lease_until, + } + ) + await redis.set(f"{_RECORD_PREFIX}{idempotency_key}", updated.model_dump_json()) + claimed.append(updated) + return claimed + + async def claim(self, idempotency_key: str) -> EffectRecord | None: + redis = await self._client() + now = _now() + lease_until = now + self._lease + claimed = await redis.eval( + _CLAIM_ONE_SCRIPT, + 1, + _DUE_KEY, + idempotency_key, + now.timestamp(), + lease_until.timestamp(), + ) + if not claimed: + return None + record = await self.get(idempotency_key) + if record is None: + await redis.zrem(_DUE_KEY, idempotency_key) + return None + updated = record.model_copy( + update={ + "status": EffectRecordStatus.RUNNING, + "attempt": record.attempt + 1, + "updated_at": now, + "lease_until": lease_until, + "next_attempt_at": lease_until, + } + ) + await redis.set(f"{_RECORD_PREFIX}{idempotency_key}", updated.model_dump_json()) + return updated + + async def complete(self, result: EffectResult) -> EffectRecord: + return await self._store_result(result, delay=None) + + async def retry(self, result: EffectResult, delay: timedelta) -> EffectRecord: + return await self._store_result(result, delay=delay) + + async def _store_result(self, result: EffectResult, delay: timedelta | None) -> EffectRecord: + redis = await self._client() + record = await self.get(result.idempotency_key) + if record is None: + raise KeyError(result.idempotency_key) + next_attempt = result.completed_at + (delay or timedelta()) + updated = record.model_copy( + update={ + "status": EffectRecordStatus(result.status.value), + "updated_at": result.completed_at, + "next_attempt_at": next_attempt, + "lease_until": None, + "result": result, + "attempt_history": [*record.attempt_history, result], + } + ) + key = f"{_RECORD_PREFIX}{result.idempotency_key}" + pipeline = redis.pipeline(transaction=True) + pipeline.set(key, updated.model_dump_json()) + if result.status is EffectResultStatus.RETRYABLE_FAILURE: + pipeline.zadd(_DUE_KEY, {result.idempotency_key: next_attempt.timestamp()}) + else: + pipeline.zrem(_DUE_KEY, result.idempotency_key) + await pipeline.execute() + return updated + + async def replay(self, idempotency_key: str) -> EffectRecord: + redis = await self._client() + record = await self.get(idempotency_key) + if record is None: + raise KeyError(idempotency_key) + if record.status not in { + EffectRecordStatus.PRECONDITION_FAILED, + EffectRecordStatus.TERMINAL_FAILURE, + }: + raise ValueError(f"Effect {idempotency_key} is not terminal") + now = _now() + updated = record.model_copy( + update={ + "status": EffectRecordStatus.PENDING, + "updated_at": now, + "next_attempt_at": now, + "lease_until": None, + "result": None, + "replay_count": record.replay_count + 1, + } + ) + pipeline = redis.pipeline(transaction=True) + pipeline.set(f"{_RECORD_PREFIX}{idempotency_key}", updated.model_dump_json()) + pipeline.zadd(_DUE_KEY, {idempotency_key: now.timestamp()}) + await pipeline.execute() + return updated + + async def purge_terminal_before(self, cutoff: datetime) -> int: + redis = await self._client() + cursor: int | bytes = 0 + removed = 0 + while True: + cursor, keys = await redis.scan(cursor=cursor, match=f"{_RECORD_PREFIX}*", count=100) + for raw_key in keys: + key = raw_key.decode() if isinstance(raw_key, bytes) else raw_key + value = await redis.get(key) + if not value: + continue + record = EffectRecord.model_validate_json(value) + if ( + record.status + in { + EffectRecordStatus.SUCCEEDED, + EffectRecordStatus.PRECONDITION_FAILED, + EffectRecordStatus.TERMINAL_FAILURE, + } + and record.updated_at < cutoff + ): + identity = record.command.idempotency_key + pipeline = redis.pipeline(transaction=True) + pipeline.delete(key) + pipeline.zrem(_DUE_KEY, identity) + pipeline.srem(f"{_WORKFLOW_PREFIX}{record.command.workflow.run_id}", identity) + await pipeline.execute() + removed += 1 + if cursor in {0, b"0", "0"}: + break + return removed diff --git a/src/forge/effects/models.py b/src/forge/effects/models.py new file mode 100644 index 000000000..8936b89dd --- /dev/null +++ b/src/forge/effects/models.py @@ -0,0 +1,32 @@ +"""Persisted state of one external effect.""" + +from __future__ import annotations + +from datetime import datetime +from enum import StrEnum + +from pydantic import Field + +from forge.domain import DomainModel, EffectCommand, EffectResult + + +class EffectRecordStatus(StrEnum): + PENDING = "pending" + RUNNING = "running" + SUCCEEDED = "succeeded" + PRECONDITION_FAILED = "precondition_failed" + RETRYABLE_FAILURE = "retryable_failure" + TERMINAL_FAILURE = "terminal_failure" + + +class EffectRecord(DomainModel): + command: EffectCommand + status: EffectRecordStatus + attempt: int = Field(ge=0) + created_at: datetime + updated_at: datetime + next_attempt_at: datetime + lease_until: datetime | None = None + result: EffectResult | None = None + attempt_history: list[EffectResult] = Field(default_factory=list) + replay_count: int = Field(default=0, ge=0) diff --git a/src/forge/effects/rendering.py b/src/forge/effects/rendering.py new file mode 100644 index 000000000..c1c4a8b7b --- /dev/null +++ b/src/forge/effects/rendering.py @@ -0,0 +1,38 @@ +"""Provider-neutral rendering used by workflow effects.""" + +import re + +_EMOJI_PREFIX_RE = re.compile(r"^\s*(?:[\u2600-\u27BF\U0001F300-\U0001FAFF]|\u2139)") + + +def format_status_comment(message: str) -> str: + """Ensure a workflow status comment starts with a matching emoji.""" + if _EMOJI_PREFIX_RE.match(message): + return message + normalized = message.lower() + emoji = "ℹ️" + if any(word in normalized for word in ("fail", "error", "conflict", "cannot", "missing")): + emoji = "⚠️" + elif any(word in normalized for word in ("complete", "success", "approved", "merged")): + emoji = "✅" + elif "prd" in normalized: + emoji = "📝" + elif "spec" in normalized or "specification" in normalized: + emoji = "📋" + elif "plan" in normalized: + emoji = "🧭" + elif "task" in normalized or "implement" in normalized: + emoji = "⚙️" + elif "pull request" in normalized or " pr " in f" {normalized} ": + emoji = "🔀" + elif " ci " in f" {normalized} ": + emoji = "🧪" + elif "review" in normalized: + emoji = "👀" + elif "question" in normalized or "q&a" in normalized: + emoji = "❓" + elif "triage" in normalized or "checking" in normalized: + emoji = "🔎" + elif "rca" in normalized or "root cause" in normalized or "analysis" in normalized: + emoji = "🔍" + return f"{emoji} {message}" diff --git a/src/forge/effects/repository.py b/src/forge/effects/repository.py new file mode 100644 index 000000000..8aaa97b14 --- /dev/null +++ b/src/forge/effects/repository.py @@ -0,0 +1,66 @@ +"""Durable effects for externally visible repository mutations.""" + +from collections.abc import Callable +from datetime import UTC, datetime +from pathlib import Path +from typing import Any + +from forge.domain import EffectCommand, EffectResult, EffectResultStatus +from forge.effects.executors import EffectExecutorRegistry +from forge.integrations.source_control.registry import Registry, get_registry +from forge.workspace.git_ops import GitOperations +from forge.workspace.manager import Workspace + +REPOSITORY_PUSH_OPERATION = "repository.ref.push" + + +class RepositoryPushExecutor: + operation = REPOSITORY_PUSH_OPERATION + + def __init__(self, registry_factory: Callable[[], Registry] = get_registry) -> None: + self._registry_factory = registry_factory + + async def execute(self, command: EffectCommand) -> EffectResult: + payload: dict[str, Any] = command.payload + resolved = self._registry_factory().resolve(str(payload["repository"])) + if resolved.adapter is None: + raise RuntimeError(f"No adapter registered for {resolved.repo_ref.provider}") + workspace = Workspace( + path=Path(str(payload["workspace_path"])), + repo_name=str(payload["repository"]), + branch_name=str(payload["branch"]), + ticket_key=str(payload["ticket_key"]), + ) + credentials = await resolved.adapter.get_git_credentials(resolved.repo_ref) + git = GitOperations(workspace, credentials) + remote = "fork" if bool(payload.get("use_fork")) else "origin" + expected_sha = str(payload["commit_sha"]) + current_sha = git.get_current_sha() + if current_sha != expected_sha: + return EffectResult( + effect_id=command.effect_id, + idempotency_key=command.idempotency_key, + status=EffectResultStatus.SUCCEEDED, + completed_at=datetime.now(UTC), + provider_reference=f"superseded-by:{current_sha}", + output={"superseded_by": current_sha}, + ) + if git.get_remote_branch_sha(workspace.branch_name, remote=remote) != expected_sha: + if remote == "fork": + git.push_to_fork(force=bool(payload.get("force"))) + else: + git.push( + force=bool(payload.get("force")), + check_conflicts=bool(payload.get("check_conflicts", True)), + ) + return EffectResult( + effect_id=command.effect_id, + idempotency_key=command.idempotency_key, + status=EffectResultStatus.SUCCEEDED, + completed_at=datetime.now(UTC), + provider_reference=f"{remote}:{workspace.branch_name}@{expected_sha}", + ) + + +def register_repository_executors(registry: EffectExecutorRegistry) -> None: + registry.register(RepositoryPushExecutor()) diff --git a/src/forge/effects/service.py b/src/forge/effects/service.py new file mode 100644 index 000000000..fd40dfe02 --- /dev/null +++ b/src/forge/effects/service.py @@ -0,0 +1,144 @@ +"""Effect submission, execution and recovery service.""" + +from __future__ import annotations + +import asyncio +import contextlib +import logging +from datetime import UTC, datetime, timedelta + +from forge.api.routes.metrics import ( + record_effect_attempt, + record_effect_replay, + record_effect_result, +) +from forge.domain import EffectCommand, EffectResult, EffectResultStatus +from forge.effects.executors import EffectExecutorRegistry +from forge.effects.journal import EffectJournal +from forge.effects.models import EffectRecord, EffectRecordStatus +from forge.integrations.source_control.errors import ConflictError, TransientProviderError +from forge.utils.redaction import redact_secrets + +logger = logging.getLogger(__name__) + + +class RequiredEffectError(RuntimeError): + def __init__(self, record: EffectRecord) -> None: + super().__init__(f"Required effect {record.command.effect_id} is {record.status.value}") + self.record = record + + +class EffectService: + def __init__( + self, + journal: EffectJournal, + executors: EffectExecutorRegistry, + *, + max_attempts: int = 3, + base_retry_delay: timedelta = timedelta(seconds=30), + ) -> None: + self.journal = journal + self.executors = executors + self.max_attempts = max_attempts + self.base_retry_delay = base_retry_delay + + async def submit(self, command: EffectCommand) -> EffectRecord: + """Persist intent before any provider call; duplicates return the first record.""" + return await self.journal.submit(command) + + async def execute_now(self, command: EffectCommand) -> EffectRecord: + """Persist and exclusively execute one workflow-critical effect.""" + submitted = await self.journal.submit(command) + if submitted.result is not None and submitted.status.value not in { + "pending", + "running", + "retryable_failure", + }: + return submitted + claimed = await self.journal.claim(command.idempotency_key) + if claimed is None: + current = await self.journal.get(command.idempotency_key) + if current is None: # pragma: no cover - journal contract violation + raise RuntimeError("submitted effect disappeared from journal") + return current + return await self._execute(claimed) + + async def execute_required(self, command: EffectCommand) -> EffectRecord: + """Execute now and fail closed until the durable effect succeeds.""" + record = await self.execute_now(command) + if record.status is not EffectRecordStatus.SUCCEEDED: + raise RequiredEffectError(record) + return record + + async def run_due(self, limit: int = 10) -> list[EffectRecord]: + completed = [] + for record in await self.journal.claim_due(limit): + completed.append(await self._execute(record)) + return completed + + async def _execute(self, record: EffectRecord) -> EffectRecord: + command = record.command + record_effect_attempt(command.operation) + try: + executor = self.executors.resolve(command.operation) + result = await executor.execute(command) + except Exception as exc: + status = self._failure_status(exc, record.attempt) + result = EffectResult( + effect_id=command.effect_id, + idempotency_key=command.idempotency_key, + status=status, + completed_at=datetime.now(UTC), + error_code=type(exc).__name__, + error_message=redact_secrets(str(exc))[:1000], + ) + + if ( + result.status is EffectResultStatus.RETRYABLE_FAILURE + and record.attempt < self.max_attempts + ): + delay = self.base_retry_delay * (2 ** (record.attempt - 1)) + retried = await self.journal.retry(result, delay) + record_effect_result(command.operation, retried.status.value) + return retried + if result.status is EffectResultStatus.RETRYABLE_FAILURE: + result = result.model_copy(update={"status": EffectResultStatus.TERMINAL_FAILURE}) + completed = await self.journal.complete(result) + record_effect_result(command.operation, completed.status.value) + return completed + + def _failure_status(self, exc: Exception, attempt: int) -> EffectResultStatus: + if isinstance(exc, (ConflictError, ValueError, KeyError)): + return EffectResultStatus.PRECONDITION_FAILED + if isinstance(exc, TransientProviderError) and attempt < self.max_attempts: + return EffectResultStatus.RETRYABLE_FAILURE + return ( + EffectResultStatus.TERMINAL_FAILURE + if attempt >= self.max_attempts + else EffectResultStatus.RETRYABLE_FAILURE + ) + + async def replay(self, idempotency_key: str) -> EffectRecord: + """Explicitly reschedule a terminal effect while retaining its history.""" + replayed = await self.journal.replay(idempotency_key) + record_effect_replay(replayed.command.operation) + return replayed + + async def purge_terminal_before(self, cutoff: datetime) -> int: + """Apply the operator-selected terminal-record retention cutoff.""" + return await self.journal.purge_terminal_before(cutoff) + + async def run_forever( + self, + stop: asyncio.Event, + *, + interval: float = 5.0, + limit: int = 10, + ) -> None: + while not stop.is_set(): + try: + await self.run_due(limit) + except Exception: + logger.exception("Durable effect sweep failed; retrying on the next interval") + with contextlib.suppress(TimeoutError): + await asyncio.wait_for(stop.wait(), timeout=interval) diff --git a/src/forge/effects/source_control.py b/src/forge/effects/source_control.py new file mode 100644 index 000000000..32b98d6c6 --- /dev/null +++ b/src/forge/effects/source_control.py @@ -0,0 +1,168 @@ +"""Provider-neutral source-control effect executors.""" + +from __future__ import annotations + +from collections.abc import Callable +from dataclasses import replace +from datetime import UTC, datetime +from typing import Any, cast + +from forge.domain import EffectCommand, EffectResult, EffectResultStatus +from forge.effects.executors import EffectExecutorRegistry +from forge.integrations.source_control.contracts import ( + ChangeRequestIdentity, + ChangeRequestState, + ResolvedRepository, + WriteTarget, +) +from forge.integrations.source_control.errors import NotFoundError +from forge.integrations.source_control.registry import Registry, get_registry + +SC_BRANCH_CREATE_OPERATION = "source_control.branch.create" +SC_FILE_PUT_OPERATION = "source_control.file.put" +SC_CHANGE_REQUEST_CREATE_OPERATION = "source_control.change_request.create" +SC_CHANGE_REQUEST_UPDATE_OPERATION = "source_control.change_request.update" +SC_COMMENT_CREATE_OPERATION = "source_control.comment.create" +SC_COMMENT_REPLY_OPERATION = "source_control.comment.reply" + + +class SourceControlMutationExecutor: + """Execute a source-control mutation after resolving its registered repository.""" + + def __init__( + self, + operation: str, + registry_factory: Callable[[], Registry] = get_registry, + ) -> None: + self.operation = operation + self._registry_factory = registry_factory + + async def execute(self, command: EffectCommand) -> EffectResult: + resolved = self._registry_factory().resolve( + str( + command.payload.get("_repository_id") + or command.target.namespace + or command.target.external_id + ) + ) + target_namespace = command.payload.get("_target_namespace") + if target_namespace: + resolved = replace( + resolved, + repo_ref=replace( + resolved.repo_ref, + id=str(target_namespace), + namespace=str(target_namespace), + ), + ) + adapter = resolved.adapter + if adapter is None: + raise RuntimeError(f"No adapter registered for {resolved.repo_ref.provider}") + + reference: str | None = command.target.external_id + output: dict[str, Any] = {} + if self.operation == SC_BRANCH_CREATE_OPERATION: + await adapter.create_branch( + resolved.repo_ref, + str(command.payload["name"]), + str(command.payload["base"]), + ) + reference = str(command.payload["name"]) + elif self.operation == SC_FILE_PUT_OPERATION: + path = str(command.payload["path"]) + content = str(command.payload["content"]) + branch = str(command.payload["branch"]) + try: + current_content = await adapter.get_file(resolved.repo_ref, path, branch) + except NotFoundError: + current_content = None + if current_content != content: + await adapter.put_file( + resolved.repo_ref, + path, + content, + str(command.payload["message"]), + branch, + ) + reference = f"{command.payload['branch']}:{command.payload['path']}" + elif self.operation == SC_CHANGE_REQUEST_CREATE_OPERATION: + target = WriteTarget(**cast(dict[str, Any], command.payload["target"])) + change = await adapter.create_change_request( + resolved.repo_ref, + target, + str(command.payload["title"]), + str(command.payload["body"]), + bool(command.payload.get("draft", False)), + ) + reference = str(change.identity.native_id) + output = {"url": change.url, "number": reference, "created": change.created} + else: + identity = _identity(resolved, command) + if self.operation == SC_CHANGE_REQUEST_UPDATE_OPERATION: + state_value = command.payload.get("state") + change = await adapter.update_change_request( + resolved.repo_ref, + identity, + title=_optional_string(command.payload.get("title")), + body=_optional_string(command.payload.get("body")), + state=ChangeRequestState(str(state_value)) if state_value else None, + ) + reference = str(change.identity.native_id) + output = {"url": change.url, "number": reference} + elif self.operation in {SC_COMMENT_CREATE_OPERATION, SC_COMMENT_REPLY_OPERATION}: + marker = f"forge-effect:{command.idempotency_key}" + if self.operation == SC_COMMENT_REPLY_OPERATION: + threads = await adapter.get_review_thread_comments(resolved.repo_ref, identity) + comments = [comment for thread in threads for comment in thread.comments] + else: + comments = await adapter.get_change_request_comments( + resolved.repo_ref, identity + ) + existing = next((item for item in comments if marker in item.body), None) + if existing is None: + body = f"{command.payload['body']}\n\n" + if self.operation == SC_COMMENT_CREATE_OPERATION: + existing = await adapter.create_comment(resolved.repo_ref, identity, body) + else: + existing = await adapter.reply_to_comment( + resolved.repo_ref, + identity, + str(command.payload["comment_id"]), + body, + ) + reference = existing.id + else: # pragma: no cover - registry construction prevents this + raise ValueError(f"Unsupported source-control effect operation: {self.operation}") + + return EffectResult( + effect_id=command.effect_id, + idempotency_key=command.idempotency_key, + status=EffectResultStatus.SUCCEEDED, + completed_at=datetime.now(UTC), + provider_reference=reference, + output=output, + ) + + +def _identity(resolved: ResolvedRepository, command: EffectCommand) -> ChangeRequestIdentity: + return ChangeRequestIdentity( + connection=resolved.repo_ref.connection, + repository_id=resolved.repo_ref.id, + native_id=command.target.external_id, + ) + + +def _optional_string(value: object) -> str | None: + return str(value) if value is not None else None + + +def register_source_control_executors(registry: EffectExecutorRegistry) -> None: + for operation in ( + SC_BRANCH_CREATE_OPERATION, + SC_FILE_PUT_OPERATION, + SC_CHANGE_REQUEST_CREATE_OPERATION, + SC_CHANGE_REQUEST_UPDATE_OPERATION, + SC_COMMENT_CREATE_OPERATION, + SC_COMMENT_REPLY_OPERATION, + ): + registry.register(SourceControlMutationExecutor(operation)) diff --git a/src/forge/integrations/github/client.py b/src/forge/integrations/github/client.py index 0e66ccbd7..985c82117 100644 --- a/src/forge/integrations/github/client.py +++ b/src/forge/integrations/github/client.py @@ -495,6 +495,18 @@ async def create_issue_comment( logger.info(f"Created comment on issue #{issue_number}") return response.json() + async def get_issue_comments( + self, owner: str, repo: str, issue_number: int + ) -> list[dict[str, Any]]: + """Get general issue/PR conversation comments.""" + client = await self._get_client() + response = await client.get( + f"/repos/{owner}/{repo}/issues/{issue_number}/comments", + params={"per_page": 100}, + ) + response.raise_for_status() + return response.json() + async def get_check_runs(self, owner: str, repo: str, ref: str) -> list[dict[str, Any]]: """Get all CI results for a commit, combining check runs and commit statuses. diff --git a/src/forge/integrations/jira/client.py b/src/forge/integrations/jira/client.py index 02e4dcc5a..93861c525 100644 --- a/src/forge/integrations/jira/client.py +++ b/src/forge/integrations/jira/client.py @@ -523,6 +523,22 @@ async def create_remote_link(self, issue_key: str, url: str, title: str) -> None response.raise_for_status() logger.info(f"Added remote link to {issue_key}: {url}") + async def get_remote_links(self, issue_key: str) -> list[dict[str, str]]: + """Return remote-link URLs and titles for idempotent reconciliation.""" + client = await self._get_client() + response = await client.get(f"/issue/{issue_key}/remotelink") + response.raise_for_status() + links: list[dict[str, str]] = [] + for item in response.json(): + remote_object = item.get("object") or {} + links.append( + { + "url": str(remote_object.get("url") or ""), + "title": str(remote_object.get("title") or ""), + } + ) + return links + async def create_issue_link( self, link_type: str, @@ -863,7 +879,7 @@ async def remove_labels(self, issue_key: str, labels: list[str]) -> None: async def set_workflow_label( self, issue_key: str, - new_label: ForgeLabel, + new_label: ForgeLabel | str, remove_prefix: str = "forge:", ) -> None: """Set a workflow label, removing other forge: labels. @@ -876,6 +892,8 @@ async def set_workflow_label( new_label: The new workflow label to set. remove_prefix: Prefix of labels to remove (default: "forge:"). """ + label_value = new_label.value if isinstance(new_label, ForgeLabel) else new_label + # Get current labels current_labels = await self.get_labels(issue_key) @@ -884,7 +902,7 @@ async def set_workflow_label( label for label in current_labels if label.startswith(remove_prefix) - and label != new_label.value + and label != label_value and label != ForgeLabel.FORGE_MANAGED.value and label != "forge:managed:task" and label != "forge:managed:task-takeover" @@ -897,7 +915,7 @@ async def set_workflow_label( operations: list[dict[str, str]] = [] for label in labels_to_remove: operations.append({"remove": label}) - operations.append({"add": new_label.value}) + operations.append({"add": label_value}) # Ensure forge:managed is set if ForgeLabel.FORGE_MANAGED.value not in current_labels: @@ -910,7 +928,7 @@ async def set_workflow_label( ) response.raise_for_status() logger.info( - f"Set workflow label {new_label.value} on {issue_key} (removed: {labels_to_remove})" + f"Set workflow label {label_value} on {issue_key} (removed: {labels_to_remove})" ) async def add_structured_comment( diff --git a/src/forge/integrations/source_control/contracts.py b/src/forge/integrations/source_control/contracts.py index 5713034e6..736213710 100644 --- a/src/forge/integrations/source_control/contracts.py +++ b/src/forge/integrations/source_control/contracts.py @@ -250,6 +250,12 @@ async def create_comment( self, repo_ref: RepositoryRef, identity: ChangeRequestIdentity, body: str ) -> ReviewComment: ... + async def get_change_request_comments( + self, repo_ref: RepositoryRef, identity: ChangeRequestIdentity + ) -> list[ReviewComment]: + """Return general conversation comments for idempotency/recovery checks.""" + ... + async def reply_to_comment( self, repo_ref: RepositoryRef, diff --git a/src/forge/integrations/source_control/github/adapter.py b/src/forge/integrations/source_control/github/adapter.py index 980db1dab..00030ff6c 100644 --- a/src/forge/integrations/source_control/github/adapter.py +++ b/src/forge/integrations/source_control/github/adapter.py @@ -703,6 +703,17 @@ async def create_comment( ) return self._map_review_comment(comment) + @_translate_provider_errors + async def get_change_request_comments( + self, repo_ref: RepositoryRef, identity: ChangeRequestIdentity + ) -> list[ReviewComment]: + """List general PR comments used to recover marker-bearing effects.""" + owner, repo = repo_ref.namespace.split("/", 1) + comments = await self._get_client().get_issue_comments( + owner, repo, _require_native_id(identity) + ) + return [self._map_review_comment(comment) for comment in comments] + @_translate_provider_errors async def reply_to_comment( self, diff --git a/src/forge/main.py b/src/forge/main.py index 7dd7c7b39..75c7d0fc2 100644 --- a/src/forge/main.py +++ b/src/forge/main.py @@ -12,7 +12,13 @@ import forge.integrations.source_control.github # noqa: F401 (registers GitHub adapter factory) from forge import __version__ from forge.api.middleware.correlation import CorrelationIdMiddleware -from forge.api.routes import github_router, health_router, jira_router, metrics_router +from forge.api.routes import ( + effects_router, + github_router, + health_router, + jira_router, + metrics_router, +) from forge.config import get_settings from forge.integrations.source_control.registry import get_registry from forge.observability.config import configure_tracing, shutdown_tracing @@ -138,6 +144,7 @@ def create_app() -> FastAPI: # Register routes app.include_router(health_router) app.include_router(metrics_router) + app.include_router(effects_router) app.include_router(jira_router) app.include_router(github_router) diff --git a/src/forge/orchestrator/worker.py b/src/forge/orchestrator/worker.py index 8a52c19ee..bf3e6d7fb 100644 --- a/src/forge/orchestrator/worker.py +++ b/src/forge/orchestrator/worker.py @@ -18,6 +18,23 @@ record_workflow_started, ) from forge.config import get_settings +from forge.domain import ( + EffectCommand, + JsonValue, + ResourceIdentity, + WorkflowIdentity, + stable_identity, +) +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 ( @@ -58,6 +75,7 @@ selected_workflow_name, ) from forge.workflow.declarative.workflow import DeclarativeWorkflow +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 ( @@ -70,12 +88,12 @@ 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 +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.utils.source_control import get_adapter, identity_for +from forge.workflow.utils.source_control import get_adapter logger = logging.getLogger(__name__) @@ -185,6 +203,7 @@ def __init__( event_adapters: EventAdapterRegistry | None = None, command_handlers: CommandHandlerRegistry | None = None, review_enrichment: ReviewEnrichmentService | None = None, + effect_service: EffectService | None = None, ) -> None: """Initialize the worker. @@ -202,6 +221,7 @@ def __init__( self.event_adapters = event_adapters or create_default_event_adapter_registry() self.command_handlers = command_handlers or create_default_command_handler_registry() self.review_enrichment = review_enrichment + self.effect_service = effect_service or create_default_effect_service() self._shutdown_event = asyncio.Event() self._checkpointer = None self._compiled_workflows: dict[str, Any] = {} # Cache compiled workflows by name @@ -217,6 +237,136 @@ def _review_enrichment(self) -> ReviewEnrichmentService: self.review_enrichment = service return service + def _event_adapter_registry(self) -> EventAdapterRegistry: + """Lazily restore adapters for legacy fixtures that bypass ``__init__``.""" + registry = getattr(self, "event_adapters", None) + if registry is None: + registry = create_default_event_adapter_registry() + self.event_adapters = registry + return registry + + def _durable_effect_service(self) -> EffectService: + """Lazily restore the effect runtime for legacy fixtures.""" + service = getattr(self, "effect_service", None) + if service is None: + service = create_default_effect_service() + self.effect_service = service + return service + + async def _invoke_workflow( + self, + compiled_workflow: Any, + invocation_input: dict[str, Any] | None, + *, + config: dict[str, Any], + ticket_key: str, + state: dict[str, Any], + ) -> dict[str, Any]: + """Invoke graph code with the durable effect port bound to this run.""" + identity = WorkflowIdentity( + run_id=str(state.get("thread_id") or ticket_key), + workflow_name=str(state.get("workflow_name") or state.get("ticket_type") or "legacy"), + definition_revision=int( + state.get("workflow_definition_revision") or state.get("workflow_revision") or 1 + ), + definition_digest=state.get("workflow_definition_digest"), + ) + with bind_effect_runtime(self._durable_effect_service(), identity): + return await compiled_workflow.ainvoke(invocation_input, config=config) + + async def _execute_required_jira_effect( + self, + *, + ticket_key: str, + state: dict[str, Any], + event_id: str, + operation: str, + payload: dict[str, JsonValue], + logical_action: str, + ) -> None: + identity_parts: dict[str, JsonValue] = { + "run_id": ticket_key, + "event_id": event_id, + "operation": operation, + "logical_action": logical_action, + "target": ticket_key, + } + effect_id = stable_identity("effect", identity_parts) + await self._durable_effect_service().execute_required( + EffectCommand( + effect_id=effect_id, + idempotency_key=effect_id, + workflow=WorkflowIdentity( + run_id=str(state.get("thread_id") or ticket_key), + workflow_name=str( + state.get("workflow_name") or state.get("ticket_type") or "legacy" + ), + definition_revision=int( + state.get("workflow_definition_revision") + or state.get("workflow_revision") + or 1 + ), + definition_digest=state.get("workflow_definition_digest"), + ), + operation=operation, + target=ResourceIdentity(resource_type="issue", external_id=ticket_key), + payload=payload, + ) + ) + + async def _execute_required_comment( + self, + ticket_key: str, + body: str, + *, + logical_action: str, + discriminator: str = "", + ) -> None: + await self._execute_required_jira_effect( + ticket_key=ticket_key, + state={}, + event_id=discriminator, + operation=JIRA_COMMENT_OPERATION, + payload={"body": body}, + logical_action=logical_action, + ) + + async def _execute_required_source_comment( + self, + repo_ref: RepositoryRef, + pr_number: int, + body: str, + *, + ticket_key: str, + logical_action: str, + ) -> None: + identity = { + "run_id": ticket_key, + "operation": SC_COMMENT_CREATE_OPERATION, + "repository": repo_ref.namespace, + "pull_request": pr_number, + "logical_action": logical_action, + } + effect_id = stable_identity("effect", identity) + await self._durable_effect_service().execute_required( + EffectCommand( + effect_id=effect_id, + idempotency_key=effect_id, + workflow=WorkflowIdentity( + run_id=ticket_key, + workflow_name="legacy", + definition_revision=1, + ), + operation=SC_COMMENT_CREATE_OPERATION, + target=ResourceIdentity( + resource_type="change_request", + external_id=str(pr_number), + namespace=repo_ref.namespace, + ), + payload={"body": body}, + ) + ) + def _deserialize_event(self, message: QueueMessage) -> NormalizedEvent | None: """Reconstruct the typed NormalizedEvent a source-control message carries. @@ -228,8 +378,7 @@ def _deserialize_event(self, message: QueueMessage) -> NormalizedEvent | None: """ if message.normalized_event is None: return None - adapters = getattr(self, "event_adapters", None) or create_default_event_adapter_registry() - return adapters.adapt(message).normalized_event + 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.""" @@ -244,34 +393,25 @@ async def _get_forge_github_login(self, repo_ref: RepositoryRef) -> str: async def _handle_terminal_failure(self, message: QueueMessage, error: str) -> None: """Post one Jira comment after queue retries are exhausted.""" - jira = JiraClient() event_marker = f"Event/correlation ID: {message.event_id}" - try: - comments = await jira.get_comments(message.ticket_key) - if any(event_marker in comment.body for comment in comments): - logger.info( - f"Terminal failure notification already exists for event {message.event_id}" - ) - return - - safe_error = redact_secrets(error) - if len(safe_error) > 500: - safe_error = f"{safe_error[:500]}..." - details = ( - f"{safe_error}\n\n" - f"Ticket: {message.ticket_key}\n" - f"{event_marker}\n" - "Recovery: inspect the dead-letter entry, resolve the root cause, " - "then requeue the event." - ) - await jira.add_error_comment( - issue_key=message.ticket_key, - error_message=details, - node_name="queue execution (retries exhausted)", - ) - logger.info(f"Posted terminal queue failure notification to {message.ticket_key}") - finally: - await jira.close() + safe_error = redact_secrets(error) + if len(safe_error) > 500: + safe_error = f"{safe_error[:500]}..." + details = ( + "**Forge error in queue execution (retries exhausted):**\n\n" + f"{safe_error}\n\n" + f"Ticket: {message.ticket_key}\n" + f"{event_marker}\n" + "Recovery: inspect the dead-letter entry, resolve the root cause, " + "then requeue the event." + ) + await self._execute_required_comment( + message.ticket_key, + details, + logical_action="terminal-queue-failure", + discriminator=message.event_id, + ) + 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. @@ -291,7 +431,7 @@ async def _handle_source_control_event(self, message: QueueMessage) -> None: async def _handle_event(self, message: QueueMessage) -> None: """Handle any registered ingress source through its adapter.""" - adapted = self.event_adapters.adapt(message) + adapted = self._event_adapter_registry().adapt(message) if adapted.requires_ticket_correlation: message = await self._resolve_ticket_from_pr_index(message) if not message.ticket_key: @@ -314,7 +454,7 @@ async def _resolve_ticket_from_pr_index(self, message: QueueMessage) -> QueueMes Returns: Message with ticket_key populated if found, otherwise unchanged. """ - adapted = self.event_adapters.adapt(message) + adapted = self._event_adapter_registry().adapt(message) pr_url = adapted.change_request_url logger.debug(f"PR URL extracted for {message.event_id}: {pr_url!r}") @@ -596,11 +736,23 @@ async def _process_workflow(self, message: QueueMessage) -> None: f"{'Retrying' if was_errored else 'Re-invoking'} workflow " f"from {updated_values.get('current_node')}" ) - result = await compiled_workflow.ainvoke(updated_values, config=config) + result = await self._invoke_workflow( + compiled_workflow, + updated_values, + config=config, + ticket_key=ticket_key, + state=updated_values, + ) else: # For normal resume (paused at approval gate): update state and continue await compiled_workflow.aupdate_state(config, updated_values) - result = await compiled_workflow.ainvoke(None, config=config) + result = await self._invoke_workflow( + compiled_workflow, + None, + config=config, + ticket_key=ticket_key, + state=updated_values, + ) else: error_before_invoke = None @@ -613,7 +765,13 @@ async def _process_workflow(self, message: QueueMessage) -> None: record_workflow_started(ticket_type=ticket_type_str) # Run the workflow from the beginning - result = await compiled_workflow.ainvoke(state, config=config) + result = await self._invoke_workflow( + compiled_workflow, + state, + config=config, + ticket_key=ticket_key, + state=state, + ) cleaned_result = await _cleanup_terminal_workspace(result) if cleaned_result != result: @@ -674,8 +832,7 @@ async def _handle_resume_event( Returns: Updated state for workflow resumption. """ - adapters = getattr(self, "event_adapters", None) or create_default_event_adapter_registry() - adapted_event = adapted_event or adapters.adapt(message) + adapted_event = adapted_event or self._event_adapter_registry().adapt(message) command_decision = command_decision or interpret_event( message, adapted_event, current_state ) @@ -770,15 +927,12 @@ async def _handle_resume_event( ) elif feedback_request.kind is FeedbackKind.OPTION_RANGE: maximum = int(feedback_request.arguments["maximum"]) - jira = JiraClient() - try: - await post_status_comment( - jira, - message.ticket_key, - f"Please reply with >option N where N is between 1 and {maximum}.", - ) - finally: - await jira.close() + 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. @@ -1028,17 +1182,25 @@ async def _handle_resume_event( is_approved = True pr_merged = True logger.info(f"PRD PR merged for {message.ticket_key}") - jira = JiraClient() - try: - await jira.set_workflow_label(message.ticket_key, ForgeLabel.PRD_APPROVED) - prd_content = current_state.get("prd_content", "") - if prd_content: - await jira.update_description(message.ticket_key, prd_content) - logger.info( - f"Copied approved PRD to Jira description for {message.ticket_key}" - ) - finally: - await jira.close() + 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 @@ -1141,47 +1303,48 @@ async def _handle_resume_event( is_approved = True pr_merged = True logger.info(f"Spec PR merged for {message.ticket_key}") - jira = JiraClient() - try: - await jira.set_workflow_label(message.ticket_key, ForgeLabel.SPEC_APPROVED) - spec_content = current_state.get("spec_content", "") - if spec_content: - settings = get_settings() - if settings.jira_store_in_comments: - await jira.add_structured_comment( - message.ticket_key, - "Technical Specification (Approved)", - spec_content, - comment_type="spec", - ) - elif settings.jira_spec_custom_field: - await jira.update_custom_field( - message.ticket_key, - settings.jira_spec_custom_field, - spec_content, - ) - else: - old_filename = f"{message.ticket_key}-spec.md" - deleted = await jira.delete_attachments_by_name( - message.ticket_key, old_filename - ) - if deleted: - logger.info( - f"Deleted {deleted} old spec attachment(s) for " - f"{message.ticket_key}" - ) - await jira.add_attachment( - message.ticket_key, - filename=old_filename, - content=spec_content, - content_type="text/markdown", - ) - logger.info( - f"Copied approved spec to configured Jira storage for " - f"{message.ticket_key}" - ) - finally: - await jira.close() + 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 @@ -1493,6 +1656,7 @@ async def _handle_resume_event( 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 @@ -1526,6 +1690,7 @@ async def _handle_resume_event( 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, @@ -1605,6 +1770,7 @@ async def _post_resume_ack_comment( signal_type: str, current_node: str, source_ticket_key: str | None = None, + event_id: str | None = None, ) -> None: """Post a best-effort Jira acknowledgement for user-visible resume signals.""" stage = self._stage_label_for_node(current_node) @@ -1630,14 +1796,27 @@ async def _post_resume_ack_comment( "and is regenerating the artifact." ) - try: - jira = JiraClient() - try: - await post_status_comment(jira, comment_target_key, message) - finally: - await jira.close() - except Exception as e: - logger.warning(f"Failed to post resume acknowledgement to {comment_target_key}: {e}") + identity_parts: dict[str, JsonValue] = { + "ticket_key": ticket_key, + "target": comment_target_key, + "signal_type": signal_type, + "current_node": current_node, + "event_id": event_id or "legacy", + } + effect_id = stable_identity("effect", identity_parts) + command = EffectCommand( + effect_id=effect_id, + idempotency_key=effect_id, + workflow=WorkflowIdentity( + run_id=ticket_key, + workflow_name="legacy", + definition_revision=1, + ), + operation="jira.comment.create", + target=ResourceIdentity(resource_type="issue", external_id=comment_target_key), + payload={"body": message}, + ) + await self._durable_effect_service().submit(command) @staticmethod def _stage_label_for_node(current_node: str) -> str: @@ -1705,40 +1884,44 @@ async def _post_skip_gate_feedback( action: "skip" or "unskip". """ try: - _, adapter = get_adapter(repo_ref.namespace) - jira = JiraClient() - try: - if action == "skip": - gh_comment = ( - f"✅ CI gate skipped by @{sender}\n\n" - f"The following check will be treated as passing for this PR:\n" - f"- `{check_name}`\n\n" - f"All other CI checks still apply. " - f"Re-evaluating CI status now." - ) - jira_comment = ( - f"CI gate skipped on GitHub PR by {sender}:\n" - f"- `{check_name}`\n\n" - f"Skipped via `/forge skip-gate` on PR #{pr_number}. " - f"Review accordingly." - ) - else: - gh_comment = ( - f"CI gate skip removed by @{sender}\n\n" - f"`{check_name}` will be re-evaluated on the next CI run." - ) - jira_comment = ( - f"CI gate skip removed on GitHub PR by {sender}:\n" - f"- `{check_name}`\n\n" - f"Check will be re-evaluated on the next CI run." - ) + if action == "skip": + gh_comment = ( + f"✅ CI gate skipped by @{sender}\n\n" + f"The following check will be treated as passing for this PR:\n" + f"- `{check_name}`\n\n" + f"All other CI checks still apply. " + f"Re-evaluating CI status now." + ) + jira_comment = ( + f"CI gate skipped on GitHub PR by {sender}:\n" + f"- `{check_name}`\n\n" + f"Skipped via `/forge skip-gate` on PR #{pr_number}. " + f"Review accordingly." + ) + else: + gh_comment = ( + f"CI gate skip removed by @{sender}\n\n" + f"`{check_name}` will be re-evaluated on the next CI run." + ) + jira_comment = ( + f"CI gate skip removed on GitHub PR by {sender}:\n" + f"- `{check_name}`\n\n" + f"Check will be re-evaluated on the next CI run." + ) - if pr_number: - identity = identity_for(repo_ref, pr_number) - await adapter.create_comment(repo_ref, identity, gh_comment) - await post_status_comment(jira, ticket_key, jira_comment) - finally: - await jira.close() + if pr_number: + await self._execute_required_source_comment( + repo_ref, + pr_number, + gh_comment, + ticket_key=ticket_key, + logical_action=f"ci-gate-{action}:{check_name}", + ) + await self._execute_required_comment( + ticket_key, + jira_comment, + logical_action=f"ci-gate-{action}:{repo_ref.namespace}:{pr_number}:{check_name}", + ) except Exception as e: logger.warning(f"Failed to post skip-gate feedback: {e}") @@ -1751,23 +1934,25 @@ async def _post_rebase_feedback( ) -> None: """Post feedback for a /forge rebase command.""" try: - _, adapter = get_adapter(repo_ref.namespace) - jira = JiraClient() - try: - gh_comment = ( - f"Rebase triggered by @{sender}\n\n" - f"Merging `main` into the PR branch and resolving any conflicts. " - f"This may take a few minutes." - ) - jira_comment = ( - f"Rebase triggered via `/forge rebase` on PR #{pr_number} by {sender}." + gh_comment = ( + f"Rebase triggered by @{sender}\n\n" + f"Merging `main` into the PR branch and resolving any conflicts. " + f"This may take a few minutes." + ) + jira_comment = f"Rebase triggered via `/forge rebase` on PR #{pr_number} by {sender}." + if pr_number: + await self._execute_required_source_comment( + repo_ref, + pr_number, + gh_comment, + ticket_key=ticket_key, + logical_action="rebase-acknowledgement", ) - if pr_number: - identity = identity_for(repo_ref, pr_number) - await adapter.create_comment(repo_ref, identity, gh_comment) - await post_status_comment(jira, ticket_key, jira_comment) - finally: - await jira.close() + await self._execute_required_comment( + ticket_key, + jira_comment, + logical_action=f"rebase-acknowledgement:{repo_ref.namespace}:{pr_number}", + ) except Exception as e: logger.warning(f"Failed to post rebase feedback: {e}") @@ -1778,10 +1963,7 @@ async def _post_terminal_error_comment(self, ticket_key: str, error: str) -> Non ticket_key: The Jira ticket key. error: The error message. """ - from forge.integrations.jira.client import JiraClient - try: - jira = JiraClient() safe_error = redact_secrets(error) if error else "Unknown error" error_preview = safe_error[:200] comment = ( @@ -1789,27 +1971,30 @@ async def _post_terminal_error_comment(self, ticket_key: str, error: str) -> Non f"```\n{error_preview}\n```\n\n" f"To retry the workflow, add the label `forge:retry` to this ticket." ) - await post_status_comment(jira, ticket_key, comment) - await jira.close() + await self._execute_required_comment( + ticket_key, + comment, + logical_action=f"terminal-workflow-error:{error_preview}", + ) logger.info(f"Posted terminal error comment to {ticket_key}") except Exception as e: logger.warning(f"Failed to post terminal error comment to {ticket_key}: {e}") async def _post_retry_acknowledgement(self, ticket_key: str, node: str) -> None: """Acknowledge an accepted retry without blocking workflow resumption.""" - jira = JiraClient() try: comment = ( f"Forge accepted the `forge:retry` request and is resuming " f"the workflow from `{node}`." ) - await post_status_comment(jira, ticket_key, comment) + await self._execute_required_comment( + ticket_key, + comment, + logical_action=f"retry-acknowledgement:{node}", + ) logger.info(f"Posted retry acknowledgement to {ticket_key}") except Exception as e: logger.warning(f"Failed to post retry acknowledgement to {ticket_key}: {e}") - finally: - with contextlib.suppress(Exception): - await jira.close() async def _find_workflow_by_state(self, ticket_key: str) -> tuple[Any, Any]: """Find a workflow that has existing checkpoint state for the given ticket. @@ -1902,19 +2087,16 @@ async def _report_custom_workflow_configuration_error( self, ticket_key: str, error: str ) -> None: """Fail closed with an actionable, redacted Jira comment.""" - jira = JiraClient() try: - await jira.add_error_comment( - issue_key=ticket_key, - error_message=redact_secrets(error)[:1000], - node_name="custom workflow configuration", + await self._execute_required_comment( + ticket_key, + f"**Forge custom workflow configuration error:**\n\n{redact_secrets(error)[:1000]}", + logical_action=f"custom-workflow-configuration:{error}", ) except Exception: logger.warning( "Could not report custom workflow error for %s", ticket_key, exc_info=True ) - finally: - await jira.close() def _extract_ticket_type(self, message: QueueMessage) -> TicketType: """Extract ticket type from queue message. @@ -1927,7 +2109,7 @@ def _extract_ticket_type(self, message: QueueMessage) -> TicketType: """ if message.source != EventSource.JIRA: return TicketType.UNKNOWN - return self.event_adapters.adapt(message).ticket_type + return self._event_adapter_registry().adapt(message).ticket_type def _get_compiled_workflow(self, workflow_instance: Any) -> Any: """Get or compile a workflow graph. @@ -2035,14 +2217,18 @@ async def start(self) -> None: # Every registered source follows the same transport path. Adding an # adapter does not require another worker branch. - for source in self.event_adapters.sources: + for source in self._event_adapter_registry().sources: self.consumer.register_handler(source, self._handle_event) + effect_stop = asyncio.Event() + effect_task = asyncio.create_task(self._durable_effect_service().run_forever(effect_stop)) try: await self.consumer.start() except asyncio.CancelledError: pass finally: + effect_stop.set() + await effect_task await self.consumer.stop() await get_registry().aclose() logger.info("Worker shut down gracefully") @@ -2137,7 +2323,15 @@ async def run_single_ticket(ticket_key: str) -> dict[str, Any]: if isinstance(workflow_instance, DeclarativeWorkflow): config["recursion_limit"] = 100 - result = await compiled_workflow.ainvoke(initial_state, config=config) + effect_service = create_default_effect_service() + identity = WorkflowIdentity( + run_id=ticket_key, + workflow_name=str(initial_state.get("workflow_name") or ticket_type_str), + definition_revision=int(initial_state.get("workflow_definition_revision") or 1), + definition_digest=initial_state.get("workflow_definition_digest"), + ) + with bind_effect_runtime(effect_service, identity): + result = await compiled_workflow.ainvoke(initial_state, config=config) logger.info(f"Workflow completed: {result.get('current_node')}") return result diff --git a/src/forge/workflow/effect_runtime.py b/src/forge/workflow/effect_runtime.py new file mode 100644 index 000000000..1e2c1f2a8 --- /dev/null +++ b/src/forge/workflow/effect_runtime.py @@ -0,0 +1,643 @@ +"""Workflow-facing provider ports backed by the durable effect journal. + +Reads are delegated to the provider client. Writes are converted to durable, +idempotent effects before an executor is allowed to call the provider. +""" + +from __future__ import annotations + +import inspect +from collections.abc import Iterator +from contextlib import contextmanager +from contextvars import ContextVar +from dataclasses import asdict, replace +from enum import Enum +from typing import Any, cast + +from forge.config import Settings +from forge.domain import EffectCommand, ResourceIdentity, WorkflowIdentity, stable_identity +from forge.domain.schema import JsonValue +from forge.effects.executors import EffectExecutorRegistry +from forge.effects.jira import ( + JIRA_ARCHIVE_OPERATION, + JIRA_ATTACHMENT_ADD_OPERATION, + JIRA_ATTACHMENT_DELETE_BY_NAME_OPERATION, + JIRA_COMMENT_OPERATION, + JIRA_CUSTOM_FIELD_OPERATION, + JIRA_DESCRIPTION_OPERATION, + JIRA_EPIC_CREATE_OPERATION, + JIRA_ERROR_COMMENT_OPERATION, + JIRA_ISSUE_LINK_CREATE_OPERATION, + JIRA_LABEL_OPERATION, + JIRA_LABELS_ADD_OPERATION, + JIRA_LABELS_REMOVE_OPERATION, + JIRA_MODEL_POLICY_ERROR_COMMENT_OPERATION, + JIRA_REMOTE_LINK_CREATE_OPERATION, + JIRA_STRUCTURED_COMMENT_OPERATION, + JIRA_TASK_CREATE_OPERATION, + JIRA_TRANSITION_OPERATION, + JiraMutationExecutor, +) +from forge.effects.journal import InMemoryEffectJournal +from forge.effects.repository import REPOSITORY_PUSH_OPERATION +from forge.effects.service import EffectService, RequiredEffectError +from forge.effects.source_control import ( + SC_BRANCH_CREATE_OPERATION, + SC_CHANGE_REQUEST_CREATE_OPERATION, + SC_CHANGE_REQUEST_UPDATE_OPERATION, + SC_COMMENT_CREATE_OPERATION, + SC_COMMENT_REPLY_OPERATION, + SC_FILE_PUT_OPERATION, + SourceControlMutationExecutor, +) +from forge.integrations.jira.client import JiraClient as ProviderJiraClient +from forge.integrations.jira.models import JiraComment +from forge.integrations.source_control.contracts import ( + ChangeRequest, + ChangeRequestIdentity, + ChangeRequestState, + RepositoryRef, + ResolvedRepository, + ReviewComment, + WriteTarget, +) +from forge.workspace.git_ops import GitOperations + +_service: ContextVar[EffectService | None] = ContextVar("workflow_effect_service", default=None) +_identity: ContextVar[WorkflowIdentity | None] = ContextVar( + "workflow_effect_identity", default=None +) + + +@contextmanager +def bind_effect_runtime(service: EffectService, identity: WorkflowIdentity) -> Iterator[None]: + """Bind the control-plane effect runtime while invoking workflow code.""" + service_token = _service.set(service) + identity_token = _identity.set(identity) + try: + yield + finally: + _identity.reset(identity_token) + _service.reset(service_token) + + +class _BorrowedJiraClient: + """Prevent a locally-owned executor from closing the node's read client.""" + + def __init__(self, client: ProviderJiraClient) -> None: + self._client = client + + def __getattr__(self, name: str) -> Any: + return getattr(self._client, name) + + async def close(self) -> None: + return None + + +class JiraClient: + """Workflow Jira port: provider reads plus journalled provider writes.""" + + def __init__(self, settings: Settings | None = None) -> None: + self._provider = ProviderJiraClient(settings) + self._local_service: EffectService | None = None + + def __getattr__(self, name: str) -> Any: + return getattr(self._provider, name) + + async def close(self) -> None: + await self._provider.close() + + def _runtime(self) -> EffectService: + bound = _service.get() + if bound is not None: + return bound + if self._local_service is None: + registry = EffectExecutorRegistry() + for operation in ( + JIRA_COMMENT_OPERATION, + JIRA_LABEL_OPERATION, + JIRA_DESCRIPTION_OPERATION, + JIRA_CUSTOM_FIELD_OPERATION, + JIRA_ATTACHMENT_ADD_OPERATION, + JIRA_ATTACHMENT_DELETE_BY_NAME_OPERATION, + JIRA_STRUCTURED_COMMENT_OPERATION, + JIRA_TRANSITION_OPERATION, + JIRA_LABELS_ADD_OPERATION, + JIRA_LABELS_REMOVE_OPERATION, + JIRA_ARCHIVE_OPERATION, + JIRA_TASK_CREATE_OPERATION, + JIRA_EPIC_CREATE_OPERATION, + JIRA_ERROR_COMMENT_OPERATION, + JIRA_ISSUE_LINK_CREATE_OPERATION, + JIRA_REMOTE_LINK_CREATE_OPERATION, + JIRA_MODEL_POLICY_ERROR_COMMENT_OPERATION, + ): + registry.register( + JiraMutationExecutor( + operation, + client_factory=cast(Any, lambda: _BorrowedJiraClient(self._provider)), + ) + ) + self._local_service = EffectService(InMemoryEffectJournal(), registry) + return self._local_service + + async def _write( + self, + operation: str, + issue_key: str, + payload: dict[str, JsonValue], + ) -> Any: + caller = inspect.currentframe() + for _ in range(2): + caller = caller.f_back if caller is not None else None + location = ( + f"{caller.f_globals.get('__name__', 'unknown')}.{caller.f_code.co_name}" + if caller is not None + else "unknown" + ) + normalized = _json_value(payload) + identity = _identity.get() or WorkflowIdentity( + run_id=issue_key, + workflow_name="local", + definition_revision=1, + ) + parts: dict[str, JsonValue] = { + "run_id": identity.run_id, + "definition_revision": identity.definition_revision, + "operation": operation, + "target": issue_key, + "payload": normalized, + "origin": location, + } + effect_id = stable_identity("effect", parts) + record = await self._runtime().execute_required( + EffectCommand( + effect_id=effect_id, + idempotency_key=effect_id, + workflow=identity, + operation=operation, + target=ResourceIdentity(resource_type="issue", external_id=issue_key), + payload=normalized, + ) + ) + if record.result is None: # pragma: no cover - execute_required contract + raise RequiredEffectError(record) + return record.result + + async def add_comment(self, issue_key: str, body: str) -> JiraComment: + result = await self._write(JIRA_COMMENT_OPERATION, issue_key, {"body": body}) + return JiraComment( + id=str(result.provider_reference or ""), body=body, author_id="", author_name="" + ) + + async def add_structured_comment( + self, issue_key: str, title: str, content: str, comment_type: str = "forge-artifact" + ) -> JiraComment: + result = await self._write( + JIRA_STRUCTURED_COMMENT_OPERATION, + issue_key, + {"title": title, "content": content, "comment_type": comment_type}, + ) + return JiraComment( + id=str(result.provider_reference or ""), body=content, author_id="", author_name="" + ) + + async def set_workflow_label( + self, issue_key: str, new_label: Any, remove_prefix: str = "forge:" + ) -> None: + await self._write( + JIRA_LABEL_OPERATION, + issue_key, + {"label": _json_value(new_label), "remove_prefix": remove_prefix}, + ) + + async def update_description(self, issue_key: str, description: str) -> None: + await self._write(JIRA_DESCRIPTION_OPERATION, issue_key, {"description": description}) + + async def update_custom_field(self, issue_key: str, field_id: str, value: str) -> None: + await self._write( + JIRA_CUSTOM_FIELD_OPERATION, issue_key, {"field": field_id, "value": value} + ) + + async def transition_issue(self, issue_key: str, transition_name: str) -> None: + await self._write(JIRA_TRANSITION_OPERATION, issue_key, {"transition": transition_name}) + + async def add_labels(self, issue_key: str, labels: list[str]) -> None: + await self._write(JIRA_LABELS_ADD_OPERATION, issue_key, {"labels": labels}) + + async def remove_labels(self, issue_key: str, labels: list[str]) -> None: + await self._write(JIRA_LABELS_REMOVE_OPERATION, issue_key, {"labels": labels}) + + async def archive_issue(self, issue_key: str, archive_subtasks: bool = True) -> None: + await self._write( + JIRA_ARCHIVE_OPERATION, + issue_key, + {"archive_subtasks": archive_subtasks}, + ) + + async def create_task( + self, + project_key: str, + summary: str, + description: str, + parent_key: str | None = None, + labels: list[str] | None = None, + ) -> str: + result = await self._write( + JIRA_TASK_CREATE_OPERATION, + parent_key or project_key, + { + "project_key": project_key, + "summary": summary, + "description": description, + "parent_key": parent_key, + "labels": labels or [], + }, + ) + return str(result.provider_reference) + + async def create_epic( + self, + project_key: str, + summary: str, + description: str, + parent_key: str, + labels: list[str] | None = None, + ) -> str: + result = await self._write( + JIRA_EPIC_CREATE_OPERATION, + parent_key, + { + "project_key": project_key, + "summary": summary, + "description": description, + "parent_key": parent_key, + "labels": labels or [], + }, + ) + return str(result.provider_reference) + + async def create_issue_link(self, link_type: str, inward_key: str, outward_key: str) -> None: + await self._write( + JIRA_ISSUE_LINK_CREATE_OPERATION, + inward_key, + { + "link_type": link_type, + "inward_key": inward_key, + "outward_key": outward_key, + }, + ) + + async def create_remote_link(self, issue_key: str, url: str, title: str) -> None: + await self._write( + JIRA_REMOTE_LINK_CREATE_OPERATION, issue_key, {"url": url, "title": title} + ) + + async def add_attachment( + self, + issue_key: str, + filename: str, + content: str | bytes, + content_type: str = "text/markdown", + ) -> dict[str, Any]: + result = await self._write( + JIRA_ATTACHMENT_ADD_OPERATION, + issue_key, + { + "filename": filename, + "content": content.decode() if isinstance(content, bytes) else content, + "content_type": content_type, + }, + ) + return dict(result.output) + + async def delete_attachments_by_name(self, issue_key: str, filename: str) -> int: + result = await self._write( + JIRA_ATTACHMENT_DELETE_BY_NAME_OPERATION, issue_key, {"filename": filename} + ) + return int(result.output.get("deleted", 0)) + + async def add_error_comment( + self, + issue_key: str, + error_message: str, + node_name: str, + mention_account_ids: list[str] | None = None, + ) -> JiraComment: + result = await self._write( + JIRA_ERROR_COMMENT_OPERATION, + issue_key, + { + "error_message": error_message, + "node_name": node_name, + "mention_account_ids": mention_account_ids or [], + }, + ) + return JiraComment( + id=str(result.provider_reference or ""), + body=error_message, + author_id="", + author_name="", + ) + + async def add_model_policy_error_comment( + self, + issue_key: str, + node_name: str, + problem: str, + available_connections: str, + fix_command: str, + mention_account_ids: list[str] | None = None, + ) -> JiraComment: + result = await self._write( + JIRA_MODEL_POLICY_ERROR_COMMENT_OPERATION, + issue_key, + { + "node_name": node_name, + "problem": problem, + "available_connections": available_connections, + "fix_command": fix_command, + "mention_account_ids": mention_account_ids or [], + }, + ) + return JiraComment( + id=str(result.provider_reference or ""), + body=problem, + author_id="", + author_name="", + ) + + +def _json_value(value: Any) -> Any: + if isinstance(value, Enum): + return value.value + if isinstance(value, dict): + return {str(key): _json_value(item) for key, item in value.items()} + if isinstance(value, (list, tuple)): + return [_json_value(item) for item in value] + return value + + +async def push_repository( + git: GitOperations, + *, + use_fork: bool, + force: bool = False, + check_conflicts: bool | None = None, +) -> None: + """Persist a ref-update intent before pushing a workspace branch.""" + service = _service.get() + if service is None: + if use_fork: + if force: + git.push_to_fork(force=True) + else: + git.push_to_fork() + elif check_conflicts is None: + git.push(force=force) + else: + git.push(force=force, check_conflicts=check_conflicts) + return + identity = _identity.get() + if identity is None: # pragma: no cover - bound as one context + raise RuntimeError("Workflow identity is not bound") + commit_sha = git.get_current_sha() + payload: dict[str, JsonValue] = { + "workspace_path": str(git.workspace.path), + "repository": git.workspace.repo_name, + "branch": git.workspace.branch_name, + "ticket_key": git.workspace.ticket_key, + "commit_sha": commit_sha, + "use_fork": use_fork, + "force": force, + "check_conflicts": True if check_conflicts is None else check_conflicts, + } + effect_id = stable_identity( + "effect", + { + "run_id": identity.run_id, + "operation": REPOSITORY_PUSH_OPERATION, + "repository": git.workspace.repo_name, + "branch": git.workspace.branch_name, + "commit_sha": commit_sha, + }, + ) + await service.execute_required( + EffectCommand( + effect_id=effect_id, + idempotency_key=effect_id, + workflow=identity, + operation=REPOSITORY_PUSH_OPERATION, + target=ResourceIdentity( + resource_type="repository_ref", + external_id=git.workspace.branch_name, + namespace=git.workspace.repo_name, + ), + payload=payload, + ) + ) + + +class _SingleRepositoryRegistry: + def __init__(self, resolved: ResolvedRepository) -> None: + self._resolved = resolved + + def resolve(self, identifier: str) -> ResolvedRepository: + return replace( + self._resolved, + repo_ref=replace(self._resolved.repo_ref, id=identifier, namespace=identifier), + ) + + +class SourceControlAdapter: + """Workflow source-control port with journalled mutations.""" + + def __init__(self, resolved: ResolvedRepository) -> None: + if resolved.adapter is None: + raise ValueError("A source-control adapter is required") + self._resolved = resolved + self._provider = resolved.adapter + self._local_service: EffectService | None = None + + def __getattr__(self, name: str) -> Any: + return getattr(self._provider, name) + + def _runtime(self) -> EffectService: + bound = _service.get() + if bound is not None: + return bound + if self._local_service is None: + registry = EffectExecutorRegistry() + local_registry = _SingleRepositoryRegistry(self._resolved) + for operation in ( + SC_BRANCH_CREATE_OPERATION, + SC_FILE_PUT_OPERATION, + SC_CHANGE_REQUEST_CREATE_OPERATION, + SC_CHANGE_REQUEST_UPDATE_OPERATION, + SC_COMMENT_CREATE_OPERATION, + SC_COMMENT_REPLY_OPERATION, + ): + registry.register( + SourceControlMutationExecutor(operation, cast(Any, lambda: local_registry)) + ) + self._local_service = EffectService(InMemoryEffectJournal(), registry) + return self._local_service + + async def _write( + self, + operation: str, + repo_ref: RepositoryRef, + external_id: str, + payload: dict[str, JsonValue], + ) -> Any: + caller = inspect.currentframe() + for _ in range(2): + caller = caller.f_back if caller is not None else None + location = ( + f"{caller.f_globals.get('__name__', 'unknown')}.{caller.f_code.co_name}" + if caller is not None + else "unknown" + ) + normalized = _json_value( + { + **payload, + "_repository_id": self._resolved.repo_ref.id, + "_target_namespace": repo_ref.namespace, + } + ) + identity = _identity.get() or WorkflowIdentity( + run_id=external_id or repo_ref.namespace, + workflow_name="local", + definition_revision=1, + ) + effect_id = stable_identity( + "effect", + { + "run_id": identity.run_id, + "definition_revision": identity.definition_revision, + "operation": operation, + "repository": repo_ref.namespace, + "target": external_id, + "payload": normalized, + "origin": location, + }, + ) + record = await self._runtime().execute_required( + EffectCommand( + effect_id=effect_id, + idempotency_key=effect_id, + workflow=identity, + operation=operation, + target=ResourceIdentity( + resource_type="change_request", + external_id=external_id, + namespace=repo_ref.namespace, + ), + payload=normalized, + ) + ) + if record.result is None: # pragma: no cover + raise RequiredEffectError(record) + return record.result + + async def create_branch(self, repo_ref: RepositoryRef, name: str, base: str) -> None: + await self._write(SC_BRANCH_CREATE_OPERATION, repo_ref, name, {"name": name, "base": base}) + + async def put_file( + self, + repo_ref: RepositoryRef, + path: str, + content: str, + message: str, + branch: str, + ) -> None: + await self._write( + SC_FILE_PUT_OPERATION, + repo_ref, + f"{branch}:{path}", + {"path": path, "content": content, "message": message, "branch": branch}, + ) + + async def create_change_request( + self, + repo_ref: RepositoryRef, + target: WriteTarget, + title: str, + body: str, + draft: bool = False, + ) -> ChangeRequest: + result = await self._write( + SC_CHANGE_REQUEST_CREATE_OPERATION, + repo_ref, + target.head_ref, + {"target": asdict(target), "title": title, "body": body, "draft": draft}, + ) + native_id = result.output.get("number") or result.provider_reference + return ChangeRequest( + identity=ChangeRequestIdentity( + connection=repo_ref.connection, + repository_id=repo_ref.id, + native_id=str(native_id) if native_id is not None else None, + ), + url=str(result.output.get("url") or ""), + title=title, + body=body, + state=ChangeRequestState.OPEN, + source_branch=target.head_ref, + target_branch=target.base_branch, + draft=draft, + created=bool(result.output.get("created", True)), + ) + + async def update_change_request( + self, + repo_ref: RepositoryRef, + identity: ChangeRequestIdentity, + *, + title: str | None = None, + body: str | None = None, + state: ChangeRequestState | None = None, + ) -> ChangeRequest: + result = await self._write( + SC_CHANGE_REQUEST_UPDATE_OPERATION, + repo_ref, + str(identity.native_id), + {"title": title, "body": body, "state": state.value if state else None}, + ) + return ChangeRequest( + identity=identity, + url=str(result.output.get("url") or ""), + title=title or "", + body=body or "", + state=state or ChangeRequestState.OPEN, + source_branch="", + target_branch="", + ) + + async def create_comment( + self, repo_ref: RepositoryRef, identity: ChangeRequestIdentity, body: str + ) -> ReviewComment: + result = await self._write( + SC_COMMENT_CREATE_OPERATION, repo_ref, str(identity.native_id), {"body": body} + ) + return ReviewComment(id=str(result.provider_reference or ""), body=body, author="forge") + + async def reply_to_comment( + self, + repo_ref: RepositoryRef, + identity: ChangeRequestIdentity, + comment_id: str, + body: str, + ) -> ReviewComment: + result = await self._write( + SC_COMMENT_REPLY_OPERATION, + repo_ref, + str(identity.native_id), + {"comment_id": comment_id, "body": body}, + ) + return ReviewComment( + id=str(result.provider_reference or ""), + body=body, + author="forge", + in_reply_to=comment_id, + ) diff --git a/src/forge/workflow/nodes/ci_evaluator.py b/src/forge/workflow/nodes/ci_evaluator.py index 48c393fbf..b1b7034f7 100644 --- a/src/forge/workflow/nodes/ci_evaluator.py +++ b/src/forge/workflow/nodes/ci_evaluator.py @@ -9,7 +9,6 @@ from forge.api.routes.metrics import record_ci_fix_attempt from forge.config import get_settings -from forge.integrations.jira.client import JiraClient from forge.integrations.source_control.contracts import ( CheckConclusion, CheckRun, @@ -21,6 +20,7 @@ from forge.models.workflow import ForgeLabel from forge.prompts import load_prompt from forge.sandbox import ContainerRunner +from forge.workflow.effect_runtime import JiraClient, push_repository from forge.workflow.feature.state import FeatureState as WorkflowState from forge.workflow.nodes.code_review import run_post_change_review, sync_pr_description from forge.workflow.nodes.error_handler import notify_error @@ -496,11 +496,9 @@ async def attempt_ci_fix(state: WorkflowState) -> WorkflowState: ) if review_result is not None: state = merge_review_exhaustion(state, review_result, ticket_key, "code_review") - if fork_owner and fork_repo: - git.push_to_fork(force=False) - else: + if not (fork_owner and fork_repo): logger.warning("Fork info not in state — pushing to origin instead") - git.push(force=False) + await push_repository(git, use_fork=bool(fork_owner and fork_repo)) logger.info(f"CI fix pushed for {ticket_key} (attempt {ci_fix_attempt})") record_ci_fix_attempt(repo=state.get("current_repo", "unknown"), result="pushed") diff --git a/src/forge/workflow/nodes/code_review.py b/src/forge/workflow/nodes/code_review.py index 778d85416..6041aa97c 100644 --- a/src/forge/workflow/nodes/code_review.py +++ b/src/forge/workflow/nodes/code_review.py @@ -12,10 +12,10 @@ from forge.config import get_settings from forge.integrations.agents import ForgeAgent -from forge.integrations.jira.client import JiraClient from forge.prompts import load_prompt from forge.sandbox import ContainerRunner from forge.sandbox.runner import ContainerResult +from forge.workflow.effect_runtime import JiraClient from forge.workflow.utils.jira_status import post_status_comment from forge.workflow.utils.source_control import get_adapter, identity_for from forge.workspace.git_ops import GitOperations diff --git a/src/forge/workflow/nodes/epic_decomposition.py b/src/forge/workflow/nodes/epic_decomposition.py index 145650060..fe1ae752f 100644 --- a/src/forge/workflow/nodes/epic_decomposition.py +++ b/src/forge/workflow/nodes/epic_decomposition.py @@ -5,8 +5,9 @@ from forge.config import get_settings from forge.integrations.agents import ForgeAgent -from forge.integrations.jira.client import JiraClient, MissingProjectConfig +from forge.integrations.jira.client import MissingProjectConfig from forge.models.workflow import ForgeLabel +from forge.workflow.effect_runtime import JiraClient from forge.workflow.feature.state import FeatureState as WorkflowState from forge.workflow.utils import update_state_timestamp from forge.workflow.utils.jira_status import post_status_comment diff --git a/src/forge/workflow/nodes/error_handler.py b/src/forge/workflow/nodes/error_handler.py index 782f4d1e7..26cbd09ff 100644 --- a/src/forge/workflow/nodes/error_handler.py +++ b/src/forge/workflow/nodes/error_handler.py @@ -6,9 +6,9 @@ import logging from typing import Any -from forge.integrations.jira.client import JiraClient from forge.integrations.source_control.errors import SourceControlError from forge.utils.redaction import redact_secrets +from forge.workflow.effect_runtime import JiraClient from forge.workflow.feature.state import FeatureState as WorkflowState from forge.workflow.utils.source_control import get_adapter, identity_for diff --git a/src/forge/workflow/nodes/git_persistence.py b/src/forge/workflow/nodes/git_persistence.py index b7d511823..6870ae8a2 100644 --- a/src/forge/workflow/nodes/git_persistence.py +++ b/src/forge/workflow/nodes/git_persistence.py @@ -4,6 +4,7 @@ import logging from enum import StrEnum +from forge.workflow.effect_runtime import push_repository from forge.workspace.git_ops import GitOperations logger = logging.getLogger(__name__) @@ -88,10 +89,7 @@ async def push_to_fork_with_retry( """ for attempt in range(1, max_attempts + 1): try: - if use_fork: - git.push_to_fork() - else: - git.push(force=False, check_conflicts=False) + await push_repository(git, use_fork=use_fork, force=False, check_conflicts=False) return except Exception as exc: kind = classify_push_failure(exc) diff --git a/src/forge/workflow/nodes/human_review.py b/src/forge/workflow/nodes/human_review.py index 194cd6b50..91e7db965 100644 --- a/src/forge/workflow/nodes/human_review.py +++ b/src/forge/workflow/nodes/human_review.py @@ -5,8 +5,8 @@ from langgraph.graph import END -from forge.integrations.jira.client import JiraClient from forge.models.workflow import ForgeLabel, JiraStatus +from forge.workflow.effect_runtime import JiraClient from forge.workflow.feature.state import FeatureState as WorkflowState from forge.workflow.utils import update_state_timestamp from forge.workflow.utils.jira_status import ( diff --git a/src/forge/workflow/nodes/implement_review.py b/src/forge/workflow/nodes/implement_review.py index 9a99038b3..65461fbcf 100644 --- a/src/forge/workflow/nodes/implement_review.py +++ b/src/forge/workflow/nodes/implement_review.py @@ -8,9 +8,9 @@ from langgraph.graph import END from forge.config import get_settings -from forge.integrations.jira.client import JiraClient from forge.prompts import load_prompt from forge.sandbox import ContainerRunner +from forge.workflow.effect_runtime import JiraClient, push_repository from forge.workflow.feature.state import FeatureState as WorkflowState from forge.workflow.nodes.code_review import run_post_change_review, sync_pr_description from forge.workflow.nodes.workspace_setup import prepare_workspace @@ -402,10 +402,7 @@ async def implement_review(state: WorkflowState) -> WorkflowState: if review_result is not None: state = merge_review_exhaustion(state, review_result, ticket_key, "code_review") - if fork_owner and fork_repo: - git.push_to_fork(force=False) - else: - git.push(force=False) + await push_repository(git, use_fork=bool(fork_owner and fork_repo)) logger.info(f"Review implementation pushed for {ticket_key}") await sync_pr_description( diff --git a/src/forge/workflow/nodes/plan_bug_fix.py b/src/forge/workflow/nodes/plan_bug_fix.py index 790c90537..37b17ce5d 100644 --- a/src/forge/workflow/nodes/plan_bug_fix.py +++ b/src/forge/workflow/nodes/plan_bug_fix.py @@ -10,11 +10,12 @@ from langgraph.graph import END from forge.config import get_settings -from forge.integrations.jira.client import JiraClient, artifact_interaction_options +from forge.integrations.jira.client import artifact_interaction_options from forge.models.workflow import ForgeLabel from forge.prompts import load_prompt from forge.sandbox import ContainerRunner from forge.workflow.bug.state import BugState +from forge.workflow.effect_runtime import JiraClient from forge.workflow.utils import ( merge_review_exhaustion, set_paused, diff --git a/src/forge/workflow/nodes/post_merge_summary.py b/src/forge/workflow/nodes/post_merge_summary.py index 0e6f2a2cf..2f0b3389b 100644 --- a/src/forge/workflow/nodes/post_merge_summary.py +++ b/src/forge/workflow/nodes/post_merge_summary.py @@ -3,8 +3,8 @@ import logging from forge.config import get_settings -from forge.integrations.jira.client import JiraClient from forge.workflow.bug.state import BugState +from forge.workflow.effect_runtime import JiraClient logger = logging.getLogger(__name__) diff --git a/src/forge/workflow/nodes/pr_creation.py b/src/forge/workflow/nodes/pr_creation.py index 1c1961812..4344ef04f 100644 --- a/src/forge/workflow/nodes/pr_creation.py +++ b/src/forge/workflow/nodes/pr_creation.py @@ -8,7 +8,6 @@ from forge.config import get_settings from forge.integrations.agents import ForgeAgent -from forge.integrations.jira.client import JiraClient from forge.integrations.source_control.contracts import ( ChangeRequest, RepositoryRef, @@ -18,6 +17,7 @@ from forge.models.workflow import ForgeLabel, TicketType from forge.orchestrator.checkpointer import set_pr_ticket_index from forge.prompts import load_prompt +from forge.workflow.effect_runtime import JiraClient, push_repository from forge.workflow.nodes.code_review import sync_pr_description from forge.workflow.nodes.post_merge_summary import _extract_impact from forge.workflow.pr_state import save_active_pull_request @@ -192,10 +192,7 @@ async def create_pull_request(state: WorkflowState) -> WorkflowState: # Push branch to the write target: the fork in fork mode, origin in # direct mode (direct-mode repos have no "fork" remote to push to). - if target.fork_owner and target.fork_repo: - git.push_to_fork() - else: - git.push(force=False) + await push_repository(git, use_fork=bool(target.fork_owner and target.fork_repo)) # Build PR title — fetch live summary from Jira as source of truth ticket_summary = "" diff --git a/src/forge/workflow/nodes/prd_generation.py b/src/forge/workflow/nodes/prd_generation.py index 008e2e3f0..fd8d62f39 100644 --- a/src/forge/workflow/nodes/prd_generation.py +++ b/src/forge/workflow/nodes/prd_generation.py @@ -7,11 +7,11 @@ from forge.config import get_settings from forge.integrations.agents import ForgeAgent from forge.integrations.jira.client import ( - JiraClient, artifact_interaction_options, pr_interaction_options, ) from forge.models.workflow import ForgeLabel +from forge.workflow.effect_runtime import JiraClient from forge.workflow.feature.state import FeatureState as WorkflowState from forge.workflow.nodes.proposal_pr import ( PRD_PROPOSAL, diff --git a/src/forge/workflow/nodes/proposal_pr.py b/src/forge/workflow/nodes/proposal_pr.py index 647608bfe..877a7d58a 100644 --- a/src/forge/workflow/nodes/proposal_pr.py +++ b/src/forge/workflow/nodes/proposal_pr.py @@ -4,10 +4,11 @@ from dataclasses import dataclass, replace from typing import Any -from forge.integrations.jira.client import JiraClient, pr_interaction_options +from forge.integrations.jira.client import pr_interaction_options from forge.integrations.source_control.errors import NotFoundError from forge.models.workflow import ForgeLabel from forge.orchestrator.checkpointer import set_pr_ticket_index +from forge.workflow.effect_runtime import JiraClient from forge.workflow.utils.jira_status import post_status_comment from forge.workflow.utils.source_control import get_adapter, identity_for diff --git a/src/forge/workflow/nodes/qa_handler.py b/src/forge/workflow/nodes/qa_handler.py index f60eb759f..b1a5abcf0 100644 --- a/src/forge/workflow/nodes/qa_handler.py +++ b/src/forge/workflow/nodes/qa_handler.py @@ -5,7 +5,7 @@ from datetime import UTC, datetime from forge.integrations.agents import ForgeAgent -from forge.integrations.jira.client import JiraClient +from forge.workflow.effect_runtime import JiraClient from forge.workflow.feature.state import FeatureState as WorkflowState from forge.workflow.utils import update_state_timestamp from forge.workflow.utils.source_control import get_adapter, identity_for diff --git a/src/forge/workflow/nodes/rca_analysis.py b/src/forge/workflow/nodes/rca_analysis.py index 1645d9284..436149d0a 100644 --- a/src/forge/workflow/nodes/rca_analysis.py +++ b/src/forge/workflow/nodes/rca_analysis.py @@ -6,11 +6,12 @@ from pathlib import Path from forge.config import get_settings -from forge.integrations.jira.client import JiraClient, MissingProjectConfig +from forge.integrations.jira.client import MissingProjectConfig from forge.models.workflow import ForgeLabel from forge.prompts import load_prompt from forge.sandbox import ContainerRunner from forge.workflow.bug.state import BugState +from forge.workflow.effect_runtime import JiraClient from forge.workflow.utils import merge_review_exhaustion, update_state_timestamp from forge.workflow.utils.jira_status import post_status_comment from forge.workflow.utils.repo_resolution import ensure_repo_labels, get_effective_repos diff --git a/src/forge/workflow/nodes/rca_option_gate.py b/src/forge/workflow/nodes/rca_option_gate.py index 3c7737a83..0ce76f3e0 100644 --- a/src/forge/workflow/nodes/rca_option_gate.py +++ b/src/forge/workflow/nodes/rca_option_gate.py @@ -4,9 +4,9 @@ from langgraph.graph import END -from forge.integrations.jira.client import JiraClient from forge.models.workflow import ForgeLabel from forge.workflow.bug.state import BugState +from forge.workflow.effect_runtime import JiraClient from forge.workflow.utils import set_paused, update_state_timestamp from forge.workflow.utils.jira_status import post_status_comment diff --git a/src/forge/workflow/nodes/rebase.py b/src/forge/workflow/nodes/rebase.py index 8695c7e4f..aa7db052f 100644 --- a/src/forge/workflow/nodes/rebase.py +++ b/src/forge/workflow/nodes/rebase.py @@ -10,7 +10,6 @@ import logging from forge.config import get_settings -from forge.integrations.jira.client import JiraClient from forge.integrations.source_control.contracts import ( ChangeRequestIdentity, RepositoryRef, @@ -18,6 +17,7 @@ ) from forge.prompts import load_prompt from forge.sandbox import ContainerRunner +from forge.workflow.effect_runtime import JiraClient, push_repository from forge.workflow.feature.state import FeatureState as WorkflowState from forge.workflow.nodes.workspace_setup import ( get_workspace_manager, @@ -126,10 +126,7 @@ async def rebase_pr(state: WorkflowState) -> WorkflowState: # Clean merge — push it logger.info(f"{ticket_key}: clean merge with main, pushing") - if use_fork: - git.push_to_fork(force=True) - else: - git.push(force=True, check_conflicts=False) + await push_repository(git, use_fork=use_fork, force=True, check_conflicts=False) await adapter.create_comment( repo_ref, @@ -240,10 +237,7 @@ async def rebase_pr(state: WorkflowState) -> WorkflowState: git.stage_all() git.commit(f"[{ticket_key}] merge: resolve conflicts with main") - if use_fork: - git.push_to_fork(force=True) - else: - git.push(force=True, check_conflicts=False) + await push_repository(git, use_fork=use_fork, force=True, check_conflicts=False) logger.info(f"{ticket_key}: conflicts resolved and pushed") await adapter.create_comment( diff --git a/src/forge/workflow/nodes/spec_generation.py b/src/forge/workflow/nodes/spec_generation.py index 32460f3ea..f3d3e69b6 100644 --- a/src/forge/workflow/nodes/spec_generation.py +++ b/src/forge/workflow/nodes/spec_generation.py @@ -7,11 +7,11 @@ from forge.config import get_settings from forge.integrations.agents import ForgeAgent from forge.integrations.jira.client import ( - JiraClient, artifact_interaction_options, pr_interaction_options, ) from forge.models.workflow import ForgeLabel +from forge.workflow.effect_runtime import JiraClient from forge.workflow.feature.state import FeatureState as WorkflowState from forge.workflow.nodes.prd_generation import ( _normalize_proposals_path, diff --git a/src/forge/workflow/nodes/task_generation.py b/src/forge/workflow/nodes/task_generation.py index c1f36de45..1a38cd3cc 100644 --- a/src/forge/workflow/nodes/task_generation.py +++ b/src/forge/workflow/nodes/task_generation.py @@ -6,9 +6,10 @@ from typing import Any from forge.integrations.agents import ForgeAgent -from forge.integrations.jira.client import JiraClient, MissingProjectConfig +from forge.integrations.jira.client import MissingProjectConfig from forge.models.workflow import ForgeLabel from forge.prompts import load_prompt +from forge.workflow.effect_runtime import JiraClient from forge.workflow.feature.state import FeatureState as WorkflowState from forge.workflow.utils import update_state_timestamp from forge.workflow.utils.jira_status import post_status_comment diff --git a/src/forge/workflow/nodes/task_takeover_planning.py b/src/forge/workflow/nodes/task_takeover_planning.py index 13cdc57de..463b4efef 100644 --- a/src/forge/workflow/nodes/task_takeover_planning.py +++ b/src/forge/workflow/nodes/task_takeover_planning.py @@ -7,9 +7,9 @@ from forge.config import get_settings from forge.integrations.agents import ForgeAgent -from forge.integrations.jira.client import JiraClient from forge.models.workflow import ForgeLabel from forge.prompts import load_prompt +from forge.workflow.effect_runtime import JiraClient from forge.workflow.task_takeover.state import TaskTakeoverState from forge.workflow.utils import set_paused, update_state_timestamp from forge.workflow.utils.jira_status import post_status_comment diff --git a/src/forge/workflow/nodes/task_takeover_triage.py b/src/forge/workflow/nodes/task_takeover_triage.py index e919bd873..78fabede3 100644 --- a/src/forge/workflow/nodes/task_takeover_triage.py +++ b/src/forge/workflow/nodes/task_takeover_triage.py @@ -10,9 +10,9 @@ from forge.config import get_settings from forge.integrations.agents import ForgeAgent -from forge.integrations.jira.client import JiraClient from forge.models.workflow import ForgeLabel from forge.prompts import load_prompt +from forge.workflow.effect_runtime import JiraClient from forge.workflow.task_takeover.state import TaskTakeoverState from forge.workflow.utils import update_state_timestamp from forge.workflow.utils.jira_status import post_status_comment diff --git a/src/forge/workflow/nodes/triage.py b/src/forge/workflow/nodes/triage.py index 7c409ae01..274861f1e 100644 --- a/src/forge/workflow/nodes/triage.py +++ b/src/forge/workflow/nodes/triage.py @@ -11,10 +11,10 @@ from forge.config import get_settings from forge.integrations.agents import ForgeAgent -from forge.integrations.jira.client import JiraClient from forge.models.workflow import ForgeLabel from forge.prompts import load_prompt from forge.workflow.bug.state import BugState +from forge.workflow.effect_runtime import JiraClient from forge.workflow.utils import set_paused, update_state_timestamp from forge.workflow.utils.jira_status import post_status_comment diff --git a/src/forge/workflow/nodes/workspace_setup.py b/src/forge/workflow/nodes/workspace_setup.py index 9b05b61cd..8d8bf16bd 100644 --- a/src/forge/workflow/nodes/workspace_setup.py +++ b/src/forge/workflow/nodes/workspace_setup.py @@ -10,8 +10,8 @@ from typing import Any from forge.config import get_settings -from forge.integrations.jira.client import JiraClient 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 diff --git a/src/forge/workflow/task_takeover/graph.py b/src/forge/workflow/task_takeover/graph.py index af10748af..cfcd420ab 100644 --- a/src/forge/workflow/task_takeover/graph.py +++ b/src/forge/workflow/task_takeover/graph.py @@ -8,8 +8,8 @@ from langgraph.graph import END, StateGraph -from forge.integrations.jira.client import JiraClient from forge.models.workflow import ForgeLabel, JiraStatus +from forge.workflow.effect_runtime import JiraClient from forge.workflow.gates.task_plan_approval import ( route_task_plan_approval, task_plan_approval_gate, diff --git a/src/forge/workflow/utils/jira_status.py b/src/forge/workflow/utils/jira_status.py index c3e071b7e..c447033b1 100644 --- a/src/forge/workflow/utils/jira_status.py +++ b/src/forge/workflow/utils/jira_status.py @@ -5,50 +5,18 @@ API issues, while logging warnings for observability. """ +from __future__ import annotations + import logging -import re +from typing import TYPE_CHECKING -from forge.integrations.jira import JiraClient +from forge.effects.rendering import format_status_comment from forge.models.workflow import ForgeLabel -logger = logging.getLogger(__name__) +if TYPE_CHECKING: + from forge.workflow.effect_runtime import JiraClient -_EMOJI_PREFIX_RE = re.compile(r"^\s*(?:[\u2600-\u27BF\U0001F300-\U0001FAFF]|\u2139)") - - -def format_status_comment(message: str) -> str: - """Ensure a workflow status comment starts with a matching emoji.""" - if _EMOJI_PREFIX_RE.match(message): - return message - - normalized = message.lower() - emoji = "ℹ️" - if any(word in normalized for word in ("fail", "error", "conflict", "cannot", "missing")): - emoji = "⚠️" - elif any(word in normalized for word in ("complete", "success", "approved", "merged")): - emoji = "✅" - elif "prd" in normalized: - emoji = "📝" - elif "spec" in normalized or "specification" in normalized: - emoji = "📋" - elif "plan" in normalized: - emoji = "🧭" - elif "task" in normalized or "implement" in normalized: - emoji = "⚙️" - elif "pull request" in normalized or " pr " in f" {normalized} ": - emoji = "🔀" - elif " ci " in f" {normalized} ": - emoji = "🧪" - elif "review" in normalized: - emoji = "👀" - elif "question" in normalized or "q&a" in normalized: - emoji = "❓" - elif "triage" in normalized or "checking" in normalized: - emoji = "🔎" - elif "rca" in normalized or "root cause" in normalized or "analysis" in normalized: - emoji = "🔍" - - return f"{emoji} {message}" +logger = logging.getLogger(__name__) async def post_status_comment( diff --git a/src/forge/workflow/utils/qa_summary.py b/src/forge/workflow/utils/qa_summary.py index 2bfe07034..33f3bb9ec 100644 --- a/src/forge/workflow/utils/qa_summary.py +++ b/src/forge/workflow/utils/qa_summary.py @@ -3,7 +3,7 @@ import logging from typing import Any -from forge.integrations.jira.client import JiraClient +from forge.workflow.effect_runtime import JiraClient logger = logging.getLogger(__name__) diff --git a/src/forge/workflow/utils/source_control.py b/src/forge/workflow/utils/source_control.py index ffdcf34e8..70bf5fac1 100644 --- a/src/forge/workflow/utils/source_control.py +++ b/src/forge/workflow/utils/source_control.py @@ -14,6 +14,7 @@ ) from forge.integrations.source_control.errors import NotFoundError from forge.integrations.source_control.registry import get_registry +from forge.workflow.effect_runtime import SourceControlAdapter def resolve_repository( @@ -37,7 +38,7 @@ def get_adapter(identifier: str) -> tuple[RepositoryRef, SourceControlProvider]: f"'{identifier}' resolved to provider '{resolved.repo_ref.provider}' " "with no registered adapter" ) - return resolved.repo_ref, resolved.adapter + return resolved.repo_ref, SourceControlAdapter(resolved) def identity_for(repo_ref: RepositoryRef, native_id: str | int | None) -> ChangeRequestIdentity: diff --git a/src/forge/workspace/git_ops.py b/src/forge/workspace/git_ops.py index bd1a26868..2955498fb 100644 --- a/src/forge/workspace/git_ops.py +++ b/src/forge/workspace/git_ops.py @@ -321,6 +321,13 @@ def remote_branch_exists(self, branch_name: str, remote: str = "origin") -> bool result = self._run_git("ls-remote", "--heads", "--", remote, branch_name, check=False) return bool(result.stdout.strip()) + def get_remote_branch_sha(self, branch_name: str, remote: str = "origin") -> str | None: + """Return the remote branch SHA without changing local repository state.""" + result = self._run_git("ls-remote", "--heads", "--", remote, branch_name, check=False) + if result.returncode != 0 or not result.stdout.strip(): + return None + return result.stdout.split()[0] + def check_for_conflicts(self, target_branch: str = "main") -> tuple[bool, list[str]]: """Check if pushing would cause conflicts with remote. diff --git a/tests/unit/api/test_effects.py b/tests/unit/api/test_effects.py new file mode 100644 index 000000000..e43f8f76f --- /dev/null +++ b/tests/unit/api/test_effects.py @@ -0,0 +1,55 @@ +from types import SimpleNamespace + +import pytest +from httpx import ASGITransport, AsyncClient +from pydantic import SecretStr + +from forge.api.routes.effects import get_effect_service +from forge.domain import EffectCommand, ResourceIdentity, WorkflowIdentity +from forge.effects import EffectExecutorRegistry, EffectService, InMemoryEffectJournal +from forge.main import app + + +def _command() -> EffectCommand: + return EffectCommand( + effect_id="effect-1", + idempotency_key="effect-1", + workflow=WorkflowIdentity(run_id="FORGE-1", workflow_name="feature", definition_revision=1), + operation="test.write", + target=ResourceIdentity(resource_type="issue", external_id="FORGE-1"), + ) + + +@pytest.mark.asyncio +async def test_effect_history_requires_configured_operator_token(monkeypatch) -> None: + monkeypatch.setattr( + "forge.api.routes.effects.get_settings", + lambda: SimpleNamespace(effect_operator_token=None), + ) + transport = ASGITransport(app=app) + async with AsyncClient(transport=transport, base_url="http://test") as client: + response = await client.get("/api/v1/effects/workflow/FORGE-1") + assert response.status_code == 503 + + +@pytest.mark.asyncio +async def test_operator_can_inspect_workflow_effect_history(monkeypatch) -> None: + journal = InMemoryEffectJournal() + service = EffectService(journal, EffectExecutorRegistry()) + await service.submit(_command()) + app.dependency_overrides[get_effect_service] = lambda: service + monkeypatch.setattr( + "forge.api.routes.effects.get_settings", + lambda: SimpleNamespace(effect_operator_token=SecretStr("operator-secret")), + ) + try: + transport = ASGITransport(app=app) + async with AsyncClient(transport=transport, base_url="http://test") as client: + response = await client.get( + "/api/v1/effects/workflow/FORGE-1", + headers={"Authorization": "Bearer operator-secret"}, + ) + assert response.status_code == 200 + assert response.json()[0]["command"]["idempotency_key"] == "effect-1" + finally: + app.dependency_overrides.pop(get_effect_service, None) diff --git a/tests/unit/architecture/test_direct_provider_effects.py b/tests/unit/architecture/test_direct_provider_effects.py new file mode 100644 index 000000000..162ee2764 --- /dev/null +++ b/tests/unit/architecture/test_direct_provider_effects.py @@ -0,0 +1,75 @@ +"""Prevent workflow execution code from bypassing durable mutation ports.""" + +import ast +from pathlib import Path + +ROOT = Path(__file__).parents[3] +WORKFLOW = ROOT / "src" / "forge" / "workflow" + + +def test_mutating_workflow_modules_do_not_import_provider_jira_client() -> None: + violations: list[str] = [] + for path in WORKFLOW.rglob("*.py"): + relative = path.relative_to(WORKFLOW).as_posix() + if relative in {"effect_runtime.py", "declarative/cli.py"}: + continue + tree = ast.parse(path.read_text(), filename=str(path)) + has_mutation = any( + isinstance(node, ast.Call) + and isinstance(node.func, ast.Attribute) + and node.func.attr.startswith( + ( + "create_", + "update_", + "delete_", + "add_", + "remove_", + "set_", + "transition_", + "archive_", + ) + ) + for node in ast.walk(tree) + ) + imports_provider_client = any( + isinstance(node, ast.ImportFrom) + and node.module in {"forge.integrations.jira", "forge.integrations.jira.client"} + and any(alias.name == "JiraClient" for alias in node.names) + for node in ast.walk(tree) + ) + if has_mutation and imports_provider_client: + violations.append(relative) + assert violations == [], f"Workflow mutations bypass the durable Jira port: {violations}" + + +def test_source_control_resolution_is_centralized_in_durable_port() -> None: + violations: list[str] = [] + for path in WORKFLOW.rglob("*.py"): + relative = path.relative_to(WORKFLOW).as_posix() + if relative in {"effect_runtime.py", "utils/source_control.py"}: + continue + tree = ast.parse(path.read_text(), filename=str(path)) + for node in ast.walk(tree): + if ( + isinstance(node, ast.ImportFrom) + and node.module == "forge.integrations.source_control.registry" + ): + violations.append(relative) + assert violations == [], f"Workflow code resolves provider adapters directly: {violations}" + + +def test_repository_pushes_only_execute_through_effect_runtime() -> None: + violations: list[str] = [] + for path in WORKFLOW.rglob("*.py"): + relative = path.relative_to(WORKFLOW).as_posix() + if relative == "effect_runtime.py": + continue + tree = ast.parse(path.read_text(), filename=str(path)) + for node in ast.walk(tree): + if ( + isinstance(node, ast.Call) + and isinstance(node.func, ast.Attribute) + and node.func.attr in {"push", "push_to_fork"} + ): + violations.append(f"{relative}:{node.lineno}") + assert violations == [], f"Workflow repository pushes bypass durable effects: {violations}" diff --git a/tests/unit/effects/__init__.py b/tests/unit/effects/__init__.py new file mode 100644 index 000000000..8d69532ed --- /dev/null +++ b/tests/unit/effects/__init__.py @@ -0,0 +1 @@ +"""Tests for durable external effects.""" diff --git a/tests/unit/effects/test_jira.py b/tests/unit/effects/test_jira.py new file mode 100644 index 000000000..30749637f --- /dev/null +++ b/tests/unit/effects/test_jira.py @@ -0,0 +1,239 @@ +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from forge.domain import EffectCommand, ResourceIdentity, WorkflowIdentity +from forge.effects.jira import ( + JIRA_ATTACHMENT_REPLACE_OPERATION, + JIRA_COMMENT_OPERATION, + JIRA_CUSTOM_FIELD_OPERATION, + JIRA_DESCRIPTION_OPERATION, + JIRA_ISSUE_LINK_CREATE_OPERATION, + JIRA_LABEL_OPERATION, + JIRA_LABELS_ADD_OPERATION, + JIRA_REMOTE_LINK_CREATE_OPERATION, + JIRA_TASK_CREATE_OPERATION, + JIRA_TRANSITION_OPERATION, + JiraCommentExecutor, + JiraMutationExecutor, +) + + +def _command() -> EffectCommand: + return EffectCommand( + effect_id="effect-1", + idempotency_key="stable-key", + workflow=WorkflowIdentity(run_id="FORGE-1", workflow_name="feature", definition_revision=1), + operation=JIRA_COMMENT_OPERATION, + target=ResourceIdentity(resource_type="issue", external_id="FORGE-1"), + payload={"body": "Work accepted"}, + ) + + +@pytest.mark.asyncio +async def test_comment_executor_adds_recovery_marker() -> None: + jira = MagicMock() + jira.get_comments = AsyncMock(return_value=[]) + jira.add_comment = AsyncMock(return_value=SimpleNamespace(id="comment-1")) + jira.close = AsyncMock() + + result = await JiraCommentExecutor(lambda: jira).execute(_command()) + + body = jira.add_comment.await_args.args[1] + assert "forge-effect:stable-key" in body + assert result.provider_reference == "comment-1" + + +@pytest.mark.parametrize( + ("operation", "payload", "method", "expected"), + [ + ( + JIRA_LABEL_OPERATION, + {"label": "forge:done"}, + "set_workflow_label", + ("FORGE-1", "forge:done"), + ), + ( + JIRA_DESCRIPTION_OPERATION, + {"description": "new"}, + "update_description", + ("FORGE-1", "new"), + ), + ( + JIRA_CUSTOM_FIELD_OPERATION, + {"field": "customfield_1", "value": "new"}, + "update_custom_field", + ("FORGE-1", "customfield_1", "new"), + ), + ], +) +@pytest.mark.asyncio +async def test_idempotent_jira_mutation_executors(operation, payload, method, expected) -> None: + jira = MagicMock() + setattr(jira, method, AsyncMock()) + jira.close = AsyncMock() + command = _command().model_copy(update={"operation": operation, "payload": payload}) + + result = await JiraMutationExecutor(operation, lambda: jira).execute(command) + + getattr(jira, method).assert_awaited_once_with(*expected) + assert result.provider_reference == "FORGE-1" + jira.close.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_attachment_replace_recovers_by_replacing_name() -> None: + jira = MagicMock() + jira.delete_attachments_by_name = AsyncMock(return_value=1) + jira.add_attachment = AsyncMock(return_value=SimpleNamespace(id="attachment-2")) + jira.close = AsyncMock() + command = _command().model_copy( + update={ + "operation": JIRA_ATTACHMENT_REPLACE_OPERATION, + "payload": { + "filename": "spec.md", + "content": "body", + "content_type": "text/markdown", + }, + } + ) + + result = await JiraMutationExecutor(JIRA_ATTACHMENT_REPLACE_OPERATION, lambda: jira).execute( + command + ) + + jira.delete_attachments_by_name.assert_awaited_once_with("FORGE-1", "spec.md") + jira.add_attachment.assert_awaited_once_with( + "FORGE-1", + filename="spec.md", + content="body", + content_type="text/markdown", + ) + assert result.provider_reference == "attachment-2" + jira.close.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_retry_after_crash_finds_provider_marker_without_duplicate() -> None: + jira = MagicMock() + jira.get_comments = AsyncMock( + return_value=[SimpleNamespace(id="comment-1", body="{forge-effect:stable-key}")] + ) + jira.add_comment = AsyncMock() + jira.close = AsyncMock() + + result = await JiraCommentExecutor(lambda: jira).execute(_command()) + + jira.add_comment.assert_not_awaited() + assert result.provider_reference == "comment-1" + + +@pytest.mark.asyncio +async def test_transition_recovers_when_target_status_was_already_reached() -> None: + jira = MagicMock() + jira.get_issue = AsyncMock(return_value=SimpleNamespace(status="Closed")) + jira.transition_issue = AsyncMock() + jira.close = AsyncMock() + command = _command().model_copy( + update={"operation": JIRA_TRANSITION_OPERATION, "payload": {"transition": "Closed"}} + ) + + await JiraMutationExecutor(JIRA_TRANSITION_OPERATION, lambda: jira).execute(command) + + jira.transition_issue.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_add_labels_only_writes_missing_values() -> None: + jira = MagicMock() + jira.get_labels = AsyncMock(return_value=["existing"]) + jira.add_labels = AsyncMock() + jira.close = AsyncMock() + command = _command().model_copy( + update={ + "operation": JIRA_LABELS_ADD_OPERATION, + "payload": {"labels": ["existing", "new"]}, + } + ) + + await JiraMutationExecutor(JIRA_LABELS_ADD_OPERATION, lambda: jira).execute(command) + + jira.add_labels.assert_awaited_once_with("FORGE-1", ["new"]) + + +@pytest.mark.asyncio +async def test_task_create_recovers_by_creation_marker() -> None: + jira = MagicMock() + jira.search_issues = AsyncMock(return_value=[SimpleNamespace(key="FORGE-9")]) + jira.create_task = AsyncMock() + jira.close = AsyncMock() + command = _command().model_copy( + update={ + "operation": JIRA_TASK_CREATE_OPERATION, + "payload": { + "project_key": "FORGE", + "summary": "Implement it", + "description": "Details", + "labels": ["team-a"], + }, + } + ) + + result = await JiraMutationExecutor(JIRA_TASK_CREATE_OPERATION, lambda: jira).execute(command) + + jira.create_task.assert_not_awaited() + assert "forge-effect-stable-key" in jira.search_issues.await_args.args[0] + assert result.provider_reference == "FORGE-9" + + +@pytest.mark.asyncio +async def test_issue_link_recovers_when_relationship_already_exists() -> None: + jira = MagicMock() + jira.get_issue_links = AsyncMock( + return_value=[ + {"type": "related", "inward_key": "FORGE-9", "outward_key": "FORGE-1"} + ] + ) + jira.create_issue_link = AsyncMock() + jira.close = AsyncMock() + command = _command().model_copy( + update={ + "operation": JIRA_ISSUE_LINK_CREATE_OPERATION, + "payload": { + "link_type": "Related", + "inward_key": "FORGE-9", + "outward_key": "FORGE-1", + }, + } + ) + + result = await JiraMutationExecutor( + JIRA_ISSUE_LINK_CREATE_OPERATION, lambda: jira + ).execute(command) + + jira.create_issue_link.assert_not_awaited() + assert result.provider_reference == "FORGE-9:Related:FORGE-1" + + +@pytest.mark.asyncio +async def test_remote_link_recovers_by_url() -> None: + jira = MagicMock() + jira.get_remote_links = AsyncMock( + return_value=[{"url": "https://example.test/pull/7", "title": "PR 7"}] + ) + jira.create_remote_link = AsyncMock() + jira.close = AsyncMock() + command = _command().model_copy( + update={ + "operation": JIRA_REMOTE_LINK_CREATE_OPERATION, + "payload": {"url": "https://example.test/pull/7", "title": "PR 7"}, + } + ) + + result = await JiraMutationExecutor( + JIRA_REMOTE_LINK_CREATE_OPERATION, lambda: jira + ).execute(command) + + jira.create_remote_link.assert_not_awaited() + assert result.provider_reference == "https://example.test/pull/7" diff --git a/tests/unit/effects/test_repository.py b/tests/unit/effects/test_repository.py new file mode 100644 index 000000000..593c6b1b4 --- /dev/null +++ b/tests/unit/effects/test_repository.py @@ -0,0 +1,77 @@ +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from forge.domain import EffectCommand, ResourceIdentity, WorkflowIdentity +from forge.effects.repository import REPOSITORY_PUSH_OPERATION, RepositoryPushExecutor + + +def _command() -> EffectCommand: + return EffectCommand( + effect_id="push-1", + idempotency_key="push-1", + workflow=WorkflowIdentity(run_id="FORGE-1", workflow_name="feature", definition_revision=1), + operation=REPOSITORY_PUSH_OPERATION, + target=ResourceIdentity( + resource_type="repository_ref", external_id="forge/forge-1", namespace="org/repo" + ), + payload={ + "workspace_path": "/tmp/forge-test", + "repository": "org/repo", + "branch": "forge/forge-1", + "ticket_key": "FORGE-1", + "commit_sha": "abc123", + "use_fork": True, + "force": False, + "check_conflicts": True, + }, + ) + + +@pytest.mark.asyncio +async def test_push_recovers_after_provider_success_without_pushing_again() -> None: + adapter = MagicMock() + adapter.get_git_credentials = AsyncMock(return_value=MagicMock()) + registry = MagicMock() + registry.resolve.return_value = MagicMock(adapter=adapter, repo_ref=MagicMock()) + git = MagicMock() + git.get_current_sha.return_value = "abc123" + git.get_remote_branch_sha.return_value = "abc123" + + with patch("forge.effects.repository.GitOperations", return_value=git): + result = await RepositoryPushExecutor(lambda: registry).execute(_command()) + + git.push_to_fork.assert_not_called() + assert result.provider_reference == "fork:forge/forge-1@abc123" + + +@pytest.mark.asyncio +async def test_push_updates_remote_when_commit_is_missing() -> None: + adapter = MagicMock() + adapter.get_git_credentials = AsyncMock(return_value=MagicMock()) + registry = MagicMock() + registry.resolve.return_value = MagicMock(adapter=adapter, repo_ref=MagicMock()) + git = MagicMock() + git.get_current_sha.return_value = "abc123" + git.get_remote_branch_sha.return_value = None + + with patch("forge.effects.repository.GitOperations", return_value=git): + await RepositoryPushExecutor(lambda: registry).execute(_command()) + + git.push_to_fork.assert_called_once_with(force=False) + + +@pytest.mark.asyncio +async def test_stale_push_is_superseded_by_newer_local_commit() -> None: + adapter = MagicMock() + adapter.get_git_credentials = AsyncMock(return_value=MagicMock()) + registry = MagicMock() + registry.resolve.return_value = MagicMock(adapter=adapter, repo_ref=MagicMock()) + git = MagicMock() + git.get_current_sha.return_value = "newer456" + + with patch("forge.effects.repository.GitOperations", return_value=git): + result = await RepositoryPushExecutor(lambda: registry).execute(_command()) + + git.push_to_fork.assert_not_called() + assert result.output == {"superseded_by": "newer456"} diff --git a/tests/unit/effects/test_service.py b/tests/unit/effects/test_service.py new file mode 100644 index 000000000..fb3700e17 --- /dev/null +++ b/tests/unit/effects/test_service.py @@ -0,0 +1,210 @@ +from datetime import UTC, datetime, timedelta + +import pytest + +from forge.domain import ( + EffectCommand, + EffectResult, + EffectResultStatus, + ResourceIdentity, + WorkflowIdentity, +) +from forge.effects import ( + EffectExecutorRegistry, + EffectRecordStatus, + EffectService, + InMemoryEffectJournal, + RequiredEffectError, +) +from forge.integrations.source_control.errors import ConflictError + + +def _command(key: str = "same-logical-effect") -> EffectCommand: + return EffectCommand( + effect_id="effect-1", + idempotency_key=key, + workflow=WorkflowIdentity(run_id="FORGE-1", workflow_name="feature", definition_revision=1), + operation="test.write", + target=ResourceIdentity(resource_type="issue", external_id="FORGE-1"), + payload={"value": "hello"}, + ) + + +class _Executor: + operation = "test.write" + + def __init__(self) -> None: + self.calls = 0 + + async def execute(self, command: EffectCommand) -> EffectResult: + self.calls += 1 + return EffectResult( + effect_id=command.effect_id, + idempotency_key=command.idempotency_key, + status=EffectResultStatus.SUCCEEDED, + completed_at=datetime.now(UTC), + provider_reference="external-1", + ) + + +@pytest.mark.asyncio +async def test_duplicate_submission_executes_once() -> None: + journal = InMemoryEffectJournal() + executor = _Executor() + registry = EffectExecutorRegistry() + registry.register(executor) + service = EffectService(journal, registry) + + first = await service.submit(_command()) + second = await service.submit(_command()) + completed = await service.run_due() + + assert first == second + assert executor.calls == 1 + assert completed[0].status is EffectRecordStatus.SUCCEEDED + assert (await journal.get("same-logical-effect")) == completed[0] + assert await journal.list_for_workflow("FORGE-1") == completed + + +@pytest.mark.asyncio +async def test_failure_is_retried_without_rerunning_originating_station() -> None: + class FlakyExecutor(_Executor): + async def execute(self, command: EffectCommand) -> EffectResult: + self.calls += 1 + if self.calls == 1: + raise TimeoutError("provider unavailable") + return await super().execute(command) + + journal = InMemoryEffectJournal() + executor = FlakyExecutor() + registry = EffectExecutorRegistry() + registry.register(executor) + service = EffectService( + journal, + registry, + base_retry_delay=timedelta(0), + ) + await service.submit(_command()) + + first = (await service.run_due())[0] + second = (await service.run_due())[0] + + assert first.status is EffectRecordStatus.RETRYABLE_FAILURE + assert second.status is EffectRecordStatus.SUCCEEDED + + +@pytest.mark.asyncio +async def test_expired_running_lease_is_recovered() -> None: + journal = InMemoryEffectJournal(lease=timedelta(0)) + await journal.submit(_command()) + + first = (await journal.claim_due())[0] + recovered = (await journal.claim_due())[0] + + assert first.attempt == 1 + assert recovered.attempt == 2 + + +@pytest.mark.asyncio +async def test_execute_now_persists_claims_and_executes_exact_effect() -> None: + journal = InMemoryEffectJournal() + executor = _Executor() + registry = EffectExecutorRegistry() + registry.register(executor) + service = EffectService(journal, registry) + + first = await service.execute_now(_command()) + duplicate = await service.execute_now(_command()) + + assert first.status is EffectRecordStatus.SUCCEEDED + assert duplicate == first + assert executor.calls == 1 + + +@pytest.mark.asyncio +async def test_claim_one_excludes_parallel_claim() -> None: + journal = InMemoryEffectJournal() + await journal.submit(_command()) + + first = await journal.claim("same-logical-effect") + competing = await journal.claim("same-logical-effect") + + assert first is not None + assert competing is None + + +@pytest.mark.asyncio +async def test_required_effect_fails_closed_on_retryable_result() -> None: + class FailingExecutor(_Executor): + async def execute(self, _command: EffectCommand) -> EffectResult: + raise TimeoutError("later") + + journal = InMemoryEffectJournal() + registry = EffectExecutorRegistry() + registry.register(FailingExecutor()) + service = EffectService(journal, registry) + + with pytest.raises(RequiredEffectError): + await service.execute_required(_command()) + + +@pytest.mark.asyncio +async def test_attempt_history_survives_retry_and_success() -> None: + class FlakyExecutor(_Executor): + async def execute(self, command: EffectCommand) -> EffectResult: + if self.calls == 0: + self.calls += 1 + raise TimeoutError("later") + return await super().execute(command) + + journal = InMemoryEffectJournal() + registry = EffectExecutorRegistry() + registry.register(FlakyExecutor()) + service = EffectService(journal, registry, base_retry_delay=timedelta(0)) + await service.submit(_command()) + + await service.run_due() + completed = (await service.run_due())[0] + + assert [attempt.status for attempt in completed.attempt_history] == [ + EffectResultStatus.RETRYABLE_FAILURE, + EffectResultStatus.SUCCEEDED, + ] + + +@pytest.mark.asyncio +async def test_precondition_failure_requires_explicit_replay() -> None: + class ConflictingExecutor(_Executor): + async def execute(self, _command: EffectCommand) -> EffectResult: + raise ConflictError("provider state changed") + + journal = InMemoryEffectJournal() + registry = EffectExecutorRegistry() + registry.register(ConflictingExecutor()) + service = EffectService(journal, registry) + await service.submit(_command()) + + failed = (await service.run_due())[0] + replayed = await service.replay(_command().idempotency_key) + + assert failed.status is EffectRecordStatus.PRECONDITION_FAILED + assert replayed.status is EffectRecordStatus.PENDING + assert replayed.replay_count == 1 + assert len(replayed.attempt_history) == 1 + + +@pytest.mark.asyncio +async def test_retention_only_purges_old_terminal_effects() -> None: + journal = InMemoryEffectJournal() + executor = _Executor() + registry = EffectExecutorRegistry() + registry.register(executor) + service = EffectService(journal, registry) + await service.execute_now(_command("old")) + await service.submit(_command("pending")) + + removed = await service.purge_terminal_before(datetime.now(UTC) + timedelta(seconds=1)) + + assert removed == 1 + assert await journal.get("old") is None + assert await journal.get("pending") is not None diff --git a/tests/unit/effects/test_source_control.py b/tests/unit/effects/test_source_control.py new file mode 100644 index 000000000..836ac7fa0 --- /dev/null +++ b/tests/unit/effects/test_source_control.py @@ -0,0 +1,163 @@ +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from forge.domain import EffectCommand, ResourceIdentity, WorkflowIdentity +from forge.effects.source_control import ( + SC_BRANCH_CREATE_OPERATION, + SC_CHANGE_REQUEST_UPDATE_OPERATION, + SC_COMMENT_CREATE_OPERATION, + SC_COMMENT_REPLY_OPERATION, + SC_FILE_PUT_OPERATION, + SourceControlMutationExecutor, +) +from forge.integrations.source_control.contracts import ( + ChangeRequest, + ChangeRequestIdentity, + ChangeRequestState, + Provider, + RepositoryRef, + ResolvedRepository, + Review, + ReviewComment, +) + + +def _fixture(operation: str, payload: dict) -> tuple[EffectCommand, MagicMock, MagicMock]: + repo = RepositoryRef( + id="repo-1", + provider=Provider.GITHUB, + connection="github", + namespace="org/repo", + default_branch="main", + change_request_mode="direct", + ) + adapter = MagicMock() + registry = MagicMock() + registry.resolve.return_value = ResolvedRepository( + repo_ref=repo, connection=MagicMock(), adapter=adapter + ) + command = EffectCommand( + effect_id="effect-1", + idempotency_key="stable-key", + workflow=WorkflowIdentity(run_id="FORGE-1", workflow_name="feature", definition_revision=1), + operation=operation, + target=ResourceIdentity( + resource_type="change_request", external_id="17", namespace="org/repo" + ), + payload=payload, + ) + return command, registry, adapter + + +@pytest.mark.asyncio +async def test_comment_effect_recovers_from_provider_marker() -> None: + command, registry, adapter = _fixture(SC_COMMENT_CREATE_OPERATION, {"body": "Done"}) + adapter.get_change_request_comments = AsyncMock( + return_value=[ReviewComment(id="9", body="", author="bot")] + ) + adapter.create_comment = AsyncMock() + + result = await SourceControlMutationExecutor( + SC_COMMENT_CREATE_OPERATION, lambda: registry + ).execute(command) + + adapter.create_comment.assert_not_awaited() + assert result.provider_reference == "9" + + +@pytest.mark.asyncio +async def test_comment_effect_leaves_recovery_marker() -> None: + command, registry, adapter = _fixture(SC_COMMENT_CREATE_OPERATION, {"body": "Done"}) + adapter.get_change_request_comments = AsyncMock(return_value=[]) + adapter.create_comment = AsyncMock(return_value=ReviewComment(id="10", body="", author="bot")) + + await SourceControlMutationExecutor(SC_COMMENT_CREATE_OPERATION, lambda: registry).execute( + command + ) + + assert "forge-effect:stable-key" in adapter.create_comment.await_args.args[2] + + +@pytest.mark.asyncio +async def test_branch_and_change_request_mutations_use_provider_contract() -> None: + command, registry, adapter = _fixture( + SC_BRANCH_CREATE_OPERATION, {"name": "forge/work", "base": "main"} + ) + adapter.create_branch = AsyncMock() + result = await SourceControlMutationExecutor( + SC_BRANCH_CREATE_OPERATION, lambda: registry + ).execute(command) + adapter.create_branch.assert_awaited_once() + assert result.provider_reference == "forge/work" + + command, registry, adapter = _fixture( + SC_CHANGE_REQUEST_UPDATE_OPERATION, {"body": "updated", "state": "open"} + ) + adapter.update_change_request = AsyncMock( + return_value=ChangeRequest( + identity=ChangeRequestIdentity("github", "repo-1", 17), + url="https://example.test/17", + title="PR", + body="updated", + state=ChangeRequestState.OPEN, + source_branch="work", + target_branch="main", + ) + ) + result = await SourceControlMutationExecutor( + SC_CHANGE_REQUEST_UPDATE_OPERATION, lambda: registry + ).execute(command) + assert result.output["number"] == "17" + + +@pytest.mark.asyncio +async def test_file_effect_recovers_after_provider_success_before_acknowledgement() -> None: + command, registry, adapter = _fixture( + SC_FILE_PUT_OPERATION, + { + "path": "docs/plan.md", + "content": "same content", + "message": "Publish plan", + "branch": "forge/work", + }, + ) + adapter.get_file = AsyncMock(return_value="same content") + adapter.put_file = AsyncMock() + + result = await SourceControlMutationExecutor( + SC_FILE_PUT_OPERATION, lambda: registry + ).execute(command) + + adapter.put_file.assert_not_awaited() + assert result.provider_reference == "forge/work:docs/plan.md" + + +@pytest.mark.asyncio +async def test_review_reply_recovers_from_inline_thread_marker() -> None: + command, registry, adapter = _fixture( + SC_COMMENT_REPLY_OPERATION, {"body": "Fixed", "comment_id": "8"} + ) + adapter.get_review_thread_comments = AsyncMock( + return_value=[ + Review( + id="thread-1", + state="commented", + body="", + author="reviewer", + comments=[ + ReviewComment( + id="9", body="", author="bot" + ) + ], + ) + ] + ) + adapter.reply_to_comment = AsyncMock() + + result = await SourceControlMutationExecutor( + SC_COMMENT_REPLY_OPERATION, lambda: registry + ).execute(command) + + adapter.reply_to_comment.assert_not_awaited() + assert result.provider_reference == "9" diff --git a/tests/unit/integrations/source_control/test_protocol.py b/tests/unit/integrations/source_control/test_protocol.py index 96015f6a9..dd8500992 100644 --- a/tests/unit/integrations/source_control/test_protocol.py +++ b/tests/unit/integrations/source_control/test_protocol.py @@ -49,6 +49,9 @@ async def update_change_request( async def create_comment(self, _repo_ref: object, _identity: object, _body: object) -> object: raise NotImplementedError + async def get_change_request_comments(self, _repo_ref: object, _identity: object) -> object: + raise NotImplementedError + async def reply_to_comment( self, _repo_ref: object, _identity: object, _comment_id: object, _body: object ) -> object: diff --git a/tests/unit/orchestrator/test_blocked_retry.py b/tests/unit/orchestrator/test_blocked_retry.py index c414b54e6..60f286e94 100644 --- a/tests/unit/orchestrator/test_blocked_retry.py +++ b/tests/unit/orchestrator/test_blocked_retry.py @@ -1,6 +1,6 @@ """Unit tests for blocked-state and forge:retry worker behaviour.""" -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, MagicMock import pytest @@ -233,44 +233,28 @@ async def test_retry_posts_acknowledgement(self, worker, base_message): @pytest.mark.asyncio async def test_retry_acknowledgement_failure_does_not_raise(self, worker): """Jira acknowledgement failures must not block workflow resumption.""" - jira = MagicMock() - jira.close = AsyncMock() - with ( - patch("forge.orchestrator.worker.JiraClient", return_value=jira), - patch( - "forge.orchestrator.worker.post_status_comment", - new=AsyncMock(side_effect=RuntimeError("Jira unavailable")), - ), - ): - await OrchestratorWorker._post_retry_acknowledgement( - worker, "TEST-123", "execute_task_changes" - ) - - jira.close.assert_awaited_once() + worker.effect_service = MagicMock() + worker.effect_service.execute_required = AsyncMock( + side_effect=RuntimeError("Jira unavailable") + ) + await OrchestratorWorker._post_retry_acknowledgement( + worker, "TEST-123", "execute_task_changes" + ) + worker.effect_service.execute_required.assert_awaited_once() @pytest.mark.asyncio async def test_retry_acknowledgement_names_resumed_node(self, worker): """The Jira acknowledgement tells the user where Forge resumed.""" - jira = MagicMock() - jira.close = AsyncMock() - with ( - patch("forge.orchestrator.worker.JiraClient", return_value=jira), - patch( - "forge.orchestrator.worker.post_status_comment", - new_callable=AsyncMock, - ) as post_comment, - ): - await OrchestratorWorker._post_retry_acknowledgement( - worker, "TEST-123", "execute_task_changes" - ) - - post_comment.assert_awaited_once_with( - jira, - "TEST-123", + worker.effect_service = MagicMock() + worker.effect_service.execute_required = AsyncMock() + await OrchestratorWorker._post_retry_acknowledgement( + worker, "TEST-123", "execute_task_changes" + ) + command = worker.effect_service.execute_required.await_args.args[0] + assert command.payload["body"] == ( "Forge accepted the `forge:retry` request and is resuming " - "the workflow from `execute_task_changes`.", + "the workflow from `execute_task_changes`." ) - jira.close.assert_awaited_once() class TestRetryOnStuckNonTerminalNode: diff --git a/tests/unit/orchestrator/test_worker.py b/tests/unit/orchestrator/test_worker.py index 8847bf134..6148dc8cd 100644 --- a/tests/unit/orchestrator/test_worker.py +++ b/tests/unit/orchestrator/test_worker.py @@ -38,6 +38,17 @@ def _patch_adapter(repo_ref: RepositoryRef, adapter): return patch("forge.orchestrator.worker.get_adapter", return_value=(repo_ref, adapter)) +@pytest.fixture(autouse=True) +def durable_effect_service_mock(): + """Keep worker unit tests infrastructure-free at the durable-effect boundary.""" + service = MagicMock() + service.submit = AsyncMock() + service.execute_required = AsyncMock() + service.run_forever = AsyncMock() + with patch("forge.orchestrator.worker.create_default_effect_service", return_value=service): + yield service + + @pytest.mark.parametrize( ("result", "error_before_invoke", "expected"), [ @@ -102,27 +113,22 @@ async def test_report_new_workflow_error_skips_non_reportable_errors( async def test_terminal_error_comment_uses_markdown_code_block(): """Terminal errors use markup supported by the Markdown-to-ADF converter.""" worker = OrchestratorWorker.__new__(OrchestratorWorker) - jira = MagicMock() - jira.close = AsyncMock() - - with ( - patch("forge.integrations.jira.client.JiraClient", return_value=jira), - patch( - "forge.orchestrator.worker.post_status_comment", new_callable=AsyncMock - ) as post_comment, - ): + worker._execute_required_comment = AsyncMock() + + with patch.object(worker, "_execute_required_comment") as post_comment: await worker._post_terminal_error_comment( "TEST-123", "Object of type set is not JSON serializable" ) post_comment.assert_awaited_once_with( - jira, "TEST-123", "**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" + ), ) - jira.close.assert_awaited_once() def _multi_repo_pr_state() -> dict: @@ -303,7 +309,9 @@ async def test_multi_repo_review_selects_earlier_pr() -> None: @pytest.mark.asyncio -async def test_terminal_failure_posts_sanitized_recovery_comment(): +async def test_terminal_failure_posts_sanitized_recovery_comment( + durable_effect_service_mock, +): worker = OrchestratorWorker(consumer_name="test-worker") message = QueueMessage( message_id="1-0", @@ -312,27 +320,21 @@ async def test_terminal_failure_posts_sanitized_recovery_comment(): event_type="issue_updated", ticket_key="TEST-123", ) - jira = AsyncMock() - jira.get_comments = AsyncMock(return_value=[]) - - with patch("forge.orchestrator.worker.JiraClient", return_value=jira): - await worker._handle_terminal_failure( - message, - "clone https://ghp_abcdefghijklmnopqrstuvwxyz123456@github.com/acme/repo failed", - ) + await worker._handle_terminal_failure( + message, + "clone https://ghp_abcdefghijklmnopqrstuvwxyz123456@github.com/acme/repo failed", + ) - jira.add_error_comment.assert_awaited_once() - kwargs = jira.add_error_comment.await_args.kwargs - assert kwargs["issue_key"] == "TEST-123" - assert "[REDACTED]" in kwargs["error_message"] - assert "ghp_" not in kwargs["error_message"] - assert "Event/correlation ID: evt-terminal-1" in kwargs["error_message"] - assert "Recovery:" in kwargs["error_message"] - jira.close.assert_awaited_once() + command = durable_effect_service_mock.execute_required.await_args.args[0] + assert command.target.external_id == "TEST-123" + assert "[REDACTED]" in command.payload["body"] + assert "ghp_" not in command.payload["body"] + assert "Event/correlation ID: evt-terminal-1" in command.payload["body"] + assert "Recovery:" in command.payload["body"] @pytest.mark.asyncio -async def test_terminal_failure_skips_existing_event_comment(): +async def test_terminal_failure_uses_stable_effect_identity(durable_effect_service_mock): worker = OrchestratorWorker(consumer_name="test-worker") message = QueueMessage( message_id="1-0", @@ -341,31 +343,21 @@ async def test_terminal_failure_skips_existing_event_comment(): event_type="issue_updated", ticket_key="TEST-123", ) - jira = AsyncMock() - jira.get_comments = AsyncMock( - return_value=[MagicMock(body="Event/correlation ID: evt-terminal-1")] - ) + await worker._handle_terminal_failure(message, "failed") + await worker._handle_terminal_failure(message, "failed") - with patch("forge.orchestrator.worker.JiraClient", return_value=jira): - await worker._handle_terminal_failure(message, "failed") - - jira.add_error_comment.assert_not_awaited() - jira.close.assert_awaited_once() + 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 class TestQuestionDetection: """Tests for Q&A mode question detection.""" @pytest.fixture(autouse=True) - def ack_comment_mocks(self): - """Mock Jira acknowledgement posting for direct resume-event tests.""" - mock_jira = AsyncMock() - mock_jira.close = AsyncMock() - with ( - patch("forge.orchestrator.worker.JiraClient", return_value=mock_jira), - patch("forge.orchestrator.worker.post_status_comment", new_callable=AsyncMock) as post, - ): - yield post + def ack_comment_mocks(self, durable_effect_service_mock): + """Expose durable acknowledgement submissions for assertions.""" + yield durable_effect_service_mock.submit @pytest.fixture def worker(self) -> OrchestratorWorker: @@ -438,8 +430,9 @@ async def test_question_comment_sets_is_question_flag( assert result["revision_requested"] is False assert result["is_paused"] is False ack_comment_mocks.assert_awaited_once() - assert ack_comment_mocks.await_args.args[1] == "TEST-123" - ack_text = ack_comment_mocks.await_args.args[2] + effect = ack_comment_mocks.await_args.args[0] + assert effect.target.external_id == "TEST-123" + ack_text = effect.payload["body"] assert "received your question" in ack_text assert "the PRD" in ack_text @@ -479,8 +472,9 @@ async def test_normal_feedback_still_works( assert result["feedback_comment"] == "Please add more detail to the security section" assert result["is_paused"] is False ack_comment_mocks.assert_awaited_once() - assert ack_comment_mocks.await_args.args[1] == "TEST-123" - ack_text = ack_comment_mocks.await_args.args[2] + effect = ack_comment_mocks.await_args.args[0] + assert effect.target.external_id == "TEST-123" + ack_text = effect.payload["body"] assert "received your revision request" in ack_text assert "regenerating" in ack_text @@ -521,8 +515,9 @@ async def test_task_phase_feedback_from_epic_sets_current_epic_key( assert result["current_epic_key"] == "TEST-124" assert result["current_task_key"] is None ack_comment_mocks.assert_awaited_once() - assert ack_comment_mocks.await_args.args[1] == "TEST-124" - ack_text = ack_comment_mocks.await_args.args[2] + effect = ack_comment_mocks.await_args.args[0] + assert effect.target.external_id == "TEST-124" + ack_text = effect.payload["body"] assert "from TEST-124" in ack_text @pytest.mark.asyncio @@ -560,8 +555,9 @@ async def test_plan_phase_feedback_from_epic_acknowledges_epic( assert result["feedback_comment"] == "Please revise this epic plan" assert result["current_epic_key"] == "TEST-124" ack_comment_mocks.assert_awaited_once() - assert ack_comment_mocks.await_args.args[1] == "TEST-124" - ack_text = ack_comment_mocks.await_args.args[2] + effect = ack_comment_mocks.await_args.args[0] + assert effect.target.external_id == "TEST-124" + ack_text = effect.payload["body"] assert "received your revision request" in ack_text assert "from TEST-124" in ack_text diff --git a/tests/unit/orchestrator/test_worker_option_detection.py b/tests/unit/orchestrator/test_worker_option_detection.py index a1ab5ccaa..1ff15448c 100644 --- a/tests/unit/orchestrator/test_worker_option_detection.py +++ b/tests/unit/orchestrator/test_worker_option_detection.py @@ -11,7 +11,9 @@ @pytest.fixture def worker() -> OrchestratorWorker: - return OrchestratorWorker(consumer_name="test-worker") + instance = OrchestratorWorker(consumer_name="test-worker") + instance.effect_service = AsyncMock() + return instance def _make_option_message(comment_body: str) -> QueueMessage: @@ -97,15 +99,10 @@ 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() - mock_jira = AsyncMock() - mock_jira.add_comment = AsyncMock() - mock_jira.close = AsyncMock() + await worker._handle_resume_event(message, state) - with patch("forge.orchestrator.worker.JiraClient", return_value=mock_jira): - await worker._handle_resume_event(message, state) - - mock_jira.add_comment.assert_called_once() - comment_text = mock_jira.add_comment.call_args[0][1] + command = worker.effect_service.execute_required.await_args.args[0] + comment_text = command.payload["body"] assert "option" in comment_text.lower() and ("1" in comment_text and "2" in comment_text) @pytest.mark.asyncio @@ -113,12 +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() - mock_jira = AsyncMock() - mock_jira.add_comment = AsyncMock() - mock_jira.close = AsyncMock() - - with patch("forge.orchestrator.worker.JiraClient", return_value=mock_jira): - result = await worker._handle_resume_event(message, state) + result = await worker._handle_resume_event(message, state) assert result["selected_fix_option"] is None assert result is state # Should return current_state unchanged diff --git a/tests/unit/orchestrator/test_worker_prd_pr.py b/tests/unit/orchestrator/test_worker_prd_pr.py index 8c2f6f5a8..b05ba72ac 100644 --- a/tests/unit/orchestrator/test_worker_prd_pr.py +++ b/tests/unit/orchestrator/test_worker_prd_pr.py @@ -236,6 +236,7 @@ def worker(): w = OrchestratorWorker.__new__(OrchestratorWorker) w._post_terminal_error_comment = AsyncMock() w._post_resume_ack_comment = AsyncMock() + w.effect_service = MagicMock(execute_required=AsyncMock()) w._forge_github_logins = {} return w @@ -322,6 +323,7 @@ async def test_pr_merge_sets_approved(self, worker): state = _prd_gate_state( automated_review_revision_count=3, automated_review_revision_pending=True, + prd_content="# PRD", ) with patch("forge.orchestrator.worker.JiraClient") as MockJira: @@ -335,7 +337,12 @@ async def test_pr_merge_sets_approved(self, worker): assert result["is_paused"] is False assert result["automated_review_revision_count"] == 0 assert result["automated_review_revision_pending"] is False - mock_jira.set_workflow_label.assert_called_once() + commands = [call.args[0] for call in worker.effect_service.execute_required.await_args_list] + assert [command.operation for command in commands] == [ + "jira.label.set", + "jira.description.update", + ] + mock_jira.set_workflow_label.assert_not_called() @pytest.mark.asyncio async def test_pr_close_without_merge_is_ignored(self, worker): diff --git a/tests/unit/orchestrator/test_worker_spec_pr.py b/tests/unit/orchestrator/test_worker_spec_pr.py index 62120f8bd..342226fff 100644 --- a/tests/unit/orchestrator/test_worker_spec_pr.py +++ b/tests/unit/orchestrator/test_worker_spec_pr.py @@ -236,6 +236,7 @@ def worker(): w = OrchestratorWorker.__new__(OrchestratorWorker) w._post_terminal_error_comment = AsyncMock() w._post_resume_ack_comment = AsyncMock() + w.effect_service = MagicMock(execute_required=AsyncMock()) w._forge_github_logins = {} return w @@ -272,12 +273,17 @@ async def test_pr_merge_uses_configured_custom_field_storage(self, worker): result = await worker._handle_resume_event(msg, state) assert result["is_paused"] is False - mock_jira.set_workflow_label.assert_called_once() - mock_jira.update_custom_field.assert_called_once_with( - "TEST-123", - "customfield_12345", - "# Spec", - ) + commands = [call.args[0] for call in worker.effect_service.execute_required.await_args_list] + assert [command.operation for command in commands] == [ + "jira.label.set", + "jira.custom_field.update", + ] + assert commands[1].payload == { + "field": "customfield_12345", + "value": "# Spec", + } + mock_jira.set_workflow_label.assert_not_called() + mock_jira.update_custom_field.assert_not_called() mock_jira.add_structured_comment.assert_not_called() mock_jira.add_attachment.assert_not_called() diff --git a/tests/unit/workflow/test_ci_gate_skip.py b/tests/unit/workflow/test_ci_gate_skip.py index fcb02f0e1..256f97e06 100644 --- a/tests/unit/workflow/test_ci_gate_skip.py +++ b/tests/unit/workflow/test_ci_gate_skip.py @@ -233,7 +233,9 @@ class TestPostSkipGateFeedback: @pytest.mark.asyncio async def test_posts_github_reply_and_jira_comment(self): """Posts a GitHub PR comment and a Jira audit comment.""" - worker = OrchestratorWorker(consumer_name="test") + effects = MagicMock() + effects.execute_required = AsyncMock() + worker = OrchestratorWorker(consumer_name="test", effect_service=effects) repo_ref = RepositoryRef( id="org/repo", @@ -243,32 +245,22 @@ async def test_posts_github_reply_and_jira_comment(self): default_branch="main", change_request_mode="fork", ) - mock_adapter = AsyncMock() - - mock_jira = MagicMock() - mock_jira.add_comment = AsyncMock() - mock_jira.close = AsyncMock() - - with ( - patch("forge.orchestrator.worker.get_adapter", return_value=(repo_ref, mock_adapter)), - patch("forge.orchestrator.worker.JiraClient", return_value=mock_jira), - ): - await worker._post_skip_gate_feedback( - ticket_key="TEST-123", - repo_ref=repo_ref, - pr_number=42, - check_name="epoxy", - sender="eshulman2", - action="skip", - ) - - mock_adapter.create_comment.assert_called_once() - mock_jira.add_comment.assert_called_once() + await worker._post_skip_gate_feedback( + ticket_key="TEST-123", + repo_ref=repo_ref, + pr_number=42, + check_name="epoxy", + sender="eshulman2", + action="skip", + ) + assert effects.execute_required.await_count == 2 @pytest.mark.asyncio async def test_unskip_posts_different_message(self): """Unskip action produces a different confirmation message.""" - worker = OrchestratorWorker(consumer_name="test") + effects = MagicMock() + effects.execute_required = AsyncMock() + worker = OrchestratorWorker(consumer_name="test", effect_service=effects) repo_ref = RepositoryRef( id="org/repo", @@ -278,26 +270,15 @@ async def test_unskip_posts_different_message(self): default_branch="main", change_request_mode="fork", ) - mock_adapter = AsyncMock() - - mock_jira = MagicMock() - mock_jira.add_comment = AsyncMock() - mock_jira.close = AsyncMock() - - with ( - patch("forge.orchestrator.worker.get_adapter", return_value=(repo_ref, mock_adapter)), - patch("forge.orchestrator.worker.JiraClient", return_value=mock_jira), - ): - await worker._post_skip_gate_feedback( - ticket_key="TEST-123", - repo_ref=repo_ref, - pr_number=42, - check_name="epoxy", - sender="eshulman2", - action="unskip", - ) - - comment = mock_adapter.create_comment.call_args[0][2] + await worker._post_skip_gate_feedback( + ticket_key="TEST-123", + repo_ref=repo_ref, + pr_number=42, + check_name="epoxy", + sender="eshulman2", + action="unskip", + ) + comment = effects.execute_required.await_args_list[0].args[0].payload["body"] assert "unskip" in comment.lower() or "removed" in comment.lower() diff --git a/tests/unit/workflow/test_effect_runtime.py b/tests/unit/workflow/test_effect_runtime.py new file mode 100644 index 000000000..cbc5b1a8d --- /dev/null +++ b/tests/unit/workflow/test_effect_runtime.py @@ -0,0 +1,34 @@ +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from forge.models.workflow import ForgeLabel +from forge.workflow.effect_runtime import JiraClient + + +async def _set_same_label(client: JiraClient) -> None: + await client.set_workflow_label("FORGE-1", ForgeLabel.BLOCKED) + + +@pytest.mark.asyncio +async def test_local_workflow_write_is_journalled_and_deduplicated() -> None: + provider = MagicMock() + provider.set_workflow_label = AsyncMock() + provider.close = AsyncMock() + with patch("forge.workflow.effect_runtime.ProviderJiraClient", return_value=provider): + client = JiraClient() + await _set_same_label(client) + await _set_same_label(client) + + provider.set_workflow_label.assert_awaited_once_with("FORGE-1", ForgeLabel.BLOCKED.value) + + +@pytest.mark.asyncio +async def test_failed_required_write_does_not_look_successful() -> None: + provider = MagicMock() + provider.set_workflow_label = AsyncMock(side_effect=TimeoutError("provider unavailable")) + provider.close = AsyncMock() + with patch("forge.workflow.effect_runtime.ProviderJiraClient", return_value=provider): + client = JiraClient() + with pytest.raises(Exception, match="Required effect"): + await _set_same_label(client)