fix(video): unconfigured platforms are skipped, not pending-forever failures (#545)

A draft targeting a platform with no credentials could never complete:
the X leg posted and committed its id, the TikTok leg failed as
'transient', and the card sat pending in the CEO queue with retry
semantics that structurally cannot succeed. Unconfigured platforms are
now an explicit skip: the draft COMPLETES when every configured
platform has posted (detail names what was skipped), an approve whose
targets are all unconfigured refuses loudly instead of silently
completing, and genuine post failures keep their partial/retry
semantics. Re-approving a parked card clears it without re-posting
(the already-posted guard is covered by a dedicated test).

Verified: 32/32 test_video_post_service (3 new: skip-and-complete,
all-unconfigured refusal, re-approve recovery), ruff/mypy/xenon clean.

Co-authored-by: Renn F <rennf93@users.noreply.github.com>

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.

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,