diff --git a/roboco/services/video_post_service.py b/roboco/services/video_post_service.py index 82534a11..a6e1ae74 100644 --- a/roboco/services/video_post_service.py +++ b/roboco/services/video_post_service.py @@ -311,7 +311,8 @@ class VideoPostService(BaseService): cancellation, or a crash can never lose the record of what already posted; a retry re-reads the committed draft and skips it via the `already_posted` check below. Only commits COMPLETED once every - platform has posted (`_finalize_post`). + CONFIGURED platform has posted (`_finalize_post`) — a platform with + no credentials is skipped, never a pending-forever failure. Each commit runs through `_commit_shielded` — a lock-loss cancellation firing while it's in flight must not interrupt it (see @@ -325,11 +326,19 @@ class VideoPostService(BaseService): ) posted: dict[str, str] = {} failures: dict[str, str] = {} + skipped: dict[str, str] = {} for platform in platforms: already_posted = draft.get(f"{platform}_posted_id") if already_posted: posted[platform] = str(already_posted) continue + # An UNCONFIGURED platform is a standing deployment fact, not a + # transient failure: treating it as a failure left every draft + # targeting it pending forever (retry semantics that can never + # succeed), parking an already-X-posted card in the CEO queue. + if not self._platform_configured(platform): + skipped[platform] = "no credentials configured" + continue posted_id, detail = await self._attempt_platform_post(platform, draft) if posted_id is None: failures[platform] = detail @@ -348,7 +357,17 @@ class VideoPostService(BaseService): # platform-native idempotency key is a future follow-up. markers.set_video_draft(task, dict(draft)) await self._commit_shielded() - return await self._finalize_post(task, posted, failures) + return await self._finalize_post(task, posted, failures, skipped) + + def _platform_configured(self, platform: str) -> bool: + """Whether `platform`'s poster holds credentials. Unknown platforms + report True so they still fall through to the explicit + unknown-platform failure in `_post_platform`.""" + if platform == "x": + return bool(self._x_poster.configured) + if platform == "tiktok": + return bool(self._tiktok_poster.configured) + return True async def _commit_shielded(self) -> None: """Commit via asyncio.shield so a lock-loss cancellation firing @@ -421,17 +440,34 @@ class VideoPostService(BaseService): return result.publish_id, result.detail async def _finalize_post( - self, task: TaskTable, posted: dict[str, str], failures: dict[str, str] + self, + task: TaskTable, + posted: dict[str, str], + failures: dict[str, str], + skipped: dict[str, str], ) -> VideoPostExecuteResult: - if not failures: + if not failures and posted: task.status = TaskStatus.COMPLETED # Commit while still holding the lock so COMPLETED is durable # before release — otherwise a racing approve could acquire the # lock the instant we drop it and double-post before a # route-level commit. Shielded — see _commit_shielded. await self._commit_shielded() + detail = "posted to all configured platforms" + if skipped: + detail += "; skipped (unconfigured): " + ", ".join(sorted(skipped)) return VideoPostExecuteResult( - status="posted", posted=dict(posted), detail="posted to all platforms" + status="posted", posted=dict(posted), detail=detail + ) + if not failures and skipped: + # Nothing posted and nothing failed — every target platform is + # unconfigured. Refuse loudly instead of completing a draft that + # never reached any audience. + detail = "no target platform has credentials configured: " + ", ".join( + sorted(skipped) + ) + return VideoPostExecuteResult( + status="post_failed", posted={}, detail=detail ) # Every successful platform's posted-id was already committed in the # loop above (see _post_all_platforms) — nothing left to persist. diff --git a/tests/unit/services/test_video_post_service.py b/tests/unit/services/test_video_post_service.py index fbf35cd8..405fa0e6 100644 --- a/tests/unit/services/test_video_post_service.py +++ b/tests/unit/services/test_video_post_service.py @@ -67,15 +67,17 @@ class _StubXPoster(XVideoPoster): posted: bool = True, video_id: str = "x-vid-1", raises: bool = False, + configured: bool = True, ) -> None: self._posted = posted self._video_id = video_id self._raises = raises + self._configured = configured self.calls: list[tuple[str, str]] = [] @property def configured(self) -> bool: - return True + return self._configured async def post_video(self, *, mp4_path: str, caption: str) -> XVideoPostResult: self.calls.append((mp4_path, caption)) @@ -93,15 +95,17 @@ class _StubTikTokPoster(TikTokPoster): uploaded: bool = True, publish_id: str = "tt-pub-1", raises: bool = False, + configured: bool = True, ) -> None: self._uploaded = uploaded self._publish_id = publish_id self._raises = raises + self._configured = configured self.calls: list[tuple[str, str]] = [] @property def configured(self) -> bool: - return True + return self._configured async def upload_to_inbox( self, *, mp4_path: str, caption: str @@ -253,6 +257,77 @@ async def test_approve_single_platform_only_calls_that_poster( assert tiktok_poster.calls == [] # never invoked — not in this draft's platforms +@pytest.mark.asyncio +async def test_approve_completes_when_unconfigured_platform_is_skipped( + db_session: AsyncSession, +) -> None: + """The live lingering-card defect: X posts, TikTok has no credentials — + the draft must COMPLETE (skipped, not pending-forever).""" + task = await _seed_video_post(db_session) + x_poster = _StubXPoster() + tiktok_poster = _StubTikTokPoster(configured=False) + with _LOCKED[0], _LOCKED[1]: + result = await _svc( + db_session, x_poster=x_poster, tiktok_poster=tiktok_poster + ).approve(_id(task)) + assert result is not None + assert result.status == "posted" + assert result.posted == {"x": "x-vid-1"} + assert "skipped (unconfigured): tiktok" in result.detail + assert tiktok_poster.calls == [] # never attempted without credentials + await db_session.refresh(task) + assert task.status == TS.COMPLETED + draft = markers.get_video_draft(task) + assert draft is not None + assert draft["x_posted_id"] == "x-vid-1" + assert "tiktok_posted_id" not in draft + + +@pytest.mark.asyncio +async def test_approve_refuses_when_no_platform_is_configured( + db_session: AsyncSession, +) -> None: + """All targets unconfigured: never silently complete a draft that + reached no audience.""" + task = await _seed_video_post(db_session) + with _LOCKED[0], _LOCKED[1]: + result = await _svc( + db_session, + x_poster=_StubXPoster(configured=False), + tiktok_poster=_StubTikTokPoster(configured=False), + ).approve(_id(task)) + assert result is not None + assert result.status == "post_failed" + assert "no target platform has credentials configured" in result.detail + await db_session.refresh(task) + assert task.status != TS.COMPLETED + + +@pytest.mark.asyncio +async def test_reapprove_after_partial_post_skips_x_and_completes( + db_session: AsyncSession, +) -> None: + """The exact recovery path for a card parked by an unconfigured + platform: X already posted on a prior approve, TikTok unconfigured — + re-approve must not re-post X and must clear the card.""" + task = await _seed_video_post(db_session) + x_poster = _StubXPoster() + tiktok_poster = _StubTikTokPoster(configured=False) + svc = _svc(db_session, x_poster=x_poster, tiktok_poster=tiktok_poster) + draft = markers.get_video_draft(task) + assert draft is not None + markers.set_video_draft(task, {**draft, "x_posted_id": "x-vid-prior"}) + await db_session.flush() + with _LOCKED[0], _LOCKED[1]: + result = await svc.approve(_id(task)) + assert result is not None + assert result.status == "posted" + assert result.posted == {"x": "x-vid-prior"} + assert x_poster.calls == [] # already-posted guard held — no double post + await db_session.refresh(task) + assert task.status == TS.COMPLETED + + @pytest.mark.asyncio async def test_approve_is_idempotent_second_call_is_noop( db_session: AsyncSession,