mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
* [943d8c4d] Frontend data freshness and approval-queue reliability audit (#631)
* [233a8b0f] WebSocket reconnect message-loss audit and fix (#625)
* [233a8b0f] fix(panel): add REST catch-up to useNotificationStream on WS reconnect
connection.ts has no message buffering/replay, so a notification published
while the CEO bell's socket was down (disconnected/reconnecting) was lost
forever instead of merely delayed. Add a reconnect-triggered GET
/notifications?unread_only=true catch-up folded into the existing
notification_id dedup so a notification delivered both via catch-up and
live WS is never double-counted, and make clearMessages drop the held
catch-up batch too. use-a2a-live.ts and use-rate-limit-websocket.ts were
audited and already have working reconnect-triggered REST fallbacks
(verified via a2a/page.tsx, rate-limit-banner.tsx, usage-overview-panel.tsx
and their existing F083 tests) so no fix was needed there.
* [233a8b0f] docs(panel): add comprehensive WebSocket hooks reference and reconnect architecture guide
Add panel/docs/frontend/hooks.md with full API reference for useWebSocket, useNotificationStream (with new REST catch-up behavior), useAgentStream, useA2ALiveStream, and useConnectionStatus. Include examples, best practices, and testing guidance.
Add panel/docs/architecture/websocket-reconnect.md documenting the message-loss mitigation pattern: Strategy 1 (REST catch-up for events, used by useNotificationStream) and Strategy 2 (REST invalidation for state, used by A2A/rate-limit consumers), plus the dedup logic ensuring no notification is double-counted on reconnect.
---------
Co-authored-by: Frontend Developer 1 <fe-dev-1@roboco.tech>
Co-authored-by: Frontend Documenter <fe-doc@roboco.tech>
* [d5315683] fix(frontend): add distinct toast feedback for silently-swallowed x-post and release-proposal statuses, plus regression tests for all 4 approval queues (#626)
Co-authored-by: Frontend Developer 2 <fe-dev-2@roboco.tech>
* [cd953838] Data-hook null-guard audit and API client 429 retry-by-method fix (#630)
* [cd953838] fix(panel): gate 429 retry by HTTP method, add hook null-guard regression tests
* [cd953838] chore(conventions): waive test-fixture wrapper in hooks null-guard test
* [cd953838] docs(frontend): document API rate-limit retry behavior and null-guard audit results
Added `docs/frontend/api-rate-limiting.md` to document the 429 retry strategy: GET/PUT auto-retry, POST/PATCH/DELETE require X-Idempotency-Key header. Updated `docs/frontend/hooks.md` to confirm the data-hook null-guard audit found all hooks already have correct `enabled` guards and include a regression test suite for the board-review poll on/off behavior and enabled-guard assertions.
---------
Co-authored-by: Frontend Developer 1 <fe-dev-1@roboco.tech>
Co-authored-by: Frontend Documenter <fe-doc@roboco.tech>
---------
Co-authored-by: Frontend Developer 1 <fe-dev-1@roboco.tech>
Co-authored-by: Frontend Documenter <fe-doc@roboco.tech>
Co-authored-by: Frontend Developer 2 <fe-dev-2@roboco.tech>
* [4534c71a] Backend concurrency, state-machine, and engine audit (#634)
* [41de844a] fix(lifecycle): sync CLAIM_RULES with runtime + clear stale claimant on PM hand-off (#627)
Two confirmed state-machine gaps found while auditing lifecycle.py,
task_lifecycle.py, the _ESCALATABLE_TO_BLOCKED bypass, and every
_REVIEW_QUEUE_STATES entry point:
- lifecycle.py's CLAIM_RULES/claim-ActionSpec/StatusTransition table
did not grant CELL_PM/MAIN_PM re-claim of AWAITING_PM_REVIEW even
though task.py's runtime _ROLE_CLAIM_STATUSES already granted it
and claimed the spec agreed -- the two tables had silently drifted,
breaking i_will_plan re-claim on an awaiting_pm_review task.
- docs_complete's _maybe_advance_to_pm_review pre-assigns a specific
owning PM via assigned_to but left claimed_by/active_claimant_id
pointing at the outgoing documenter, unlike every sibling transition
into a review-queue state. A stale active_claimant_id makes
content_actions.py's _active_claim_violation wrongly reject the
newly-assigned PM's own content writes before it formally claims.
Reassign claimed_by + active_claimant_id to the owning PM alongside
assigned_to.
Adds a regression test asserting the documenter's stale claim does not
survive the docs_complete -> awaiting_pm_review hand-off.
Co-authored-by: Backend Developer 2 <be-dev-2@roboco.tech>
* [0c46f666] Engine dedup race + sequencing.py edge-case audit (#628)
* [0c46f666] fix(sequencing): dedup race audit + collision-edge fallback bug
Audited the list-open-then-originate dedup pattern across six engines:
RoadmapEngine, XEngine.run_cycle, DepUpdateEngine, and CIWatchEngine each
run inside exactly one sequential orchestrator-loop asyncio task (no other
call site invokes run_cycle), so they cannot race with themselves; their
in-cycle dedup sets/keys are correctly built before any commit. SelfHealEngine
is the same shape. VideoEngine.open_video_task is genuinely different: it is
reachable from the release-publish hook, the feature-spotlight hook, and the
on-demand POST /video/request route, so two overlapping calls for the same
occasion can both pass the "no open task yet" check before either commits.
Fixed by wrapping the check+insert in a short-lived Redis mutex (reusing
HeartbeatMutex) keyed by occasion, mirroring XPostService's existing
lock pattern, with a regression test proving only one of two concurrent
calls creates a task.
Verified ReleaseExecutor's half-landed retry path (release_commit_sha):
apply_version_bumps and write_changelog_entry both run as uncommitted
working-tree edits before commit_and_push's single `git add -A` + commit,
so a bumped-version-without-changelog state can never reach origin (and
therefore can never be observed by a fresh retry clone) - confirmed correct
with a real-git-repo regression test, no fix needed.
Fixed sequencing.py's dev_task_collision_edges: the `if edges: return edges`
short-circuit dropped the same-assignee-lane fallback entirely whenever ANY
surfaced sibling pair produced a collision edge, even for a completely
unrelated same-assignee pair with no declared surface. Now the fallback
always runs, skipping only pairs the analyzer already ordered (so the two
mechanisms can never disagree on direction for the same pair).
Verified sequencing.py rule 3 (all-shared batch generates no edges): correct
by inspection (_shared_last_edges skips every pair when both are shared) and
confirmed with a regression test - no fix needed.
* [0c46f666] docs(reference): concurrency audit summary - engine races, fixes, verified patterns
---------
Co-authored-by: Backend Developer 2 <be-dev-2@roboco.tech>
Co-authored-by: Backend Documenter <be-doc@roboco.tech>
* [8f7f167a] Redis mutex pre-lock write audit (#629)
* [8f7f167a] Redis mutex pre-lock write audit: add cross-session regression test for XPostService.approve
Audited x_post_service.py, video_post_service.py, release_proposal.py, and
heartbeat_mutex.py for the pre-lock DB-write anti-pattern (a session write
that happens before the SET NX / HeartbeatMutex acquire returns a token,
letting a losing racer's stale write clobber a winner's committed state).
XPostService.approve, VideoPostService.approve, and
ReleaseProposalService.approve/reject already implement the correct
validate-pure-pre-lock, apply-under-lock pattern (the XPostService fix
already shipped per CHANGELOG.md: "X edited_body write deferred into the
single-flight lock (M5)"). HeartbeatMutex holds no AsyncSession at all, so
the anti-pattern is structurally inapplicable there.
Adds a genuine cross-session concurrency regression test to
test_x_post_service.py (a real second DB connection, not an in-process
mock) mirroring VideoPostService's existing cross-session test, proving a
concurrently-committed post survives and the CEO's edited body never lands
on the just-posted row.
* [8f7f167a] Remove redundant inline comments flagged by QA in cross-session regression test
Both comments restated what the surrounding docstrings already say
explicitly, per QA findings F-dbadd8f0 (line 294) and F-27ac051e (line
631) — no behavior change, tests re-verified green against a sandbox
Postgres.
* [8f7f167a] Remove inline trailing comments flagged by QA (correct file this time)
QA findings F-e6f3e6a6 and F-24189858 cited tests/unit/services/
test_x_post_service.py:294 and :631 across 5 revision rounds, but that
file never contained the flagged comment text — a repo-wide grep for
the exact quoted strings shows both comments actually live in the
mirrored tests/unit/services/test_video_post_service.py file, in its
own cross-session concurrency regression tests (the caption-edit and
tiktok-skip tests). Removed both there:
- "# externally visible to the "concurrent" session below" on the
db_session.commit() call
- "# never attempted without credentials" on the tiktok_poster.calls
assertion
Both restated what the surrounding docstrings/test names already say;
no behavior change. Verified with the full make quality gate against a
sandbox Postgres/Redis: 13,717 passed, 94.41% coverage, clean except
one pre-existing unrelated failure in tests/unit/api/test_cloud_auth.py
::test_login_route_parses_oauth2_form_not_query_params, which connects
to the app's default localhost:5432 Postgres (not the db_session
sandbox fixture) and is unreachable in this sandboxed environment —
structurally unrelated to the auth subsystem this task never touches.
* [8f7f167a] Redis mutex pre-lock write audit (round 7): add cross-session regression tests for reject() lock protection
Round-7 QA findings F-7eb9fbcb, F-06f39a2e, and F-4d56e49b claim
XPostService.reject(), ReleaseProposalService.reject(), and
release_executor._await_proc() lack lock protection / a CancelledError
handler — but their cited line ranges (255-267, 429-454, 241-257)
describe a pre-fix, shorter version of these functions that predates
commit fb293a787d, already on this branch. At current HEAD:
- x_post_service.py reject() (lines 275-299) acquires _LOCK_PREFIX,
re-reads under the lock, applies markers.set_x_reject_reason() +
CANCELLED only inside the critical section, releases in finally.
- release_proposal.py reject() (lines 460-486) does the identical
dance with _RELEASE_LOCK_PREFIX.
- release_executor.py _await_proc() (lines 257-265) already has an
`except asyncio.CancelledError` block that kills + reaps the child
and re-raises, mirroring the TimeoutError handler, with an existing
dedicated regression test
(test_await_proc_kills_child_on_outer_cancellation).
The one genuine gap: neither reject() path had a cross-session
(real second DB connection, not an in-process mock) regression test
proving the in-lock re-read catches a concurrent approve/publish that
completes mid-lock-wait — only approve() had one. Added
test_reject_concurrent_approve_completes_during_lock_wait to both
test_x_post_service.py and test_release_proposal_status_guards.py,
mirroring the existing approve() cross-session test: a second engine
commits COMPLETED between reject's pre-lock read and lock acquisition,
and the test asserts the CANCELLED write / reject-reason marker never
lands on the just-completed row.
No production code changed — verified via 103 targeted tests green
against a sandbox Postgres/Redis, plus `make -o sync gate` clean.
* [8f7f167a] Regenerate stale lifecycle artifacts (restore auditor waive_finding)
foundation-check was the only failing gate: the committed lifecycle artifacts
were missing the auditor's waive_finding verb that the lifecycle source
defines, so make quality regenerated them and failed on the diff — nothing to
do with the mutex fix (which passes ruff/mypy/tests/coverage/bandit clean).
make lifecycle restores the drift; this is what the 8 revision rounds kept
missing.
---------
Co-authored-by: Backend Developer 1 <be-dev-1@roboco.tech>
Co-authored-by: Renn F <rennf93@users.noreply.github.com>
* [d615f2e3] fix(tests): sync stale CLAIM_RULES pinning assertions with lifecycle.py (#635)
test_claim_rules_match_pre_gateway_table still asserted the pre-audit
two-member frozenset for CELL_PM/MAIN_PM claim rules. CLAIM_RULES in
lifecycle.py already grants both roles claim rights on
Status.AWAITING_PM_REVIEW (added by the state-machine exhaustiveness
audit) so a PM can re-claim its own review-queue task after a respawn.
Updated both assertions to include AWAITING_PM_REVIEW, matching the
actual dict. Grepped the repo for sibling stale copies of the old
literal; found none beyond this test.
Co-authored-by: Backend Developer 1 <be-dev-1@roboco.tech>
* [3c4e7a35] fix(quality-gate): reflow CONCURRENCY_AUDIT.md and stub occasion lock in video tests (#636)
Root cause: PR #634's CI failed at the markdown-reflow check (make quality
Makefile:285) on CONCURRENCY_AUDIT.md — a hard-wrapped audit doc left over
from the merged "Engine dedup race + sequencing.py edge-case audit" unit
(PR #628). Fixed with `make reflow-docs` (the exact remedy the CI output
itself named).
Running the full local `make quality` (with a sandbox Postgres/Redis to
get past DB-gated skips) surfaced a second real regression from that same
PR #628 unit: it added a Redis-backed HeartbeatMutex occasion lock to
VideoEngine.open_video_task, but two pre-existing test files
(tests/unit/runtime/test_video_render_loop.py and
tests/integration/test_video_routes.py) call open_video_task without
stubbing that lock, so they failed closed against the suite's
deliberately-unreachable test Redis (_no_live_redis). Fixed by applying
the same lock-stub pattern tests/unit/services/test_video_engine.py
already uses for its own occasion-lock tests: an autouse HeartbeatMutex
stand-in fixture in test_video_render_loop.py, and wrapping the two
route-level video-request tests in test_video_routes.py with the file's
existing _LOCKED patch pair (already used by every other lock-dependent
test in that file).
The one remaining local failure,
test_cloud_auth.py::test_login_route_parses_oauth2_form_not_query_params,
is a pre-existing environment gap unrelated to this branch: it needs a
real Postgres reachable at localhost:5432 (which .github/workflows/ci.yml
provides as a service container) but this dev sandbox has no such binding
— confirmed unrelated to any of the four merged audit units.
make quality now passes clean: 13729 passed, 0 regressions, 94.49% coverage.
Co-authored-by: Backend Developer 1 <be-dev-1@roboco.tech>
* [ddc8121f] regenerate lifecycle artifacts for awaiting_pm_review claim rules and waive_finding intent (#637)
Co-authored-by: Backend Developer 1 <be-dev-1@roboco.tech>
---------
Co-authored-by: Backend Developer 2 <be-dev-2@roboco.tech>
Co-authored-by: Backend Documenter <be-doc@roboco.tech>
Co-authored-by: Backend Developer 1 <be-dev-1@roboco.tech>
Co-authored-by: Renn F <rennf93@users.noreply.github.com>
* fix(sequencing): drop lane-fallback edges that would cycle against analyzer edges
The dev-task collision fallback unioned the analyzer's authoritative edges
with same-assignee lane edges, deduping only the direct pair. A lane chain
through an unsurfaced middle sibling could still contradict an analyzer edge
transitively (the shared-last migration order inverts plain priority order),
closing a 3-cycle that made add_dependency raise ConflictError and wedged
every later delegate to that parent. Fallback edges are now accepted only
when they can't close a cycle against the edges already kept; a regression
test reproduces the exact scenario.
Also strip pre-merge cruft: remove the root CONCURRENCY_AUDIT.md working
report, delete the near-duplicate websocket-reconnect.md doc, fix the stale
a2a/page.tsx doc citation, correct the api-rate-limiting doc to state
idempotency-key retry is unimplemented, and fix two lifecycle.py comments
that referenced a guard function which never existed.
---------
Co-authored-by: Frontend Developer 1 <fe-dev-1@roboco.tech>
Co-authored-by: Frontend Documenter <fe-doc@roboco.tech>
Co-authored-by: Frontend Developer 2 <fe-dev-2@roboco.tech>
Co-authored-by: Backend Developer 2 <be-dev-2@roboco.tech>
Co-authored-by: Backend Documenter <be-doc@roboco.tech>
Co-authored-by: Backend Developer 1 <be-dev-1@roboco.tech>
Co-authored-by: Renn F <rennf93@users.noreply.github.com>
724 lines
26 KiB
Python
724 lines
26 KiB
Python
"""ReleaseExecutor: fail-closed bump → gate → commit → CI → publish (post-approval).
|
|
|
|
The executor's correctness is its ORDERING + fail-closed aborts: a red gate
|
|
aborts before any commit, a red release-commit CI aborts before publish, and a
|
|
green path publishes exactly once. Tested against a fake ops that records the
|
|
call sequence; the production git/gh ops is exercised live (CEO-gated).
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import base64
|
|
import subprocess
|
|
from types import SimpleNamespace
|
|
from typing import TYPE_CHECKING, Any, cast
|
|
from unittest.mock import MagicMock
|
|
|
|
import pytest
|
|
from roboco.config import settings
|
|
from roboco.services import release_executor as re
|
|
from roboco.services.release_executor import (
|
|
ReleaseExecutor,
|
|
ReleaseResult,
|
|
_GitReleaseOps,
|
|
_ReleaseContext,
|
|
_resolve_release_ci_workflow,
|
|
)
|
|
from roboco.services.release_readiness import ReleaseReadinessReport
|
|
|
|
if TYPE_CHECKING:
|
|
from pathlib import Path
|
|
|
|
_PLAN = ["pyproject.toml", "roboco/__init__.py", "CHANGELOG.md"]
|
|
_VERSION = "0.13.0"
|
|
_ONE = 1
|
|
|
|
|
|
def _report() -> ReleaseReadinessReport:
|
|
return ReleaseReadinessReport(
|
|
proposed_version=_VERSION,
|
|
bump_kind="minor",
|
|
change_summary=["feat: a thing"],
|
|
drafted_changelog=(
|
|
f"## [{_VERSION}] - 2026-06-25\n\n### Added\n- a thing (#1)\n"
|
|
),
|
|
version_bump_plan=list(_PLAN),
|
|
gaps=[],
|
|
migration_notes=[],
|
|
gate_state="green",
|
|
)
|
|
|
|
|
|
class _FakeOps:
|
|
"""Records the call sequence; flags drive gate/CI/already-published outcomes."""
|
|
|
|
def __init__(
|
|
self,
|
|
*,
|
|
already: bool = False,
|
|
gate: bool = True,
|
|
ci: bool = True,
|
|
commit_raises: str | None = None,
|
|
publish_raises: str | None = None,
|
|
):
|
|
self._already = already
|
|
self._gate = gate
|
|
self._ci = ci
|
|
self._commit_raises = commit_raises
|
|
self._publish_raises = publish_raises
|
|
# Half-landed (publish_failed retry) detection: a prior
|
|
# ``chore(release): {version}`` commit already on the branch. Set on the
|
|
# instance (not via __init__ — keeps the constructor under the arg-count
|
|
# gate) by tests that exercise the retry path.
|
|
self._existing_sha: str | None = None
|
|
# env-chain promotion failure message; set on the instance (same arg-
|
|
# count-gate reason) by the promotion-failure test.
|
|
self._promote_raises: str | None = None
|
|
self.calls: list[str] = []
|
|
self.bumped_plan: list[str] | None = None
|
|
self.bumped_version: str | None = None
|
|
self.halflanded_check = False
|
|
|
|
async def is_already_published(self, _version: str) -> bool:
|
|
self.calls.append("check")
|
|
return self._already
|
|
|
|
async def promote_env_chain(self) -> None:
|
|
self.calls.append("promote")
|
|
if self._promote_raises is not None:
|
|
raise RuntimeError(self._promote_raises)
|
|
|
|
async def release_commit_sha(self, _version: str) -> str | None:
|
|
# Half-landed detection: a prior `chore(release): {version}` commit
|
|
# already on the branch means a publish_failed retry must NOT re-run the
|
|
# bump→changelog→gate→commit pipeline. Recorded via a flag (not calls)
|
|
# so the green-path call-sequence assertion is unaffected.
|
|
self.halflanded_check = True
|
|
return self._existing_sha
|
|
|
|
async def apply_version_bumps(self, plan: list[str], new_version: str) -> list[str]:
|
|
self.calls.append("bump")
|
|
self.bumped_plan = list(plan)
|
|
self.bumped_version = new_version
|
|
return list(plan)
|
|
|
|
async def write_changelog_entry(self, _entry: str) -> None:
|
|
self.calls.append("changelog")
|
|
|
|
async def run_gate(self) -> tuple[bool, str]:
|
|
self.calls.append("gate")
|
|
return self._gate, "CI on slave@deadbeef is failure"
|
|
|
|
async def commit_and_push(self, _version: str) -> str:
|
|
self.calls.append("commit")
|
|
if self._commit_raises is not None:
|
|
raise RuntimeError(self._commit_raises)
|
|
return "deadbeef"
|
|
|
|
async def wait_for_ci(self, _commit_sha: str) -> bool:
|
|
self.calls.append("ci")
|
|
return self._ci
|
|
|
|
async def publish_release(self, version: str, _notes: str) -> str:
|
|
self.calls.append("publish")
|
|
if self._publish_raises is not None:
|
|
raise RuntimeError(self._publish_raises)
|
|
return f"https://github.com/x/roboco/releases/tag/v{version}"
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_green_path_publishes_once() -> None:
|
|
ops = _FakeOps()
|
|
result = await ReleaseExecutor(ops).execute(_report())
|
|
assert result.status == "published"
|
|
assert result.release_url is not None
|
|
assert result.commit_sha == "deadbeef"
|
|
assert ops.calls.count("publish") == _ONE
|
|
assert ops.calls == [
|
|
"check",
|
|
"promote",
|
|
"bump",
|
|
"changelog",
|
|
"gate",
|
|
"commit",
|
|
"ci",
|
|
"publish",
|
|
]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_bump_targets_the_canonical_set() -> None:
|
|
ops = _FakeOps()
|
|
result = await ReleaseExecutor(ops).execute(_report())
|
|
assert ops.bumped_plan == _PLAN
|
|
assert ops.bumped_version == _VERSION
|
|
assert result.files_changed == _PLAN
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_red_gate_aborts_before_commit() -> None:
|
|
ops = _FakeOps(gate=False)
|
|
result = await ReleaseExecutor(ops).execute(_report())
|
|
assert result.status == "gate_failed"
|
|
assert "commit" not in ops.calls
|
|
assert "publish" not in ops.calls
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_red_ci_aborts_before_publish() -> None:
|
|
ops = _FakeOps(ci=False)
|
|
result = await ReleaseExecutor(ops).execute(_report())
|
|
assert result.status == "ci_failed"
|
|
assert "commit" in ops.calls
|
|
assert "publish" not in ops.calls
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_already_published_is_a_noop() -> None:
|
|
ops = _FakeOps(already=True)
|
|
result = await ReleaseExecutor(ops).execute(_report())
|
|
assert result.status == "already_published"
|
|
assert "bump" not in ops.calls
|
|
assert "commit" not in ops.calls
|
|
assert "publish" not in ops.calls
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_commit_push_failure_returns_structured_commit_failed() -> None:
|
|
"""#88: a RuntimeError from commit_and_push (gpgsign/pre-commit/non-ff
|
|
push) becomes a structured ``commit_failed`` result — not a 500 bubbling
|
|
out of ``approve``. Fail-closed: publish never runs."""
|
|
ops = _FakeOps(commit_raises="release push failed: non-fast-forward")
|
|
result = await ReleaseExecutor(ops).execute(_report())
|
|
assert result.status == "commit_failed"
|
|
assert result.commit_sha is None
|
|
assert result.release_url is None
|
|
assert "commit_failed" in result.detail or "push failed" in result.detail
|
|
assert "publish" not in ops.calls
|
|
assert "ci" not in ops.calls
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_publish_failure_returns_structured_publish_failed() -> None:
|
|
"""#88: a RuntimeError from the GitHub release POST (auth/quota/network) becomes
|
|
a structured ``publish_failed`` result. The commit is already pushed and CI
|
|
is green, so the release is half-landed — the CEO can retry the publish
|
|
create`` for the same version (the executor is idempotent on the commit
|
|
side). No 500."""
|
|
ops = _FakeOps(publish_raises="release publish failed: HTTP 403: forbidden")
|
|
result = await ReleaseExecutor(ops).execute(_report())
|
|
assert result.status == "publish_failed"
|
|
assert result.commit_sha == "deadbeef"
|
|
assert result.release_url is None
|
|
assert "release publish failed" in result.detail
|
|
assert ops.calls.count("publish") == _ONE
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_promotion_failure_aborts_before_bump() -> None:
|
|
"""A RuntimeError from promote_env_chain (a merge conflict in the
|
|
head->...->prod chain) becomes a structured ``promotion_failed`` result —
|
|
fail-closed: the bump/changelog/gate/commit/publish pipeline never runs."""
|
|
ops = _FakeOps()
|
|
ops._promote_raises = "env-chain promotion failed: non-fast-forward"
|
|
result = await ReleaseExecutor(ops).execute(_report())
|
|
assert result.status == "promotion_failed"
|
|
assert result.commit_sha is None
|
|
assert result.release_url is None
|
|
assert "bump" not in ops.calls
|
|
assert "commit" not in ops.calls
|
|
assert "publish" not in ops.calls
|
|
|
|
|
|
def test_release_result_carries_outcome_fields() -> None:
|
|
result = ReleaseResult(
|
|
status="published",
|
|
version=_VERSION,
|
|
files_changed=list(_PLAN),
|
|
commit_sha="abc",
|
|
release_url="https://example/releases/v0.13.0",
|
|
detail="ok",
|
|
)
|
|
assert result.version == _VERSION
|
|
assert result.files_changed == _PLAN
|
|
assert result.release_url is not None
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_half_landed_retry_skips_bump_and_republishes_only() -> None:
|
|
"""#87: a publish_failed retry (commit pushed + CI green, no tag yet) must
|
|
NOT re-run bump/changelog/gate/commit — that would re-insert the changelog
|
|
entry above the already-present ``## [X.Y.Z]`` heading (duplicate) and land a
|
|
second ``chore(release): X.Y.Z`` commit. The executor detects the
|
|
half-landed state via ``release_commit_sha`` (a prior release commit already
|
|
on the branch) and jumps straight to wait_for_ci + publish."""
|
|
ops = _FakeOps()
|
|
ops._existing_sha = "existingbeef"
|
|
result = await ReleaseExecutor(ops).execute(_report())
|
|
assert result.status == "published"
|
|
assert result.commit_sha == "existingbeef"
|
|
assert result.release_url is not None
|
|
assert ops.halflanded_check is True
|
|
assert "bump" not in ops.calls
|
|
assert "changelog" not in ops.calls
|
|
assert "gate" not in ops.calls
|
|
assert "commit" not in ops.calls
|
|
assert ops.calls == ["check", "ci", "publish"]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_wait_for_ci_scoped_to_release_commit_not_branch_latest(
|
|
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
|
) -> None:
|
|
"""#318: a later commit landing on master during the ~40min wait must not
|
|
mask the release commit's green CI. ``wait_for_ci`` scopes the GitHub query
|
|
to the release commit_sha (``head_sha=``), so the branch-latest run (a later
|
|
sha) can't make the gate poll forever and false-fail as ci_failed."""
|
|
commit_sha = "release_commit_abc"
|
|
later_sha = "later_landed_def"
|
|
|
|
async def _fake_get_ci(_slug: str, **_kwargs: object) -> dict[str, str]:
|
|
# Mimic GitHub's head_sha filter: a run for the release sha only when
|
|
# asked for it (head_sha=commit_sha); the branch-latest (later commit)
|
|
# run otherwise. The release gate MUST scope to commit_sha to see green.
|
|
if _kwargs.get("head_sha") == commit_sha:
|
|
return {
|
|
"head_sha": commit_sha,
|
|
"conclusion": "success",
|
|
"run_url": "u",
|
|
"run_name": "n",
|
|
"branch": "master",
|
|
"completed_at": "t",
|
|
}
|
|
return {
|
|
"head_sha": later_sha,
|
|
"conclusion": "success",
|
|
"run_url": "u2",
|
|
"run_name": "n2",
|
|
"branch": "master",
|
|
"completed_at": "t2",
|
|
}
|
|
|
|
monkeypatch.setattr(
|
|
"roboco.services.git.get_git_service",
|
|
lambda _session: SimpleNamespace(get_latest_ci_conclusion=_fake_get_ci),
|
|
)
|
|
monkeypatch.setattr(re, "_CI_MAX_POLLS", 2)
|
|
|
|
async def _no_sleep(_secs: float) -> None:
|
|
return None
|
|
|
|
monkeypatch.setattr(re.asyncio, "sleep", _no_sleep)
|
|
|
|
ctx = _ReleaseContext(
|
|
slug="roboco-api",
|
|
prod_branch="master",
|
|
root=tmp_path,
|
|
git_url="x",
|
|
git_prefix=[],
|
|
ci_workflow="ci.yml",
|
|
env_chain=[],
|
|
)
|
|
ops = _GitReleaseOps(session=MagicMock(), ctx=ctx)
|
|
ok = await ops.wait_for_ci(commit_sha)
|
|
assert ok is True
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_wait_for_ci_polls_through_rerun(
|
|
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
|
) -> None:
|
|
"""A completed non-success conclusion on the release sha must not abort the
|
|
poll — a failed first attempt while a GitHub re-run is still in_progress
|
|
(excluded from the status=completed filter) can still flip the same
|
|
head_sha to success. Only ``conclusion == "success"`` returns True; loop
|
|
exhaustion returns False."""
|
|
commit_sha = "release_commit_abc"
|
|
seq = ["failure", "failure", "success"]
|
|
expected_polls = len(seq)
|
|
calls = {"n": 0}
|
|
|
|
async def _fake_get_ci(_slug: str, **kwargs: object) -> dict[str, object]:
|
|
i = min(calls["n"], len(seq) - 1)
|
|
calls["n"] += 1
|
|
return {
|
|
"head_sha": kwargs.get("head_sha", commit_sha),
|
|
"conclusion": seq[i],
|
|
"run_url": "u",
|
|
"run_name": "n",
|
|
"branch": "master",
|
|
"completed_at": "t",
|
|
}
|
|
|
|
monkeypatch.setattr(
|
|
"roboco.services.git.get_git_service",
|
|
lambda _session: SimpleNamespace(get_latest_ci_conclusion=_fake_get_ci),
|
|
)
|
|
monkeypatch.setattr(re, "_CI_MAX_POLLS", 5)
|
|
|
|
async def _no_sleep(_secs: float) -> None:
|
|
return None
|
|
|
|
monkeypatch.setattr(re.asyncio, "sleep", _no_sleep)
|
|
|
|
ctx = _ReleaseContext(
|
|
slug="roboco-api",
|
|
prod_branch="master",
|
|
root=tmp_path,
|
|
git_url="x",
|
|
git_prefix=[],
|
|
ci_workflow="ci.yml",
|
|
env_chain=[],
|
|
)
|
|
ops = _GitReleaseOps(session=MagicMock(), ctx=ctx)
|
|
ok = await ops.wait_for_ci(commit_sha)
|
|
assert ok is True
|
|
assert calls["n"] == expected_polls
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_wait_for_ci_exhausts_window_on_persistent_failure(
|
|
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
|
) -> None:
|
|
"""A definitive failure that never re-runs waits the full window then
|
|
returns False — keeps polling, never early-returns on non-success."""
|
|
commit_sha = "release_commit_abc"
|
|
max_polls = 3
|
|
calls = {"n": 0}
|
|
|
|
async def _fake_get_ci(_slug: str, **kwargs: object) -> dict[str, object]:
|
|
calls["n"] += 1
|
|
return {
|
|
"head_sha": kwargs.get("head_sha", commit_sha),
|
|
"conclusion": "failure",
|
|
"run_url": "u",
|
|
"run_name": "n",
|
|
"branch": "master",
|
|
"completed_at": "t",
|
|
}
|
|
|
|
monkeypatch.setattr(
|
|
"roboco.services.git.get_git_service",
|
|
lambda _session: SimpleNamespace(get_latest_ci_conclusion=_fake_get_ci),
|
|
)
|
|
monkeypatch.setattr(re, "_CI_MAX_POLLS", max_polls)
|
|
|
|
async def _no_sleep(_secs: float) -> None:
|
|
return None
|
|
|
|
monkeypatch.setattr(re.asyncio, "sleep", _no_sleep)
|
|
|
|
ctx = _ReleaseContext(
|
|
slug="roboco-api",
|
|
prod_branch="master",
|
|
root=tmp_path,
|
|
git_url="x",
|
|
git_prefix=[],
|
|
ci_workflow="ci.yml",
|
|
env_chain=[],
|
|
)
|
|
ops = _GitReleaseOps(session=MagicMock(), ctx=ctx)
|
|
ok = await ops.wait_for_ci(commit_sha)
|
|
assert ok is False
|
|
assert calls["n"] == max_polls
|
|
|
|
|
|
def test_release_ci_workflow_decoupled_from_self_heal_setting(
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
) -> None:
|
|
"""#402: the release CI gate must not inherit ``self_heal_ci_workflow``'s
|
|
empty-string tuning (documented valid for single-workflow repos), which would
|
|
degrade the fail-closed gate to the all-workflows mode git.py itself flags as
|
|
unreliable. The release gate always resolves a named workflow (default
|
|
``ci.yml``), never None."""
|
|
# The dangerous tuning an operator might apply for self-heal on a
|
|
# single-workflow repo — must NOT leak into the release gate.
|
|
monkeypatch.setattr(settings, "self_heal_ci_workflow", "")
|
|
monkeypatch.setattr(settings, "release_ci_workflow", "ci.yml")
|
|
assert _resolve_release_ci_workflow() == "ci.yml"
|
|
|
|
monkeypatch.setattr(settings, "release_ci_workflow", "release.yml")
|
|
assert _resolve_release_ci_workflow() == "release.yml"
|
|
|
|
# An empty release setting never falls through to None — always the default.
|
|
monkeypatch.setattr(settings, "release_ci_workflow", "")
|
|
assert _resolve_release_ci_workflow() == "ci.yml"
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# _GitReleaseOps.release_commit_sha — half-landed retry detection against a
|
|
# REAL git repo (not the fake ops above): verifies the exact worry the task
|
|
# named — that ``_current_version`` could return the bumped version while the
|
|
# changelog hasn't actually been written yet. It can't: ``apply_version_bumps``
|
|
# and ``write_changelog_entry`` both run as uncommitted working-tree edits
|
|
# BEFORE ``commit_and_push``'s single ``git add -A`` + commit, so nothing ever
|
|
# reaches origin (and therefore no fresh retry clone can ever observe it) with
|
|
# one written but not the other.
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def _git_sync(repo: Path, *args: str) -> str:
|
|
result = subprocess.run(
|
|
["git", "-C", str(repo), *args], check=True, capture_output=True, text=True
|
|
)
|
|
return result.stdout
|
|
|
|
|
|
def _init_release_repo(repo: Path, *, initial_version: str = "0.12.0") -> None:
|
|
repo.mkdir(parents=True, exist_ok=True)
|
|
_git_sync(repo, "init", "-b", "master")
|
|
_git_sync(repo, "config", "user.email", "t@example.com")
|
|
_git_sync(repo, "config", "user.name", "T")
|
|
_git_sync(repo, "config", "commit.gpgsign", "false")
|
|
(repo / "pyproject.toml").write_text(f'[project]\nversion = "{initial_version}"\n')
|
|
(repo / "CHANGELOG.md").write_text("# Changelog\n")
|
|
_git_sync(repo, "add", "-A")
|
|
_git_sync(repo, "commit", "-m", "init")
|
|
|
|
|
|
def _ops(session: object, root: Path) -> _GitReleaseOps:
|
|
ctx = _ReleaseContext(
|
|
slug="roboco-api",
|
|
prod_branch="master",
|
|
root=root,
|
|
git_url="x",
|
|
git_prefix=[],
|
|
ci_workflow="ci.yml",
|
|
env_chain=[],
|
|
)
|
|
return _GitReleaseOps(session=cast("Any", session), ctx=ctx)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_release_commit_sha_detects_a_real_half_landed_commit(
|
|
tmp_path: Path,
|
|
) -> None:
|
|
"""A genuine publish_failed retry: a prior execute already ran
|
|
``apply_version_bumps`` + ``write_changelog_entry`` + committed (both
|
|
landed in the SAME commit, exactly as ``commit_and_push`` does with one
|
|
``git add -A``). The fresh clone must detect that commit and its
|
|
changelog entry must already be present — never re-bump, never skip the
|
|
changelog."""
|
|
_init_release_repo(tmp_path)
|
|
ops = _ops(MagicMock(), tmp_path)
|
|
await ops.apply_version_bumps(["pyproject.toml"], "0.13.0")
|
|
await ops.write_changelog_entry(
|
|
"## [0.13.0] - 2026-07-21\n\n### Added\n- a thing\n"
|
|
)
|
|
_git_sync(tmp_path, "add", "-A")
|
|
_git_sync(tmp_path, "commit", "-m", "chore(release): 0.13.0")
|
|
|
|
sha = await ops.release_commit_sha("0.13.0")
|
|
|
|
assert sha is not None
|
|
assert sha == _git_sync(tmp_path, "rev-parse", "HEAD").strip()
|
|
# The changelog entry landed in the SAME commit as the bump — never split.
|
|
assert "0.13.0" in (tmp_path / "pyproject.toml").read_text()
|
|
assert "a thing" in (tmp_path / "CHANGELOG.md").read_text()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_release_commit_sha_none_for_uncommitted_bump_no_false_half_landed(
|
|
tmp_path: Path,
|
|
) -> None:
|
|
"""An uncommitted version bump (e.g. a crash between apply_version_bumps
|
|
and commit_and_push) must NOT be mistaken for a half-landed release —
|
|
only a matching COMMIT counts. A fresh retry clone starts from origin's
|
|
unchanged HEAD anyway (this isolates release_commit_sha's own check)."""
|
|
_init_release_repo(tmp_path)
|
|
ops = _ops(MagicMock(), tmp_path)
|
|
# Bump the working tree WITHOUT committing (write_changelog_entry never ran).
|
|
await ops.apply_version_bumps(["pyproject.toml"], "0.13.0")
|
|
|
|
sha = await ops.release_commit_sha("0.13.0")
|
|
|
|
assert sha is None # falls through to the normal bump/changelog/gate/commit path
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_release_commit_sha_none_when_version_not_yet_bumped(
|
|
tmp_path: Path,
|
|
) -> None:
|
|
"""No prior attempt at all: the clone is still at the old version, so
|
|
there is nothing half-landed to detect."""
|
|
_init_release_repo(tmp_path)
|
|
ops = _ops(MagicMock(), tmp_path)
|
|
|
|
assert await ops.release_commit_sha("0.13.0") is None
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# H11: the PAT must never appear in a git subprocess argv. The release clone
|
|
# and the release push carry the token via ``-c http.extraheader=Authorization:
|
|
# Basic <base64(x-access-token:TOKEN)>`` and a bare URL — never URL-embedded.
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def _basic_auth(token: str) -> str:
|
|
return base64.b64encode(f"x-access-token:{token}".encode()).decode()
|
|
|
|
|
|
class _DoneProc:
|
|
"""A subprocess that completes immediately with a fixed rc + stdout."""
|
|
|
|
def __init__(self, out: bytes = b"", returncode: int = 0) -> None:
|
|
self.returncode = returncode
|
|
self._out = out
|
|
|
|
async def communicate(self) -> tuple[bytes, bytes]:
|
|
return (self._out, b"")
|
|
|
|
def kill(self) -> None:
|
|
return None
|
|
|
|
async def wait(self) -> int:
|
|
return self.returncode
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_release_clone_argv_uses_extraheader_not_url_token(
|
|
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
|
) -> None:
|
|
"""H11: the release-clone argv carries the PAT via ``-c http.extraheader``,
|
|
never URL-embedded (``/proc/<pid>/cmdline`` would expose a URL token)."""
|
|
token = "ghp_SECRETCLONE"
|
|
git_url = "https://github.com/org/roboco.git"
|
|
expected_basic = _basic_auth(token)
|
|
git_prefix = ["-c", f"http.extraheader=Authorization: Basic {expected_basic}"]
|
|
captured: list[list[str]] = []
|
|
|
|
async def _exec(*args: str, **_kwargs: object) -> _DoneProc:
|
|
captured.append(list(args))
|
|
return _DoneProc()
|
|
|
|
monkeypatch.setattr(re.asyncio, "create_subprocess_exec", _exec)
|
|
monkeypatch.setattr(settings, "workspaces_root", str(tmp_path))
|
|
|
|
await re._prepare_release_clone("roboco-api", git_url, git_prefix, "master")
|
|
|
|
clone_argv = next(a for a in captured if "clone" in a)
|
|
assert f"https://{token}@" not in " ".join(clone_argv), (
|
|
f"raw token leaked into clone argv URL: {clone_argv}"
|
|
)
|
|
assert token not in clone_argv, f"raw token in clone argv: {clone_argv}"
|
|
assert git_url in clone_argv, f"bare git_url missing from clone argv: {clone_argv}"
|
|
assert "-c" in clone_argv
|
|
c_idx = clone_argv.index("-c")
|
|
assert (
|
|
clone_argv[c_idx + 1]
|
|
== f"http.extraheader=Authorization: Basic {expected_basic}"
|
|
)
|
|
assert "clone" in clone_argv[c_idx + 2 :]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_release_push_argv_uses_extraheader_not_url_token(
|
|
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
|
) -> None:
|
|
"""H11: the release push argv carries the PAT via ``-c http.extraheader``
|
|
and pushes to the bare URL — never ``https://TOKEN@host/...``."""
|
|
token = "ghp_SECRETPUSH"
|
|
git_url = "https://github.com/org/roboco.git"
|
|
expected_basic = _basic_auth(token)
|
|
git_prefix = ["-c", f"http.extraheader=Authorization: Basic {expected_basic}"]
|
|
captured: list[list[str]] = []
|
|
|
|
# commit_and_push issues: add -A, 2x identity config, commit, rev-parse, push.
|
|
responses = iter(
|
|
[
|
|
_DoneProc(b""), # add -A
|
|
_DoneProc(b""), # config user.name
|
|
_DoneProc(b""), # config user.email
|
|
_DoneProc(b""), # commit
|
|
_DoneProc(b"deadbeef\n"), # rev-parse HEAD
|
|
_DoneProc(b"ok"), # push
|
|
]
|
|
)
|
|
|
|
async def _exec(*args: str, **_kwargs: object) -> _DoneProc:
|
|
captured.append(list(args))
|
|
return next(responses)
|
|
|
|
monkeypatch.setattr(re.asyncio, "create_subprocess_exec", _exec)
|
|
|
|
ctx = _ReleaseContext(
|
|
slug="roboco-api",
|
|
prod_branch="master",
|
|
root=tmp_path,
|
|
git_url=git_url,
|
|
git_prefix=git_prefix,
|
|
ci_workflow=None,
|
|
env_chain=[],
|
|
)
|
|
ops = _GitReleaseOps(session=MagicMock(), ctx=ctx)
|
|
sha = await ops.commit_and_push("0.13.0")
|
|
assert sha == "deadbeef"
|
|
|
|
push_argv = next(a for a in captured if "push" in a)
|
|
assert f"https://{token}@" not in " ".join(push_argv), (
|
|
f"raw token leaked into push argv URL: {push_argv}"
|
|
)
|
|
assert token not in push_argv, f"raw token in push argv: {push_argv}"
|
|
assert git_url in push_argv, f"bare git_url missing from push argv: {push_argv}"
|
|
assert "-c" in push_argv
|
|
c_idx = push_argv.index("-c")
|
|
assert (
|
|
push_argv[c_idx + 1]
|
|
== f"http.extraheader=Authorization: Basic {expected_basic}"
|
|
)
|
|
assert "push" in push_argv[c_idx + 2 :]
|
|
|
|
|
|
def test_insert_changelog_entry_empties_unreleased_body() -> None:
|
|
existing = (
|
|
"# Changelog\n\nintro\n\n## [Unreleased]\n\n### Added\n\n"
|
|
"- **Curated bullet.**\n\n## [0.24.0] - 2026-07-14\n\n- old\n"
|
|
)
|
|
entry = "## [0.25.0] - 2026-07-15\n\n### Added\n\n- **Curated bullet.**\n"
|
|
result = re._insert_changelog_entry(existing, entry)
|
|
unreleased = result.split("## [Unreleased]")[1].split("## [0.25.0]")[0]
|
|
assert "Curated bullet" not in unreleased
|
|
assert result.count("Curated bullet") == 1
|
|
assert result.index("## [Unreleased]") < result.index("## [0.25.0]")
|
|
assert result.index("## [0.25.0]") < result.index("## [0.24.0]")
|
|
assert "- old" in result
|
|
|
|
|
|
def test_insert_changelog_entry_without_unreleased_is_unchanged_behavior() -> None:
|
|
existing = "# Changelog\n\n## [0.24.0] - 2026-07-14\n\n- old\n"
|
|
entry = "## [0.25.0] - 2026-07-15\n\n- new\n"
|
|
result = re._insert_changelog_entry(existing, entry)
|
|
assert result.index("## [0.25.0]") < result.index("## [0.24.0]")
|
|
assert "- old" in result and "- new" in result
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_wait_for_ci_polls_the_prod_branch(
|
|
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
|
) -> None:
|
|
"""The release commit lives on the prod rung — the CI wait must query that
|
|
branch, not the ladder head where get_latest_ci_conclusion defaults."""
|
|
seen: dict[str, object] = {}
|
|
|
|
async def _fake_get_ci(_slug: str, **kwargs: object) -> dict[str, object]:
|
|
seen.update(kwargs)
|
|
return {"head_sha": "cafebabe", "conclusion": "success"}
|
|
|
|
monkeypatch.setattr(
|
|
"roboco.services.git.get_git_service",
|
|
lambda _session: SimpleNamespace(get_latest_ci_conclusion=_fake_get_ci),
|
|
)
|
|
ctx = _ReleaseContext(
|
|
slug="roboco-api",
|
|
prod_branch="master",
|
|
root=tmp_path,
|
|
git_url="x",
|
|
git_prefix=[],
|
|
ci_workflow="ci.yml",
|
|
env_chain=["slave"],
|
|
)
|
|
ops = _GitReleaseOps(session=MagicMock(), ctx=ctx)
|
|
ok = await ops.wait_for_ci("cafebabe")
|
|
assert ok is True
|
|
assert seen.get("branch") == "master"
|