From 4856ad9f3461dd290d12e8e07eeb00a0e977db9e Mon Sep 17 00:00:00 2001 From: ProtocolWarden Date: Tue, 7 Jul 2026 08:12:18 -0400 Subject: [PATCH] fix(execution): push failure fails the task; success comments state what shipped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first live autonomy-lane task reached In Review with no pushed branch: finalize swallowed push failures (warning, success unchanged) and the workspace tempdir is deleted on return, so the work product was silently lost with "Implementation complete — moved to In Review" on the board. Push failure now fails the task as backend_error (transient- retryable); no-new-commits remains a success but sets branch_pushed False explicitly; goal-task comments name the pushed branch and PR, or state "no branch pushed — nothing to review". Co-Authored-By: Claude Fable 5 --- .console/log.md | 13 +++++++++ .../entrypoints/board_worker/dispatch.py | 10 +++---- .../entrypoints/board_worker/outcomes.py | 19 +++++++++++-- src/operations_center/execution/workspace.py | 25 +++++++++++++++-- .../board_worker/test_outcomes_cov.py | 25 +++++++++++++++++ tests/unit/execution/test_workspace_cov.py | 28 +++++++++++++++++-- 6 files changed, 107 insertions(+), 13 deletions(-) diff --git a/.console/log.md b/.console/log.md index d4b067b3..cd7105be 100644 --- a/.console/log.md +++ b/.console/log.md @@ -1,3 +1,16 @@ +## 2026-07-07 — fix(execution): push failure fails the task; comments state what shipped + +First autonomy-lane task (73b1e016, 58min, status=succeeded) reached In +Review with NO pushed branch and no evidence — the ephemeral workspace is +deleted on return, and WorkspaceManager.finalize swallowed push failures +(warning + success unchanged) and let "no new commits" claim "Implementation +complete". Now: push failure → FAILED/backend_error (transient-retryable, so +network blips hit the existing retry); no-new-commits stays a success but +branch_pushed=False is explicit; handle_success comments name the pushed +branch + PR, or say "no branch pushed — nothing to review". Rewrote +test_finalize_push_failure_is_nonfatal (it encoded the defect) + 3 new tests. +(C29: condensed two dispatch.py comments — the new kwargs tipped it to 501.) + ## 2026-07-07 — docs(config): document task_admission in the example config trusted_label_authors (Track A1) was configurable but undocumented in diff --git a/src/operations_center/entrypoints/board_worker/dispatch.py b/src/operations_center/entrypoints/board_worker/dispatch.py index f354d5f4..f05c75d6 100644 --- a/src/operations_center/entrypoints/board_worker/dispatch.py +++ b/src/operations_center/entrypoints/board_worker/dispatch.py @@ -306,9 +306,8 @@ def dispatch_issue( and result.get("failure_category") == "scope_too_wide" ) - # Track A8: out-of-process scope verification. The executor's own scope - # gates run INSIDE the sandbox; the parent re-diffs the PUSHED branch - # from the remote (the committed tree) against the declared allowlist. + # Track A8: out-of-process scope verification — the parent re-diffs the + # PUSHED branch from the remote against the declared allowlist. if success and not scope_too_wide: scope_error = verify_pushed_scope( bundle=bundle, @@ -337,9 +336,10 @@ def dispatch_issue( settings, improve_suggestions=improve_suggestions, pr_url=result.get("pull_request_url") or None, + branch_pushed=bool(result.get("branch_pushed")), + branch_name=result.get("branch_name") or None, ) - # F1: populate the durable lineage tier on a real completion, so the - # read-model has an actual producer (the writer lives with the tier). + # F1: the durable lineage tier gets its producer on a real completion. from operations_center.lineage.durable import record_task_completion record_task_completion(oc_root, task_id, result) diff --git a/src/operations_center/entrypoints/board_worker/outcomes.py b/src/operations_center/entrypoints/board_worker/outcomes.py index 2409a6d9..ae3fbc0e 100644 --- a/src/operations_center/entrypoints/board_worker/outcomes.py +++ b/src/operations_center/entrypoints/board_worker/outcomes.py @@ -110,6 +110,8 @@ def handle_success( *, improve_suggestions: list[dict] | None = None, pr_url: str | None = None, + branch_pushed: bool = False, + branch_name: str | None = None, ) -> None: task_id = str(issue["id"]) labels = issue.get("labels", []) @@ -122,6 +124,15 @@ def handle_success( try: if role == "goal": + # Claims-vs-reality: say what was actually delivered. A goal task + # can succeed with nothing pushed (no new commits — already done / + # analysis outcome); the comment must not imply a reviewable branch. + if branch_pushed and branch_name: + delivered = f"branch `{branch_name}` pushed" + if pr_url: + delivered += f", PR: {pr_url}" + else: + delivered = "no branch pushed — no new commits, nothing to review" if needs_verification: follow_id = create_follow_up( client, @@ -132,17 +143,19 @@ def handle_success( ) client.comment_issue( task_id, - f"Implementation complete — created verification task #{follow_id}", + f"Implementation complete ({delivered}) — created verification task #{follow_id}", ) client.transition_issue(task_id, STATE_DONE) elif await_review: client.transition_issue(task_id, STATE_REVIEW) if pr_url: add_label(client, issue, f"pr-url: {pr_url}") - client.comment_issue(task_id, "Implementation complete — moved to In Review") + client.comment_issue( + task_id, f"Implementation complete ({delivered}) — moved to In Review" + ) else: client.transition_issue(task_id, STATE_DONE) - client.comment_issue(task_id, "Implementation complete") + client.comment_issue(task_id, f"Implementation complete ({delivered})") elif role == "test": client.transition_issue(task_id, STATE_DONE) diff --git a/src/operations_center/execution/workspace.py b/src/operations_center/execution/workspace.py index fcb537f6..5fc2f516 100644 --- a/src/operations_center/execution/workspace.py +++ b/src/operations_center/execution/workspace.py @@ -466,7 +466,10 @@ def finalize( request.task_branch, request.base_branch, ) - return result + # Success is preserved (a legitimate "already done / analysis only" + # outcome), but make the no-push explicit so downstream messaging + # doesn't claim a deliverable that was never pushed. + return result.model_copy(update={"branch_pushed": False}) # Squash all stage commits into one before pushing (ADR 0009 P4). # Rewritten history requires force-push; single-commit branches use @@ -508,8 +511,26 @@ def finalize( else: self._git.push_branch(ws, request.task_branch) except Exception as exc: + # The workspace is ephemeral: an unpushed work product is LOST when + # dispatch tears the tempdir down. Reporting success here let tasks + # reach In Review with nothing to review (claims-vs-reality gap). + # Fail the task instead — backend_error is transient-retryable, so + # network blips get the existing retry machinery. logger.warning("WorkspaceManager.finalize: push failed — %s", exc) - return result + return result.model_copy( + update={ + "status": ExecutionStatus.FAILED, + "success": False, + "branch_pushed": False, + "failure_category": FailureReasonCategory.BACKEND_ERROR, + "failure_reason": ( + f"push of {request.task_branch} failed after execution " + f"succeeded: {exc}. The ephemeral workspace is deleted on " + "return, so the work product would be lost — failing the " + "task so it retries." + ), + } + ) # Cross-repo impact warning: if the changed files touch any other # repo's declared impact_report_paths, log the affected neighbours diff --git a/tests/unit/entrypoints/board_worker/test_outcomes_cov.py b/tests/unit/entrypoints/board_worker/test_outcomes_cov.py index bd595987..b7bf47b3 100644 --- a/tests/unit/entrypoints/board_worker/test_outcomes_cov.py +++ b/tests/unit/entrypoints/board_worker/test_outcomes_cov.py @@ -175,6 +175,31 @@ def test_handle_success_goal_plain_done(): assert any("Implementation complete" in c.args[1] for c in client.comment_issue.mock_calls) +def test_handle_success_goal_comment_names_pushed_branch(): + client = _make_client() + issue = {"id": "g5", "labels": [{"name": "repo: web"}]} + settings = _make_settings({"web": _make_repo_cfg(await_review=True)}) + outcomes.handle_success( + client, issue, "goal", "goal", False, settings, + pr_url="http://pr/2", branch_pushed=True, branch_name="goal/abc12345", + ) + comments = [c.args[1] for c in client.comment_issue.mock_calls] + assert any("goal/abc12345" in c and "http://pr/2" in c for c in comments) + + +def test_handle_success_goal_comment_says_nothing_pushed(): + """Claims-vs-reality: In Review with no pushed branch must say so, not + imply a reviewable deliverable exists.""" + client = _make_client() + issue = {"id": "g6", "labels": [{"name": "repo: web"}]} + settings = _make_settings({"web": _make_repo_cfg(await_review=True)}) + outcomes.handle_success( + client, issue, "goal", "goal", False, settings, branch_pushed=False, + ) + comments = [c.args[1] for c in client.comment_issue.mock_calls] + assert any("no branch pushed" in c for c in comments) + + def test_handle_success_goal_no_repo_key(): client = _make_client() issue = {"id": "g5", "labels": []} diff --git a/tests/unit/execution/test_workspace_cov.py b/tests/unit/execution/test_workspace_cov.py index 0179c75b..9a1a26f5 100644 --- a/tests/unit/execution/test_workspace_cov.py +++ b/tests/unit/execution/test_workspace_cov.py @@ -314,7 +314,8 @@ def test_finalize_no_new_commits_returns_early(tmp_path): mock.patch.object(mgr, "_has_new_commits", return_value=False), ): out = mgr.finalize(req, result) - assert out is result + assert out.success is True + assert out.branch_pushed is False # explicit: nothing pushed git.commit_all.assert_called_once() git.push_branch.assert_not_called() @@ -376,7 +377,10 @@ def test_finalize_push_force_when_squashed(tmp_path): assert out.pull_request_url is None -def test_finalize_push_failure_is_nonfatal(tmp_path): +def test_finalize_push_failure_fails_the_task(tmp_path): + """Claims-vs-reality: the workspace is deleted on return, so an unpushed + work product is lost — a failed push must FAIL the task (transient- + retryable), not report success with nothing to review.""" ws = _git_repo(tmp_path) git = _finalize_git_ok() git.squash_commits.return_value = False @@ -389,8 +393,26 @@ def test_finalize_push_failure_is_nonfatal(tmp_path): mock.patch.object(mgr, "_has_new_commits", return_value=True), ): out = mgr.finalize(req, result) - assert out is result + assert out.success is False + assert out.branch_pushed is False + assert out.failure_category == FailureReasonCategory.BACKEND_ERROR + assert "push of goal/fix-widget failed" in (out.failure_reason or "") + + +def test_finalize_no_new_commits_reports_branch_not_pushed(tmp_path): + ws = _git_repo(tmp_path) + git = _finalize_git_ok() + mgr = WorkspaceManager(git_client=git) + req = _make_request(ws) + result = _make_result(success=True) + with ( + mock.patch.object(mgr, "_diff_oversized", return_value=None), + mock.patch.object(mgr, "_has_new_commits", return_value=False), + ): + out = mgr.finalize(req, result) + assert out.success is True # legitimate already-done/analysis outcome assert out.branch_pushed is False + git.push_branch.assert_not_called() # ── _diff_oversized ──────────────────────────────────────────────────────────